From 86d2924a115c2604e4facdf285a7b764938fae0f Mon Sep 17 00:00:00 2001 From: Maarten Jacobs Date: Tue, 29 Sep 2015 13:53:39 +0200 Subject: fixes #21815 The default timestamp used for AR is `updated_at` in nanoseconds! (:nsec) This causes issues on any machine that runs an OS that supports nanoseconds timestamps, i.e. not-OS X, where the cache_key of the record persisted in the database (milliseconds precision) is out-of-sync with the cache_key in the ruby VM. This commit adds: A test that shows the issue, it can be found in the separate file `cache_key_test.rb`, because - model couldn't be defined inline - transactional testing needed to be turned off to get it to pass the MySQL tests This seemed cleaner than putting it in an existing testcase file. It adds :usec as a dateformat that calculates datetime in microseconds It sets precision of cache_key to :usec instead of :nsec, as no db supports nsec precision on timestamps --- activerecord/lib/active_record/integration.rb | 4 ++-- activerecord/test/cases/cache_key_test.rb | 25 ++++++++++++++++++++++ .../test/cases/collection_cache_key_test.rb | 2 +- activerecord/test/cases/integration_test.rb | 18 +++++++++++----- .../active_support/core_ext/time/conversions.rb | 1 + activesupport/test/core_ext/time_ext_test.rb | 1 + 6 files changed, 43 insertions(+), 8 deletions(-) create mode 100644 activerecord/test/cases/cache_key_test.rb diff --git a/activerecord/lib/active_record/integration.rb b/activerecord/lib/active_record/integration.rb index 19d2589db3..466c8509a4 100644 --- a/activerecord/lib/active_record/integration.rb +++ b/activerecord/lib/active_record/integration.rb @@ -10,9 +10,9 @@ module ActiveRecord # Indicates the format used to generate the timestamp in the cache key. # Accepts any of the symbols in Time::DATE_FORMATS. # - # This is +:nsec+, by default. + # This is +:usec+, by default. class_attribute :cache_timestamp_format, :instance_writer => false - self.cache_timestamp_format = :nsec + self.cache_timestamp_format = :usec end # Returns a String, which Action Pack uses for constructing a URL to this diff --git a/activerecord/test/cases/cache_key_test.rb b/activerecord/test/cases/cache_key_test.rb new file mode 100644 index 0000000000..bb2829b3c1 --- /dev/null +++ b/activerecord/test/cases/cache_key_test.rb @@ -0,0 +1,25 @@ +require "cases/helper" + +module ActiveRecord + class CacheKeyTest < ActiveRecord::TestCase + self.use_transactional_tests = false + + class CacheMe < ActiveRecord::Base; end + + setup do + @connection = ActiveRecord::Base.connection + @connection.create_table(:cache_mes) { |t| t.timestamps } + end + + teardown do + @connection.drop_table :cache_mes, if_exists: true + end + + test "test_cache_key_format_is_not_too_precise" do + record = CacheMe.create + key = record.cache_key + + assert_equal key, record.reload.cache_key + end + end +end diff --git a/activerecord/test/cases/collection_cache_key_test.rb b/activerecord/test/cases/collection_cache_key_test.rb index 724234d7f4..53058c5a4a 100644 --- a/activerecord/test/cases/collection_cache_key_test.rb +++ b/activerecord/test/cases/collection_cache_key_test.rb @@ -53,7 +53,7 @@ module ActiveRecord test "cache_key with custom timestamp column" do topics = Topic.where("title like ?", "%Topic%") - last_topic_timestamp = topics(:fifth).written_on.utc.to_s(:nsec) + last_topic_timestamp = topics(:fifth).written_on.utc.to_s(:usec) assert_match(last_topic_timestamp, topics.cache_key(:written_on)) end diff --git a/activerecord/test/cases/integration_test.rb b/activerecord/test/cases/integration_test.rb index 9169207b0a..08a186ae07 100644 --- a/activerecord/test/cases/integration_test.rb +++ b/activerecord/test/cases/integration_test.rb @@ -81,7 +81,7 @@ class IntegrationTest < ActiveRecord::TestCase def test_cache_key_format_for_existing_record_with_updated_at dev = Developer.first - assert_equal "developers/#{dev.id}-#{dev.updated_at.utc.to_s(:nsec)}", dev.cache_key + assert_equal "developers/#{dev.id}-#{dev.updated_at.utc.to_s(:usec)}", dev.cache_key end def test_cache_key_format_for_existing_record_with_updated_at_and_custom_cache_timestamp_format @@ -111,19 +111,19 @@ class IntegrationTest < ActiveRecord::TestCase def test_cache_key_for_updated_on dev = Developer.first dev.updated_at = nil - assert_equal "developers/#{dev.id}-#{dev.updated_on.utc.to_s(:nsec)}", dev.cache_key + assert_equal "developers/#{dev.id}-#{dev.updated_on.utc.to_s(:usec)}", dev.cache_key end def test_cache_key_for_newer_updated_at dev = Developer.first dev.updated_at += 3600 - assert_equal "developers/#{dev.id}-#{dev.updated_at.utc.to_s(:nsec)}", dev.cache_key + assert_equal "developers/#{dev.id}-#{dev.updated_at.utc.to_s(:usec)}", dev.cache_key end def test_cache_key_for_newer_updated_on dev = Developer.first dev.updated_on += 3600 - assert_equal "developers/#{dev.id}-#{dev.updated_on.utc.to_s(:nsec)}", dev.cache_key + assert_equal "developers/#{dev.id}-#{dev.updated_on.utc.to_s(:usec)}", dev.cache_key end def test_cache_key_format_is_precise_enough @@ -134,8 +134,16 @@ class IntegrationTest < ActiveRecord::TestCase assert_not_equal key, dev.cache_key end + def test_cache_key_format_is_not_too_precise + skip("Subsecond precision is not supported") unless subsecond_precision_supported? + dev = Developer.first + dev.touch + key = dev.cache_key + assert_equal key, dev.reload.cache_key + end + def test_named_timestamps_for_cache_key owner = owners(:blackbeard) - assert_equal "owners/#{owner.id}-#{owner.happy_at.utc.to_s(:nsec)}", owner.cache_key(:updated_at, :happy_at) + assert_equal "owners/#{owner.id}-#{owner.happy_at.utc.to_s(:usec)}", owner.cache_key(:updated_at, :happy_at) end end diff --git a/activesupport/lib/active_support/core_ext/time/conversions.rb b/activesupport/lib/active_support/core_ext/time/conversions.rb index eecbac2c20..536c4bf525 100644 --- a/activesupport/lib/active_support/core_ext/time/conversions.rb +++ b/activesupport/lib/active_support/core_ext/time/conversions.rb @@ -6,6 +6,7 @@ class Time :db => '%Y-%m-%d %H:%M:%S', :number => '%Y%m%d%H%M%S', :nsec => '%Y%m%d%H%M%S%9N', + :usec => '%Y%m%d%H%M%S%6N', :time => '%H:%M', :short => '%d %b %H:%M', :long => '%B %d, %Y %H:%M', diff --git a/activesupport/test/core_ext/time_ext_test.rb b/activesupport/test/core_ext/time_ext_test.rb index b14c04fba6..2d0fb70a6b 100644 --- a/activesupport/test/core_ext/time_ext_test.rb +++ b/activesupport/test/core_ext/time_ext_test.rb @@ -534,6 +534,7 @@ class TimeExtCalculationsTest < ActiveSupport::TestCase assert_equal "17:44", time.to_s(:time) assert_equal "20050221174430", time.to_s(:number) assert_equal "20050221174430123456789", time.to_s(:nsec) + assert_equal "20050221174430123456", time.to_s(:usec) assert_equal "February 21, 2005 17:44", time.to_s(:long) assert_equal "February 21st, 2005 17:44", time.to_s(:long_ordinal) with_env_tz "UTC" do -- cgit v1.2.3