aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--activerecord/CHANGELOG.md4
-rw-r--r--activerecord/lib/active_record/counter_cache.rb22
-rw-r--r--activerecord/lib/active_record/relation.rb5
-rw-r--r--activerecord/lib/active_record/timestamp.rb22
-rw-r--r--activerecord/test/cases/counter_cache_test.rb12
-rw-r--r--activerecord/test/cases/locking_test.rb24
-rw-r--r--activerecord/test/cases/persistence_test.rb52
-rw-r--r--activerecord/test/models/car.rb2
-rw-r--r--activerecord/test/models/wheel.rb2
-rw-r--r--activerecord/test/schema/schema.rb4
10 files changed, 98 insertions, 51 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 231837dcb7..fad09182c9 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,7 @@
+* Fix `touch` option to behave consistently with `Persistence#touch` method.
+
+ *Ryuta Kamizono*
+
* Migrations raise when duplicate column definition.
Fixes #33024.
diff --git a/activerecord/lib/active_record/counter_cache.rb b/activerecord/lib/active_record/counter_cache.rb
index faaedf1d4a..0d8748d7e6 100644
--- a/activerecord/lib/active_record/counter_cache.rb
+++ b/activerecord/lib/active_record/counter_cache.rb
@@ -47,8 +47,12 @@ module ActiveRecord
reflection = child_class._reflections.values.find { |e| e.belongs_to? && e.foreign_key.to_s == foreign_key && e.options[:counter_cache].present? }
counter_name = reflection.counter_cache_column
- updates = { counter_name.to_sym => object.send(counter_association).count(:all) }
- updates.merge!(touch_updates(touch)) if touch
+ updates = { counter_name => object.send(counter_association).count(:all) }
+
+ if touch
+ names = touch if touch != true
+ updates.merge!(touch_attributes_with_time(*names))
+ end
unscoped.where(primary_key => object.id).update_all(updates)
end
@@ -68,8 +72,8 @@ module ActiveRecord
# * +counters+ - A Hash containing the names of the fields
# to update as keys and the amount to update the field by as values.
# * <tt>:touch</tt> option - Touch timestamp columns when updating.
- # Pass +true+ to touch +updated_at+ and/or +updated_on+. Pass a symbol to
- # touch that column or an array of symbols to touch just those ones.
+ # If attribute names are passed, they are updated along with updated_at/on
+ # attributes.
#
# ==== Examples
#
@@ -107,7 +111,8 @@ module ActiveRecord
end
if touch
- touch_updates = touch_updates(touch)
+ names = touch if touch != true
+ touch_updates = touch_attributes_with_time(*names)
updates << sanitize_sql_for_assignment(touch_updates) unless touch_updates.empty?
end
@@ -171,13 +176,6 @@ module ActiveRecord
def decrement_counter(counter_name, id, touch: nil)
update_counters(id, counter_name => -1, touch: touch)
end
-
- private
- def touch_updates(touch)
- touch = timestamp_attributes_for_update_in_model if touch == true
- touch_time = current_time_from_proper_timezone
- Array(touch).map { |column| [ column, touch_time ] }.to_h
- end
end
private
diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index c055b97061..c911cbe5ec 100644
--- a/activerecord/lib/active_record/relation.rb
+++ b/activerecord/lib/active_record/relation.rb
@@ -393,10 +393,7 @@ module ActiveRecord
# Person.where(name: 'David').touch_all
# # => "UPDATE \"people\" SET \"updated_at\" = '2018-01-04 22:55:23.132670' WHERE \"people\".\"name\" = 'David'"
def touch_all(*names, time: nil)
- attributes = Array(names) + klass.timestamp_attributes_for_update_in_model
- time ||= klass.current_time_from_proper_timezone
- updates = {}
- attributes.each { |column| updates[column] = time }
+ updates = touch_attributes_with_time(*names, time: time)
if klass.locking_enabled?
quoted_locking_column = connection.quote_column_name(klass.locking_column)
diff --git a/activerecord/lib/active_record/timestamp.rb b/activerecord/lib/active_record/timestamp.rb
index e47f06bf3a..d32f971ad1 100644
--- a/activerecord/lib/active_record/timestamp.rb
+++ b/activerecord/lib/active_record/timestamp.rb
@@ -53,12 +53,10 @@ module ActiveRecord
end
module ClassMethods # :nodoc:
- def timestamp_attributes_for_update_in_model
- timestamp_attributes_for_update.select { |c| column_names.include?(c) }
- end
-
- def current_time_from_proper_timezone
- default_timezone == :utc ? Time.now.utc : Time.now
+ def touch_attributes_with_time(*names, time: nil)
+ attribute_names = timestamp_attributes_for_update_in_model
+ attribute_names |= names.map(&:to_s)
+ attribute_names.index_with(time ||= current_time_from_proper_timezone)
end
private
@@ -66,6 +64,10 @@ module ActiveRecord
timestamp_attributes_for_create.select { |c| column_names.include?(c) }
end
+ def timestamp_attributes_for_update_in_model
+ timestamp_attributes_for_update.select { |c| column_names.include?(c) }
+ end
+
def all_timestamp_attributes_in_model
timestamp_attributes_for_create_in_model + timestamp_attributes_for_update_in_model
end
@@ -77,6 +79,10 @@ module ActiveRecord
def timestamp_attributes_for_update
["updated_at", "updated_on"]
end
+
+ def current_time_from_proper_timezone
+ default_timezone == :utc ? Time.now.utc : Time.now
+ end
end
private
@@ -116,7 +122,7 @@ module ActiveRecord
end
def timestamp_attributes_for_update_in_model
- self.class.timestamp_attributes_for_update_in_model
+ self.class.send(:timestamp_attributes_for_update_in_model)
end
def all_timestamp_attributes_in_model
@@ -124,7 +130,7 @@ module ActiveRecord
end
def current_time_from_proper_timezone
- self.class.current_time_from_proper_timezone
+ self.class.send(:current_time_from_proper_timezone)
end
def max_updated_column_timestamp(timestamp_names = timestamp_attributes_for_update_in_model)
diff --git a/activerecord/test/cases/counter_cache_test.rb b/activerecord/test/cases/counter_cache_test.rb
index e0948f90ac..99d286dc52 100644
--- a/activerecord/test/cases/counter_cache_test.rb
+++ b/activerecord/test/cases/counter_cache_test.rb
@@ -280,38 +280,38 @@ class CounterCacheTest < ActiveRecord::TestCase
end
test "update counters with touch: :written_on" do
- assert_touching @topic, :written_on do
+ assert_touching @topic, :updated_at, :written_on do
Topic.update_counters(@topic.id, replies_count: -1, touch: :written_on)
end
end
test "update multiple counters with touch: :written_on" do
- assert_touching @topic, :written_on do
+ assert_touching @topic, :updated_at, :written_on do
Topic.update_counters(@topic.id, replies_count: 2, unique_replies_count: 2, touch: :written_on)
end
end
test "reset counters with touch: :written_on" do
- assert_touching @topic, :written_on do
+ assert_touching @topic, :updated_at, :written_on do
Topic.reset_counters(@topic.id, :replies, touch: :written_on)
end
end
test "reset multiple counters with touch: :written_on" do
- assert_touching @topic, :written_on do
+ assert_touching @topic, :updated_at, :written_on do
Topic.update_counters(@topic.id, replies_count: 1, unique_replies_count: 1)
Topic.reset_counters(@topic.id, :replies, :unique_replies, touch: :written_on)
end
end
test "increment counters with touch: :written_on" do
- assert_touching @topic, :written_on do
+ assert_touching @topic, :updated_at, :written_on do
Topic.increment_counter(:replies_count, @topic.id, touch: :written_on)
end
end
test "decrement counters with touch: :written_on" do
- assert_touching @topic, :written_on do
+ assert_touching @topic, :updated_at, :written_on do
Topic.decrement_counter(:replies_count, @topic.id, touch: :written_on)
end
end
diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb
index 8513edb0ab..33bd74e114 100644
--- a/activerecord/test/cases/locking_test.rb
+++ b/activerecord/test/cases/locking_test.rb
@@ -445,32 +445,38 @@ class OptimisticLockingTest < ActiveRecord::TestCase
assert_equal 0, car.wheels_count
assert_equal 0, car.lock_version
- previously_car_updated_at = car.updated_at
- travel(2.second) do
+ previously_updated_at = car.updated_at
+ previously_wheels_owned_at = car.wheels_owned_at
+ travel(1.second) do
Wheel.create!(wheelable: car)
end
assert_equal 1, car.reload.wheels_count
- assert_not_equal previously_car_updated_at, car.updated_at
assert_equal 1, car.lock_version
+ assert_operator previously_updated_at, :<, car.updated_at
+ assert_operator previously_wheels_owned_at, :<, car.wheels_owned_at
- previously_car_updated_at = car.updated_at
- travel(1.day) do
+ previously_updated_at = car.updated_at
+ previously_wheels_owned_at = car.wheels_owned_at
+ travel(2.second) do
car.wheels.first.update(size: 42)
end
assert_equal 1, car.reload.wheels_count
- assert_not_equal previously_car_updated_at, car.updated_at
assert_equal 2, car.lock_version
+ assert_operator previously_updated_at, :<, car.updated_at
+ assert_operator previously_wheels_owned_at, :<, car.wheels_owned_at
- previously_car_updated_at = car.updated_at
- travel(2.second) do
+ previously_updated_at = car.updated_at
+ previously_wheels_owned_at = car.wheels_owned_at
+ travel(3.second) do
car.wheels.first.destroy!
end
assert_equal 0, car.reload.wheels_count
- assert_not_equal previously_car_updated_at, car.updated_at
assert_equal 3, car.lock_version
+ assert_operator previously_updated_at, :<, car.updated_at
+ assert_operator previously_wheels_owned_at, :<, car.wheels_owned_at
end
def test_polymorphic_destroy_with_dependencies_and_lock_version
diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb
index 4c332e30aa..7348a22dd3 100644
--- a/activerecord/test/cases/persistence_test.rb
+++ b/activerecord/test/cases/persistence_test.rb
@@ -206,12 +206,28 @@ class PersistenceTest < ActiveRecord::TestCase
assert_equal initial_credit + 2, a1.reload.credit_limit
end
- def test_increment_updates_timestamps
+ def test_increment_with_touch_updates_timestamps
topic = topics(:first)
- topic.update_columns(updated_at: 5.minutes.ago)
- previous_updated_at = topic.updated_at
- topic.increment!(:replies_count, touch: true)
- assert_operator previous_updated_at, :<, topic.reload.updated_at
+ assert_equal 1, topic.replies_count
+ previously_updated_at = topic.updated_at
+ travel(1.second) do
+ topic.increment!(:replies_count, touch: true)
+ end
+ assert_equal 2, topic.reload.replies_count
+ assert_operator previously_updated_at, :<, topic.updated_at
+ end
+
+ def test_increment_with_touch_an_attribute_updates_timestamps
+ topic = topics(:first)
+ assert_equal 1, topic.replies_count
+ previously_updated_at = topic.updated_at
+ previously_written_on = topic.written_on
+ travel(1.second) do
+ topic.increment!(:replies_count, touch: :written_on)
+ end
+ assert_equal 2, topic.reload.replies_count
+ assert_operator previously_updated_at, :<, topic.updated_at
+ assert_operator previously_written_on, :<, topic.written_on
end
def test_destroy_all
@@ -333,12 +349,28 @@ class PersistenceTest < ActiveRecord::TestCase
assert_equal 41, accounts(:signals37, :reload).credit_limit
end
- def test_decrement_updates_timestamps
+ def test_decrement_with_touch_updates_timestamps
topic = topics(:first)
- topic.update_columns(updated_at: 5.minutes.ago)
- previous_updated_at = topic.updated_at
- topic.decrement!(:replies_count, touch: true)
- assert_operator previous_updated_at, :<, topic.reload.updated_at
+ assert_equal 1, topic.replies_count
+ previously_updated_at = topic.updated_at
+ travel(1.second) do
+ topic.decrement!(:replies_count, touch: true)
+ end
+ assert_equal 0, topic.reload.replies_count
+ assert_operator previously_updated_at, :<, topic.updated_at
+ end
+
+ def test_decrement_with_touch_an_attribute_updates_timestamps
+ topic = topics(:first)
+ assert_equal 1, topic.replies_count
+ previously_updated_at = topic.updated_at
+ previously_written_on = topic.written_on
+ travel(1.second) do
+ topic.decrement!(:replies_count, touch: :written_on)
+ end
+ assert_equal 0, topic.reload.replies_count
+ assert_operator previously_updated_at, :<, topic.updated_at
+ assert_operator previously_written_on, :<, topic.written_on
end
def test_create
diff --git a/activerecord/test/models/car.rb b/activerecord/test/models/car.rb
index 3d6a7a96c2..8614926626 100644
--- a/activerecord/test/models/car.rb
+++ b/activerecord/test/models/car.rb
@@ -20,6 +20,8 @@ class Car < ActiveRecord::Base
scope :incl_engines, -> { includes(:engines) }
scope :order_using_new_style, -> { order("name asc") }
+
+ attribute :wheels_owned_at, :datetime, default: -> { Time.now }
end
class CoolCar < Car
diff --git a/activerecord/test/models/wheel.rb b/activerecord/test/models/wheel.rb
index 8db57d181e..22fc74995f 100644
--- a/activerecord/test/models/wheel.rb
+++ b/activerecord/test/models/wheel.rb
@@ -1,5 +1,5 @@
# frozen_string_literal: true
class Wheel < ActiveRecord::Base
- belongs_to :wheelable, polymorphic: true, counter_cache: true, touch: true
+ belongs_to :wheelable, polymorphic: true, counter_cache: true, touch: :wheels_owned_at
end
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index 8bf2f93c26..266e55f682 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -36,6 +36,7 @@ ActiveRecord::Schema.define do
create_table :aircraft, force: true do |t|
t.string :name
t.integer :wheels_count, default: 0, null: false
+ t.datetime :wheels_owned_at
end
create_table :articles, force: true do |t|
@@ -126,7 +127,8 @@ ActiveRecord::Schema.define do
create_table :cars, force: true do |t|
t.string :name
t.integer :engines_count
- t.integer :wheels_count, default: 0
+ t.integer :wheels_count, default: 0, null: false
+ t.datetime :wheels_owned_at
t.column :lock_version, :integer, null: false, default: 0
t.timestamps null: false
end
generated by cgit v1.2.3 (git 2.25.1) at 2024-06-26 18:35:41 +0000