Merge pull request rails#46602 from hmcguire-shopify/fix-lazy-readonly-writer

Fix undefined attribute method with attr_readonly

Author: rafaelfranca

activerecord/lib/active_record/associations/collection_association.rb

@@ -324,7 +324,7 @@ def merge_target_lists(persisted, memory)
           persisted.map! do |record|
             if mem_record = memory.delete(record)
 
-              ((record.attribute_names & mem_record.attribute_names) - mem_record.changed_attribute_names_to_save).each do |name|
+              ((record.attribute_names & mem_record.attribute_names) - mem_record.changed_attribute_names_to_save - mem_record.class._attr_readonly).each do |name|
                 mem_record._write_attribute(name, record[name])
               end
 

activerecord/lib/active_record/readonly_attributes.rb

@@ -28,21 +28,11 @@ module ClassMethods
       #   post.title = "a different title" # raises ActiveRecord::ReadonlyAttributeError
       #   post.update(title: "a different title") # raises ActiveRecord::ReadonlyAttributeError
       def attr_readonly(*attributes)
-        new_attributes = attributes.map(&:to_s).reject { |a| _attr_readonly.include?(a) }
+        self._attr_readonly |= attributes.map(&:to_s)
 
         if ActiveRecord.raise_on_assign_to_attr_readonly
-          new_attributes.each do |attribute|
-            define_method("#{attribute}=") do |value|
-              raise ReadonlyAttributeError.new(attribute) unless new_record?
-
-              super(value)
-            end
-          end
-
           include(HasReadonlyAttributes)
         end
-
-        self._attr_readonly = Set.new(new_attributes) + _attr_readonly
       end
 
       # Returns an array of all the attributes that have been specified as readonly.
@@ -56,12 +46,14 @@ def readonly_attribute?(name) # :nodoc:
     end
 
     module HasReadonlyAttributes # :nodoc:
-      def write_attribute(attr_name, value)
-        if !new_record? && self.class.readonly_attribute?(attr_name.to_s)
-          raise ReadonlyAttributeError.new(attr_name)
-        end
+      [:write_attribute, :_write_attribute].each do |name|
+        define_method(name) do |attr_name, value|
+          if !new_record? && self.class.readonly_attribute?(attr_name.to_s)
+            raise ReadonlyAttributeError.new(attr_name)
+          end
 
-        super
+          super(attr_name, value)
+        end
       end
     end
   end

activerecord/test/cases/base_test.rb

@@ -66,6 +66,16 @@ class ReadonlyTitlePost < Post
   attr_readonly :title
 end
 
+class ReadonlyTitleAbstractPost < ActiveRecord::Base
+  self.abstract_class = true
+
+  attr_readonly :title
+end
+
+class ReadonlyTitlePostWithAbstractParent < ReadonlyTitleAbstractPost
+  self.table_name = "posts"
+end
+
 previous_value, ActiveRecord.raise_on_assign_to_attr_readonly = ActiveRecord.raise_on_assign_to_attr_readonly, false
 
 class NonRaisingPost < Post
@@ -699,7 +709,7 @@ def test_comparison_with_different_objects_in_array
   end
 
   def test_readonly_attributes
-    assert_equal Set.new([ "title" ]), ReadonlyTitlePost.readonly_attributes
+    assert_equal [ "title" ], ReadonlyTitlePost.readonly_attributes
 
     post = ReadonlyTitlePost.create(title: "cannot change this", body: "changeable")
     assert_equal "cannot change this", post.title
@@ -750,7 +760,7 @@ def test_readonly_attributes
   end
 
   def test_readonly_attributes_on_a_new_record
-    assert_equal Set.new([ "title" ]), ReadonlyTitlePost.readonly_attributes
+    assert_equal [ "title" ], ReadonlyTitlePost.readonly_attributes
 
     post = ReadonlyTitlePost.new(title: "can change this until you save", body: "changeable")
     assert_equal "can change this until you save", post.title
@@ -766,7 +776,6 @@ def test_readonly_attributes_on_a_new_record
     assert_equal "changed via write_attribute", post.title
     assert_equal "changed via write_attribute", post.body
 
-
     post.assign_attributes(body: "changed via assign_attributes", title: "changed via assign_attributes")
     assert_equal "changed via assign_attributes", post.title
     assert_equal "changed via assign_attributes", post.body
@@ -783,8 +792,16 @@ def test_readonly_attributes_on_a_new_record
     assert_equal "changed via []=", post.body
   end
 
+  def test_readonly_attributes_in_abstract_class_descendant
+    assert_equal [ "title" ], ReadonlyTitlePostWithAbstractParent.readonly_attributes
+
+    assert_nothing_raised do
+      ReadonlyTitlePostWithAbstractParent.new(title: "can change this until you save")
+    end
+  end
+
   def test_readonly_attributes_when_configured_to_not_raise
-    assert_equal Set.new([ "title" ]), NonRaisingPost.readonly_attributes
+    assert_equal [ "title" ], NonRaisingPost.readonly_attributes
 
     post = NonRaisingPost.create(title: "cannot change this", body: "changeable")
     assert_equal "cannot change this", post.title

activerecord/test/cases/locking_test.rb

@@ -455,7 +455,7 @@ def test_lock_with_custom_column_without_default_queries_count
   end
 
   def test_readonly_attributes
-    assert_equal Set.new([ "name" ]), ReadonlyNameShip.readonly_attributes
+    assert_equal [ "name" ], ReadonlyNameShip.readonly_attributes
 
     s = ReadonlyNameShip.create(name: "unchangeable name")
     s.reload