From d29d69e10d2e1d8a3d0c53505d3938b096e4335e Mon Sep 17 00:00:00 2001 From: David Taylor Date: Fri, 29 May 2020 13:04:51 +0100 Subject: [PATCH] FIX: Invalidate database theme cache when hostname changes (#9908) Hostname can vary per-site on a multisite cluster, so this change requires converting the compiler_version from a constant into a class method which is evaluated at runtime. The value is stored in the theme DistributedCache, so performance impact should be negligible. --- app/models/theme.rb | 19 ++++++++++++++++--- app/models/theme_field.rb | 20 ++++++-------------- spec/models/theme_spec.rb | 15 ++++++++++++++- 3 files changed, 36 insertions(+), 18 deletions(-) diff --git a/app/models/theme.rb b/app/models/theme.rb index 4f54f9a5180..da9d3577298 100644 --- a/app/models/theme.rb +++ b/app/models/theme.rb @@ -122,6 +122,19 @@ class Theme < ActiveRecord::Base SvgSprite.expire_cache end + BASE_COMPILER_VERSION = 16 + def self.compiler_version + get_set_cache "compiler_version" do + dependencies = [ + BASE_COMPILER_VERSION, + Ember::VERSION, + GlobalSetting.cdn_url, + Discourse.current_hostname + ] + Digest::SHA1.hexdigest(dependencies.join) + end + end + def self.get_set_cache(key, &blk) if @cache.hash.key? key.to_s return @cache[key] @@ -248,7 +261,7 @@ class Theme < ActiveRecord::Base theme_ids = [theme_ids] unless Array === theme_ids theme_ids = transform_ids(theme_ids) - cache_key = "#{theme_ids.join(",")}:#{target}:#{field}:#{ThemeField::COMPILER_VERSION}" + cache_key = "#{theme_ids.join(",")}:#{target}:#{field}:#{Theme.compiler_version}" lookup = @cache[cache_key] return lookup.html_safe if lookup @@ -262,7 +275,7 @@ class Theme < ActiveRecord::Base theme_ids = [theme_ids] unless Array === theme_ids theme_ids = transform_ids(theme_ids) - get_set_cache("#{theme_ids.join(",")}:modifier:#{modifier_name}:#{ThemeField::COMPILER_VERSION}") do + get_set_cache("#{theme_ids.join(",")}:modifier:#{modifier_name}:#{Theme.compiler_version}") do ThemeModifierSet.resolve_modifier_for_themes(theme_ids, modifier_name) end end @@ -321,7 +334,7 @@ class Theme < ActiveRecord::Base def self.resolve_baked_field(theme_ids, target, name) if target == :extra_js require_rebake = ThemeField.where(theme_id: theme_ids, target_id: Theme.targets[:extra_js]). - where("compiler_version <> ?", ThemeField::COMPILER_VERSION) + where("compiler_version <> ?", Theme.compiler_version) require_rebake.each { |tf| tf.ensure_baked! } require_rebake.map(&:theme_id).uniq.each do |theme_id| Theme.find(theme_id).update_javascript_cache! diff --git a/app/models/theme_field.rb b/app/models/theme_field.rb index c2e8d06a72c..a15d42ec9e5 100644 --- a/app/models/theme_field.rb +++ b/app/models/theme_field.rb @@ -60,14 +60,6 @@ class ThemeField < ActiveRecord::Base validates :name, format: { with: /\A[a-z_][a-z0-9_-]*\z/i }, if: Proc.new { |field| ThemeField.theme_var_type_ids.include?(field.type_id) } - BASE_COMPILER_VERSION = 16 - DEPENDENT_CONSTANTS = [ - BASE_COMPILER_VERSION, - Ember::VERSION, - GlobalSetting.cdn_url - ] - COMPILER_VERSION = Digest::SHA1.hexdigest(DEPENDENT_CONSTANTS.join) - belongs_to :theme def process_html(html) @@ -311,18 +303,18 @@ class ThemeField < ActiveRecord::Base end def ensure_baked! - needs_baking = !self.value_baked || compiler_version != COMPILER_VERSION + needs_baking = !self.value_baked || compiler_version != Theme.compiler_version return unless needs_baking if basic_html_field? || translation_field? self.value_baked, self.error = translation_field? ? process_translation : process_html(self.value) self.error = nil unless self.error.present? - self.compiler_version = COMPILER_VERSION + self.compiler_version = Theme.compiler_version DB.after_commit { CSP::Extension.clear_theme_extensions_cache! } elsif extra_js_field? self.value_baked, self.error = process_extra_js(self.value) self.error = nil unless self.error.present? - self.compiler_version = COMPILER_VERSION + self.compiler_version = Theme.compiler_version elsif basic_scss_field? ensure_scss_compiles! DB.after_commit { Stylesheet::Manager.clear_theme_cache! } @@ -332,12 +324,12 @@ class ThemeField < ActiveRecord::Base DB.after_commit { CSP::Extension.clear_theme_extensions_cache! } DB.after_commit { SvgSprite.expire_cache } self.value_baked = "baked" - self.compiler_version = COMPILER_VERSION + self.compiler_version = Theme.compiler_version elsif svg_sprite_field? DB.after_commit { SvgSprite.expire_cache } self.error = nil self.value_baked = "baked" - self.compiler_version = COMPILER_VERSION + self.compiler_version = Theme.compiler_version end if self.will_save_change_to_value_baked? || @@ -370,7 +362,7 @@ class ThemeField < ActiveRecord::Base rescue SassC::SyntaxError => e self.error = e.message unless self.destroyed? end - self.compiler_version = COMPILER_VERSION + self.compiler_version = Theme.compiler_version self.value_baked = Digest::SHA1.hexdigest(result.join(",")) # We don't use the compiled CSS here, we just use it to invalidate the stylesheet cache end diff --git a/spec/models/theme_spec.rb b/spec/models/theme_spec.rb index 04b452ccfc6..f32dc33760d 100644 --- a/spec/models/theme_spec.rb +++ b/spec/models/theme_spec.rb @@ -716,7 +716,7 @@ HTML first_common_value = Theme.lookup_field(child.id, :desktop, "header") first_extra_js_value = Theme.lookup_field(child.id, :extra_js, nil) - stub_const(ThemeField, :COMPILER_VERSION, "SOME_NEW_HASH") do + Theme.stubs(:compiler_version).returns("SOME_NEW_HASH") do second_common_value = Theme.lookup_field(child.id, :desktop, "header") second_extra_js_value = Theme.lookup_field(child.id, :extra_js, nil) @@ -730,5 +730,18 @@ HTML expect(new_extra_js_compiler_version).to eq("SOME_NEW_HASH") end end + + it 'recompiles when the hostname changes' do + theme.set_field(target: :settings, name: :yaml, value: "name: bob") + theme_field = theme.set_field(target: :common, name: :after_header, value: '') + theme.save! + + expect(Theme.lookup_field(theme.id, :common, :after_header)).to include("_ws=#{Discourse.current_hostname}") + + SiteSetting.force_hostname = "someotherhostname.com" + Theme.clear_cache! + + expect(Theme.lookup_field(theme.id, :common, :after_header)).to include("_ws=someotherhostname.com") + end end end