summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGreg Stark <stark@gitlab.com>2017-12-11 16:40:03 +0000
committerGreg Stark <stark@gitlab.com>2017-12-12 14:01:53 +0000
commite8ab4d92b410f46ca87d31a1376acd94f90d1dd9 (patch)
treed55dac3b8663232bfe1c2ff893147cd0b6cd44bf
parentbe8ca260dadae948d09a87664baed8c85d133434 (diff)
downloadgitlab-ce-e8ab4d92b410f46ca87d31a1376acd94f90d1dd9.tar.gz
Treat empty markdown and html strings as valid cached text, not missing cache that needs to be updatedoptimize-issues-avoid-noop-empty-cache-updates2
-rw-r--r--app/models/concerns/cache_markdown_field.rb3
-rw-r--r--changelogs/unreleased/optimize-issues-avoid-noop-empty-cache-updates2.yml6
-rw-r--r--spec/models/concerns/cache_markdown_field_spec.rb20
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'