Create refresh_markdown_cache that doesn't write to database

`CacheMarkdownField#refresh_markdown_cache!` is splitted out in:
- `CacheMarkdownField#refresh_markdown_cache`: This does the same as
  before, but does never write to database.
- `CacheMarkdownField#refresh_markdown_cache!`: This calls the
  previous method, and tries to write to database, if possible.

app/models/concerns/cache_markdown_field.rb

@@ -59,7 +59,7 @@ module CacheMarkdownField
 
   # Update every column in a row if any one is invalidated, as we only store
   # one version per row
-  def refresh_markdown_cache!(do_update: false)
+  def refresh_markdown_cache
     options = { skip_project_check: skip_project_check? }
 
     updates = cached_markdown_fields.markdown_fields.map do |markdown_field|
@@ -71,8 +71,14 @@ module CacheMarkdownField
     updates['cached_markdown_version'] = CacheMarkdownField::CACHE_VERSION
 
     updates.each {|html_field, data| write_attribute(html_field, data) }
+  end
+
+  def refresh_markdown_cache!
+    updates = refresh_markdown_cache
+
+    return unless persisted? && Gitlab::Database.read_write?
 
-    update_columns(updates) if persisted? && do_update
+    update_columns(updates)
   end
 
   def cached_html_up_to_date?(markdown_field)
@@ -124,8 +130,8 @@ module CacheMarkdownField
     end
 
     # Using before_update here conflicts with elasticsearch-model somehow
-    before_create :refresh_markdown_cache!, if: :invalidated_markdown_cache?
-    before_update :refresh_markdown_cache!, if: :invalidated_markdown_cache?
+    before_create :refresh_markdown_cache, if: :invalidated_markdown_cache?
+    before_update :refresh_markdown_cache, if: :invalidated_markdown_cache?
   end
 
   class_methods do

lib/banzai/renderer.rb

@@ -40,7 +40,7 @@ module Banzai
         return cacheless_render_field(object, field)
       end
 
-      object.refresh_markdown_cache!(do_update: update_object?(object)) unless object.cached_html_up_to_date?(field)
+      object.refresh_markdown_cache! unless object.cached_html_up_to_date?(field)
 
       object.cached_html_for(field)
     end
@@ -162,10 +162,5 @@ module Banzai
       return unless cache_key
       Rails.cache.__send__(:expanded_key, full_cache_key(cache_key, pipeline_name)) # rubocop:disable GitlabSecurity/PublicSend
     end
-
-    # GitLab needs to disable updates on GET requests if database is readonly
-    def self.update_object?(object)
-      Gitlab::Database.read_write?
-    end
   end
 end

spec/lib/banzai/renderer_spec.rb

@@ -31,14 +31,14 @@ describe Banzai::Renderer do
         let(:object) { fake_object(fresh: false) }
 
         it 'caches and returns the result' do
-          expect(object).to receive(:refresh_markdown_cache!).with(do_update: true)
+          expect(object).to receive(:refresh_markdown_cache!)
 
           is_expected.to eq('field_html')
         end
 
         it "skips database caching on a GitLab readonly instance" do
           allow(Gitlab::Database).to receive(:read_only?).and_return(true)
-          expect(object).to receive(:refresh_markdown_cache!).with(do_update: false)
+          expect(object).to receive(:refresh_markdown_cache!)
 
           is_expected.to eq('field_html')
         end

spec/models/concerns/cache_markdown_field_spec.rb

@@ -178,57 +178,59 @@ describe CacheMarkdownField do
     end
   end
 
-  describe '#refresh_markdown_cache!' do
+  describe '#refresh_markdown_cache' do
     before do
       thing.foo = updated_markdown
     end
 
-    context 'do_update: false' do
-      it 'fills all html fields' do
-        thing.refresh_markdown_cache!
+    it 'fills all html fields' do
+      thing.refresh_markdown_cache
 
-        expect(thing.foo_html).to eq(updated_html)
-        expect(thing.foo_html_changed?).to be_truthy
-        expect(thing.baz_html_changed?).to be_truthy
-      end
+      expect(thing.foo_html).to eq(updated_html)
+      expect(thing.foo_html_changed?).to be_truthy
+      expect(thing.baz_html_changed?).to be_truthy
+    end
 
-      it 'does not save the result' do
-        expect(thing).not_to receive(:update_columns)
+    it 'does not save the result' do
+      expect(thing).not_to receive(:update_columns)
 
-        thing.refresh_markdown_cache!
-      end
+      thing.refresh_markdown_cache
+    end
 
-      it 'updates the markdown cache version' do
-        thing.cached_markdown_version = nil
-        thing.refresh_markdown_cache!
+    it 'updates the markdown cache version' do
+      thing.cached_markdown_version = nil
+      thing.refresh_markdown_cache
 
-        expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_VERSION)
-      end
+      expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_VERSION)
     end
+  end
 
-    context 'do_update: true' do
-      it 'fills all html fields' do
-        thing.refresh_markdown_cache!(do_update: true)
+  describe '#refresh_markdown_cache!' do
+    before do
+      thing.foo = updated_markdown
+    end
 
-        expect(thing.foo_html).to eq(updated_html)
-        expect(thing.foo_html_changed?).to be_truthy
-        expect(thing.baz_html_changed?).to be_truthy
-      end
+    it 'fills all html fields' do
+      thing.refresh_markdown_cache!
 
-      it 'skips saving if not persisted' do
-        expect(thing).to receive(:persisted?).and_return(false)
-        expect(thing).not_to receive(:update_columns)
+      expect(thing.foo_html).to eq(updated_html)
+      expect(thing.foo_html_changed?).to be_truthy
+      expect(thing.baz_html_changed?).to be_truthy
+    end
 
-        thing.refresh_markdown_cache!(do_update: true)
-      end
+    it 'skips saving if not persisted' do
+      expect(thing).to receive(:persisted?).and_return(false)
+      expect(thing).not_to receive(:update_columns)
 
-      it 'saves the changes using #update_columns' do
-        expect(thing).to receive(:persisted?).and_return(true)
-        expect(thing).to receive(:update_columns)
-          .with("foo_html" => updated_html, "baz_html" => "", "cached_markdown_version" => CacheMarkdownField::CACHE_VERSION)
+      thing.refresh_markdown_cache!
+    end
 
-        thing.refresh_markdown_cache!(do_update: true)
-      end
+    it 'saves the changes using #update_columns' do
+      expect(thing).to receive(:persisted?).and_return(true)
+      expect(thing).to receive(:update_columns)
+        .with("foo_html" => updated_html, "baz_html" => "", "cached_markdown_version" => CacheMarkdownField::CACHE_VERSION)
+
+      thing.refresh_markdown_cache!
     end
   end