FIX: Invalidate cache when updating color scheme colors (#10417)

app/controllers/stylesheets_controller.rb

@@ -29,6 +29,10 @@ class StylesheetsController < ApplicationController
       # TODO add theme
       # calling this method ensures we have a cache for said target
       # we hold of re-compilation till someone asks for asset
+      if target.include?("color_definitions")
+        split_target, color_scheme_id = target.split(/_(-?[0-9]+)/)
+        Stylesheet::Manager.color_scheme_stylesheet_link_tag(color_scheme_id)
+      else
         if target.include?("theme")
           split_target, theme_id = target.split(/_(-?[0-9]+)/)
           theme = Theme.find_by(id: theme_id) if theme_id.present?
@@ -38,6 +42,7 @@ class StylesheetsController < ApplicationController
         end
         Stylesheet::Manager.stylesheet_link_tag(split_target, nil, theme&.id)
       end
+    end
 
     cache_time = request.env["HTTP_IF_MODIFIED_SINCE"]
 

app/models/color_scheme.rb

@@ -271,6 +271,8 @@ class ColorScheme < ActiveRecord::Base
 
   def publish_discourse_stylesheet
     if self.id
+      Stylesheet::Manager.color_scheme_cache_clear(self)
+
       theme_ids = Theme.where(color_scheme_id: self.id).pluck(:id)
       if theme_ids.present?
         Stylesheet::Manager.cache.clear

lib/stylesheet/manager.rb

@@ -106,19 +106,18 @@ class Stylesheet::Manager
 
     target = COLOR_SCHEME_STYLESHEET.to_sym
     current_hostname = Discourse.current_hostname
-    color_scheme_name = Slug.for(color_scheme.name) + color_scheme&.id.to_s
+    cache_key = color_scheme_cache_key(color_scheme)
-    array_cache_key = "color_scheme_stylesheet_#{color_scheme_name}_#{current_hostname}"
+    stylesheets = cache[cache_key]
-    stylesheets = cache[array_cache_key]
     return stylesheets if stylesheets.present?
 
-    stylesheet = { color_scheme_name: color_scheme_name }
+    stylesheet = { color_scheme_id: color_scheme&.id }
 
     builder = self.new(target, nil, color_scheme)
-
     builder.compile unless File.exists?(builder.stylesheet_fullpath)
+
     href = builder.stylesheet_path(current_hostname)
     stylesheet[:new_href] = href
-    cache[array_cache_key] = stylesheet.freeze
+    cache[cache_key] = stylesheet.freeze
     stylesheet
   end
 
@@ -130,6 +129,16 @@ class Stylesheet::Manager
     %[<link href="#{href}" media="#{media}" rel="stylesheet"/>].html_safe
   end
 
+  def self.color_scheme_cache_key(color_scheme)
+    color_scheme_name = Slug.for(color_scheme.name) + color_scheme&.id.to_s
+    "#{COLOR_SCHEME_STYLESHEET}_#{color_scheme_name}_#{Discourse.current_hostname}"
+  end
+
+  def self.color_scheme_cache_clear(color_scheme)
+    cache_key = color_scheme_cache_key(color_scheme)
+    cache[cache_key] = nil
+  end
+
   def self.precompile_css
     themes = Theme.where('user_selectable OR id = ?', SiteSetting.default_theme_id).pluck(:id, :name)
     themes << nil
@@ -149,11 +158,11 @@ class Stylesheet::Manager
     cs_ids = Theme.where('user_selectable OR id = ?', SiteSetting.default_theme_id).pluck(:color_scheme_id)
     ColorScheme.where(id: cs_ids).each do |cs|
       target = COLOR_SCHEME_STYLESHEET
-      cache_key = "#{target}_#{cs.id}"
       STDERR.puts "precompile target: #{target} #{cs.name}"
 
       builder = self.new(target, nil, cs)
       builder.compile(force: true)
+      cache_key = color_scheme_cache_key(cs)
       cache[cache_key] = nil
     end
 
@@ -300,7 +309,7 @@ class Stylesheet::Manager
     if is_theme?
       "#{@target}_#{theme.id}"
     elsif @color_scheme
-      "#{@target}_#{Slug.for(@color_scheme.name) + @color_scheme&.id.to_s}"
+      "#{@target}_#{Slug.for(@color_scheme.name)}_#{@color_scheme&.id.to_s}"
     else
       scheme_string = theme && theme.color_scheme ? "_#{theme.color_scheme.id}" : ""
       "#{@target}#{scheme_string}"
@@ -384,7 +393,8 @@ class Stylesheet::Manager
 
   def color_scheme_digest
 
-    cs = theme&.color_scheme
+    cs = @color_scheme || theme&.color_scheme
+
     category_updated = Category.where("uploaded_background_id IS NOT NULL").pluck(:updated_at).map(&:to_i).sum
 
     if cs || category_updated > 0

spec/components/stylesheet/manager_spec.rb

@@ -163,6 +163,18 @@ describe Stylesheet::Manager do
       expect(digest3).to_not eq(digest2)
       expect(digest3).to_not eq(digest1)
     end
+
+    it "updates digest when updating a color scheme" do
+      scheme = ColorScheme.create_from_base(name: "Neutral", base_scheme_id: "Neutral")
+      manager = Stylesheet::Manager.new(:color_definitions, nil, scheme)
+      digest1 = manager.color_scheme_digest
+
+      ColorSchemeRevisor.revise(scheme, colors: [{ name: "primary", hex: "CC0000" }])
+
+      digest2 = manager.color_scheme_digest
+
+      expect(digest1).to_not eq(digest2)
+    end
   end
 
   describe 'color_scheme_stylesheets' do
@@ -193,12 +205,12 @@ describe Stylesheet::Manager do
       SiteSetting.default_theme_id = theme.id
 
       link = Stylesheet::Manager.color_scheme_stylesheet_link_tag()
-      expect(link).to include("/stylesheets/color_definitions_funky#{cs.id}_")
+      expect(link).to include("/stylesheets/color_definitions_funky_#{cs.id}_")
     end
 
     it "uses the correct scheme when colors are passed" do
       link = Stylesheet::Manager.color_scheme_stylesheet_link_tag(ColorScheme.first.id)
-      slug = Slug.for(ColorScheme.first.name) + ColorScheme.first.id.to_s
+      slug = Slug.for(ColorScheme.first.name) + "_" + ColorScheme.first.id.to_s
       expect(link).to include("/stylesheets/color_definitions_#{slug}_")
     end
 
@@ -208,7 +220,21 @@ describe Stylesheet::Manager do
       SiteSetting.default_theme_id = theme.id
 
       link = Stylesheet::Manager.color_scheme_stylesheet_link_tag()
-      expect(link).to include("/stylesheets/color_definitions_funky-bunch#{cs.id}_")
+      expect(link).to include("/stylesheets/color_definitions_funky-bunch_#{cs.id}_")
+    end
+
+    it "updates outputted colors when updating a color scheme" do
+      scheme = ColorScheme.create_from_base(name: "Neutral", base_scheme_id: "Neutral")
+      manager = Stylesheet::Manager.new(:color_definitions, nil, scheme)
+      stylesheet = manager.compile
+
+      ColorSchemeRevisor.revise(scheme, colors: [{ name: "primary", hex: "CC0000" }])
+
+      manager2 = Stylesheet::Manager.new(:color_definitions, nil, scheme)
+      stylesheet2 = manager2.compile
+
+      expect(stylesheet).not_to eq(stylesheet2)
+      expect(stylesheet2).to include("--primary: #c00;")
     end
 
   end
@@ -237,7 +263,7 @@ describe Stylesheet::Manager do
       scheme2 = ColorScheme.create!(name: "scheme2")
       core_targets = [:desktop, :mobile, :desktop_rtl, :mobile_rtl, :admin]
       theme_targets = [:desktop_theme, :mobile_theme]
-      color_scheme_targets = ["color_definitions_scheme1", "color_definitions_scheme2"]
+      color_scheme_targets = ["color_definitions_scheme1_#{scheme1.id}", "color_definitions_scheme2_#{scheme2.id}"]
 
       Theme.update_all(user_selectable: false)
       user_theme = Fabricate(:theme, user_selectable: true, color_scheme: scheme1)