Merge branch 'optimize-issues-avoid-noop-empty-cache-updates2' into 'master'

Treat empty markdown and html strings as valid cached text, not missing cache…

See merge request gitlab-org/gitlab-ce!15856

3 files changed, 27 insertions, 2 deletions

diff --git a/app/models/concerns/cache_markdown_field.rb b/app/models/concerns/cache_markdown_field.rb
index 98776eab424..90ad644ce34 100644
--- a/app/models/concerns/cache_markdown_field.rb
+++ b/app/models/concerns/cache_markdown_field.rb
@@ -85,8 +85,7 @@ module CacheMarkdownField
   def cached_html_up_to_date?(markdown_field)
     html_field = cached_markdown_fields.html_field(markdown_field)
 
-    cached = cached_html_for(markdown_field).present? && __send__(markdown_field).present? # rubocop:disable GitlabSecurity/PublicSend
-    return false unless cached
+    return false if cached_html_for(markdown_field).nil? && !__send__(markdown_field).nil? # rubocop:disable GitlabSecurity/PublicSend
 
     markdown_changed = attribute_changed?(markdown_field) || false
     html_changed = attribute_changed?(html_field) || false
diff --git a/changelogs/unreleased/optimize-issues-avoid-noop-empty-cache-updates2.yml b/changelogs/unreleased/optimize-issues-avoid-noop-empty-cache-updates2.yml
new file mode 100644
index 00000000000..e0c3136be69
--- /dev/null
+++ b/changelogs/unreleased/optimize-issues-avoid-noop-empty-cache-updates2.yml
@@ -0,0 +1,6 @@
+---
+title: Treat empty markdown and html strings as valid cached text, not missing cache
+  that needs to be updated
+merge_request:
+author:
+type: performance
diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb
index 129dfa07f15..3c7f578975b 100644
--- a/spec/models/concerns/cache_markdown_field_spec.rb
+++ b/spec/models/concerns/cache_markdown_field_spec.rb
@@ -102,6 +102,26 @@ describe CacheMarkdownField do
     it { expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_VERSION) }
   end
 
+  context 'when a markdown field is set repeatedly to an empty string' do
+    it do
+      expect(thing).to receive(:refresh_markdown_cache).once
+      thing.foo = ''
+      thing.save
+      thing.foo = ''
+      thing.save
+    end
+  end
+
+  context 'when a markdown field is set repeatedly to a string which renders as empty html' do
+    it do
+      expect(thing).to receive(:refresh_markdown_cache).once
+      thing.foo = '[//]: # (This is also a comment.)'
+      thing.save
+      thing.foo = '[//]: # (This is also a comment.)'
+      thing.save
+    end
+  end
+
   context 'a non-markdown field changed' do
     before do
       thing.bar = 'OK'