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

@@ -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)

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
 

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
 

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!

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

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`