From e6f99ebb2aa168aa8fa36392382ea89a51c02db9 Mon Sep 17 00:00:00 2001
From: Yogesh Khater <yogeshjain999@gmail.com>
Date: Sun, 13 Jun 2021 23:49:02 +0530
Subject: [PATCH] Use cell's name instead of `controller_path` in cache key
 generation

---
 lib/cell/caching.rb |  7 ++++-
 test/cache_test.rb  | 73 +++++++++++++++++++++++++--------------------
 2 files changed, 47 insertions(+), 33 deletions(-)

diff --git a/lib/cell/caching.rb b/lib/cell/caching.rb
index 10d97d8d..f939d516 100644
--- a/lib/cell/caching.rb
+++ b/lib/cell/caching.rb
@@ -5,6 +5,7 @@ module Caching
     def self.included(includer)
       includer.class_eval do
         extend ClassMethods
+        extend Util
         extend Uber::InheritableAttr
         inheritable_attr :version_procs
         inheritable_attr :conditional_procs
@@ -25,7 +26,7 @@ def cache(state, *args, **kwargs, &block)
 
       # Computes the complete, namespaced cache key for +state+.
       def state_cache_key(state, key_parts={})
-        expand_cache_key([controller_path, state, key_parts])
+        expand_cache_key([formatted_name, state, key_parts])
       end
 
       def expire_cache_key_for(key, cache_store, *args)
@@ -37,6 +38,10 @@ def expire_cache_key_for(key, cache_store, *args)
       def expand_cache_key(key)
         key.join("/")
       end
+
+      def formatted_name
+        util.underscore(name)
+      end
     end
 
     def render_state(state, *args)
diff --git a/test/cache_test.rb b/test/cache_test.rb
index 5e671a3c..2ada4df2 100644
--- a/test/cache_test.rb
+++ b/test/cache_test.rb
@@ -3,17 +3,18 @@
 class CacheTest < Minitest::Spec
   STORE = Class.new(Hash) do
     def fetch(key, options, &block)
-      self[key] || self[key] = yield
+      value = self[key] || self[key] = yield
+      [value, options]
     end
   end.new
 
-  module Cache
+  module InstanceMethods
     def show(*)
       "#{@model}"
     end
 
-    def cache_store
-      STORE
+    def new(*)
+      "#{@model}"
     end
 
     def has_changed?(*)
@@ -21,18 +22,22 @@ def has_changed?(*)
     end
   end
 
-  Component = ->(*args, **kwargs, &block) {
+  Component = ->(*args, store: STORE, **kwargs, &block) {
     Class.new(Cell::ViewModel) do
       cache :show, *args, **kwargs, &block
-      include Cache
+      cache :new
+
+      define_method(:cache_store) { store }
+
+      include InstanceMethods
     end
   }
 
   it "without any options" do
     WithoutOptions = Component.()
 
-    _(WithoutOptions.new(1).()).must_equal("1")
-    _(WithoutOptions.new(2).()).must_equal("1")
+    _(WithoutOptions.new(1).()).must_equal(%{["1", {}]})
+    _(WithoutOptions.new(2).()).must_equal(%{["1", {}]})
   end
 
   it "with specified version" do
@@ -41,48 +46,52 @@ def has_changed?(*)
     # Cache invalidation using version as a proc
     WithVersionArg = Component.(version)
 
-    _(WithVersionArg.new(1).(:show, version: 1)).must_equal("1")
-    _(WithVersionArg.new(2).(:show, version: 1)).must_equal("1")
+    _(WithVersionArg.new(1).(:show, version: 1)).must_equal(%{["1", {}]})
+    _(WithVersionArg.new(2).(:show, version: 1)).must_equal(%{["1", {}]})
 
-    _(WithVersionArg.new(3).(:show, version: 2)).must_equal("3")
+    _(WithVersionArg.new(3).(:show, version: 2)).must_equal(%{["3", {}]})
 
     # Cache invalidation using version as a block
     WithVersionBlock = Component.(&version)
 
-    _(WithVersionBlock.new(1).(:show, version: 1)).must_equal("1")
-    _(WithVersionBlock.new(2).(:show, version: 1)).must_equal("1")
+    _(WithVersionBlock.new(1).(:show, version: 1)).must_equal(%{["1", {}]})
+    _(WithVersionBlock.new(2).(:show, version: 1)).must_equal(%{["1", {}]})
 
-    _(WithVersionBlock.new(3).(:show, version: 2)).must_equal("3")
+    _(WithVersionBlock.new(3).(:show, version: 2)).must_equal(%{["3", {}]})
   end
 
   it "with conditional" do
     WithConditional = Component.(if: :has_changed?)
 
-    _(WithConditional.new(1).()).must_equal("1")
-    _(WithConditional.new(2).()).must_equal("1")
+    _(WithConditional.new(1).()).must_equal(%{["1", {}]})
+    _(WithConditional.new(2).()).must_equal(%{["1", {}]})
 
     _(WithConditional.new(3).()).must_equal("3")
   end
 
-  it "forwards remaining options to cache store" do
-    WithOptions = Class.new(Cell::ViewModel) do
-      cache :show, if: :has_changed?, expires_in: 10, tags: ->(*args) { Hash(args.first)[:tags] }
-      include Cache
-
-      CACHE_WITH_OPTIONS_STORE = Class.new(Hash) do
-        def fetch(key, options)
-          value = self[key] || self[key] = yield
-          [value, options]
-        end
-      end.new
-
-      def cache_store
-        CACHE_WITH_OPTIONS_STORE
-      end
-    end
+  it "forwards additional options to the store" do
+    WithOptions = Component.(if: :has_changed?, expires_in: 10, tags: ->(*args) { Hash(args.first)[:tags] })
 
     _(WithOptions.new(1).()).must_equal(%{["1", {:expires_in=>10, :tags=>nil}]})
     _(WithOptions.new(2).()).must_equal(%{["1", {:expires_in=>10, :tags=>nil}]})
+
     _(WithOptions.new(2).(:show, tags: [:a, :b])).must_equal(%{["1", {:expires_in=>10, :tags=>[:a, :b]}]})
   end
+
+  it "generates different cache key per cell and per action" do
+    store = Class.new(STORE.class).new
+
+    CellOne = Component.(store: store, expires_in: 10)
+    CellTwo = Component.(store: store, expires_in: 10)
+
+    CellOne.new(1).(:new)
+    CellOne.new(2).()
+    CellTwo.new(3).()
+
+    _(store).must_equal({
+      "cache_test/cell_one/new/"=>"1",
+      "cache_test/cell_one/show/"=>"2",
+      "cache_test/cell_two/show/"=>"3"
+    })
+  end
 end