BUGFIX: redis-rails has always been a problem child

implemented an ActiveSupport::Cache::Store for our internal use.
* allows for expire by family
* works correctly in multisite
* namespaced correctly

Removed redis-rails from the project, no longer needed

Gemfile

@@ -65,7 +65,7 @@ else
   gem 'active_attr'
 end
 
-gem 'redis-rails'
+#gem 'redis-rails'
 gem 'hiredis'
 gem 'redis', :require => ["redis", "redis/connection/hiredis"]
 

Gemfile_rails4.lock

@@ -268,24 +268,8 @@ GEM
       trollop (>= 1.16.2)
     redcarpet (3.0.0)
     redis (3.0.6)
-    redis-actionpack (4.0.0)
-      actionpack (~> 4)
-      redis-rack (~> 1.5.0)
-      redis-store (~> 1.1.0)
-    redis-activesupport (4.0.0)
-      activesupport (~> 4)
-      redis-store (~> 1.1.0)
     redis-namespace (1.3.2)
       redis (~> 3.0.4)
-    redis-rack (1.5.0)
-      rack (~> 1.5)
-      redis-store (~> 1.1.0)
-    redis-rails (4.0.0)
-      redis-actionpack (~> 4)
-      redis-activesupport (~> 4)
-      redis-store (~> 1.1.0)
-    redis-store (1.1.4)
-      redis (>= 2.2)
     ref (1.0.5)
     rest-client (1.6.7)
       mime-types (>= 1.16)
@@ -466,7 +450,6 @@ DEPENDENCIES
   rbtrace
   redcarpet
   redis
-  redis-rails
   rest-client
   rinku
   rspec-given

config/application.rb

@@ -1,6 +1,5 @@
 require File.expand_path('../boot', __FILE__)
 require 'rails/all'
-require 'redis-store' # HACK
 
 # Plugin related stuff
 require_relative '../lib/discourse_plugin_registry'

lib/cache.rb

@@ -1,55 +1,72 @@
-# Standard Rails.cache is lacking support for this interface, possibly yank all in from redis:cache and start using this instead
+# Discourse specific cache supports expire by family missing from standard cache
-#
 
-class Cache
+class Cache < ActiveSupport::Cache::Store
-  def initialize(redis=nil)
+
-    @redis = redis
+  def initialize(opts = {})
+    opts[:namespace] ||= "_CACHE_"
+    super(opts)
   end
 
   def redis
-    @redis || $redis
+    $redis
-  end
-
-  def fetch(key, options={})
-    result = redis.get key
-    if result.nil?
-      if expiry = options[:expires_in]
-        if block_given?
-          result = yield
-          redis.setex(key, expiry, result)
-        end
-      else
-        if block_given?
-          result = yield
-          redis.set(key, result)
-        end
-      end
-    end
-
-    if family = family_key(options[:family])
-      redis.sadd(family, key)
-    end
-
-    result
-  end
-
-  def delete(key)
-    redis.del(key)
   end
 
   def delete_by_family(key)
-    k = family_key(key)
+    k = family_key(key, options)
     redis.smembers(k).each do |member|
-      delete(member)
+      redis.del(member)
     end
     redis.del(k)
   end
 
-  private
+  def reconnect
+    redis.reconnect
+  end
 
-  def family_key(name)
+  def clear
-    if name
+    redis.keys.each do |k|
-      "FAMILY_#{name}"
+      redis.del(k) if k =~ /^_CACHE_:/
     end
   end
+
+  def namespaced_key(key, opts=nil)
+    opts ||= options
+    super(key,opts)
+  end
+
+  protected
+
+  def read_entry(key, options)
+    if data = redis.get(key)
+      ActiveSupport::Cache::Entry.new data
+    end
+  end
+
+  def write_entry(key, entry, options)
+    if expiry = options[:expires_in]
+      redis.setex(key, expiry, entry.value)
+    else
+      redis.set(key, entry.value)
+    end
+
+    if family = family_key(options[:family], options)
+      redis.sadd(family, key)
+    end
+
+    true
+  end
+
+  def delete_entry(key, options)
+    redis.del key
+  end
+
+  private
+
+  def family_key(name, options)
+    if name
+      key = namespaced_key(name, options)
+      key << "FAMILY:#{name}"
+    end
+  end
+
 end

lib/discourse_redis.rb

@@ -1,6 +1,7 @@
 #
 #  A wrapper around redis that namespaces keys with the current site id
 #
+require_dependency 'cache'
 class DiscourseRedis
 
   def self.raw_connection(config = nil)
@@ -43,7 +44,7 @@ class DiscourseRedis
   end
 
   # Proxy key methods through, but prefix the keys with the namespace
