diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index dc1e0473bf..8ab2d7e7bd 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,21 @@ +* Add support for `strict_loading` mode on association declarations. + + Raise an error if attempting to load a record from an association that has been marked as `strict_loading` unless it was explicitly eager loaded. + + Usage: + + ``` + >> class Developer < ApplicationRecord + >> has_many :projects, strict_loading: true + >> end + >> + >> dev = Developer.first + >> dev.projects.first + => ActiveRecord::StrictLoadingViolationError: The projects association is marked as strict_loading and cannot be lazily loaded. + ``` + + *Kevin Deisz* + * Add support for `strict_loading` mode to prevent lazy loading of records. Raise an error if a parent record is marked as `strict_loading` and attempts to lazily load its associations. This is useful for finding places you may want to preload an association and avoid additional queries. diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 1e1205abb9..7ff3b2b890 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1355,6 +1355,8 @@ module ActiveRecord # Specifies a module or array of modules that will be extended into the association object returned. # Useful for defining methods on associations, especially when they should be shared between multiple # association objects. + # [:strict_loading] + # Enforces strict loading every time the associated record is loaded through this association. # # Option examples: # has_many :comments, -> { order("posted_on") } @@ -1365,6 +1367,7 @@ module ActiveRecord # has_many :tags, as: :taggable # has_many :reports, -> { readonly } # has_many :subscribers, through: :subscriptions, source: :user + # has_many :comments, strict_loading: true def has_many(name, scope = nil, **options, &extension) reflection = Builder::HasMany.build(self, name, scope, options, &extension) Reflection.add_reflection self, name, reflection @@ -1494,6 +1497,8 @@ module ActiveRecord # When set to +true+, the association will also have its presence validated. # This will validate the association itself, not the id. You can use # +:inverse_of+ to avoid an extra query during validation. + # [:strict_loading] + # Enforces strict loading every time the associated record is loaded through this association. # # Option examples: # has_one :credit_card, dependent: :destroy # destroys the associated credit card @@ -1506,6 +1511,7 @@ module ActiveRecord # has_one :club, through: :membership # has_one :primary_address, -> { where(primary: true) }, through: :addressables, source: :addressable # has_one :credit_card, required: true + # has_one :credit_card, strict_loading: true def has_one(name, scope = nil, **options) reflection = Builder::HasOne.build(self, name, scope, options) Reflection.add_reflection self, name, reflection @@ -1639,6 +1645,8 @@ module ActiveRecord # [:default] # Provide a callable (i.e. proc or lambda) to specify that the association should # be initialized with a particular record before validation. + # [:strict_loading] + # Enforces strict loading every time the associated record is loaded through this association. # # Option examples: # belongs_to :firm, foreign_key: "client_of" @@ -1653,6 +1661,7 @@ module ActiveRecord # belongs_to :company, touch: :employees_last_updated_at # belongs_to :user, optional: true # belongs_to :account, default: -> { company.account } + # belongs_to :account, strict_loading: true def belongs_to(name, scope = nil, **options) reflection = Builder::BelongsTo.build(self, name, scope, options) Reflection.add_reflection self, name, reflection @@ -1815,6 +1824,8 @@ module ActiveRecord # # Note that NestedAttributes::ClassMethods#accepts_nested_attributes_for sets # :autosave to true. + # [:strict_loading] + # Enforces strict loading every time an associated record is loaded through this association. # # Option examples: # has_and_belongs_to_many :projects @@ -1822,6 +1833,7 @@ module ActiveRecord # has_and_belongs_to_many :nations, class_name: "Country" # has_and_belongs_to_many :categories, join_table: "prods_cats" # has_and_belongs_to_many :categories, -> { readonly } + # has_and_belongs_to_many :categories, strict_loading: true def has_and_belongs_to_many(name, scope = nil, **options, &extension) habtm_reflection = ActiveRecord::Reflection::HasAndBelongsToManyReflection.new(name, scope, options, self) @@ -1852,7 +1864,7 @@ module ActiveRecord hm_options[:through] = middle_reflection.name hm_options[:source] = join_model.right_reflection.name - [:before_add, :after_add, :before_remove, :after_remove, :autosave, :validate, :join_table, :class_name, :extend].each do |k| + [:before_add, :after_add, :before_remove, :after_remove, :autosave, :validate, :join_table, :class_name, :extend, :strict_loading].each do |k| hm_options[k] = options[k] if options.key? k end diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb index 71d6095845..50d586d290 100644 --- a/activerecord/lib/active_record/associations/association.rb +++ b/activerecord/lib/active_record/associations/association.rb @@ -211,6 +211,10 @@ module ActiveRecord raise StrictLoadingViolationError, "#{owner.class} is marked as strict_loading and #{klass} cannot be lazily loaded." end + if reflection.strict_loading? + raise StrictLoadingViolationError, "The #{reflection.name} association is marked as strict_loading and cannot be lazily loaded." + end + scope = self.scope return scope.to_a if skip_statement_cache?(scope) diff --git a/activerecord/lib/active_record/associations/builder/association.rb b/activerecord/lib/active_record/associations/builder/association.rb index ff6ece8bb7..3e062575be 100644 --- a/activerecord/lib/active_record/associations/builder/association.rb +++ b/activerecord/lib/active_record/associations/builder/association.rb @@ -19,7 +19,7 @@ module ActiveRecord::Associations::Builder # :nodoc: self.extensions = [] VALID_OPTIONS = [ - :class_name, :anonymous_class, :primary_key, :foreign_key, :dependent, :validate, :inverse_of + :class_name, :anonymous_class, :primary_key, :foreign_key, :dependent, :validate, :inverse_of, :strict_loading ].freeze # :nodoc: def self.build(model, name, scope, options, &block) diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 825b201071..902f737c7f 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -309,6 +309,7 @@ module ActiveRecord def find_from_target? loaded? || owner.strict_loading? || + reflection.strict_loading? || owner.new_record? || target.any? { |record| record.new_record? || record.changed? } end diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index e986d8b6f1..d111d00bc9 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -306,6 +306,10 @@ module ActiveRecord active_record_primary_key end + def strict_loading? + options[:strict_loading] + end + protected def actual_source_reflection # FIXME: this is a horrible name self diff --git a/activerecord/test/cases/strict_loading_test.rb b/activerecord/test/cases/strict_loading_test.rb index 87b555e29c..113d16cac6 100644 --- a/activerecord/test/cases/strict_loading_test.rb +++ b/activerecord/test/cases/strict_loading_test.rb @@ -3,9 +3,14 @@ require "cases/helper" require "models/developer" require "models/computer" +require "models/mentor" +require "models/project" +require "models/ship" class StrictLoadingTest < ActiveRecord::TestCase fixtures :developers + fixtures :projects + fixtures :ships def test_strict_loading Developer.all.each { |d| assert_not d.strict_loading? } @@ -67,4 +72,88 @@ class StrictLoadingTest < ActiveRecord::TestCase dev.audit_logs.first end end + + def test_raises_on_lazy_loading_a_strict_loading_belongs_to_relation + mentor = Mentor.create!(name: "Mentor") + + developer = Developer.first + developer.update_column(:mentor_id, mentor.id) + + assert_raises ActiveRecord::StrictLoadingViolationError do + developer.strict_loading_mentor + end + end + + def test_does_not_raise_on_eager_loading_a_strict_loading_belongs_to_relation + mentor = Mentor.create!(name: "Mentor") + + Developer.first.update_column(:mentor_id, mentor.id) + developer = Developer.includes(:strict_loading_mentor).first + + assert_nothing_raised { developer.strict_loading_mentor } + end + + def test_raises_on_lazy_loading_a_strict_loading_has_one_relation + developer = Developer.first + ship = Ship.first + + ship.update_column(:developer_id, developer.id) + + assert_raises ActiveRecord::StrictLoadingViolationError do + developer.strict_loading_ship + end + end + + def test_does_not_raise_on_eager_loading_a_strict_loading_has_one_relation + Ship.first.update_column(:developer_id, Developer.first.id) + developer = Developer.includes(:strict_loading_ship).first + + assert_nothing_raised { developer.strict_loading_ship } + end + + def test_raises_on_lazy_loading_a_strict_loading_has_many_relation + developer = Developer.first + + AuditLog.create( + 3.times.map do + { developer_id: developer.id, message: "I am message" } + end + ) + + assert_raises ActiveRecord::StrictLoadingViolationError do + developer.strict_loading_opt_audit_logs.first + end + end + + def test_does_not_raise_on_eager_loading_a_strict_loading_has_many_relation + developer = Developer.first + + AuditLog.create( + 3.times.map do + { developer_id: developer.id, message: "I am message" } + end + ) + + developer = Developer.includes(:strict_loading_opt_audit_logs).first + + assert_nothing_raised { developer.strict_loading_opt_audit_logs.first } + end + + def test_raises_on_lazy_loading_a_strict_loading_habtm_relation + developer = Developer.first + developer.projects << Project.first + + assert_not developer.strict_loading_projects.loaded? + + assert_raises ActiveRecord::StrictLoadingViolationError do + developer.strict_loading_projects.first + end + end + + def test_does_not_raise_on_eager_loading_a_strict_loading_habtm_relation + Developer.first.projects << Project.first + developer = Developer.includes(:strict_loading_projects).first + + assert_nothing_raised { developer.strict_loading_projects.first } + end end diff --git a/activerecord/test/models/developer.rb b/activerecord/test/models/developer.rb index 0bc2de078e..81fa3ad713 100644 --- a/activerecord/test/models/developer.rb +++ b/activerecord/test/models/developer.rb @@ -18,6 +18,7 @@ class Developer < ActiveRecord::Base end belongs_to :mentor + belongs_to :strict_loading_mentor, strict_loading: true, foreign_key: :mentor_id, class_name: "Mentor" accepts_nested_attributes_for :projects @@ -45,6 +46,12 @@ class Developer < ActiveRecord::Base end end + has_and_belongs_to_many :strict_loading_projects, + join_table: :developers_projects, + association_foreign_key: :project_id, + class_name: "Project", + strict_loading: true + has_and_belongs_to_many :special_projects, join_table: "developers_projects", association_foreign_key: "project_id" has_and_belongs_to_many :sym_special_projects, join_table: :developers_projects, @@ -53,11 +60,14 @@ class Developer < ActiveRecord::Base has_many :audit_logs has_many :strict_loading_audit_logs, -> { strict_loading }, class_name: "AuditLog" + has_many :strict_loading_opt_audit_logs, strict_loading: true, class_name: "AuditLog" has_many :contracts has_many :firms, through: :contracts, source: :firm has_many :comments, ->(developer) { where(body: "I'm #{developer.name}") } has_many :ratings, through: :comments + has_one :ship, dependent: :nullify + has_one :strict_loading_ship, strict_loading: true, class_name: "Ship" belongs_to :firm has_many :contracted_projects, class_name: "Project"