Time columns should support time zone aware attributes

The types that are affected by `time_zone_aware_attributes` (which is on
by default) have been made configurable, in case this is a breaking
change for existing applications.

8 files changed, 116 insertions, 8 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 8875b7ae25..c777af342f 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,17 @@
+*   `time` columns can now affected by `time_zone_aware_attributes`. If you have
+    set `config.time_zone` to a value other than `'UTC'`, they will be treated
+    as in that time zone by default in Rails 5.0. If this is not the desired
+    behavior, you can set
+
+        ActiveRecord::Base.time_zone_aware_types = [:datetime]
+
+    A deprecation warning will be emitted if you have a `:time` column, and have
+    not explicitly opted out.
+
+    Fixes #3145
+
+    *Sean Griffin*
+
 *   `nil` as a value for a binary column in a query no longer logs as
     "<NULL binary data>", and instead logs as just "nil".
 
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 777f7ab4d7..e10024b2ec 100644
--- a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb
+++ b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb
@@ -13,7 +13,7 @@ module ActiveRecord
             value.map { |v| type_cast_from_user(v) }
           elsif value.respond_to?(:in_time_zone)
             begin
-              value.in_time_zone || super
+              user_input_in_time_zone(value) || super
             rescue ArgumentError
               nil
             end
@@ -39,6 +39,9 @@ module ActiveRecord
 
         class_attribute :skip_time_zone_conversion_for_attributes, instance_writer: false
         self.skip_time_zone_conversion_for_attributes = []
+
+        class_attribute :time_zone_aware_types, instance_writer: false
+        self.time_zone_aware_types = [:datetime, :not_explicitly_configured]
       end
 
       module ClassMethods
@@ -59,9 +62,31 @@ module ActiveRecord
         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) &&
-            (:datetime == cast_type.type)
+          enabled_for_column = time_zone_aware_attributes &&
+            !self.skip_time_zone_conversion_for_attributes.include?(name.to_sym)
+          result = enabled_for_column &&
+            time_zone_aware_types.include?(cast_type.type)
+
+          if enabled_for_column &&
+            !result &&
+            cast_type.type == :time &&
+            time_zone_aware_types.include?(:not_explicitly_configured)
+            ActiveSupport::Deprecation.warn(<<-MESSAGE)
+              Time columns will become time zone aware in Rails 5.1. This
+              sill cause `String`s to be parsed as if they were in `Time.zone`,
+              and `Time`s to be converted to `Time.zone`.
+
+              To keep the old behavior, you must add the following to your initializer:
+
+                  config.active_record.time_zone_aware_types = [:datetime]
+
+              To silence this deprecation warning, add the following:
+
+                  config.active_record.time_zone_aware_types << :time
+            MESSAGE
+          end
+
+          result
         end
       end
     end
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid/array.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid/array.rb
index c203e6c604..e45a2f59d9 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/oid/array.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid/array.rb
@@ -18,7 +18,7 @@ module ActiveRecord
           end
 
           attr_reader :subtype, :delimiter
-          delegate :type, to: :subtype
+          delegate :type, :user_input_in_time_zone, to: :subtype
 
           def initialize(subtype, delimiter = ',')
             @subtype = subtype
diff --git a/activerecord/lib/active_record/type/time.rb b/activerecord/lib/active_record/type/time.rb
index 41f7d97f0c..cab1c7bf1e 100644
--- a/activerecord/lib/active_record/type/time.rb
+++ b/activerecord/lib/active_record/type/time.rb
@@ -7,6 +7,19 @@ module ActiveRecord
         :time
       end
 
+      def user_input_in_time_zone(value)
+        return unless value.present?
+
+        case value
+        when ::String
+          value = "2000-01-01 #{value}"
+        when ::Time
+          value = value.change(year: 2000, day: 1, month: 1)
+        end
+
+        super(value)
+      end
+
       private
 
       def cast_value(value)
diff --git a/activerecord/lib/active_record/type/time_value.rb b/activerecord/lib/active_record/type/time_value.rb
index d611d72dd4..8d9ac25643 100644
--- a/activerecord/lib/active_record/type/time_value.rb
+++ b/activerecord/lib/active_record/type/time_value.rb
@@ -9,6 +9,10 @@ module ActiveRecord
         "'#{value.to_s(:db)}'"
       end
 
+      def user_input_in_time_zone(value)
+        value.in_time_zone
+      end
+
       private
 
       def new_time(year, mon, mday, hour, min, sec, microsec, offset = nil)
diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb
index 359503db98..4036294121 100644
--- a/activerecord/test/cases/attribute_methods_test.rb
+++ b/activerecord/test/cases/attribute_methods_test.rb
@@ -657,7 +657,7 @@ class AttributeMethodsTest < ActiveRecord::TestCase
     end
   end
 
-  def test_setting_time_zone_aware_attribute_in_current_time_zone
+  def test_setting_time_zone_aware_datetime_in_current_time_zone
     utc_time = Time.utc(2008, 1, 1)
     in_time_zone "Pacific Time (US & Canada)" do
       record   = @target.new
@@ -676,6 +676,47 @@ class AttributeMethodsTest < ActiveRecord::TestCase
     end
   end
 
+  def test_setting_time_zone_aware_time_in_current_time_zone
+    in_time_zone "Pacific Time (US & Canada)" do
+      record = @target.new
+      time_string = "10:00:00"
+      expected_time = Time.zone.parse("2000-01-01 #{time_string}")
+
+      record.bonus_time = time_string
+      assert_equal expected_time, record.bonus_time
+      assert_equal ActiveSupport::TimeZone["Pacific Time (US & Canada)"], record.bonus_time.time_zone
+
+      record.bonus_time = ''
+      assert_nil record.bonus_time
+    end
+  end
+
+  def test_setting_time_zone_aware_time_with_dst
+    in_time_zone "Pacific Time (US & Canada)" do
+      current_time = Time.zone.local(2014, 06, 15, 10)
+      record = @target.new(bonus_time: current_time)
+      time_before_save = record.bonus_time
+
+      record.save
+      record.reload
+
+      assert_equal time_before_save, record.bonus_time
+      assert_equal ActiveSupport::TimeZone["Pacific Time (US & Canada)"], record.bonus_time.time_zone
+    end
+  end
+
+  def test_removing_time_zone_aware_types
+    with_time_zone_aware_types(:datetime) do
+      in_time_zone "Pacific Time (US & Canada)" do
+        record = @target.new(bonus_time: "10:00:00")
+        expected_time = Time.utc(2000, 01, 01, 10)
+
+        assert_equal expected_time, record.bonus_time
+        assert record.bonus_time.utc?
+      end
+    end
+  end
+
   def test_setting_time_zone_conversion_for_attributes_should_write_value_on_class_variable
     Topic.skip_time_zone_conversion_for_attributes = [:field_a]
     Minimalistic.skip_time_zone_conversion_for_attributes = [:field_b]
@@ -908,6 +949,14 @@ class AttributeMethodsTest < ActiveRecord::TestCase
     klass
   end
 
+  def with_time_zone_aware_types(*types)
+    old_types = ActiveRecord::Base.time_zone_aware_types
+    ActiveRecord::Base.time_zone_aware_types = types
+    yield
+  ensure
+    ActiveRecord::Base.time_zone_aware_types = old_types
+  end
+
   def cached_columns
     Topic.columns.map(&:name)
   end
diff --git a/activerecord/test/cases/helper.rb b/activerecord/test/cases/helper.rb
index 925491acbd..0a577fa2f5 100644
--- a/activerecord/test/cases/helper.rb
+++ b/activerecord/test/cases/helper.rb
@@ -30,6 +30,9 @@ ARTest.connect
 # Quote "type" if it's a reserved word for the current connection.
 QUOTED_TYPE = ActiveRecord::Base.connection.quote_column_name('type')
 
+# FIXME: Remove this when the deprecation cycle on TZ aware types by default ends.
+ActiveRecord::Base.time_zone_aware_types << :time
+
 def current_adapter?(*types)
   types.any? do |type|
     ActiveRecord::ConnectionAdapters.const_defined?(type) &&
diff --git a/activerecord/test/cases/multiparameter_attributes_test.rb b/activerecord/test/cases/multiparameter_attributes_test.rb
index 14d4ef457d..4aaf6f8b5f 100644
--- a/activerecord/test/cases/multiparameter_attributes_test.rb
+++ b/activerecord/test/cases/multiparameter_attributes_test.rb
@@ -250,8 +250,8 @@ class MultiParameterAttributeTest < ActiveRecord::TestCase
         }
         topic = Topic.find(1)
         topic.attributes = attributes
-        assert_equal Time.utc(2000, 1, 1, 16, 24, 0), topic.bonus_time
-        assert topic.bonus_time.utc?
+        assert_equal Time.zone.local(2000, 1, 1, 16, 24, 0), topic.bonus_time
+        assert_not topic.bonus_time.utc?
       end
     end
   end