From 7e91242f56835b61c5a32af08ed5cc762063f636 Mon Sep 17 00:00:00 2001 From: Chris Barton Date: Wed, 16 Nov 2022 17:23:54 -0800 Subject: [PATCH] fix: use DateTime instead of Time for time data type In Rails 8.1, the Time datatype will truncate the date component unless the column is excluded from conversion with `skip_time_zone_conversion_for_attributes`. This is not helpful for timestamps in a localized application. The time data type supports datetime, so it should use datetime, not time type. --- acceptance/cases/type/time_test.rb | 36 +++++++++++++++++-- lib/active_record/type/spanner/time.rb | 17 +-------- ...ner_active_record_with_mock_server_test.rb | 6 ++-- 3 files changed, 37 insertions(+), 22 deletions(-) diff --git a/acceptance/cases/type/time_test.rb b/acceptance/cases/type/time_test.rb index 4ed86950..2e278566 100644 --- a/acceptance/cases/type/time_test.rb +++ b/acceptance/cases/type/time_test.rb @@ -44,10 +44,28 @@ def test_set_and_save_time assert_equal expected_time, record.start_time end - def test_date_time_string_value_with_subsecond_precision + def test_date_time_string_value_with_timezone_aware_attributes + TestTypeModel.time_zone_aware_attributes = true + TestTypeModel.reset_column_information + + string_value = "2017-07-04 14:19:10.897761" expected_time = ::Time.local 2017, 07, 4, 14, 19, 10, 897761 + record = TestTypeModel.new start_time: string_value + assert_equal expected_time, record.start_time.utc + + record.save! + assert_equal expected_time, record.start_time.utc + + assert_equal record, TestTypeModel.find_by(start_time: string_value) + ensure + TestTypeModel.time_zone_aware_attributes = false + TestTypeModel.reset_column_information + end + + def test_date_time_string_value_with_subsecond_precision string_value = "2017-07-04 14:19:10.897761" + expected_time = ::Time.local 2017, 07, 4, 14, 19, 10, 897761 record = TestTypeModel.new start_time: string_value assert_equal expected_time, record.start_time.utc @@ -71,7 +89,7 @@ def test_date_time_with_string_value_with_non_iso_format assert_equal record, TestTypeModel.find_by(start_time: string_value) end - def test_default_year_is_correct + def test_multiparameter_time expected_time = ::Time.utc(2000, 1, 1, 10, 30, 0) record = TestTypeModel.new start_time: { 4 => 10, 5 => 30 } @@ -82,6 +100,18 @@ def test_default_year_is_correct assert_equal expected_time, record.start_time end + + def test_multiparameter_datetime + expected_time = ::Time.utc(2020, 12, 25, 10, 30, 0) + record = TestTypeModel.new start_time: { 1 => 2020, 2 => 12, 3 => 25, 4 => 10, 5 => 30 } + + assert_equal expected_time, record.start_time + + record.save! + record.reload + + assert_equal expected_time, record.start_time + end end end -end \ No newline at end of file +end diff --git a/lib/active_record/type/spanner/time.rb b/lib/active_record/type/spanner/time.rb index e47fa97d..b64d7682 100644 --- a/lib/active_record/type/spanner/time.rb +++ b/lib/active_record/type/spanner/time.rb @@ -9,7 +9,7 @@ module ActiveRecord module Type module Spanner - class Time < ActiveRecord::Type::Time + class Time < ActiveRecord::Type::DateTime def serialize_with_isolation_level value, isolation_level if value == :commit_timestamp return "PENDING_COMMIT_TIMESTAMP()" if isolation_level == :dml @@ -23,21 +23,6 @@ def serialize value val = super value val.acts_like?(:time) ? val.utc.rfc3339(9) : val end - - def user_input_in_time_zone value - return value.in_time_zone if value.is_a? ::Time - super value - end - - private - - def cast_value value - if value.is_a? ::String - value = value.empty? ? nil : ::Time.parse(value) - end - - value - end end end end diff --git a/test/activerecord_spanner_mock_server/spanner_active_record_with_mock_server_test.rb b/test/activerecord_spanner_mock_server/spanner_active_record_with_mock_server_test.rb index 897315eb..5a1b19e3 100644 --- a/test/activerecord_spanner_mock_server/spanner_active_record_with_mock_server_test.rb +++ b/test/activerecord_spanner_mock_server/spanner_active_record_with_mock_server_test.rb @@ -670,7 +670,7 @@ def test_create_singer_with_last_performance_as_non_iso_string # could represent both 4th of July and 7th of April. This test verifies that it is encoded to the # same value as ::Time.parse(..) would encode it to. timestamp_string = "04/07/2017 2:19pm" - timestamp = ::Time.parse(timestamp_string) + timestamp = Time.find_zone('UTC').parse(timestamp_string) Singer.transaction do Singer.create(first_name: "Dave", last_name: "Allison", last_performance: timestamp_string) @@ -689,12 +689,12 @@ def test_find_singer_by_last_performance_as_non_iso_string # could represent both 4th of July and 7th of April. This test verifies that it is encoded to the # same value as ::Time.parse(..) would encode it to. timestamp_string = "04/07/2017 2:19pm" - timestamp = ::Time.parse(timestamp_string) + timestamp = Time.find_zone('UTC').parse(timestamp_string) Singer.find_by(last_performance: timestamp_string) request = @mock.requests.select {|req| req.is_a?(Google::Cloud::Spanner::V1::ExecuteSqlRequest) && req.sql == select_sql }.first assert_equal :TIMESTAMP, request.param_types["p1"].code - assert_equal timestamp.utc.rfc3339(9), request.params["p1"] + assert_equal timestamp.rfc3339(9), request.params["p1"] end def test_create_singer_with_picture