Fix collection partial leaking fields between iterations (#64)

CHANGELOG.md

@@ -1,5 +1,6 @@
 ## Unreleased
 
+- Fix `partial:`/`collection:` shorthand reusing one sub-message across iterations, which leaked unset fields and accumulated repeated message blocks between elements (#64)
 - Remove official support for Rails 7.0 and Rails 6.x, add Appraisal config for Rails 8.x
 - Bump actions/checkout from 3 to 4 (#40)
 - Drop support for rails v6.* versions (#60)

lib/pbbuilder.rb

@@ -197,6 +197,13 @@ def new_message_for(field)
     _new_message_from_descriptor(descriptor)
   end
 
+  # Replaces the underlying message. Used by Pbbuilder::CollectionRenderer to
+  # give each iteration of a `partial:`/`collection:` render a clean message
+  # so fields cannot leak between elements.
+  def _reset_message(message)
+    @message = message
+  end
+
   private
 
   # Lookup the field name (or 'attribute' name, for ex "best_friend") on the Google descriptor (Google::Protobuf::Descriptor) of our

lib/pbbuilder/collection_renderer.rb

@@ -12,7 +12,12 @@ def initialize(lookup_context, options, &scope)
     private
 
     def build_rendered_template(content, template, layout = nil)
-      super(content || pb.attributes!, template)
+      body = content || pb.attributes!
+      # Reset the working message so the next iteration starts with a clean
+      # sub-message and cannot inherit fields written or pushed by this one.
+      # See https://github.com/cheddar-me/pbbuilder/issues/64.
+      pb._reset_message(pb_parent.new_message_for(field))
+      super(body, template)
     end
 
     def build_rendered_collection(templates, _spacer)

test/pbbuilder_template_test.rb

@@ -21,11 +21,25 @@ class PbbuilderTemplateTest < ActiveSupport::TestCase
     pb.url_3x asset.url
   PBBUILDER
 
+  TAGGABLE_PARTIAL = <<-PBBUILDER
+    pb.name taggable.name
+    pb.tags taggable.tags if taggable.tags.any?
+  PBBUILDER
+
+  FRIEND_HOLDER_PARTIAL = <<-PBBUILDER
+    pb.name holder.name
+    pb.friends holder.friends do |f|
+      pb.name f.name
+    end
+  PBBUILDER
+
   PARTIALS = {
     "_partial.pb.pbbuilder" => "pb.name name",
     "_person.pb.pbbuilder" => PERSON_PARTIAL,
     "racers/_racer.pb.pbbuilder" => RACER_PARTIAL,
     "_asset.pb.pbbuilder" => ASSET_PARTIAL,
+    "_taggable.pb.pbbuilder" => TAGGABLE_PARTIAL,
+    "_friend_holder.pb.pbbuilder" => FRIEND_HOLDER_PARTIAL,
 
     # Ensure we find only Pbbuilder partials from within Pbbuilder templates.
     "_person.html.erb" => "Hello world!"
@@ -89,6 +103,51 @@ class PbbuilderTemplateTest < ActiveSupport::TestCase
     assert_equal 2, result.friends.count
   end
 
+  # Regression test for https://github.com/cheddar-me/pbbuilder/issues/64.
+  # A partial that conditionally writes a repeated scalar field used to leak
+  # the previous element's value into elements that took the silent branch
+  # because the underlying message was reused across iterations.
+  test "collection partial does not leak conditional repeated scalar fields between iterations" do
+    template = <<-PBBUILDER
+      taggable_struct = Struct.new(:name, :tags)
+      items = [
+        taggable_struct.new("alice", ["x", "y"]),
+        taggable_struct.new("bob", []),
+      ]
+      pb.friends partial: "taggable", as: :taggable, collection: items
+    PBBUILDER
+    result = render(template)
+
+    assert_equal 2, result.friends.count
+    assert_equal "alice", result.friends[0].name
+    assert_equal ["x", "y"], result.friends[0].tags.to_a
+    assert_equal "bob", result.friends[1].name
+    assert_equal [], result.friends[1].tags.to_a
+  end
+
+  # Regression test for https://github.com/cheddar-me/pbbuilder/issues/64.
+  # `pb.field collection do |x| ... end` inside a collection partial used to
+  # accumulate elements across iterations because _append_repeated pushed onto
+  # the shared message's repeated field.
+  test "collection partial does not accumulate repeated message blocks between iterations" do
+    template = <<-PBBUILDER
+      holder_struct = Struct.new(:name, :friends)
+      friend_struct = Struct.new(:name)
+      items = [
+        holder_struct.new("alice", [friend_struct.new("a1"), friend_struct.new("a2")]),
+        holder_struct.new("bob", []),
+      ]
+      pb.friends partial: "friend_holder", as: :holder, collection: items
+    PBBUILDER
+    result = render(template)
+
+    assert_equal 2, result.friends.count
+    assert_equal "alice", result.friends[0].name
+    assert_equal ["a1", "a2"], result.friends[0].friends.map(&:name)
+    assert_equal "bob", result.friends[1].name
+    assert_equal 0, result.friends[1].friends.count
+  end
+
   test "partial by name with top-level locals" do
     result = render('pb.partial! "partial", name: "hello"')
     assert_equal "hello", result.name