FIX: emoji cache could get corrupt

FEATURE: enforce 1 day expiry by default on discourse cache

remove family expiry concept as the implementation was fragile

app/models/emoji.rb

@@ -20,15 +20,15 @@ class Emoji
   end
 
   def self.all
-    Discourse.cache.fetch("all", family: "emoji") { standard | custom }
+    Discourse.cache.fetch("all_emojis") { standard | custom }
   end
 
   def self.standard
-    Discourse.cache.fetch("standard", family: "emoji") { load_standard }
+    Discourse.cache.fetch("standard_emojis") { load_standard }
   end
 
   def self.custom
-    Discourse.cache.fetch("custom", family: "emoji") { load_custom }
+    Discourse.cache.fetch("custom_emojis") { load_custom }
   end
 
   def self.exists?(name)
@@ -72,7 +72,9 @@ class Emoji
   end
 
   def self.clear_cache
-    Discourse.cache.delete_by_family("emoji")
+    Discourse.cache.delete("custom_emojis")
+    Discourse.cache.delete("standard_emojis")
+    Discourse.cache.delete("all_emojis")
   end
 
   def self.db_file

lib/cache.rb

@@ -1,9 +1,15 @@
-# Discourse specific cache supports expire by family missing from standard cache
+# Discourse specific cache, enforces 1 day expiry
 
 class Cache < ActiveSupport::Cache::Store
 
+  # nothing is cached for longer than 1 day EVER
+  # there is no reason to have data older than this clogging redis
+  # it is dangerous cause if we rename keys we will be stuck with
+  # pointless data
+  MAX_CACHE_AGE = 1.day unless defined? MAX_CACHE_AGE
+
   def initialize(opts = {})
-    opts[:namespace] ||= "_CACHE_"
+    @namespace = opts[:namespace] || "_CACHE_"
     super(opts)
   end
 
@@ -11,27 +17,18 @@ class Cache < ActiveSupport::Cache::Store
     $redis
   end
 
-  def delete_by_family(key)
-    k = family_key(key, options)
-    redis.smembers(k).each do |member|
-      redis.del(member)
-    end
-    redis.del(k)
-  end
-
   def reconnect
     redis.reconnect
   end
 
   def clear
-    redis.keys.each do |k|
-      redis.del(k) if k =~ /^_CACHE_:/
+    redis.keys("#{@namespace}:*").each do |k|
+      redis.del(k)
     end
   end
 
   def namespaced_key(key, opts=nil)
-    opts ||= options
-    super(key,opts)
+    "#{@namespace}:" << key
   end
 
   protected
@@ -47,17 +44,8 @@ class Cache < ActiveSupport::Cache::Store
 
   def write_entry(key, entry, options)
     dumped = Marshal.dump(entry.value)
-
-    if expiry = options[:expires_in]
-      redis.setex(key, expiry, dumped)
-    else
-      redis.set(key, dumped)
-    end
-
-    if family = family_key(options[:family], options)
-      redis.sadd(family, key)
-    end
-
+    expiry = options[:expires_in] || MAX_CACHE_AGE
+    redis.setex(key, expiry, dumped)
     true
   end
 
@@ -65,13 +53,4 @@ class Cache < ActiveSupport::Cache::Store
     redis.del key
   end
 
-  private
-
-  def family_key(name, options)
-    if name
-      key = namespaced_key(name, options)
-      key << "FAMILY:#{name}"
-    end
-  end
-
 end

spec/components/cache_spec.rb

@@ -19,24 +19,15 @@ describe Cache do
   end
 
   it "can be cleared" do
+    $redis.set("boo", "boo")
     cache.write("hello0", "world")
     cache.write("hello1", "world")
     cache.clear
 
+    expect($redis.get("boo")).to eq("boo")
     expect(cache.read("hello0")).to eq(nil)
   end
 
-  it "can delete by family" do
-    cache.write("key2", "test", family: "my_family")
-    cache.write("key", "test", expires_in: 1.minute, family: "my_family")
-
-    cache.delete_by_family("my_family")
-
-    expect(cache.fetch("key")).to eq(nil)
-    expect(cache.fetch("key2")).to eq(nil)
-
-  end
-
   it "can delete correctly" do
     cache.fetch("key", expires_in: 1.minute) do
       "test"
@@ -46,16 +37,23 @@ describe Cache do
     expect(cache.fetch("key")).to eq(nil)
   end
 
-  #TODO yuck on this mock
   it "calls setex in redis" do
     cache.delete("key")
+    cache.delete("bla")
 
     key = cache.namespaced_key("key")
-    $redis.expects(:setex).with(key, 60 , Marshal.dump("bob"))
 
     cache.fetch("key", expires_in: 1.minute) do
       "bob"
     end
+
+    expect($redis.ttl(key)).to be_within(2.seconds).of(1.minute)
+
+    # we always expire withing a day
+    cache.fetch("bla"){ "hi" }
+
+    key = cache.namespaced_key("bla")
+    expect($redis.ttl(key)).to be_within(2.seconds).of(1.day)
   end
 
   it "can store and fetch correctly" do