[Bug] Data is lost on nested associations when preload is enabled for new records

[jamesst20]

Demonstration

As you can see, the first time I render the blueprint, it's like there is no data at all. Then I load the data from the model itself and rerender the exact same blueprint with the exact same record. Now the data is present.

Code

Blueprinter.configure do |config|
  config.extensions << BlueprinterActiveRecord::Preloader.new(auto: true)
end

class PostBlueprint < ApplicationBlueprint
  identifier :id

  fields :name

  association :locale, blueprint: Rcf::Admin::LocaleBlueprint
  association :post_group, blueprint: Rcf::Admin::PostGroupBlueprint
  association :field_values, blueprint: Rcf::Admin::FieldValueBlueprint

  view :list do
    excludes :field_values
    association :post_group, blueprint: Rcf::Admin::PostGroupBlueprint, view: :list
  end
end


class FieldValueBlueprint < ApplicationBlueprint
  identifier :id

  fields :position, :value, :file_url, :meta

  association :field, blueprint: Rcf::Admin::FieldBlueprint
  association :subfield_values, blueprint: Rcf::Admin::FieldValueBlueprint
end

Edit: Reproduction exemple

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  gem "rails"
  # If you want to test against edge Rails replace the previous line with this:
  # gem "rails", github: "rails/rails", branch: "main"

  gem "sqlite3", "~> 1.4"

  gem "blueprinter"
  gem "blueprinter-activerecord"
end

require "active_record"
require "blueprinter"
require "blueprinter-activerecord"
require "minitest/autorun"
require "logger"

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table "posts", force: :cascade do |t|
    t.string :name
  end

  create_table "post_values", force: :cascade do |t|
    t.references :post, null: true
    t.bigint "parent_id"
    t.string :name
    t.string :value
  end
end

class Post < ActiveRecord::Base
  has_many :post_values, dependent: :destroy

  accepts_nested_attributes_for :post_values, allow_destroy: true, reject_if: :all_blank
end

class PostValue < ActiveRecord::Base
  belongs_to :post, inverse_of: :post_values, optional: true

  belongs_to :parent, class_name: "PostValue", inverse_of: :subvalues, optional: true

  has_many :subvalues,
    -> { order(position: :asc) },
    foreign_key: :parent_id,
    class_name: "PostValue",
    dependent: :destroy,
    inverse_of: :parent

  accepts_nested_attributes_for :subvalues, allow_destroy: true, reject_if: :all_blank
end

class PostValueBlueprint < Blueprinter::Base
  identifier :id

  fields :name, :value

  association :subvalues, blueprint: PostValueBlueprint
end

class PostBlueprint < Blueprinter::Base
  identifier :id

  fields :name

  association :post_values, blueprint: PostValueBlueprint
end

class BugTest < Minitest::Test
  def expected_output
    {
      id: nil,
      name: "Test",
      post_values: [
        {
          id: nil,
          name: "Root value",
          subvalues: [
            { id: nil, name: "Child 1 level 1", subvalues: [], value: "asdasdads" },
            { id: nil, name: "Child 2 level 1", subvalues: [], value: "hjgghj" },
            { id: nil, name: "Child 3 level 1", subvalues: [], value: "tyrhtfghdf" }
          ],
          value: "root"
        }
      ]
    }
  end

  def test_preloading_enabled
    Blueprinter.configure do |config|
      config.extensions << BlueprinterActiveRecord::Preloader.new(auto: true)
    end

    post = Post.new

    post.assign_attributes(
      id: nil,
      name: "Test",
      post_values_attributes:[
        {
          id: nil,
          name: "Root value",
          value: "root",
          subvalues_attributes: [
            { id: nil, name: "Child 1 level 1", value: "asdasdads" },
            { id: nil, name: "Child 2 level 1", value: "hjgghj" },
            { id: nil, name: "Child 3 level 1", value: "tyrhtfghdf" },
          ]
         }
      ]
    )

    assert_equal expected_output, PostBlueprint.render_as_hash(post)
  end

  def test_preloading_disabled
    post = Post.new

    post.assign_attributes(
      id: nil,
      name: "Test",
      post_values_attributes:[
        {
          id: nil,
          name: "Root value",
          value: "root",
          subvalues_attributes: [
            { id: nil, name: "Child 1 level 1", value: "asdasdads" },
            { id: nil, name: "Child 2 level 1", value: "hjgghj" },
            { id: nil, name: "Child 3 level 1", value: "tyrhtfghdf" },
          ]
         }
      ]
    )

    assert_equal expected_output, PostBlueprint.render_as_hash(post)
  end
end

[jhollinger]

I don't think the issue is related to this extension. Remember, the extension only runs when the object is an ActiveRecord::Relation or descendant. In your example, it looks like you're passing in a single ActiveRecord object, so the extension isn't run.

There could be an issue in Blueprinter itself.

[jamesst20]

I don't think the issue is related to this extension. Remember, the extension only runs when the object is an ActiveRecord::Relation or descendant. In your example, it looks like you're passing in a single ActiveRecord object, so the extension isn't run.

There could be an issue in Blueprinter itself.

I added a reproduction example. Hope this helps investigate faster :)

If I remove the line

config.extensions << BlueprinterActiveRecord::Preloader.new(auto: true)

the issue is gone. So I do believe it is related.

Technically associations within a blueprint are ActiveRecord relation, no?

[jhollinger]

Thanks for the example, will look. And you're right, associations would trigger the extension.

[jhollinger]

I've duplicated this in our test suite in the fix-new-records branch. I think what's happening is that when a preload for the child association is run, ActiveRecord wipes out any unsaved records on that association if they were added via attributes.

