Fix that models can clobber each others' attachment reflections

Consider the following model definitions:

    class User < ApplicationRecord
      has_one_attached :avatar
    end

    class Group < ApplicationRecord
      has_one_attached :avatar
    end

If you attempt to reflect on the User model's avatar attachment via User.reflect_on_attachment, you could receive a reflection for the Group model's avatar attachment. Fix this by ensuring that each model class uses its own Hash object to track attachment reflections.
This commit is contained in:
George Claghorn 2018-07-07 17:04:47 -04:00
parent 13c66d4626
commit 03afddd2eb
3 changed files with 25 additions and 16 deletions

View File

@ -19,8 +19,8 @@ module ActiveStorage
end
module ReflectionExtension # :nodoc:
def add_attachment_reflection(ar, name, reflection)
ar.attachment_reflections.merge!(name.to_s => reflection)
def add_attachment_reflection(model, name, reflection)
model.attachment_reflections = model.attachment_reflections.merge(name.to_s => reflection)
end
private

View File

@ -3,27 +3,32 @@
require "test_helper"
class ActiveStorage::ReflectionTest < ActiveSupport::TestCase
test "allows reflecting for all attachment" do
expected_classes =
User.reflect_on_all_attachments.all? do |reflection|
reflection.is_a?(ActiveStorage::Reflection::HasOneAttachedReflection) ||
reflection.is_a?(ActiveStorage::Reflection::HasManyAttachedReflection)
end
assert expected_classes
end
test "allows reflecting on a singular has_one_attached attachment" do
test "reflecting on a singular attachment" do
reflection = User.reflect_on_attachment(:avatar)
assert_equal User, reflection.active_record
assert_equal :avatar, reflection.name
assert_equal :has_one_attached, reflection.macro
assert_equal :purge_later, reflection.options[:dependent]
end
test "allows reflecting on a singular has_many_attached attachment" do
reflection = User.reflect_on_attachment(:highlights)
test "reflection on a singular attachment with the same name as an attachment on another model" do
reflection = Group.reflect_on_attachment(:avatar)
assert_equal Group, reflection.active_record
end
test "reflecting on a collection attachment" do
reflection = User.reflect_on_attachment(:highlights)
assert_equal User, reflection.active_record
assert_equal :highlights, reflection.name
assert_equal :has_many_attached, reflection.macro
assert_equal :purge_later, reflection.options[:dependent]
end
test "reflecting on all attachments" do
reflections = User.reflect_on_all_attachments.sort_by(&:name)
assert_equal [ User ], reflections.collect(&:active_record).uniq
assert_equal %i[ avatar cover_photo highlights vlogs ], reflections.collect(&:name)
assert_equal %i[ has_one_attached has_one_attached has_many_attached has_many_attached ], reflections.collect(&:macro)
assert_equal [ :purge_later, false, :purge_later, false ], reflections.collect { |reflection| reflection.options[:dependent] }
end
end

View File

@ -92,3 +92,7 @@ class User < ActiveRecord::Base
has_many_attached :highlights
has_many_attached :vlogs, dependent: false
end
class Group < ActiveRecord::Base
has_one_attached :avatar
end