ensure MultiCache has a TTL, even if falling back

to regular cache Change-Id: I69dd9738027ae83252cce526baa1293c3ce3792f Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/262260 Tested-by: Service Cloud Jenkins <[email protected]> Reviewed-by: Rob Orton <[email protected]> QA-Review: Cody Cutrer <[email protected]> Product-Review: Cody Cutrer <[email protected]>
rthbound · Apr 5, 2021 · 69b0387 · 69b0387
1 parent 5dd1df2
commit 69b0387
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 7 deletions.
diff --git a/lib/multi_cache.rb b/lib/multi_cache.rb
@@ -24,15 +24,19 @@ class << self
 
     def cache
       unless defined?(@multi_cache)
-        ha_cache_config = YAML.load(Canvas::DynamicSettings.find(tree: :private, cluster: Canvas.cluster)["ha_cache.yml"] || "{}").symbolize_keys || {}
-        if (ha_cache_config[:cache_store])
+        ha_cache_config = YAML.safe_load(Canvas::DynamicSettings.find(tree: :private, cluster: Canvas.cluster)["ha_cache.yml"] || "{}").symbolize_keys || {}
+        @multi_cache = if ha_cache_config[:cache_store]
           ha_cache_config[:url] = ha_cache_config[:servers] if ha_cache_config[:servers]
-          store = ActiveSupport::Cache.lookup_store(ha_cache_config[:cache_store].to_sym, ha_cache_config)
-          store.options.delete(:namespace)
-          @multi_cache = store
+          ActiveSupport::Cache.lookup_store(ha_cache_config[:cache_store].to_sym, ha_cache_config)
+        else
+          config = Canvas.cache_store_config_for(Rails.env).dup || {}
+          # MultiCache has to have an expiration
+          config[:expires_in] ||= 300
+          Canvas.lookup_cache_store(config, Rails.env)
         end
+        @multi_cache.options.delete(:namespace) # remove the namespace that switchman added; MultiCache is global
       end
-      @multi_cache || Rails.cache
+      @multi_cache
     end
 
     def reset

diff --git a/spec/lib/canvas/vault_spec.rb b/spec/lib/canvas/vault_spec.rb
@@ -159,6 +159,7 @@ module Canvas
             redis_port: 6379,
             redis_db: 6 # intentionally one probably not used elsewhere
           })
+          allow(ConfigFile).to receive(:load).and_call_original
           @lock_stub = stub_request(:get, "#{addr}/v1/#{credential_path}").
             to_return(status: 200, body: {
             data: credential_data,

diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
@@ -586,7 +586,7 @@ def set_cache(new_cache)
     allow_any_instance_of(ActionController::Base).to receive(:cache_store).and_return(new_cache)
     allow(ActionController::Base).to receive(:perform_caching).and_return(true)
     allow_any_instance_of(ActionController::Base).to receive(:perform_caching).and_return(true)
-    MultiCache.reset
+    allow(MultiCache).to receive(:cache).and_return(new_cache)
   end
 
   def specs_require_cache(new_cache=:memory_store)
@@ -598,6 +598,7 @@ def specs_require_cache(new_cache=:memory_store)
   def enable_cache(new_cache=:memory_store)
     previous_cache = Rails.cache
     previous_perform_caching = ActionController::Base.perform_caching
+    previous_multicache = MultiCache.cache
     set_cache(new_cache)
     if block_given?
       begin
@@ -608,6 +609,7 @@ def enable_cache(new_cache=:memory_store)
         allow_any_instance_of(ActionController::Base).to receive(:cache_store).and_return(previous_cache)
         allow(ActionController::Base).to receive(:perform_caching).and_return(previous_perform_caching)
         allow_any_instance_of(ActionController::Base).to receive(:perform_caching).and_return(previous_perform_caching)
+        allow(MultiCache).to receive(:cache).and_return(previous_multicache)
       end
     end
   end