From 92b73ffb1e5cdfb001da684c463856f79aff6e11 Mon Sep 17 00:00:00 2001 From: Stefan Daschek Date: Fri, 5 Jul 2013 04:22:35 +0200 Subject: Make relationship ids more reliable. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Relationship instances now keep track of their own id – this should be much more reliable than the old way of more or less “guessing” the relationship id based on the position of some object in some array. Fixes https://github.com/randym/axlsx/issues/212, especially. Each relationship now has its own, unique id – except for the cases when it doesn’t: Some relationships need to share the same id, see `Relation#should_use_same_id_as?` for the gory details. All tests pass, and the full example.xlsx is generated without errors and looks fine in Excel for Mac 2011. The pivot table example still has the problems mentioned in https://github.com/randym/axlsx/issues/168 – but as far as I can tell I didn’t make it worse (Excel is still be able to “repair” the file, and the repaired file then contains the pivot table). --- lib/axlsx/drawing/chart.rb | 6 +- lib/axlsx/drawing/drawing.rb | 16 +----- lib/axlsx/drawing/graphic_frame.rb | 11 +--- lib/axlsx/drawing/hyperlink.rb | 17 ++---- lib/axlsx/drawing/pic.rb | 13 ++--- lib/axlsx/package.rb | 6 +- lib/axlsx/rels/relationship.rb | 67 +++++++++++++++++++--- lib/axlsx/rels/relationships.rb | 9 ++- lib/axlsx/workbook/workbook.rb | 13 ++--- lib/axlsx/workbook/worksheet/comments.rb | 6 +- lib/axlsx/workbook/worksheet/pivot_table.rb | 10 +--- .../worksheet/pivot_table_cache_definition.rb | 5 +- lib/axlsx/workbook/worksheet/pivot_tables.rb | 2 +- lib/axlsx/workbook/worksheet/table.rb | 5 +- lib/axlsx/workbook/worksheet/tables.rb | 2 +- lib/axlsx/workbook/worksheet/worksheet.rb | 13 +---- lib/axlsx/workbook/worksheet/worksheet_comments.rb | 11 ++-- lib/axlsx/workbook/worksheet/worksheet_drawing.rb | 12 +--- .../workbook/worksheet/worksheet_hyperlink.rb | 15 ++--- 19 files changed, 122 insertions(+), 117 deletions(-) (limited to 'lib') diff --git a/lib/axlsx/drawing/chart.rb b/lib/axlsx/drawing/chart.rb index 1da3a253..c1d408e6 100644 --- a/lib/axlsx/drawing/chart.rb +++ b/lib/axlsx/drawing/chart.rb @@ -80,10 +80,10 @@ module Axlsx # Default :gap (although this really should vary by chart type and grouping) attr_reader :display_blanks_as - # returns a relationship object for the chart - # @return [Axlsx::Relationship] + # The relationship object for this chart. + # @return [Relationship] def relationship - Relationship.new(CHART_R, "../#{pn}") + Relationship.new(self, CHART_R, "../#{pn}") end # The index of this chart in the workbooks charts collection diff --git a/lib/axlsx/drawing/drawing.rb b/lib/axlsx/drawing/drawing.rb index 58f0f81e..1748e9f4 100644 --- a/lib/axlsx/drawing/drawing.rb +++ b/lib/axlsx/drawing/drawing.rb @@ -121,12 +121,6 @@ module Axlsx @worksheet.workbook.drawings.index(self) end - # The relation reference id for this drawing - # @return [String] - def rId - "rId#{index+1}" - end - # The part name for this drawing # @return [String] def pn @@ -140,15 +134,7 @@ module Axlsx "#{DRAWING_RELS_PN % (index+1)}" end - # The index of a chart, image or hyperlink object this drawing contains - def index_of(object) - child_objects.index(object) - end - - - # An ordered list of objects this drawing holds - # It is important that the objects are returned in the same order each time for - # releationship indexing in the package + # A list of objects this drawing holds. # @return [Array] def child_objects charts + images + hyperlinks diff --git a/lib/axlsx/drawing/graphic_frame.rb b/lib/axlsx/drawing/graphic_frame.rb index 943caeae..9b921275 100644 --- a/lib/axlsx/drawing/graphic_frame.rb +++ b/lib/axlsx/drawing/graphic_frame.rb @@ -22,15 +22,10 @@ module Axlsx @chart = chart_type.new(self, options) end - # The relationship id for this graphic + # The relationship id for this graphic frame. # @return [String] - # - # NOTE: Discontinued. This should not be part of GraphicFrame. - # The drawing object maintains relationships and needs to be queried to determine the relationship id of any given graphic data child object. - # def rId - warn('axlsx::DEPRECIATED: GraphicFrame#rId has been depreciated. relationship id is determed by the drawing object') - "rId#{@anchor.index+1}" + @anchor.drawing.relationships.for(chart).Id end # Serializes the object @@ -49,7 +44,7 @@ module Axlsx str << '' str << '' str << '' - str << '' + str << '' str << '' str << '' str << '' diff --git a/lib/axlsx/drawing/hyperlink.rb b/lib/axlsx/drawing/hyperlink.rb index e886f766..850baeef 100644 --- a/lib/axlsx/drawing/hyperlink.rb +++ b/lib/axlsx/drawing/hyperlink.rb @@ -83,27 +83,20 @@ module Axlsx # @return [String] attr_accessor :tooltip - # Returns a relationship object for this hyperlink - # @return [Axlsx::Relationship] + # The relationship object for this hyperlink. + # @return [Relationship] def relationship - Relationship.new(HYPERLINK_R, href, :target_mode => :External) + Relationship.new(self, HYPERLINK_R, href, :target_mode => :External) end + # Serializes the object # @param [String] str # @return [String] def to_xml_string(str = '') str << ' "rId#{id}", :'xmlns:r' => XML_NS_R } + serialized_attributes str, {:'r:id' => relationship.Id, :'xmlns:r' => XML_NS_R } str << '/>' end - private - - # The relational ID for this hyperlink - # @return [Integer] - def id - @parent.anchor.drawing.index_of(self)+1 - end - end end diff --git a/lib/axlsx/drawing/pic.rb b/lib/axlsx/drawing/pic.rb index 8ed8d042..89f4c1ea 100644 --- a/lib/axlsx/drawing/pic.rb +++ b/lib/axlsx/drawing/pic.rb @@ -102,15 +102,10 @@ module Axlsx "#{IMAGE_PN % [(index+1), extname]}" end - # The relational id withing the drawing's relationships - def id - @anchor.drawing.charts.size + @anchor.drawing.images.index(self) + 1 - end - - # Returns a relationship object for this object - # @return Axlsx::Relationship + # The relationship object for this pic. + # @return [Relationship] def relationship - Relationship.new(IMAGE_R, "../#{pn}") + Relationship.new(self, IMAGE_R, "../#{pn}") end # providing access to the anchor's width attribute @@ -177,7 +172,7 @@ module Axlsx picture_locking.to_xml_string(str) str << '' str << '' - str << '' + str << '' str << '' str << '' str << '' diff --git a/lib/axlsx/package.rb b/lib/axlsx/package.rb index 7e4bad60..a78b5f5d 100644 --- a/lib/axlsx/package.rb +++ b/lib/axlsx/package.rb @@ -339,9 +339,9 @@ module Axlsx # @private def relationships rels = Axlsx::Relationships.new - rels << Relationship.new(WORKBOOK_R, WORKBOOK_PN) - rels << Relationship.new(CORE_R, CORE_PN) - rels << Relationship.new(APP_R, APP_PN) + rels << Relationship.new(self, WORKBOOK_R, WORKBOOK_PN) + rels << Relationship.new(self, CORE_R, CORE_PN) + rels << Relationship.new(self, APP_R, APP_PN) rels.lock rels end diff --git a/lib/axlsx/rels/relationship.rb b/lib/axlsx/rels/relationship.rb index 385059f1..64356d46 100644 --- a/lib/axlsx/rels/relationship.rb +++ b/lib/axlsx/rels/relationship.rb @@ -3,7 +3,29 @@ module Axlsx # A relationship defines a reference between package parts. # @note Packages automatically manage relationships. class Relationship + + class << self + # Keeps track of all instances of this class. + # @return [Array] + def instances + @instances ||= [] + end + + # Generate and return a unique id. Used for setting {#Id}. + # @return [String] + def next_free_id + @next_free_id_counter ||= 0 + @next_free_id_counter += 1 + "rId#{@next_free_id_counter}" + end + end + # The id of the relationship (eg. "rId123"). Most instances get their own unique id. + # However, some instances need to share the same id – see {#should_use_same_id_as?} + # for details. + # @return [String] + attr_reader :Id + # The location of the relationship target # @return [String] attr_reader :Target @@ -30,14 +52,26 @@ module Axlsx # Target mode must be :external for now. attr_reader :TargetMode - # creates a new relationship + # The source object the relations belongs to (e.g. a hyperlink, drawing, ...). Needed when + # looking up the relationship for a specific object (see {Relationships#for}). + attr_reader :source_obj + + # Initializes a new relationship. + # @param [Object] source_obj see {#source_obj} # @param [String] type The type of the relationship # @param [String] target The target for the relationship # @option [Symbol] :target_mode only accepts :external. - def initialize(type, target, options={}) + def initialize(source_obj, type, target, options={}) + @source_obj = source_obj self.Target=target self.Type=type - self.TargetMode = options.delete(:target_mode) if options[:target_mode] + self.TargetMode = options[:target_mode] if options[:target_mode] + @Id = if (existing = self.class.instances.find{ |i| should_use_same_id_as?(i) }) + existing.Id + else + self.class.next_free_id + end + self.class.instances << self end # @see Target @@ -50,15 +84,32 @@ module Axlsx # serialize relationship # @param [String] str - # @param [Integer] rId the id for this relationship # @return [String] - def to_xml_string(rId, str = '') - h = self.instance_values - h[:Id] = 'rId' << rId.to_s + def to_xml_string(str = '') + h = self.instance_values.reject{|k, _| k == "source_obj"} str << '' end - + + # Whether this relationship should use the same id as `other`. + # + # Instances designating the same relationship need to use the same id. We can not simply + # compare the {#Target} attribute, though: `foo/bar.xml`, `../foo/bar.xml`, + # `../../foo/bar.xml` etc. are all different but probably mean the same file (this + # is especially an issue for relationships in the context of pivot tables). So lets + # just ignore this attribute for now (except when {#TargetMode} is set to `:External` – + # then {#Target} will be an absolute URL and thus can safely be compared). + # + # @todo Implement comparison of {#Target} based on normalized path names. + # @param other [Relationship] + def should_use_same_id_as?(other) + result = self.source_obj == other.source_obj && self.Type == other.Type && self.TargetMode == other.TargetMode + if self.TargetMode == :External + result &&= self.Target == other.Target + end + result + end + end end diff --git a/lib/axlsx/rels/relationships.rb b/lib/axlsx/rels/relationships.rb index 521d7689..a9c73f7d 100644 --- a/lib/axlsx/rels/relationships.rb +++ b/lib/axlsx/rels/relationships.rb @@ -11,10 +11,17 @@ require 'axlsx/rels/relationship.rb' super Relationship end + # The relationship instance for the given source object, or nil if none exists. + # @see Relationship#source_obj + # @return [Relationship] + def for(source_obj) + @list.find{ |rel| rel.source_obj == source_obj } + end + def to_xml_string(str = '') str << '' str << '' - each_with_index { |rel, index| rel.to_xml_string(index+1, str) } + each{ |rel| rel.to_xml_string(str) } str << '' end diff --git a/lib/axlsx/workbook/workbook.rb b/lib/axlsx/workbook/workbook.rb index 1397552a..d122f253 100644 --- a/lib/axlsx/workbook/workbook.rb +++ b/lib/axlsx/workbook/workbook.rb @@ -270,14 +270,14 @@ require 'axlsx/workbook/worksheet/selection.rb' def relationships r = Relationships.new @worksheets.each do |sheet| - r << Relationship.new(WORKSHEET_R, WORKSHEET_PN % (r.size+1)) + r << Relationship.new(sheet, WORKSHEET_R, WORKSHEET_PN % (r.size+1)) end pivot_tables.each_with_index do |pivot_table, index| - r << Relationship.new(PIVOT_TABLE_CACHE_DEFINITION_R, PIVOT_TABLE_CACHE_DEFINITION_PN % (index+1)) + r << Relationship.new(pivot_table.cache_definition, PIVOT_TABLE_CACHE_DEFINITION_R, PIVOT_TABLE_CACHE_DEFINITION_PN % (index+1)) end - r << Relationship.new(STYLES_R, STYLES_PN) + r << Relationship.new(self, STYLES_R, STYLES_PN) if use_shared_strings - r << Relationship.new(SHARED_STRINGS_R, SHARED_STRINGS_PN) + r << Relationship.new(self, SHARED_STRINGS_R, SHARED_STRINGS_PN) end r end @@ -336,9 +336,8 @@ require 'axlsx/workbook/worksheet/selection.rb' defined_names.to_xml_string(str) unless pivot_tables.empty? str << '' - pivot_tables.each_with_index do |pivot_table, index| - rId = "rId#{@worksheets.size + index + 1 }" - str << '' + pivot_tables.each do |pivot_table| + str << '' end str << '' end diff --git a/lib/axlsx/workbook/worksheet/comments.rb b/lib/axlsx/workbook/worksheet/comments.rb index 25da9de2..03cce339 100644 --- a/lib/axlsx/workbook/worksheet/comments.rb +++ b/lib/axlsx/workbook/worksheet/comments.rb @@ -56,9 +56,9 @@ module Axlsx # The relationships required by this object # @return [Array] def relationships - [Relationship.new(VML_DRAWING_R, "../#{vml_drawing.pn}"), - Relationship.new(COMMENT_R, "../#{pn}"), - Relationship.new(COMMENT_R_NULL, "NULL")] + [Relationship.new(self, VML_DRAWING_R, "../#{vml_drawing.pn}"), + Relationship.new(self, COMMENT_R, "../#{pn}"), + Relationship.new(self, COMMENT_R_NULL, "NULL")] end # serialize the object diff --git a/lib/axlsx/workbook/worksheet/pivot_table.rb b/lib/axlsx/workbook/worksheet/pivot_table.rb index da87162d..632765f9 100644 --- a/lib/axlsx/workbook/worksheet/pivot_table.rb +++ b/lib/axlsx/workbook/worksheet/pivot_table.rb @@ -135,20 +135,14 @@ module Axlsx @cache_definition ||= PivotTableCacheDefinition.new(self) end - # The worksheet relationships. This is managed automatically by the worksheet + # The relationships for this pivot table. # @return [Relationships] def relationships r = Relationships.new - r << Relationship.new(PIVOT_TABLE_CACHE_DEFINITION_R, "../#{cache_definition.pn}") + r << Relationship.new(cache_definition, PIVOT_TABLE_CACHE_DEFINITION_R, "../#{cache_definition.pn}") r end - # The relation reference id for this table - # @return [String] - def rId - "rId#{index+1}" - end - # Serializes the object # @param [String] str # @return [String] diff --git a/lib/axlsx/workbook/worksheet/pivot_table_cache_definition.rb b/lib/axlsx/workbook/worksheet/pivot_table_cache_definition.rb index c1683520..340b94ff 100644 --- a/lib/axlsx/workbook/worksheet/pivot_table_cache_definition.rb +++ b/lib/axlsx/workbook/worksheet/pivot_table_cache_definition.rb @@ -35,10 +35,11 @@ module Axlsx index + 1 end - # The relation reference id for this table + # The relationship id for this pivot table cache definition. + # @see Relationship#Id # @return [String] def rId - "rId#{index + 1}" + pivot_table.relationships.for(self).Id end # Serializes the object diff --git a/lib/axlsx/workbook/worksheet/pivot_tables.rb b/lib/axlsx/workbook/worksheet/pivot_tables.rb index f5625fc0..912d9f41 100644 --- a/lib/axlsx/workbook/worksheet/pivot_tables.rb +++ b/lib/axlsx/workbook/worksheet/pivot_tables.rb @@ -17,7 +17,7 @@ module Axlsx # returns the relationships required by this collection def relationships return [] if empty? - map{ |pivot_table| Relationship.new(PIVOT_TABLE_R, "../#{pivot_table.pn}") } + map{ |pivot_table| Relationship.new(pivot_table, PIVOT_TABLE_R, "../#{pivot_table.pn}") } end end diff --git a/lib/axlsx/workbook/worksheet/table.rb b/lib/axlsx/workbook/worksheet/table.rb index dce1a40e..94474c57 100644 --- a/lib/axlsx/workbook/worksheet/table.rb +++ b/lib/axlsx/workbook/worksheet/table.rb @@ -47,10 +47,11 @@ module Axlsx "#{TABLE_PN % (index+1)}" end - # The relation reference id for this table + # The relationship id for this table. + # @see Relationship#Id # @return [String] def rId - "rId#{index+1}" + @sheet.relationships.for(self).Id end # The name of the Table. diff --git a/lib/axlsx/workbook/worksheet/tables.rb b/lib/axlsx/workbook/worksheet/tables.rb index 2d9a1f3c..814f995f 100644 --- a/lib/axlsx/workbook/worksheet/tables.rb +++ b/lib/axlsx/workbook/worksheet/tables.rb @@ -17,7 +17,7 @@ module Axlsx # returns the relationships required by this collection def relationships return [] if empty? - map{ |table| Relationship.new(TABLE_R, "../#{table.pn}") } + map{ |table| Relationship.new(table, TABLE_R, "../#{table.pn}") } end def to_xml_string(str = "") diff --git a/lib/axlsx/workbook/worksheet/worksheet.rb b/lib/axlsx/workbook/worksheet/worksheet.rb index 7324181a..ecba9254 100644 --- a/lib/axlsx/workbook/worksheet/worksheet.rb +++ b/lib/axlsx/workbook/worksheet/worksheet.rb @@ -348,10 +348,11 @@ module Axlsx "#{WORKSHEET_RELS_PN % (index+1)}" end - # The relationship Id of thiw worksheet + # The relationship id of this worksheet. # @return [String] + # @see Relationship#Id def rId - "rId#{index+1}" + @workbook.relationships.for(self).Id end # The index of this worksheet in the owning Workbook's worksheets list. @@ -574,14 +575,6 @@ module Axlsx r end - # identifies the index of an object withing the collections used in generating relationships for the worksheet - # @param [Any] object the object to search for - # @return [Integer] The index of the object - def relationships_index_of(object) - objects = [tables.to_a, worksheet_comments.comments.to_a, hyperlinks.to_a, worksheet_drawing.drawing].flatten.compact || [] - objects.index(object) - end - # Returns the cell or cells defined using excel style A1:B3 references. # @param [String|Integer] cell_def the string defining the cell or range of cells, or the rownumber # @return [Cell, Array] diff --git a/lib/axlsx/workbook/worksheet/worksheet_comments.rb b/lib/axlsx/workbook/worksheet/worksheet_comments.rb index 8c700aa0..f9e4c8cd 100644 --- a/lib/axlsx/workbook/worksheet/worksheet_comments.rb +++ b/lib/axlsx/workbook/worksheet/worksheet_comments.rb @@ -40,10 +40,11 @@ module Axlsx !comments.empty? end - # The index in the worksheet's relationships for the VML drawing that will render the comments - # @return [Integer] - def index - worksheet.relationships.index { |r| r.Type == VML_DRAWING_R } + 1 + # The relationship id of the VML drawing that will render the comments. + # @see Relationship#Id + # @return [String] + def drawing_rId + comments.relationships.find{ |r| r.Type == VML_DRAWING_R }.Id end # Seraalize the object @@ -51,7 +52,7 @@ module Axlsx # @return [String] def to_xml_string(str = '') return unless has_comments? - str << "" + str << "" end end end diff --git a/lib/axlsx/workbook/worksheet/worksheet_drawing.rb b/lib/axlsx/workbook/worksheet/worksheet_drawing.rb index 9deeeec3..08cad1f7 100644 --- a/lib/axlsx/workbook/worksheet/worksheet_drawing.rb +++ b/lib/axlsx/workbook/worksheet/worksheet_drawing.rb @@ -41,24 +41,18 @@ module Axlsx @drawing.is_a? Drawing end - # The relationship required by this object + # The relationship instance for this drawing. # @return [Relationship] def relationship return unless has_drawing? - Relationship.new(DRAWING_R, "../#{drawing.pn}") - end - - # returns the index of the worksheet releationship that defines this drawing. - # @return [Integer] - def index - worksheet.relationships.index{ |r| r.Type == DRAWING_R } +1 + Relationship.new(self, DRAWING_R, "../#{drawing.pn}") end # Serialize the drawing for the worksheet # @param [String] str def to_xml_string(str = '') return unless has_drawing? - str << "" + str << "" end end end diff --git a/lib/axlsx/workbook/worksheet/worksheet_hyperlink.rb b/lib/axlsx/workbook/worksheet/worksheet_hyperlink.rb index d352ec90..2a241287 100644 --- a/lib/axlsx/workbook/worksheet/worksheet_hyperlink.rb +++ b/lib/axlsx/workbook/worksheet/worksheet_hyperlink.rb @@ -45,18 +45,13 @@ module Axlsx @ref = cell_reference end - # The relationship required by this hyperlink when the taget is :external + # The relationship instance for this hyperlink. + # A relationship is only required if `@target` is `:external`. If not, this method will simply return `nil`. + # @see #target= # @return [Relationship] def relationship return unless @target == :external - Relationship.new HYPERLINK_R, location, :target_mode => :External - end - - # The id of the relationship for this object - # @return [String] - def id - return unless @target == :external - "rId#{(@worksheet.relationships_index_of(self)+1)}" + Relationship.new(self, HYPERLINK_R, location, :target_mode => :External) end # Seralize the object @@ -73,7 +68,7 @@ module Axlsx # r:id should only be specified for external targets. # @return [Hash] def location_or_id - @target == :external ? { :"r:id" => id } : { :location => Axlsx::coder.encode(location) } + @target == :external ? { :"r:id" => relationship.Id } : { :location => Axlsx::coder.encode(location) } end end end -- cgit v1.2.3 From 2a4c39f7ae863ac7e4d14e0110aba11fce4a822f Mon Sep 17 00:00:00 2001 From: Stefan Daschek Date: Mon, 8 Jul 2013 16:34:28 +0200 Subject: Simply Relationship.next_free_id by depending on number of cached instances. Only drawback: Setting @next_freed_id_counter to 1000 in tc_helper.rb is no longer possible. But this was useful mainly while adding / fixing test cases when implementing the Relationship instance cache. --- lib/axlsx/rels/relationship.rb | 4 +--- test/tc_helper.rb | 4 ---- 2 files changed, 1 insertion(+), 7 deletions(-) (limited to 'lib') diff --git a/lib/axlsx/rels/relationship.rb b/lib/axlsx/rels/relationship.rb index 64356d46..5e75a98a 100644 --- a/lib/axlsx/rels/relationship.rb +++ b/lib/axlsx/rels/relationship.rb @@ -14,9 +14,7 @@ module Axlsx # Generate and return a unique id. Used for setting {#Id}. # @return [String] def next_free_id - @next_free_id_counter ||= 0 - @next_free_id_counter += 1 - "rId#{@next_free_id_counter}" + "rId#{@instances.size + 1}" end end diff --git a/test/tc_helper.rb b/test/tc_helper.rb index c09446b7..af40a1e4 100644 --- a/test/tc_helper.rb +++ b/test/tc_helper.rb @@ -8,7 +8,3 @@ end require 'test/unit' require "timecop" require "axlsx.rb" - -# Make sure all valid rIds are > 1000 - this should help catching the cases where rId is still -# determined by looking at the index of an object in an array etc. -Axlsx::Relationship.instance_variable_set :@next_free_id_counter, 1000 -- cgit v1.2.3 From e5e79814670569838621e67249927c104006bcca Mon Sep 17 00:00:00 2001 From: Stefan Daschek Date: Mon, 8 Jul 2013 16:36:42 +0200 Subject: Implement Relationship.clear_cached_instances, use it before serializing the package. This is necessary to make serialization idempotent (i.e. make sure that Relationship instances are generated with the same IDs everytime the package is serialized). It also fixes a memory leak if Axlsx is used in a long running server process (eg a Rails app). --- lib/axlsx/package.rb | 2 ++ lib/axlsx/rels/relationship.rb | 18 +++++++++++++++++- test/tc_package.rb | 11 +++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/axlsx/package.rb b/lib/axlsx/package.rb index a78b5f5d..0a1bceaa 100644 --- a/lib/axlsx/package.rb +++ b/lib/axlsx/package.rb @@ -100,6 +100,7 @@ module Axlsx # File.open('example_streamed.xlsx', 'w') { |f| f.write(s.read) } def serialize(output, confirm_valid=false) return false unless !confirm_valid || self.validate.empty? + Relationship.clear_cached_instances Zip::ZipOutputStream.open(output) do |zip| write_parts(zip) end @@ -112,6 +113,7 @@ module Axlsx # @return [StringIO|Boolean] False if confirm_valid and validation errors exist. rewound string IO if not. def to_stream(confirm_valid=false) return false unless !confirm_valid || self.validate.empty? + Relationship.clear_cached_instances zip = write_parts(Zip::ZipOutputStream.new("streamed", true)) stream = zip.close_buffer stream.rewind diff --git a/lib/axlsx/rels/relationship.rb b/lib/axlsx/rels/relationship.rb index 5e75a98a..a6b7bdd2 100644 --- a/lib/axlsx/rels/relationship.rb +++ b/lib/axlsx/rels/relationship.rb @@ -11,7 +11,23 @@ module Axlsx @instances ||= [] end - # Generate and return a unique id. Used for setting {#Id}. + # Clear cached instances. + # + # This should be called before serializing a package (see {Package#serialize} and + # {Package#to_stream}) to make sure that serialization is idempotent (i.e. + # Relationship instances are generated with the same IDs everytime the package + # is serialized). + # + # Also, calling this avoids memory leaks (cached instances lingering around + # forever). + def clear_cached_instances + @instances = [] + end + + # Generate and return a unique id (eg. `rId123`) Used for setting {#Id}. + # + # The generated id depends on the number of cached instances, so using + # {clear_cached_instances} will automatically reset the generated ids, too. # @return [String] def next_free_id "rId#{@instances.size + 1}" diff --git a/test/tc_package.rb b/test/tc_package.rb index 86f11dd4..d7aa6904 100644 --- a/test/tc_package.rb +++ b/test/tc_package.rb @@ -133,6 +133,17 @@ class TestPackage < Test::Unit::TestCase assert zip_content_then == zip_content_now, "zip files are not identical" end end + + def test_serialization_creates_identical_files_for_identical_packages + package_1, package_2 = 2.times.map do + Axlsx::Package.new(created_at: Time.utc(2013, 1, 1)).tap do |p| + p.workbook.add_worksheet(:name => "Basic Worksheet") do |sheet| + sheet.add_row [1, 2, 3] + end + end + end + assert package_1.to_stream.string == package_2.to_stream.string, "zip files are not identical" + end def test_validation assert_equal(@package.validate.size, 0, @package.validate) -- cgit v1.2.3