ar_lazy_preload
ar_lazy_preload copied to clipboard
Unexpected behavior with accepts_nested_attributes_for
Hi @DmitryTsepelev,
Could you please help with this case?
require 'bundler/inline'
gemfile(true) do
source 'https://rubygems.org'
git_source(:github) { |repo| "https://github.com/#{repo}.git" }
gem 'activerecord', '= 7.0.6'
require "active_record"
gem 'ar_lazy_preload'
gem 'sqlite3'
end
require "ar_lazy_preload"
ArLazyPreload.install_hooks
require 'minitest/autorun'
require 'logger'
ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')
ActiveRecord::Base.logger = Logger.new($stdout)
ActiveRecord::Schema.define do
create_table :users, force: true do |t|
end
create_table :invitations, force: true do |t|
t.integer :user_id
t.integer :company_id
end
create_table :companies, force: true do |t|
end
end
class User < ActiveRecord::Base
has_many :invitations, dependent: :destroy
has_many :companies, through: :invitations
accepts_nested_attributes_for :invitations, allow_destroy: true
end
class Invitation < ActiveRecord::Base
belongs_to :company
belongs_to :user
validate :something
def something
# do something
# if we comment out this but keep ArLazyPreload on the test passes.
user.invitations.each(&:itself)
end
end
class Company < ActiveRecord::Base
has_many :invitations, dependent: :destroy
has_many :users, through: :invitations
end
ArLazyPreload.config.auto_preload = true
class BugTest < ActiveSupport::TestCase
def test_nested_preloading
user = User.create
company = Company.create
# This test is failing but if we comment out ArLazyPreload.config.auto_preload = true it will pass as expected
assert_difference "Invitation.count" do
user.update(invitations_attributes: [{company_id: company.id}])
# => true regardless of ArLazyPreload mode
# It is funny that if we do update twice (with ArLazyPreload on), the test passes by creating only single record
# user.update(invitations_attributes: [{company_id: company.id}])
end
end
end
P.S. I apologize for the messy code above, but it should show the problem.
I can confirm this is a real issue.
This just bit us off-guard too. We've added a lot of lazy_preload calls across our multiple projects and this has started getting reported from multiple users.
I've run into the same issue and it appears the problem is due to the fact that the association.target is being used when detecting records in the nested attributes:
`def assign_nested_attributes_for_collection_association(association_name, attributes_collection) options = nested_attributes_options[association_name] if attributes_collection.respond_to?(:permitted?) attributes_collection = attributes_collection.to_h end
unless attributes_collection.is_a?(Hash) || attributes_collection.is_a?(Array)
raise ArgumentError, "Hash or Array expected for attribute #{association_name}, got #{attributes_collection.class.name} (#{attributes_collection.inspect})"
end
check_record_limit!(options[:limit], attributes_collection)
if attributes_collection.is_a? Hash
keys = attributes_collection.keys
attributes_collection = if keys.include?("id") || keys.include?(:id)
[attributes_collection]
else
attributes_collection.values
end
end
association = association(association_name)
existing_records = if association.loaded?
association.target
else
attribute_ids = attributes_collection.filter_map { |a| a["id"] || a[:id] }
attribute_ids.empty? ? [] : association.scope.where(association.klass.primary_key => attribute_ids)
end
attributes_collection.each do |attributes|
if attributes.respond_to?(:permitted?)
attributes = attributes.to_h
end
attributes = attributes.with_indifferent_access
if attributes["id"].blank?
unless reject_new_record?(association_name, attributes)
association.reader.build(attributes.except(*UNASSIGNABLE_KEYS))
end
elsif existing_record = existing_records.detect { |record| record.id.to_s == attributes["id"].to_s }
unless call_reject_if(association_name, attributes)
# Make sure we are operating on the actual object which is in the association's
# proxy_target array (either by finding it, or adding it if not found)
# Take into account that the proxy_target may have changed due to callbacks
target_record = association.target.detect { |record| record.id.to_s == attributes["id"].to_s }
if target_record
existing_record = target_record
else
association.add_to_target(existing_record, skip_callbacks: true)
end
assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy])
end
else
raise_nested_attributes_record_not_found!(association_name, attributes["id"])
end
end
end`
The issue is here:
target_record = association.target.detect { |record| record.id.to_s == attributes["id"].to_s } if target_record existing_record = target_record else association.add_to_target(existing_record, skip_callbacks: true) end
where the association target is being uses to find if there is an existing record to update. I don't exactly know how to fix it as load_target is overriden by the lazy preloaded and in this case it doesn't detect that it needs to load that association.
IMHO this is makes the auto preloading unusable if you use nested attributes.
ar_lazy_preload-2.1.0/lib/ar_lazy_preload/active_record/association.rb
# frozen_string_literal: true
module ArLazyPreload
# ActiveRecord::Association patch with a hook for lazy preloading
module Association
def load_target
owner.try_preload_lazily(reflection.name)
super
end
end
end
was there any specific reason that we call owner.try_preload_lazily(reflection.name)
first and then super
? Shouldn't we first let active record to set the target and then we go for preloading?
I tried below and it did solved the problem of accepts_nested_attributes_for
# frozen_string_literal: true
module ArLazyPreload
# ActiveRecord::Association patch with a hook for lazy preloading
module Association
def load_target
records = super
owner.try_preload_lazily(reflection.name)
records
end
end
end