Skip to content

Commit

Permalink
Handle false in relation strict loading checks
Browse files Browse the repository at this point in the history
Fixes: #41453
Closes: #41461

Previously when a model had strict loading set to true and then had a
relation set strict_loading to false the false wasn't considered when
deciding whether to raise/warn about strict loading.

Code example:

```ruby
class Dog < ActiveRecord::Base
  self.strict_loading_by_default = true

  has_many :treats, strict_loading: false
end
```

In the example, `dog.treats` would still raise even though
`strict_loading` was set to false. This is a bug effecting more than
Active Storage which is why I made this PR superceeding #41461. We need
to fix this for all applications since the behavior is a little
surprising. I took the test from ##41461 and the code suggestion from #41453
with some additions.

Co-authored-by: Radamés Roriz <radamesroriz@gmail.com>
  • Loading branch information
eileencodes and Roriz committed Mar 16, 2021
1 parent 1baa068 commit 226007d
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 1 deletion.
8 changes: 7 additions & 1 deletion activerecord/lib/active_record/associations/association.rb
Expand Up @@ -213,7 +213,7 @@ def create!(attributes = nil, &block)

private
def find_target
if (owner.strict_loading? || reflection.strict_loading?) && owner.validation_context.nil?
if strict_loading? && owner.validation_context.nil?
Base.strict_loading_violation!(owner: owner.class, reflection: reflection)
end

Expand All @@ -229,6 +229,12 @@ def find_target
sc.execute(binds, klass.connection) { |record| set_inverse_instance(record) }
end

def strict_loading?
return reflection.strict_loading? if reflection.options.key?(:strict_loading)

owner.strict_loading?
end

# The scope for this association.
#
# Note that the association_scope is merged into the target_scope only when the
Expand Down
14 changes: 14 additions & 0 deletions activerecord/test/cases/strict_loading_test.rb
Expand Up @@ -238,6 +238,20 @@ def test_raises_on_lazy_loading_a_belongs_to_relation_if_strict_loading_by_defau
end
end

def test_raises_on_lazy_loading_a_belongs_to_relation_if_strict_loading_by_default
with_strict_loading_by_default(Developer) do
mentor = Mentor.create!(name: "Mentor")

developer = Developer.first
developer.update_column(:mentor_id, mentor.id)

assert_nothing_raised do
developer.strict_loading_off_mentor
end
end
end


def test_does_not_raise_on_eager_loading_a_strict_loading_belongs_to_relation
mentor = Mentor.create!(name: "Mentor")

Expand Down
1 change: 1 addition & 0 deletions activerecord/test/models/developer.rb
Expand Up @@ -32,6 +32,7 @@ def find_most_recent

belongs_to :mentor
belongs_to :strict_loading_mentor, strict_loading: true, foreign_key: :mentor_id, class_name: "Mentor"
belongs_to :strict_loading_off_mentor, strict_loading: false, foreign_key: :mentor_id, class_name: "Mentor"

accepts_nested_attributes_for :projects

Expand Down
9 changes: 9 additions & 0 deletions activestorage/test/models/attachment_test.rb
Expand Up @@ -108,6 +108,15 @@ class ActiveStorage::AttachmentTest < ActiveSupport::TestCase
assert_equal blob, ActiveStorage::Blob.find_signed!(signed_id_generated_old_way)
end

test "attaching with strict_loading and getting a signed blob ID from an attachment" do
blob = create_blob
@user.strict_loading!(true)
@user.avatar.attach(blob)

signed_id = @user.avatar.signed_id
assert_equal blob, ActiveStorage::Blob.find_signed(signed_id)
end

private
def assert_blob_identified_before_owner_validated(owner, blob, content_type)
validated_content_type = nil
Expand Down

0 comments on commit 226007d

Please sign in to comment.