Fix spacing call sites for collection cardinality

CHANGELOG.md

@@ -30,6 +30,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- **Breaking:** `ParagraphProperties#spacing` now returns an Array of
+  `Properties::Spacing` (was a single `Spacing` or nil), because Word
+  can emit more than one `w:spacing` inside a single `w:pPr`. Use
+  `ensure_spacing` to mutate spacing fields and the `Style` readers
+  (`spacing_before`/`spacing_after`) to read them. The readers resolve
+  conflicting values last-wins, matching Word.
 - `Uniword::Diff` module now autoloads `Semantic` (element-level
   diff sits alongside the existing text-level `DocumentDiffer`).
 - `Uniword::Batch` module now autoloads `Operation` (sibling to

lib/uniword/builder/paragraph_builder.rb

@@ -98,12 +98,11 @@ def align=(value)
       # @param rule [String, nil] Line rule ('auto', 'exact', 'atLeast')
       # @return [self]
       def spacing(before: nil, after: nil, line: nil, rule: nil)
-        ensure_properties.spacing ||= Properties::Spacing.new
-        props = @model.properties.spacing
-        props.before = before if before
-        props.after = after if after
-        props.line = line if line
-        props.line_rule = rule if rule
+        written = {
+          before: before, after: after, line: line, line_rule: rule
+        }.select { |_, value| value }
+        sp = ensure_properties.ensure_spacing(*written.keys)
+        written.each { |field, value| sp.public_send(:"#{field}=", value) }
         self
       end
 

lib/uniword/builder/style_builder.rb

@@ -102,11 +102,11 @@ def align(value)
       # @param line [Integer, nil] Line spacing in twips
       # @return [self]
       def spacing(before: nil, after: nil, line: nil)
-        ensure_para_props
-        @model.pPr.spacing ||= Properties::Spacing.new
-        @model.pPr.spacing.before = before if before
-        @model.pPr.spacing.after = after if after
-        @model.pPr.spacing.line = line if line
+        written = {
+          before: before, after: after, line: line
+        }.select { |_, value| value }
+        sp = ensure_para_props.ensure_spacing(*written.keys)
+        written.each { |field, value| sp.public_send(:"#{field}=", value) }
         self
       end
 

lib/uniword/wordprocessingml/paragraph_properties.rb

@@ -22,8 +22,9 @@ class ParagraphProperties < Lutaml::Model::Serializable
       attribute :outline_level, Properties::OutlineLevel
       attribute :numbering_properties, Properties::NumberingProperties
 
-      # Complex spacing object
-      attribute :spacing, Properties::Spacing
+      # Complex spacing objects. Word can emit more than one w:spacing in a
+      # single w:pPr, so this is a collection.
+      attribute :spacing, Properties::Spacing, collection: true
 
       # Complex indentation object
       attribute :indentation, Properties::Indentation
@@ -296,20 +297,50 @@ def initialize(attrs = {})
         convert_flat_attributes!
       end
 
+      # First w:spacing entry, appending one when absent
+      #
+      # Word can emit more than one w:spacing inside a single w:pPr, so the
+      # attribute is a collection. Callers setting individual spacing fields
+      # want the first entry.
+      #
+      # Each field named in +clearing+ is dropped from the later entries. A
+      # value written into the first entry while a later one still carries its
+      # own would emit two contradictory values, and a last-wins reader keeps
+      # the stale one.
+      #
+      # @param clearing [Array<Symbol>] fields the caller is about to write
+      # @return [Properties::Spacing] the first spacing entry
+      def ensure_spacing(*clearing)
+        entries = Array(spacing)
+        if entries.empty?
+          entries = [Properties::Spacing.new]
+          self.spacing = entries
+        end
+
+        entries.drop(1).each do |stale|
+          clearing.each { |field| stale.public_send(:"#{field}=", nil) }
+        end
+
+        entries.first
+      end
+
       # Convert flat convenience attributes to proper wrapper objects
       # This handles cases like ParagraphProperties.new(spacing_before: 120)
       # where the flat attribute is set but the wrapper object is not
       def convert_flat_attributes!
         # Alignment - convert string to Alignment wrapper
         self.alignment = Properties::Alignment.new(value: @alignment) if @alignment.is_a?(String)
 
-        # Spacing - create spacing object from flat spacing attributes
-        if (@spacing_before || @spacing_after || @line_spacing || @line_rule) && !@spacing
-          self.spacing = Properties::Spacing.new
-          spacing.before = @spacing_before if @spacing_before
-          spacing.after = @spacing_after if @spacing_after
-          spacing.line = @line_spacing.to_i if @line_spacing
-          spacing.line_rule = @line_rule if @line_rule
+        # Spacing - create spacing object from flat spacing attributes.
+        # Array() covers both absent forms: nil when built, [] when parsed.
+        if (@spacing_before || @spacing_after || @line_spacing || @line_rule) &&
+            Array(@spacing).empty?
+          entry = Properties::Spacing.new
+          entry.before = @spacing_before if @spacing_before
+          entry.after = @spacing_after if @spacing_after
+          entry.line = @line_spacing.to_i if @line_spacing
+          entry.line_rule = @line_rule if @line_rule
+          self.spacing = [entry]
         end
 
         # Indentation - create indentation object from flat indent attributes

lib/uniword/wordprocessingml/style.rb

@@ -159,12 +159,15 @@ def quick_format
         val == true
       end
 
+      # Word splits one paragraph's spacing across several w:spacing elements,
+      # and Word readers resolve conflicts last-wins, so take the last entry
+      # that actually carries the value.
       def spacing_before
-        pPr&.spacing&.before || pPr&.spacing_before
+        Array(pPr&.spacing).filter_map(&:before).last || pPr&.spacing_before
       end
 
       def spacing_after
-        pPr&.spacing&.after || pPr&.spacing_after
+        Array(pPr&.spacing).filter_map(&:after).last || pPr&.spacing_after
       end
 
       def alignment

spec/uniword/builder/paragraph_builder_spec.rb

@@ -180,15 +180,61 @@
     it "sets spacing before and after" do
       builder = described_class.new
       builder.spacing(before: 240, after: 120)
-      expect(builder.model.properties.spacing.before).to eq(240)
-      expect(builder.model.properties.spacing.after).to eq(120)
+      spacing = builder.model.properties.spacing.first
+      expect(spacing.before).to eq(240)
+      expect(spacing.after).to eq(120)
     end
 
     it "sets line spacing with rule" do
       builder = described_class.new
       builder.spacing(line: 360, rule: "exact")
-      expect(builder.model.properties.spacing.line).to eq(360)
-      expect(builder.model.properties.spacing.line_rule).to eq("exact")
+      spacing = builder.model.properties.spacing.first
+      expect(spacing.line).to eq(360)
+      expect(spacing.line_rule).to eq("exact")
+    end
+
+    it "reuses the existing entry instead of appending a second" do
+      builder = described_class.new
+      builder.spacing(before: 240)
+      builder.spacing(after: 120)
+      expect(builder.model.properties.spacing.size).to eq(1)
+      expect(builder.model.properties.spacing.first.before).to eq(240)
+      expect(builder.model.properties.spacing.first.after).to eq(120)
+    end
+
+    # A pPr whose spacing Word split across two w:spacing elements.
+    context "when a later entry already carries the field" do
+      let(:two_entries_xml) do
+        <<~XML
+          <w:pPr xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main">
+            <w:spacing w:before="0"/>
+            <w:spacing w:line="240" w:lineRule="auto"/>
+          </w:pPr>
+        XML
+      end
+      let(:properties) do
+        Uniword::Wordprocessingml::ParagraphProperties.from_xml(two_entries_xml)
+      end
+      let(:paragraph) do
+        para = Uniword::Wordprocessingml::Paragraph.new
+        para.properties = properties
+        para
+      end
+
+      # Leaving the stale 240 standing emits two w:line, and a last-wins
+      # reader keeps 240 rather than the 360 that was asked for.
+      it "emits only the value it was asked for" do
+        described_class.new(paragraph).spacing(line: 360)
+
+        expect(paragraph.properties.to_xml(prefix: true).scan(/w:line="\d+"/))
+          .to eq(['w:line="360"'])
+      end
+
+      it "leaves fields it was not asked to set alone" do
+        described_class.new(paragraph).spacing(after: 120)
+
+        expect(paragraph.properties.spacing.map(&:line)).to eq([nil, 240])
+      end
     end
   end
 

spec/uniword/builder/style_builder_spec.rb

@@ -55,8 +55,34 @@
     it "sets spacing" do
       sb = described_class.new("S")
       sb.spacing(before: 240, after: 120)
-      expect(sb.model.pPr.spacing.before).to eq(240)
-      expect(sb.model.pPr.spacing.after).to eq(120)
+      spacing = sb.model.pPr.spacing.first
+      expect(spacing.before).to eq(240)
+      expect(spacing.after).to eq(120)
+    end
+
+    # A .dotx style whose spacing Word split across two w:spacing elements.
+    context "when a later entry already carries the field" do
+      let(:two_entries_xml) do
+        <<~XML
+          <w:pPr xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main">
+            <w:spacing w:before="0"/>
+            <w:spacing w:before="777" w:line="240"/>
+          </w:pPr>
+        XML
+      end
+      let(:style_builder) do
+        sb = described_class.new("S")
+        sb.model.pPr =
+          Uniword::Wordprocessingml::ParagraphProperties.from_xml(two_entries_xml)
+        sb
+      end
+
+      it "sets spacing without leaving the stale value standing" do
+        style_builder.spacing(before: 111)
+
+        expect(style_builder.model.pPr.spacing.map(&:before)).to eq([111, nil])
+        expect(style_builder.model.pPr.spacing.map(&:line)).to eq([nil, 240])
+      end
     end
   end
 

spec/uniword/docx/reconciler_spec.rb

@@ -44,7 +44,7 @@ def build_package(settings: nil, footnotes: nil, endnotes: nil)
       para = sep.paragraphs.first
       expect(para).not_to be_nil
       expect(para.properties).not_to be_nil
-      spacing = para.properties.spacing
+      spacing = Array(para.properties.spacing).first
       expect(spacing).not_to be_nil
       expect(spacing.after).to eq(0)
       expect(spacing.line).to eq(240)

spec/uniword/properties/paragraph_properties_spec.rb

@@ -20,8 +20,16 @@
       )
       expect(Array(props.style).first.value).to eq("Heading1")
       expect(props.alignment.value).to eq("center")
