From b7404373cfe667e809a7f065c9612e86db8547a9 Mon Sep 17 00:00:00 2001 From: Daniel Waterworth Date: Wed, 12 Jul 2023 09:49:28 -0500 Subject: [PATCH] FIX: Always clear caches after committing the current transaction (#22550) Instead of having to remember every time, just always wait until the current transaction (if it exists) has committed before clearing any DistributedCache. The only exception to this is caches that aren't caching things from postgres. This means we have to do the test setup after setting the test transaction, because doing the test setup involves clearing caches. Reapplying this - it now doesn't use after_commit if skip_db is set --- app/models/theme.rb | 8 ++++---- app/models/theme_field.rb | 14 +++++++------- app/models/theme_svg_sprite.rb | 2 +- lib/discourse.rb | 4 ++-- lib/distributed_cache.rb | 8 ++++++++ spec/rails_helper.rb | 5 ++--- 6 files changed, 24 insertions(+), 17 deletions(-) diff --git a/app/models/theme.rb b/app/models/theme.rb index a21de2c5398..73225814553 100644 --- a/app/models/theme.rb +++ b/app/models/theme.rb @@ -117,7 +117,7 @@ class Theme < ActiveRecord::Base update_javascript_cache! remove_from_cache! - DB.after_commit { ColorScheme.hex_cache.clear } + ColorScheme.hex_cache.clear notify_theme_change(with_scheme: notify_with_scheme) if theme_setting_requests_refresh @@ -358,7 +358,7 @@ class Theme < ActiveRecord::Base end def self.clear_cache! - DB.after_commit { @cache.clear } + @cache.clear end def self.targets @@ -529,12 +529,12 @@ class Theme < ActiveRecord::Base def child_theme_ids=(theme_ids) super(theme_ids) - DB.after_commit { Theme.clear_cache! } + Theme.clear_cache! end def parent_theme_ids=(theme_ids) super(theme_ids) - DB.after_commit { Theme.clear_cache! } + Theme.clear_cache! end def add_relative_theme!(kind, theme) diff --git a/app/models/theme_field.rb b/app/models/theme_field.rb index 1972d2e3734..b63a796c522 100644 --- a/app/models/theme_field.rb +++ b/app/models/theme_field.rb @@ -394,22 +394,22 @@ class ThemeField < ActiveRecord::Base translation_field? ? process_translation : process_html(self.value) self.error = nil unless self.error.present? self.compiler_version = Theme.compiler_version - DB.after_commit { CSP::Extension.clear_theme_extensions_cache! } + CSP::Extension.clear_theme_extensions_cache! elsif extra_js_field? || js_tests_field? self.error = nil self.value_baked = "baked" self.compiler_version = Theme.compiler_version elsif basic_scss_field? ensure_scss_compiles! - DB.after_commit { Stylesheet::Manager.clear_theme_cache! } + Stylesheet::Manager.clear_theme_cache! elsif settings_field? validate_yaml! - DB.after_commit { CSP::Extension.clear_theme_extensions_cache! } - DB.after_commit { SvgSprite.expire_cache } + CSP::Extension.clear_theme_extensions_cache! + SvgSprite.expire_cache self.value_baked = "baked" self.compiler_version = Theme.compiler_version elsif svg_sprite_field? - DB.after_commit { SvgSprite.expire_cache } + SvgSprite.expire_cache self.error = validate_svg_sprite_xml self.value_baked = "baked" self.compiler_version = Theme.compiler_version @@ -682,7 +682,7 @@ class ThemeField < ActiveRecord::Base if upload && svg_sprite_field? upsert_svg_sprite! - DB.after_commit { SvgSprite.expire_cache } + SvgSprite.expire_cache end end @@ -690,7 +690,7 @@ class ThemeField < ActiveRecord::Base if svg_sprite_field? ThemeSvgSprite.where(theme_id: theme_id).delete_all - DB.after_commit { SvgSprite.expire_cache } + SvgSprite.expire_cache end end diff --git a/app/models/theme_svg_sprite.rb b/app/models/theme_svg_sprite.rb index be8e087aeaa..0bc9581bd54 100644 --- a/app/models/theme_svg_sprite.rb +++ b/app/models/theme_svg_sprite.rb @@ -5,7 +5,7 @@ class ThemeSvgSprite < ActiveRecord::Base def self.refetch! ThemeField.svg_sprite_fields.find_each(&:upsert_svg_sprite!) - DB.after_commit { SvgSprite.expire_cache } + SvgSprite.expire_cache end end diff --git a/lib/discourse.rb b/lib/discourse.rb index 1e5d85e66da..521b3ec2186 100644 --- a/lib/discourse.rb +++ b/lib/discourse.rb @@ -773,14 +773,14 @@ module Discourse def self.received_postgres_readonly! time = Time.zone.now redis.set(LAST_POSTGRES_READONLY_KEY, time.to_i.to_s) - postgres_last_read_only.clear + postgres_last_read_only.clear(after_commit: false) time end def self.clear_postgres_readonly! redis.del(LAST_POSTGRES_READONLY_KEY) - postgres_last_read_only.clear + postgres_last_read_only.clear(after_commit: false) end def self.received_redis_readonly! diff --git a/lib/distributed_cache.rb b/lib/distributed_cache.rb index 80fa7bf1f82..b041b39d1d4 100644 --- a/lib/distributed_cache.rb +++ b/lib/distributed_cache.rb @@ -19,4 +19,12 @@ class DistributedCache < MessageBus::DistributedCache self.defer_set(k, value) value end + + def clear(after_commit: true) + if after_commit && !GlobalSetting.skip_db? + DB.after_commit { super() } + else + super() + end + end end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index e498a3fd45f..8717536d347 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -151,8 +151,8 @@ end TestProf::BeforeAll.configure do |config| config.after(:begin) do - TestSetup.test_setup DB.test_transaction = ActiveRecord::Base.connection.current_transaction + TestSetup.test_setup end end @@ -350,8 +350,6 @@ RSpec.configure do |config| FileUtils.remove_dir(concurrency_safe_tmp_dir, true) if SpecSecureRandom.value end - config.before :each, &TestSetup.method(:test_setup) - config.around :each do |example| before_event_count = DiscourseEvent.events.values.sum(&:count) example.run @@ -386,6 +384,7 @@ RSpec.configure do |config| config.before :each do # This allows DB.transaction_open? to work in tests. See lib/mini_sql_multisite_connection.rb DB.test_transaction = ActiveRecord::Base.connection.current_transaction + TestSetup.test_setup end # Match the request hostname to the value in `database.yml`