Merge pull request #15938 from sgrif/sg-stop-instance-execing

Stop using instance exec for type decorators

3 files changed, 38 insertions, 17 deletions

diff --git a/activerecord/lib/active_record/attribute_decorators.rb b/activerecord/lib/active_record/attribute_decorators.rb
index 5745bbe0e3..5b96623b6e 100644
--- a/activerecord/lib/active_record/attribute_decorators.rb
+++ b/activerecord/lib/active_record/attribute_decorators.rb
@@ -26,7 +26,7 @@ module ActiveRecord
 
       def add_user_provided_columns(*)
         super.map do |column|
-          decorated_type = attribute_type_decorations.apply(self, column.name, column.cast_type)
+          decorated_type = attribute_type_decorations.apply(column.name, column.cast_type)
           column.with_type(decorated_type)
         end
       end
@@ -43,8 +43,8 @@ module ActiveRecord
         TypeDecorator.new(@decorations.merge(*args))
       end
 
-      def apply(context, name, type)
-        decorations = decorators_for(context, name, type)
+      def apply(name, type)
+        decorations = decorators_for(name, type)
         decorations.inject(type) do |new_type, block|
           block.call(new_type)
         end
@@ -52,13 +52,13 @@ module ActiveRecord
 
       private
 
-      def decorators_for(context, name, type)
-        matching(context, name, type).map(&:last)
+      def decorators_for(name, type)
+        matching(name, type).map(&:last)
       end
 
-      def matching(context, name, type)
+      def matching(name, type)
         @decorations.values.select do |(matcher, _)|
-          context.instance_exec(name, type, &matcher)
+          matcher.call(name, type)
         end
       end
     end
diff --git a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb
index 5f1dc8bc9f..f439bd1ffe 100644
--- a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb
+++ b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb
@@ -33,15 +33,25 @@ module ActiveRecord
 
         class_attribute :skip_time_zone_conversion_for_attributes, instance_writer: false
         self.skip_time_zone_conversion_for_attributes = []
-
-        matcher = ->(name, type) { create_time_zone_conversion_attribute?(name, type) }
-        decorate_matching_attribute_types(matcher, :_time_zone_conversion) do |type|
-          TimeZoneConverter.new(type)
-        end
       end
 
       module ClassMethods
         private
+
+        def inherited(subclass)
+          # We need to apply this decorator here, rather than on module inclusion. The closure
+          # created by the matcher would otherwise evaluate for `ActiveRecord::Base`, not the
+          # sub class being decorated. As such, changes to `time_zone_aware_attributes`, or
+          # `skip_time_zone_conversion_for_attributes` would not be picked up.
+          subclass.class_eval do
+            matcher = ->(name, type) { create_time_zone_conversion_attribute?(name, type) }
+            decorate_matching_attribute_types(matcher, :_time_zone_conversion) do |type|
+              TimeZoneConverter.new(type)
+            end
+          end
+          super
+        end
+
         def create_time_zone_conversion_attribute?(name, cast_type)
           time_zone_aware_attributes &&
             !self.skip_time_zone_conversion_for_attributes.include?(name.to_sym) &&
diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb
index f4ec13a177..52eeb8ae1f 100644
--- a/activerecord/lib/active_record/locking/optimistic.rb
+++ b/activerecord/lib/active_record/locking/optimistic.rb
@@ -53,11 +53,6 @@ module ActiveRecord
       included do
         class_attribute :lock_optimistically, instance_writer: false
         self.lock_optimistically = true
-
-        is_lock_column = ->(name, _) { lock_optimistically && name == locking_column }
-        decorate_matching_attribute_types(is_lock_column, :_optimistic_locking) do |type|
-          LockingType.new(type)
-        end
       end
 
       def locking_enabled? #:nodoc:
@@ -167,6 +162,22 @@ module ActiveRecord
           counters = counters.merge(locking_column => 1) if locking_enabled?
           super
         end
+
+        private
+
+        # We need to apply this decorator here, rather than on module inclusion. The closure
+        # created by the matcher would otherwise evaluate for `ActiveRecord::Base`, not the
+        # sub class being decorated. As such, changes to `lock_optimistically`, or
+        # `locking_column` would not be picked up.
+        def inherited(subclass)
+          subclass.class_eval do
+            is_lock_column = ->(name, _) { lock_optimistically && name == locking_column }
+            decorate_matching_attribute_types(is_lock_column, :_optimistic_locking) do |type|
+              LockingType.new(type)
+            end
+          end
+          super
+        end
       end
     end