Ideally this extension wouldn't be called at all in cases like this (i.e. non-ActiveRecord::Relation objects being rendered). It's been a recurring challenge for this extension, given how extension hooks interact with rendering. Some ideas have been tossed around for BP 2.0 that could fix it. At the moment, it's not clear how we could fix this in BP 1.0. I'll keep thinking about it, though.

$ bundle exec rake test N=/test_auto_preload_with_new_records/
Run options: -n /test_auto_preload_with_new_records/ --seed 25987

# Running:

F.

Finished in 0.038318s, 52.1948 runs/s, 52.1948 assertions/s.

  1) Failure:
PreloaderExtensionTest#test_auto_preload_with_new_records_via_attributes [test/preloader_extension_test.rb:181]:
--- expected
+++ actual
@@ -1 +1 @@
-{:id=>nil, :name=>"asdf", :widgets=>[{:id=>nil, :name=>"A", :price=>nil}, {:id=>nil, :name=>"B", :price=>nil}, {:id=>nil, :name=>"C", :price=>nil}]}
+{:id=>nil, :name=>"asdf", :widgets=>[]}

2 runs, 2 assertions, 1 failures, 0 errors, 0 skips

[jamesst20]

I've duplicated this in our test suite in the fix-new-records branch. I think what's happening is that when a preload for the child association is run, ActiveRecord wipes out any unsaved records on that association if they were added via attributes.

Ideally this extension wouldn't be called at all in cases like this (i.e. non-ActiveRecord::Relation objects being rendered). It's been a recurring challenge for this extension, given how extension hooks interact with rendering. Some ideas have been tossed around for BP 2.0 that could fix it. At the moment, it's not clear how we could fix this in BP 1.0. I'll keep thinking about it, though.

$ bundle exec rake test N=/test_auto_preload_with_new_records/
Run options: -n /test_auto_preload_with_new_records/ --seed 25987

# Running:

F.

Finished in 0.038318s, 52.1948 runs/s, 52.1948 assertions/s.

  1) Failure:
PreloaderExtensionTest#test_auto_preload_with_new_records_via_attributes [test/preloader_extension_test.rb:181]:
--- expected
+++ actual
@@ -1 +1 @@
-{:id=>nil, :name=>"asdf", :widgets=>[{:id=>nil, :name=>"A", :price=>nil}, {:id=>nil, :name=>"B", :price=>nil}, {:id=>nil, :name=>"C", :price=>nil}]}
+{:id=>nil, :name=>"asdf", :widgets=>[]}

2 runs, 2 assertions, 1 failures, 0 errors, 0 skips

Thanks for the quick investigation. I am not sure if it's actually wiped out by active record because if I were to do these in those step, the blueprint would extract all attributes fine:

MyBlueprint.render_as_hash(record)
--> empty association

# Simply navigating the attributes
record.my_associations.each(&:any_value)
--> any value[]

MyBlueprint.render_as_hash(record)
--> Now associations are filled properly

To demonstrate that, here is another test that passes successfully with preload

def test_preloading_enabled_with_navigation
    Blueprinter.configure do |config|
      config.extensions = [BlueprinterActiveRecord::Preloader.new(auto: true)]
    end

    post = Post.new

    post.assign_attributes(
      id: nil,
      name: "Test",
      post_values_attributes:[
        {
          id: nil,
          name: "Root value",
          value: "root",
          subvalues_attributes: [
            { id: nil, name: "Child 1 level 1", value: "asdasdads" },
            { id: nil, name: "Child 2 level 1", value: "hjgghj" },
            { id: nil, name: "Child 3 level 1", value: "tyrhtfghdf" },
          ]
         }
      ]
    )

    # This here makes the test pass
    post.post_values.each { |post_value| post_value.subvalues.each { |subvalue| subvalue.name } }

    assert_equal expected_output, PostBlueprint.render_as_hash(post)
  end

Maybe it gives you some ideas?

I am not quite sure why it works, but maybe a guess is that by navigating the objects, this here https://github.com/procore-oss/blueprinter-activerecord/blob/main/lib/blueprinter-activerecord/preloader.rb#L41 !object.loaded? becomes false? (I didn't verify)

If so, maybe it's actually an ActiveRecord bug?

[jhollinger]

@jamesst20 Thanks, I'll play around with that. But yes, I think it's quite possible it's an AR bug - or at least quirky behavior that "makes sense" to those with a perfect understanding of AR's internals.

[jamesst20]

Hey @jhollinger just checking if you had some time to play with it in the past month? :)

[jhollinger]

Hi @jamesst20, yes I've been looking at this off and on and sadly don't see a good solution. I wouldn't call it a Rails "bug" - rather unexpected behavior when attribute assignment and preloading are both used. I've not been able to find a way to detect this case (an association with unsaved members) without potentially triggering an unwanted query. It wouldn't surprise me if an undocumented way exists, but that would have its own risks.

We're currently working on some significant updates to Blueprinter's internals which should give us some relief here (i.e. preloading wouldn't happen at all in the above example), but there's no ETA on that.

[github-actions]

This issue is stale because it has been open for 30 days with no activity and will be closed in 14 days unless you add a comment.

[jhollinger]

IMHO for the 2.0 verison of the Blueprint renderer and extensions, we need a way to ensure this extension is only called once per "outer" MyBlueprint.render call. "Inner" rendering calls (like rendering an association's Blueprint) shouldn't call the hook this extension uses. This would ensure the extension only runs if the "outer" render call site passes an ActiveRecord::Relation.

[github-actions]

This issue is stale because it has been open for 30 days with no activity and will be closed in 14 days unless you add a comment.

[github-actions]

This issue was closed because it has been stalled for 14 days with no activity.