-  [:append, :blpop, :brpop, :brpoplpush, :decr, :decrby, :del, :exists, :expire, :expireat, :get, :getbit, :getrange, :getset,
+  [:append, :blpop, :brpop, :brpoplpush, :decr, :decrby, :exists, :expire, :expireat, :get, :getbit, :getrange, :getset,
    :hdel, :hexists, :hget, :hgetall, :hincrby, :hincrbyfloat, :hkeys, :hlen, :hmget, :hmset, :hset, :hsetnx, :hvals, :incr,
    :incrby, :incrbyfloat, :lindex, :linsert, :llen, :lpop, :lpush, :lpushx, :lrange, :lrem, :lset, :ltrim,
    :mapped_hmset, :mapped_hmget, :mapped_mget, :mapped_mset, :mapped_msetnx, :mget, :move, :mset,
@@ -57,20 +58,32 @@ class DiscourseRedis
     end
   end
 
+  def del(k)
+    k = "#{DiscourseRedis.namespace}:#{k}"
+    @redis.del k
+  end
+
+  def keys
+    len = DiscourseRedis.namespace.length + 1
+    @redis.keys("#{DiscourseRedis.namespace}:*").map{
+      |k| k[len..-1]
+    }
+  end
+
+  def flushdb
+    keys.each{|k| del(k)}
+  end
+
+  def reconnect
+    @redis.client.reconnect
+  end
+
   def self.namespace
     RailsMultisite::ConnectionManagement.current_db
   end
 
   def self.new_redis_store
-    redis_config = YAML.load(ERB.new(File.new("#{Rails.root}/config/redis.yml").read).result)[Rails.env]
+    Cache.new
-    unless redis_config
-      puts '', "Redis config for environment '#{Rails.env}' was not found in #{Rails.root}/config/redis.yml."
-      puts "Did you forget to do RAILS_ENV=production?"
-      puts "Check your redis.yml and make sure it has configuration for the environment you're trying to use.", ''
-      raise 'Redis config not found'
-    end
-    ActiveSupport::Cache::RedisStore.new host:redis_config['host'], port:redis_config['port'], password:redis_config['password'], db:redis_config['db'], namespace:->{ DiscourseRedis.namespace + "_cache" }
   end
 
-
 end

lib/pretty_text.rb

@@ -59,7 +59,7 @@ module PrettyText
               "vendor/assets/javascripts/rsvp.js",
               Rails.configuration.ember.handlebars_location)
 
-    ctx.eval("var Discourse = {}; Discourse.SiteSettings = #{SiteSetting.client_settings_json};")
+    ctx.eval("var Discourse = {}; Discourse.SiteSettings = {};")
     ctx.eval("var window = {}; window.devicePixelRatio = 2;") # hack to make code think stuff is retina
     ctx.eval("var I18n = {}; I18n.t = function(a,b){ return helpers.t(a,b); }");
 

spec/components/cache_spec.rb

@@ -7,23 +7,27 @@ describe Cache do
     Cache.new
   end
 
-  it "can delete by family" do
+  it "can be cleared" do
-    cache.fetch("key2", family: "my_family") do
+    cache.write("hello0", "world")
-      "test"
+    cache.write("hello1", "world")
-    end
+    cache.clear
 
-    cache.fetch("key", expires_in: 1.minute, family: "my_family") do
+    cache.read("hello0").should be_nil
-      "test"
+  end
-    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")
+
     cache.fetch("key").should be_nil
     cache.fetch("key2").should be_nil
 
   end
 
   it "can delete correctly" do
-    r = cache.fetch("key", expires_in: 1.minute) do
+    cache.fetch("key", expires_in: 1.minute) do
       "test"
     end
 
@@ -32,8 +36,9 @@ describe Cache do
   end
 
   it "can store with expiry correctly" do
-    $redis.expects(:get).with("key").returns nil
+    key = cache.namespaced_key("key")
-    $redis.expects(:setex).with("key", 60 , "bob")
+    $redis.expects(:get).with(key).returns nil
+    $redis.expects(:setex).with(key, 60 , "bob")
 
     r = cache.fetch("key", expires_in: 1.minute) do
       "bob"
@@ -42,8 +47,9 @@ describe Cache do
   end
 
   it "can store and fetch correctly" do
-    $redis.expects(:get).with("key").returns nil
+    key = cache.namespaced_key("key")
-    $redis.expects(:set).with("key", "bob")
+    $redis.expects(:get).with(key).returns nil
+    $redis.expects(:set).with(key, "bob")
 
     r = cache.fetch "key" do
       "bob"
@@ -52,8 +58,9 @@ describe Cache do
   end
 
   it "can fetch existing correctly" do
+    key = cache.namespaced_key("key")
 
-    $redis.expects(:get).with("key").returns "bill"
+    $redis.expects(:get).with(key).returns "bill"
 
     r = cache.fetch "key" do
       "bob"

spec/components/redis_store_spec.rb

@@ -4,7 +4,7 @@ require 'cache'
 describe "Redis Store" do
 
   let :cache do
-    Cache.new
+    Cache.new(namespace: 'foo')
   end
 
   let :store do
@@ -27,16 +27,17 @@ describe "Redis Store" do
   end
 
   it "doesn't collide with our Cache" do
+
     store.fetch "key" do
       "key in store"
     end
-   
+
     cache.fetch "key" do
       "key in cache"
     end
- 
+
     r = store.read "key"
-    
+
     r.should == "key in store"
   end
 
@@ -52,6 +53,7 @@ describe "Redis Store" do
     store.clear
     store.read("key").should be_nil
     cache.fetch("key").should == "key in cache"
+
   end
 
 end