Skip to content

Commit

Permalink
Add conditional api
Browse files Browse the repository at this point in the history
Adds a conditional API similar to [ActiveRecord's Callbacks](https://guides.rubyonrails.org/active_record_callbacks.html#conditional-callbacks).

Resolves magnusvk#389.
  • Loading branch information
tatethurston committed Feb 17, 2024
1 parent af30dc4 commit 9097eae
Show file tree
Hide file tree
Showing 6 changed files with 139 additions and 63 deletions.
18 changes: 18 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,24 @@ Now, the ```Category``` model will keep the counter cache in ```special_count```

If you would like to use this with `counter_culture_fix_counts`, make sure to also provide [the `column_names` configuration](#handling-dynamic-column-names).

### Conditional counter cache shorthand

You may also use a conditional API identical to [ActiveRecord's Callbacks](https://guides.rubyonrails.org/active_record_callbacks.html#conditional-callbacks) to conditionally count columns:

```ruby
class Product < ActiveRecord::Base
belongs_to :category
counter_culture :category, column_name: :special_count, if: :special?
end

class Category < ActiveRecord::Base
has_many :products
end

Now, the ```Category``` model will keep the counter cache in ```special_count``` up-to-date. Only products where ```special?``` returns true will affect the special_count.

If you would like to use this with `counter_culture_fix_counts`, make sure to also provide [the `column_names` configuration](#handling-dynamic-column-names).

### Temporarily skipping counter cache updates

If you would like to temporarily pause counter_culture, for example in a backfill script, you can do so as follows:
Expand Down
148 changes: 85 additions & 63 deletions lib/counter_culture/counter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ def initialize(model, relation, options)
@delta_magnitude = options[:delta_magnitude] || 1
@with_papertrail = options.fetch(:with_papertrail, false)
@execute_after_commit = options.fetch(:execute_after_commit, false)
@if_conditions = Array.wrap(options[:if])
@unless_conditions = Array.wrap(options[:unless])

if @execute_after_commit
begin
Expand Down Expand Up @@ -49,78 +51,79 @@ def change_counter_cache(obj, options)
# allow overwriting of foreign key value by the caller
id_to_change = foreign_key_values.call(id_to_change) if foreign_key_values

if id_to_change && change_counter_column
delta_magnitude = if delta_column
(options[:was] ? attribute_was(obj, delta_column) : obj.public_send(delta_column)) || 0
else
counter_delta_magnitude_for(obj)
end
# increment or decrement?
operator = options[:increment] ? '+' : '-'

klass = relation_klass(relation, source: obj, was: options[:was])

# MySQL throws an ambiguous column error if any joins are present and we don't include the
# table name. We isolate this change to MySQL because sqlite has the opposite behavior and
# throws an exception if the table name is present after UPDATE.
quoted_column = if klass.connection.adapter_name == 'Mysql2'
"#{klass.quoted_table_name}.#{model.connection.quote_column_name(change_counter_column)}"
return unless conditions_allow_change?(obj)
return unless id_to_change && change_counter_column

delta_magnitude = if delta_column
(options[:was] ? attribute_was(obj, delta_column) : obj.public_send(delta_column)) || 0
else
"#{model.connection.quote_column_name(change_counter_column)}"
counter_delta_magnitude_for(obj)
end

column_type = klass.type_for_attribute(change_counter_column).type

# we don't use Rails' update_counters because we support changing the timestamp
updates = []
# this updates the actual counter
if column_type == :money
updates << "#{quoted_column} = COALESCE(CAST(#{quoted_column} as NUMERIC), 0) #{operator} #{delta_magnitude}"
else
updates << "#{quoted_column} = COALESCE(#{quoted_column}, 0) #{operator} #{delta_magnitude}"
# increment or decrement?
operator = options[:increment] ? '+' : '-'

klass = relation_klass(relation, source: obj, was: options[:was])

# MySQL throws an ambiguous column error if any joins are present and we don't include the
# table name. We isolate this change to MySQL because sqlite has the opposite behavior and
# throws an exception if the table name is present after UPDATE.
quoted_column = if klass.connection.adapter_name == 'Mysql2'
"#{klass.quoted_table_name}.#{model.connection.quote_column_name(change_counter_column)}"
else
"#{model.connection.quote_column_name(change_counter_column)}"
end

column_type = klass.type_for_attribute(change_counter_column).type

# we don't use Rails' update_counters because we support changing the timestamp
updates = []
# this updates the actual counter
if column_type == :money
updates << "#{quoted_column} = COALESCE(CAST(#{quoted_column} as NUMERIC), 0) #{operator} #{delta_magnitude}"
else
updates << "#{quoted_column} = COALESCE(#{quoted_column}, 0) #{operator} #{delta_magnitude}"
end
# and here we update the timestamp, if so desired
if touch
current_time = klass.send(:current_time_from_proper_timezone)
timestamp_columns = klass.send(:timestamp_attributes_for_update_in_model)
if touch != true
# starting in Rails 6 this is frozen
timestamp_columns = timestamp_columns.dup
timestamp_columns << touch
end
# and here we update the timestamp, if so desired
if touch
current_time = klass.send(:current_time_from_proper_timezone)
timestamp_columns = klass.send(:timestamp_attributes_for_update_in_model)
if touch != true
# starting in Rails 6 this is frozen
timestamp_columns = timestamp_columns.dup
timestamp_columns << touch
end
timestamp_columns.each do |timestamp_column|
updates << "#{timestamp_column} = '#{current_time.to_formatted_s(:db)}'"
end
timestamp_columns.each do |timestamp_column|
updates << "#{timestamp_column} = '#{current_time.to_formatted_s(:db)}'"
end
end

primary_key = relation_primary_key(relation, source: obj, was: options[:was])

if @with_papertrail
instance = klass.where(primary_key => id_to_change).first
if instance
if instance.paper_trail.respond_to?(:save_with_version)
# touch_with_version is deprecated starting in PaperTrail 9.0.0

primary_key = relation_primary_key(relation, source: obj, was: options[:was])

if @with_papertrail
instance = klass.where(primary_key => id_to_change).first
if instance
if instance.paper_trail.respond_to?(:save_with_version)
# touch_with_version is deprecated starting in PaperTrail 9.0.0

current_time = obj.send(:current_time_from_proper_timezone)
timestamp_columns = obj.send(:timestamp_attributes_for_update_in_model)
timestamp_columns.each do |timestamp_column|
instance.send("#{timestamp_column}=", current_time)
end

execute_now_or_after_commit(obj) do
instance.paper_trail.save_with_version(validate: false)
end
else
execute_now_or_after_commit(obj) do
instance.paper_trail.touch_with_version
end
current_time = obj.send(:current_time_from_proper_timezone)
timestamp_columns = obj.send(:timestamp_attributes_for_update_in_model)
timestamp_columns.each do |timestamp_column|
instance.send("#{timestamp_column}=", current_time)
end

execute_now_or_after_commit(obj) do
instance.paper_trail.save_with_version(validate: false)
end
else
execute_now_or_after_commit(obj) do
instance.paper_trail.touch_with_version
end
end
end
end

execute_now_or_after_commit(obj) do
klass.where(primary_key => id_to_change).update_all updates.join(', ')
end
execute_now_or_after_commit(obj) do
klass.where(primary_key => id_to_change).update_all updates.join(', ')
end
end

Expand Down Expand Up @@ -342,5 +345,24 @@ def attribute_was(obj, attr)
end
obj.public_send("#{attr}#{changes_method}")
end

# Returns whether the given conditions allow the counter cache column to update.
# The callback only runs when all the :if conditions and none of the :unless conditions are evaluated to true.
def conditions_allow_change?(obj)
@if_conditions.all? { |condition| evaluate_condition(condition, obj) } &&
@unless_conditions.none? { |condition| evaluate_condition(condition, obj) }
end

# Evaluate the conditions in the context of the object
def evaluate_condition(condition, obj)
case condition
when Symbol
obj.send(condition)
when Proc
obj.instance_exec(&condition)
else
raise ArgumentError, "Condition must be a symbol or a proc"
end
end
end
end
19 changes: 19 additions & 0 deletions spec/counter_culture_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
require 'models/simple_dependent'
require 'models/conditional_main'
require 'models/conditional_dependent'
require 'models/conditional_dependent_shorthand'
require 'models/post'
require 'models/post_comment'
require 'models/post_like'
Expand Down Expand Up @@ -1711,6 +1712,24 @@ def yaml_load(yaml)
ConditionalMain.find_each { |main| expect(main.conditional_dependents_count).to eq(main.id % 2 == 0 ? 3 : 0) }
end

it "should correctly fix the counter caches for thousands of records when counter is conditional using :if/:unless" do
# first, clean up
ConditionalDependentShorthand.delete_all
ConditionalMain.delete_all

MANY.times do |i|
main = ConditionalMain.create
3.times { main.conditional_dependent_shorthands.create(:condition => main.id % 2 == 0) }
end

ConditionalMain.find_each { |main| expect(main.conditional_dependent_shorthands_count).to eq(main.id % 2 == 0 ? 3 : 0) }

ConditionalMain.order(db_random).limit(A_FEW).update_all :conditional_dependent_shorthands_count => 1
ConditionalDependentShorthand.counter_culture_fix_counts :batch_size => A_BATCH

ConditionalMain.find_each { |main| expect(main.conditional_dependent_shorthands_count).to eq(main.id % 2 == 0 ? 3 : 0) }
end

it "should correctly fix the counter caches when no dependent record exists for some of main records" do
# first, clean up
SimpleDependent.delete_all
Expand Down
8 changes: 8 additions & 0 deletions spec/models/conditional_dependent_shorthand.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class ConditionalDependentShorthand < ActiveRecord::Base
belongs_to :conditional_main
scope :condition, -> { where(condition: true) }

counter_culture :conditional_main, if: :condition?, column_names: -> { {
ConditionalDependentShorthand.condition => :conditional_dependent_shorthands_count,
} }
end
1 change: 1 addition & 0 deletions spec/models/conditional_main.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
class ConditionalMain < ActiveRecord::Base
has_many :conditional_dependents
has_many :conditional_dependent_shorthands
end
8 changes: 8 additions & 0 deletions spec/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@

create_table "conditional_mains", :force => true do |t|
t.integer "conditional_dependents_count", :null => false, :default => 0
t.integer "conditional_dependent_shorthands_count", :null => false, :default => 0
t.datetime "created_at"
t.datetime "updated_at"
end
Expand All @@ -150,6 +151,13 @@
t.datetime "updated_at"
end

create_table "conditional_dependent_shorthands", :force => true do |t|
t.integer "conditional_main_id"
t.boolean "condition", default: false
t.datetime "created_at"
t.datetime "updated_at"
end

create_table "categs", :primary_key => "cat_id", :force => true do |t|
t.integer "posts_count", :default => 0, :null => false
t.datetime "created_at"
Expand Down

0 comments on commit 9097eae

Please sign in to comment.