diff --git a/.gitignore b/.gitignore index 44e2e656..77900dd1 100644 --- a/.gitignore +++ b/.gitignore @@ -16,3 +16,4 @@ test/version_tmp tmp .rubocop-http* .byebug_history +gemfiles/*.lock diff --git a/CHANGELOG.md b/CHANGELOG.md index bdd12c7c..1b3a3ec5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,9 +2,13 @@ ## Unreleased +## 1.6.3 + +- Split the `with_deferred_parent_expiration` and `with_deferred_parent_expiration`. (#578) + ## 1.6.2 -- Support deferred expiry of associations and attributes. Add a rake task to create test database. +- Support deferred expiry of associations and attributes. Add a rake task to create test database. (#577) ## 1.6.1 diff --git a/Gemfile.lock b/Gemfile.lock index 6b392fac..3bcbd6ca 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -8,7 +8,7 @@ GIT PATH remote: . specs: - identity_cache (1.6.2) + identity_cache (1.6.3) activerecord (>= 7.0) ar_transaction_changes (~> 1.1) diff --git a/Rakefile b/Rakefile index 1742ce5a..202c0bad 100644 --- a/Rakefile +++ b/Rakefile @@ -9,14 +9,82 @@ require "rdoc/task" desc("Default: run tests and style checks.") task(default: [:test, :rubocop]) -desc("Test the identity_cache plugin.") -Rake::TestTask.new(:test) do |t| - t.libs << "lib" - t.libs << "test" - t.pattern = "test/**/*_test.rb" - t.verbose = true +namespace :test do + desc "Test the identity_cache plugin with default Gemfile" + Rake::TestTask.new(:default) do |t| + t.libs << "lib" + t.libs << "test" + t.pattern = "test/**/*_test.rb" + t.verbose = true + end + + desc "Test the identity_cache plugin with minimum supported dependencies" + task :min_supported do + gemfile = File.expand_path("gemfiles/Gemfile.min-supported", __dir__) + ENV["BUNDLE_GEMFILE"] = gemfile + + puts "\nInstalling dependencies for #{gemfile}..." + Bundler.with_unbundled_env do + system("bundle install --gemfile #{gemfile}") || abort("Bundle install failed") + end + + puts "Running tests with #{gemfile}..." + Rake::TestTask.new(:run_min_supported) do |t| + t.libs << "lib" + t.libs << "test" + t.pattern = "test/**/*_test.rb" + t.verbose = true + end + Rake::Task["run_min_supported"].invoke + end + + desc "Test the identity_cache plugin with latest released dependencies" + task :latest_release do + gemfile = File.expand_path("gemfiles/Gemfile.latest-release", __dir__) + ENV["BUNDLE_GEMFILE"] = gemfile + + puts "\nInstalling dependencies for #{gemfile}..." + Bundler.with_unbundled_env do + system("bundle install --gemfile #{gemfile}") || abort("Bundle install failed") + end + + puts "Running tests with #{gemfile}..." + Rake::TestTask.new(:run_latest_release) do |t| + t.libs << "lib" + t.libs << "test" + t.pattern = "test/**/*_test.rb" + t.verbose = true + end + Rake::Task["run_latest_release"].invoke + end + + desc "Test the identity_cache plugin with rails edge dependencies" + task :rails_edge do + gemfile = File.expand_path("gemfiles/Gemfile.rails-edge", __dir__) + ENV["BUNDLE_GEMFILE"] = gemfile + + puts "\nInstalling dependencies for #{gemfile}..." + Bundler.with_unbundled_env do + system("bundle install --gemfile #{gemfile}") || abort("Bundle install failed") + end + + puts "Running tests with #{gemfile}..." + Rake::TestTask.new(:run_rails_edge) do |t| + t.libs << "lib" + t.libs << "test" + t.pattern = "test/**/*_test.rb" + t.verbose = true + end + Rake::Task["run_rails_edge"].invoke + end end +desc "Run default tests" +task test: ["test:default"] + +desc "Run all tests (default, min_supported, latest_release, rails_edge)" +task test_all: ["test:default", "test:min_supported", "test:latest_release", "test:rails_edge"] + task :rubocop do require "rubocop/rake_task" RuboCop::RakeTask.new diff --git a/dev.yml b/dev.yml index 98ec8ab3..a5fd5d5e 100644 --- a/dev.yml +++ b/dev.yml @@ -24,6 +24,10 @@ commands: bundle exec ruby -I test "$@" fi + test-all: + desc: "Run tests for all gemfiles (default, min_supported, latest_release, rails_edge)" + run: bundle exec rake test_all + style: desc: "Run rubocop checks" run: bundle exec rubocop "$@" diff --git a/lib/identity_cache.rb b/lib/identity_cache.rb index 7cd3e142..4f19e003 100644 --- a/lib/identity_cache.rb +++ b/lib/identity_cache.rb @@ -60,6 +60,8 @@ class AssociationError < StandardError; end class InverseAssociationError < StandardError; end + class NestedDeferredParentBlockError < StandardError; end + class NestedDeferredCacheExpirationBlockError < StandardError; end class UnsupportedScopeError < StandardError; end @@ -202,6 +204,48 @@ def fetch_multi(*keys) result end + # Executes a block with deferred parent expiration, ensuring that the parent + # records' cache expiration is deferred until the block completes. When the block + # completes, it triggers expiration of the primary index for the parent records. + # Raises a NestedDeferredParentBlockError if a deferred parent expiration block + # is already active on the current thread. + # + # == Parameters: + # No parameters. + # + # == Raises: + # NestedDeferredParentBlockError if a deferred parent expiration block is already active. + # + # == Yield: + # Runs the provided block with deferred parent expiration. + # + # == Returns: + # The result of executing the provided block. + # + # == Ensures: + # Cleans up thread-local variables related to deferred parent expiration regardless + # of whether the block raises an exception. + def with_deferred_parent_expiration + raise NestedDeferredParentBlockError if Thread.current[:idc_deferred_parent_expiration] + + if Thread.current[:idc_deferred_expiration] + deprecator.deprecation_warning("`with_deferred_parent_expiration`") + end + + Thread.current[:idc_deferred_parent_expiration] = true + Thread.current[:idc_parent_records_for_cache_expiry] = Set.new + + result = yield + + Thread.current[:idc_deferred_parent_expiration] = nil + Thread.current[:idc_parent_records_for_cache_expiry].each(&:expire_primary_index) + + result + ensure + Thread.current[:idc_deferred_parent_expiration] = nil + Thread.current[:idc_parent_records_for_cache_expiry]&.clear + end + # Executes a block with deferred cache expiration, ensuring that the records' (parent, # children and attributes) cache expiration is deferred until the block completes. When # the block completes, it issues delete_multi calls for all the records and attributes @@ -225,6 +269,10 @@ def fetch_multi(*keys) def with_deferred_expiration raise NestedDeferredCacheExpirationBlockError if Thread.current[:idc_deferred_expiration] + if Thread.current[:idc_deferred_parent_expiration] + deprecator.deprecation_warning("`with_deferred_parent_expiration`") + end + Thread.current[:idc_deferred_expiration] = true Thread.current[:idc_records_to_expire] = Set.new Thread.current[:idc_attributes_to_expire] = Set.new @@ -268,6 +316,10 @@ def eager_load! ParentModelExpiration.install_all_pending_parent_expiry_hooks end + def deprecator + @deprecator ||= ActiveSupport::Deprecation.new("1.7.0", "IdentityCache") + end + private def fetch_in_batches(keys, &block) diff --git a/lib/identity_cache/parent_model_expiration.rb b/lib/identity_cache/parent_model_expiration.rb index 1993a21c..499e6d0b 100644 --- a/lib/identity_cache/parent_model_expiration.rb +++ b/lib/identity_cache/parent_model_expiration.rb @@ -47,6 +47,10 @@ def expire_parent_caches add_parents_to_cache_expiry_set(parents_to_expire) parents_to_expire.select! { |parent| parent.class.primary_cache_index_enabled } parents_to_expire.reduce(true) do |all_expired, parent| + if Thread.current[:idc_deferred_parent_expiration] + Thread.current[:idc_parent_records_for_cache_expiry] << parent + next parent + end parent.expire_primary_index && all_expired end end diff --git a/lib/identity_cache/version.rb b/lib/identity_cache/version.rb index f100f710..8634ac34 100644 --- a/lib/identity_cache/version.rb +++ b/lib/identity_cache/version.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true module IdentityCache - VERSION = "1.6.2" + VERSION = "1.6.3" CACHE_VERSION = 8 end diff --git a/test/index_cache_test.rb b/test/index_cache_test.rb index e9ffaa1f..dcf5843c 100644 --- a/test/index_cache_test.rb +++ b/test/index_cache_test.rb @@ -166,6 +166,129 @@ def test_unique_cache_index_with_non_id_primary_key assert_equal(123, KeyedRecord.fetch_by_value("a").id) end + def test_with_deferred_parent_expiration_expires_parent_index_once + Item.send(:cache_has_many, :associated_records, embed: true) + + @parent = Item.create!(title: "bob") + @records = @parent.associated_records.create!([{ name: "foo" }, { name: "bar" }, { name: "baz" }]) + + @memcached_spy = Spy.on(backend, :write).and_call_through + + expected_item_expiration_count = Array(@parent).count + expected_associated_record_expiration_count = @records.count + + expected_return_value = "Some text that we expect to see returned from the block" + + result = IdentityCache.with_deferred_parent_expiration do + @parent.transaction do + @parent.associated_records.destroy_all + end + assert_equal(expected_associated_record_expiration_count, @memcached_spy.calls.count) + expected_return_value + end + + expired_cache_keys = @memcached_spy.calls.map(&:args).map(&:first) + item_expiration_count = expired_cache_keys.count { _1.include?("Item") } + associated_record_expiration_count = expired_cache_keys.count { _1.include?("AssociatedRecord") } + + assert_operator(@memcached_spy.calls.count, :>, 0) + assert_equal(expected_item_expiration_count, item_expiration_count) + assert_equal(expected_associated_record_expiration_count, associated_record_expiration_count) + assert_equal(expected_return_value, result) + end + + def test_double_nested_deferred_parent_expiration_will_raise_error + Item.send(:cache_has_many, :associated_records, embed: true) + + @parent = Item.create!(title: "bob") + @records = @parent.associated_records.create!([{ name: "foo" }, { name: "bar" }, { name: "baz" }]) + + @memcached_spy = Spy.on(backend, :write).and_call_through + + assert_raises(IdentityCache::NestedDeferredParentBlockError) do + IdentityCache.with_deferred_parent_expiration do + IdentityCache.with_deferred_parent_expiration do + @parent.transaction do + @parent.associated_records.destroy_all + end + end + end + end + + assert_equal(0, @memcached_spy.calls.count) + end + + def test_deep_association_with_deferred_parent_expiration_expires_parent_once + AssociatedRecord.send(:has_many, :deeply_associated_records, dependent: :destroy) + Item.send(:cache_has_many, :associated_records, embed: true) + + @parent = Item.create!(title: "bob") + @records = @parent.associated_records.create!([{ name: "foo" }, { name: "bar" }, { name: "baz" }]) + @records.each do + _1.deeply_associated_records.create!([ + { name: "a", item: @parent }, + { name: "b", item: @parent }, + { name: "c", item: @parent }, + ]) + end + + @memcached_spy = Spy.on(backend, :write).and_call_through + + expected_item_expiration_count = Array(@parent).count + expected_associated_record_expiration_count = @records.count + expected_deeply_associated_record_expiration_count = @records.flat_map(&:deeply_associated_records).count + + IdentityCache.with_deferred_parent_expiration do + @parent.transaction do + @parent.associated_records.destroy_all + end + end + + expired_cache_keys = @memcached_spy.calls.map(&:args).map(&:first) + item_expiration_count = expired_cache_keys.count { _1.include?("Item") } + associated_record_expiration_count = expired_cache_keys.count { _1.include?(":AssociatedRecord:") } + deeply_associated_record_expiration_count = expired_cache_keys.count { _1.include?("DeeplyAssociatedRecord") } + + assert_operator(@memcached_spy.calls.count, :>, 0) + assert_equal(expected_item_expiration_count, item_expiration_count) + assert_equal(expected_associated_record_expiration_count, associated_record_expiration_count) + assert_equal(expected_deeply_associated_record_expiration_count, deeply_associated_record_expiration_count) + end + + def test_with_deferred_expiration_and_deferred_parent_expiration_is_compatible + Item.send(:cache_has_many, :associated_records, embed: true) + + @parent = Item.create!(title: "bob") + @records = @parent.associated_records.create!([{ name: "foo" }, { name: "bar" }, { name: "baz" }]) + + @memcached_spy_write_multi = Spy.on(backend, :write_multi).and_call_through + @memcached_spy_write = Spy.on(backend, :write).and_call_through + expected_item_expiration_count = Array(@parent).count + expected_associated_record_expiration_count = @records.count + + expected_return_value = "Some text that we expect to see returned from the block" + + result = + IdentityCache.with_deferred_parent_expiration do + IdentityCache.with_deferred_expiration do + @parent.transaction do + @parent.associated_records.destroy_all + end + expected_return_value + end + end + + all_keys_write = @memcached_spy_write.calls.map(&:args).map(&:first) + all_keys_write_multi = @memcached_spy_write_multi.calls.flat_map { |call| call.args.first.keys } + item_expiration_count = all_keys_write.count { _1.include?(":blob:Item:") } + associated_record_expiration_count = all_keys_write_multi.count { _1.include?(":blob:AssociatedRecord:") } + + assert_equal(1, @memcached_spy_write_multi.calls.count) + assert_equal(expected_item_expiration_count, item_expiration_count) + assert_equal(expected_associated_record_expiration_count, associated_record_expiration_count) + assert_equal(expected_return_value, result) + end + def test_with_deferred_expiration_for_parent_records_expires_parent_index_once Item.send(:cache_has_many, :associated_records, embed: true) @@ -173,6 +296,7 @@ def test_with_deferred_expiration_for_parent_records_expires_parent_index_once @records = @parent.associated_records.create!([{ name: "foo" }, { name: "bar" }, { name: "baz" }]) @memcached_spy_write_multi = Spy.on(backend, :write_multi).and_call_through + @memcached_spy_write = Spy.on(backend, :write).and_call_through expected_item_expiration_count = Array(@parent).count expected_associated_record_expiration_count = @records.count @@ -193,6 +317,7 @@ def test_with_deferred_expiration_for_parent_records_expires_parent_index_once assert_equal(expected_item_expiration_count, item_expiration_count) assert_equal(expected_associated_record_expiration_count, associated_record_expiration_count) assert_equal(expected_return_value, result) + assert(@memcached_spy_write.calls.empty?) end def test_double_nested_deferred_expiration_for_parent_records_will_raise_error