Merge pull request rails#53359 from kamipo/fix_time_zone_aware_custom_attributes

Fix time zone-aware custom attributes not to hit the circuit breaker for infinite recursion

Author: kamipo

activemodel/lib/active_model/type/value.rb

@@ -114,8 +114,8 @@ def force_equality?(_value) # :nodoc:
         false
       end
 
-      def map(value) # :nodoc:
-        yield value
+      def map(value, &) # :nodoc:
+        value
       end
 
       def ==(other)

activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb

@@ -28,7 +28,7 @@ def cast(value)
           elsif value.respond_to?(:infinite?) && value.infinite?
             value
           else
-            map_avoiding_infinite_recursion(super) { |v| cast(v) }
+            map(super) { |v| cast(v) }
           end
         end
 
@@ -45,23 +45,13 @@ def convert_time_to_time_zone(value)
             elsif value.respond_to?(:infinite?) && value.infinite?
               value
             else
-              map_avoiding_infinite_recursion(value) { |v| convert_time_to_time_zone(v) }
+              map(value) { |v| convert_time_to_time_zone(v) }
             end
           end
 
           def set_time_zone_without_conversion(value)
             ::Time.zone.local_to_utc(value).try(:in_time_zone) if value
           end
-
-          def map_avoiding_infinite_recursion(value)
-            map(value) do |v|
-              if value.equal?(v)
-                nil
-              else
-                yield(v)
-              end
-            end
-          end
       end
 
       extend ActiveSupport::Concern

activerecord/lib/active_record/connection_adapters/postgresql/oid/array.rb

@@ -65,7 +65,7 @@ def type_cast_for_schema(value)
           end
 
           def map(value, &block)
-            value.map { |v| subtype.map(v, &block) }
+            value.is_a?(::Array) ? value.map(&block) : subtype.map(value, &block)
           end
 
           def changed_in_place?(raw_old_value, new_value)

activerecord/test/cases/attribute_methods_test.rb

@@ -19,6 +19,18 @@
 class AttributeMethodsTest < ActiveRecord::TestCase
   include InTimeZone
 
+  class EpochTimestamp < ActiveRecord::Type::DateTime
+    def deserialize(time_or_int)
+      Time.at(time_or_int).utc if time_or_int
+    end
+
+    def serialize(time)
+      time.to_i if time
+    end
+  end
+
+  ActiveRecord::Type.register(:epoch_timestamp, EpochTimestamp)
+
   fixtures :topics, :developers, :companies, :computers
 
   def setup
@@ -911,9 +923,67 @@ def topic.approved; false; end
   end
 
   test "time zone-aware attributes do not recurse infinitely on invalid values" do
+    model = new_topic_like_ar_class { }
+
+    type = model.type_for_attribute(:bonus_time)
+    assert_kind_of ActiveRecord::Type::Time, type
+
+    invalid_time = []
+    record = model.new(bonus_time: invalid_time)
+    assert_equal invalid_time, record.bonus_time
+
+    invalid_time = Time.current.utc.to_i
+    record = model.new(bonus_time: invalid_time)
+    assert_equal invalid_time, record.bonus_time
+
     in_time_zone "Pacific Time (US & Canada)" do
-      record = @target.new(bonus_time: [])
-      assert_nil record.bonus_time
+      model = new_topic_like_ar_class { }
+
+      type = model.type_for_attribute(:bonus_time)
+      assert_kind_of ActiveRecord::AttributeMethods::TimeZoneConversion::TimeZoneConverter, type
+
+      invalid_time = []
+      record = model.new(bonus_time: invalid_time)
+      assert_equal invalid_time, record.bonus_time
+
+      invalid_time = Time.current.utc.to_i
+      record = model.new(bonus_time: invalid_time)
+      assert_equal invalid_time, record.bonus_time
+    end
+  end
+
+  test "time zone-aware custom attributes" do
+    timestamp = Time.current.utc.to_i
+
+    model = Class.new(ActiveRecord::Base)
+    model.table_name = "minimalistics"
+
+    model.attribute :expires_at, :epoch_timestamp
+
+    type = model.type_for_attribute(:expires_at)
+    assert_kind_of EpochTimestamp, type
+
+    record_1 = model.create!(expires_at: timestamp)
+    assert_equal timestamp, record_1.expires_at.to_i
+
+    model.insert!({ expires_at: timestamp })
+    record_2 = model.last
+    assert_not_equal record_1, record_2
+    assert_equal timestamp, record_2.expires_at.to_i
+
+    in_time_zone "Pacific Time (US & Canada)" do
+      model.attribute :expires_at, :epoch_timestamp
+
+      type = model.type_for_attribute(:expires_at)
+      assert_kind_of ActiveRecord::AttributeMethods::TimeZoneConversion::TimeZoneConverter, type
+
+      record_1 = model.create!(expires_at: timestamp)
+      assert_equal timestamp, record_1.expires_at.to_i
+
+      model.insert!({ expires_at: timestamp })
+      record_2 = model.last
+      assert_not_equal record_1, record_2
+      assert_equal timestamp, record_2.expires_at.to_i
     end
   end
 
@@ -1433,7 +1503,6 @@ def self.name
     assert_equal "Text", comment.text
   end
 
-
   test "#alias_attribute with a manually defined method raises an error" do
     class_with_aliased_manually_defined_method = Class.new(ActiveRecord::Base) do
       def self.name

activerecord/test/schema/schema.rb

@@ -809,6 +809,7 @@
   end
 
   create_table :minimalistics, force: true do |t|
+    t.bigint :expires_at
   end
 
   create_table :mixed_case_monkeys, force: true, id: false do |t|