From e8ab4d92b410f46ca87d31a1376acd94f90d1dd9 Mon Sep 17 00:00:00 2001 From: Greg Stark Date: Mon, 11 Dec 2017 16:40:03 +0000 Subject: Treat empty markdown and html strings as valid cached text, not missing cache that needs to be updated --- app/models/concerns/cache_markdown_field.rb | 3 +-- ...timize-issues-avoid-noop-empty-cache-updates2.yml | 6 ++++++ spec/models/concerns/cache_markdown_field_spec.rb | 20 ++++++++++++++++++++ 3 files changed, 27 insertions(+), 2 deletions(-) create mode 100644 changelogs/unreleased/optimize-issues-avoid-noop-empty-cache-updates2.yml 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' -- cgit v1.2.1