Merge the two methods into one

lib/identity_cache.rb

@@ -60,9 +60,7 @@ class AssociationError < StandardError; end
 
   class InverseAssociationError < StandardError; end
 
-  class NestedDeferredParentBlockError < StandardError; end
-
-  class NestedDeferredAttributeExpirationBlockError < StandardError; end
+  class NestedDeferredCacheExpirationBlockError < StandardError; end
 
   class UnsupportedScopeError < StandardError; end
 
@@ -204,59 +202,45 @@ 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.
+    # 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
+    # that were marked for expiration.
     #
     # == Parameters:
     # No parameters.
     #
     # == Raises:
-    # NestedDeferredParentBlockError if a deferred parent expiration block is already active.
+    # NestedDeferredCacheExpirationBlockError if a deferred cache expiration block is already active.
     #
     # == Yield:
-    # Runs the provided block with deferred parent expiration.
+    # Runs the provided block with deferred cache expiration.
     #
     # == Returns:
     # The result of executing the provided block.
     #
     # == Ensures:
-    # Cleans up thread-local variables related to deferred parent expiration regardless
+    # Cleans up thread-local variables related to deferred cache expiration regardless
     # of whether the block raises an exception.
-    def with_deferred_parent_expiration
-      raise NestedDeferredParentBlockError if Thread.current[:idc_deferred_parent_expiration]
-
-      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
-
-    def with_deferred_attribute_expiration
-      raise NestedDeferredAttributeExpirationBlockError if Thread.current[:identity_cache_deferred_attribute_expiration]
+    def with_deferred_expiration
+      raise NestedDeferredCacheExpirationBlockError if Thread.current[:idc_deferred_expiration]
 
-      Thread.current[:idc_deferred_attribute_expiration] = true
-      Thread.current[:idc_records_to_expire] = Set.new
+      Thread.current[:idc_deferred_expiration] = true
+      Thread.current[:idc_parent_records_to_expire] = Set.new
+      Thread.current[:idc_child_records_to_expire] = Set.new
       Thread.current[:idc_attributes_to_expire] = Set.new
 
       result = yield
 
-      Thread.current[:idc_deferred_attribute_expiration] = nil
-      IdentityCache.cache.delete_multi(Thread.current[:idc_records_to_expire])
-      IdentityCache.cache.delete_multi(Thread.current[:idc_attributes_to_expire])
+      Thread.current[:idc_deferred_expiration] = nil
+      IdentityCache.cache.delete_multi(Thread.current[:idc_parent_records_to_expire]) if Thread.current[:idc_parent_records_to_expire].any?
+      IdentityCache.cache.delete_multi(Thread.current[:idc_child_records_to_expire]) if Thread.current[:idc_child_records_to_expire].any?
+      IdentityCache.cache.delete_multi(Thread.current[:idc_attributes_to_expire]) if Thread.current[:idc_attributes_to_expire].any?
       result
     ensure
-      Thread.current[:idc_deferred_attribute_expiration] = nil
+      Thread.current[:idc_deferred_expiration] = nil
+      Thread.current[:idc_parent_records_to_expire].clear
+      Thread.current[:idc_child_records_to_expire].clear
       Thread.current[:idc_attributes_to_expire].clear
     end
 

lib/identity_cache/cached/attribute.rb

@@ -40,7 +40,7 @@ def expire(record)
         unless record.send(:was_new_record?)
           old_key = old_cache_key(record)
 
-          if Thread.current[:idc_deferred_attribute_expiration]
+          if Thread.current[:idc_deferred_expiration]
             Thread.current[:idc_attributes_to_expire] << old_key
             # defer the deletion, and don't block the following deletion
             all_deleted = true
@@ -51,7 +51,7 @@ def expire(record)
         unless record.destroyed?
           new_key = new_cache_key(record)
           if new_key != old_key
-            if Thread.current[:idc_deferred_attribute_expiration]
+            if Thread.current[:idc_deferred_expiration]
               Thread.current[:idc_attributes_to_expire] << new_key
               all_deleted = true
             else

lib/identity_cache/cached/primary_index.rb

@@ -45,8 +45,8 @@ def fetch_multi(ids)
 
       def expire(id)
         id = cast_id(id)
-        if Thread.current[:idc_deferred_attribute_expiration]
-          Thread.current[:idc_records_to_expire] << cache_key(id)
+        if Thread.current[:idc_deferred_expiration]
+          Thread.current[:idc_child_records_to_expire] << cache_key(id)
         else
           IdentityCache.cache.delete(cache_key(id))
         end