-      expect(props.spacing&.before).to eq(240)
-      expect(props.spacing&.after).to eq(120)
+      expect(props.spacing.first.before).to eq(240)
+      expect(props.spacing.first.after).to eq(120)
+    end
+
+    it "builds a single spacing entry from flat attributes" do
+      props = described_class.new(spacing_before: 120, spacing_after: 240)
+
+      expect(props.spacing.size).to eq(1)
+      expect(props.spacing.first.before).to eq(120)
+      expect(props.spacing.first.after).to eq(240)
     end
 
     it "allows mutation for test compatibility" do
@@ -129,6 +137,93 @@
     end
   end
 
+  describe "#ensure_spacing" do
+    let(:ns) do
+      "http://schemas.openxmlformats.org/wordprocessingml/2006/main"
+    end
+    let(:two_entries_xml) do
+      <<~XML
+        <w:pPr xmlns:w="#{ns}">
+          <w:spacing w:before="0" w:after="0"/>
+          <w:spacing w:line="240" w:lineRule="auto"/>
+        </w:pPr>
+      XML
+    end
+
+    it "appends an entry when the properties were built without spacing" do
+      props = described_class.new
+
+      props.ensure_spacing.before = 240
+
+      expect(props.spacing.size).to eq(1)
+      expect(props.spacing.first.before).to eq(240)
+    end
+
+    it "appends an entry when parsing left the collection empty" do
+      props = described_class.from_xml(%(<w:pPr xmlns:w="#{ns}"/>))
+
+      props.ensure_spacing.before = 240
+
+      expect(props.spacing.size).to eq(1)
+      expect(props.spacing.first.before).to eq(240)
+    end
+
+    # Reconciler builds properties this way (notes.rb, parts.rb), passing a
+    # single Spacing rather than an array.
+    #
+    # Assert the entry and the emitted XML, not the shape of `spacing` itself:
+    # whether lutaml-model wraps a scalar assignment into a one-element array
+    # differs between lutaml-model releases, and that is not our contract.
+    it "handles properties constructed with a scalar spacing" do
+      props = described_class.new(
+        spacing: Uniword::Properties::Spacing.new(after: 0, line: 240),
+      )
+
+      entry = props.ensure_spacing
+
+      expect(entry.after).to eq(0)
+      expect(entry.line).to eq(240)
+      expect(props.to_xml(prefix: true)).to include('w:line="240"')
+    end
+
+    it "returns the existing first entry instead of appending" do
+      props = described_class.from_xml(
+        %(<w:pPr xmlns:w="#{ns}"><w:spacing w:before="60"/></w:pPr>),
+      )
+
+      props.ensure_spacing.after = 120
+
+      expect(props.spacing.size).to eq(1)
+      expect(props.spacing.first.before).to eq(60)
+      expect(props.spacing.first.after).to eq(120)
+    end
+
+    it "returns the first of several entries Word emitted" do
+      props = described_class.from_xml(two_entries_xml)
+
+      expect(props.ensure_spacing).to equal(props.spacing.first)
+      expect(props.spacing.size).to eq(2)
+    end
+
+    it "clears the named fields from the later entries" do
+      props = described_class.from_xml(two_entries_xml)
+
+      props.ensure_spacing(:line).line = 360
+
+      expect(props.spacing.map(&:line)).to eq([360, nil])
+      expect(props.to_xml(prefix: true).scan(/w:line="\d+"/))
+        .to eq(['w:line="360"'])
+    end
+
+    it "leaves the later entries alone when no field is named" do
+      props = described_class.from_xml(two_entries_xml)
+
+      props.ensure_spacing.before = 60
+
+      expect(props.spacing.map(&:line)).to eq([nil, 240])
+    end
+  end
+
   describe "inheritance" do
     it "inherits from Lutaml::Model::Serializable" do
       expect(described_class.ancestors)