diff options
author | Greg Stark <stark@gitlab.com> | 2017-12-11 16:40:03 +0000 |
---|---|---|
committer | Greg Stark <stark@gitlab.com> | 2017-12-12 14:01:53 +0000 |
commit | e8ab4d92b410f46ca87d31a1376acd94f90d1dd9 (patch) | |
tree | d55dac3b8663232bfe1c2ff893147cd0b6cd44bf | |
parent | be8ca260dadae948d09a87664baed8c85d133434 (diff) | |
download | gitlab-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
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' |