lib/identity_cache/parent_model_expiration.rb

@@ -47,8 +47,8 @@ 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
+        if Thread.current[:idc_deferred_expiration]
+          Thread.current[:idc_parent_records_to_expire] << parent.cache_key
           next parent
         end
         parent.expire_primary_index && all_expired

test/index_cache_test.rb

@@ -166,48 +166,46 @@ 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
+  def test_with_deferred_expiration_for_parent_records_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
-
+    @memcached_spy_write_multi = Spy.on(backend, :write_multi).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
+    result = IdentityCache.with_deferred_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") }
+    all_keys = @memcached_spy_write_multi.calls.flat_map { |call| call.args.first.keys }
+    item_expiration_count = all_keys.count { _1.start_with?("items/") }
+    associated_record_expiration_count = all_keys.count { _1.include?(":AssociatedRecord:") }
 
-    assert_operator(@memcached_spy.calls.count, :>, 0)
+    assert_operator(@memcached_spy_write_multi.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
+  def test_double_nested_deferred_expiration_for_parent_records_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
+    assert_raises(IdentityCache::NestedDeferredCacheExpirationBlockError) do
+      IdentityCache.with_deferred_expiration do
+        IdentityCache.with_deferred_expiration do
           @parent.transaction do
             @parent.associated_records.destroy_all
           end
@@ -218,7 +216,7 @@ def test_double_nested_deferred_parent_expiration_will_raise_error
     assert_equal(0, @memcached_spy.calls.count)
   end
 
-  def test_deep_association_with_deferred_parent_expiration_expires_parent_once
+  def test_deep_association_with_deferred_expiration_expires_parent_once
     AssociatedRecord.send(:has_many, :deeply_associated_records, dependent: :destroy)
     Item.send(:cache_has_many, :associated_records, embed: true)
 
@@ -232,27 +230,27 @@ def test_deep_association_with_deferred_parent_expiration_expires_parent_once
       ])
     end
 
-    @memcached_spy = Spy.on(backend, :write).and_call_through
+    @memcached_spy_write_multi = Spy.on(backend, :write_multi).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
+    IdentityCache.with_deferred_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") }
+    all_keys = @memcached_spy_write_multi.calls.flat_map { |call| call.args.first.keys }
+    item_expiration_count = all_keys.count { |key| key.start_with?("items/") }
+    associated_record_keys = all_keys.count { |key| key.include?(":AssociatedRecord:") }
+    deeply_associated_record_keys = all_keys.count { |key| key.include?(":DeeplyAssociatedRecord:") }
 
-    assert_operator(@memcached_spy.calls.count, :>, 0)
+    assert_operator(@memcached_spy_write_multi.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)
+    assert_equal(expected_associated_record_expiration_count, associated_record_keys)
+    assert_equal(expected_deeply_associated_record_expiration_count, deeply_associated_record_keys)
   end
 
   private

test/save_test.rb

@@ -111,22 +111,12 @@ def test_touch_with_batched_calls
     ])
     expect_cache_deletes([@record1.primary_cache_index_key, @record2.primary_cache_index_key])
 
-    IdentityCache.with_deferred_attribute_expiration do
+    IdentityCache.with_deferred_expiration do
       ActiveRecord::Base.transaction do
         @record1.touch
         @record2.touch
       end
     end
   end
 
-  private
-
-  def expect_cache_delete(key)
-    @backend.expects(:write).with(key, IdentityCache::DELETED, anything)
-  end
-
-  def expect_cache_deletes(keys)
-    key_values = keys.map { |key| [key, IdentityCache::DELETED] }
-    @backend.expects(:write_multi).with(key_values, anything)
-  end
 end

test/test_helper.rb

@@ -9,6 +9,7 @@
 require "helpers/active_record_objects"
 require "helpers/mocked_cache_backend"
 require "spy/integration"
+require "byebug"
 
 require File.dirname(__FILE__) + "/../lib/identity_cache"
 
@@ -128,6 +129,15 @@ def assert_memcache_operations(num, &block)
       ret
     end
 
+    def expect_cache_delete(key)
+      @backend.expects(:write).with(key, IdentityCache::DELETED, anything)
+    end
+
+    def expect_cache_deletes(keys)
+      key_values = keys.map { |key| [key, IdentityCache::DELETED] }.to_h
+      @backend.expects(:write_multi).with(key_values, anything)
+    end
+
     def assert_no_queries(**subscribe_opts, &block)
       subscribe_to_sql_queries(->(sql) { raise "Unexpected SQL query: #{sql}" }, **subscribe_opts, &block)
     end