From 3853e3cfdc17ae778ea578a6f8ece2f1bca65685 Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Wed, 23 Sep 2015 16:47:17 +1000
Subject: [PATCH] PERF: omit 2 queries on every full page load

---
 app/models/color_scheme.rb       | 22 ++++++++++++++++++++--
 spec/models/color_scheme_spec.rb |  9 +++++++--
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/app/models/color_scheme.rb b/app/models/color_scheme.rb
index d4a1b02dec5..7e431079409 100644
--- a/app/models/color_scheme.rb
+++ b/app/models/color_scheme.rb
@@ -1,7 +1,12 @@
 require_dependency 'sass/discourse_stylesheets'
+require_dependency 'distributed_cache'
 
 class ColorScheme < ActiveRecord::Base
 
+  def self.hex_cache
+    @hex_cache ||= DistributedCache.new("scheme_hex_for_name")
+  end
+
   attr_accessor :is_base
 
   has_many :color_scheme_colors, -> { order('id ASC') }, dependent: :destroy
@@ -12,6 +17,8 @@ class ColorScheme < ActiveRecord::Base
 
   after_destroy :destroy_versions
   after_save :publish_discourse_stylesheet
+  after_save :dump_hex_cache
+  after_destroy :dump_hex_cache
 
   validates_associated :color_scheme_colors
 
@@ -64,8 +71,14 @@ class ColorScheme < ActiveRecord::Base
   end
 
   def self.hex_for_name(name)
-    # Can't use `where` here because base doesn't allow it
-    (enabled || base).colors.find {|c| c.name == name }.try(:hex)
+    val = begin
+      hex_cache[name] ||= begin
+        # Can't use `where` here because base doesn't allow it
+        (enabled || base).colors.find {|c| c.name == name }.try(:hex) || :nil
+      end
+    end
+
+    val == :nil ? nil : val
   end
 
   def colors=(arr)
@@ -101,6 +114,11 @@ class ColorScheme < ActiveRecord::Base
     DiscourseStylesheets.cache.clear
   end
 
+
+  def dump_hex_cache
+    self.class.hex_cache.clear
+  end
+
 end
 
 # == Schema Information
diff --git a/spec/models/color_scheme_spec.rb b/spec/models/color_scheme_spec.rb
index 5ae8fface08..bdc55ee13b0 100644
--- a/spec/models/color_scheme_spec.rb
+++ b/spec/models/color_scheme_spec.rb
@@ -42,6 +42,10 @@ describe ColorScheme do
     end
 
     context "hex_for_name without anything enabled" do
+      before do
+        ColorScheme.hex_cache.clear
+      end
+
       it "returns nil for a missing attribute" do
         expect(described_class.hex_for_name('undefined')).to eq nil
       end
@@ -55,8 +59,8 @@ describe ColorScheme do
   describe "destroy" do
     it "also destroys old versions" do
       c1 = described_class.create(valid_params.merge(version: 2))
-      c2 = described_class.create(valid_params.merge(versioned_id: c1.id, version: 1))
-      other = described_class.create(valid_params)
+      _c2 = described_class.create(valid_params.merge(versioned_id: c1.id, version: 1))
+      _other = described_class.create(valid_params)
       expect {
         c1.destroy
       }.to change { described_class.count }.by(-2)
@@ -69,6 +73,7 @@ describe ColorScheme do
     end
 
     it "returns the enabled color scheme" do
+      ColorScheme.hex_cache.clear
       expect(described_class.hex_for_name('$primary_background_color')).to eq nil
       c = described_class.create(valid_params.merge(enabled: true))
       expect(described_class.enabled.id).to eq c.id