diff options
-rw-r--r-- | app/models/concerns/cache_markdown_field.rb | 30 | ||||
-rw-r--r-- | spec/lib/banzai/object_renderer_spec.rb | 2 | ||||
-rw-r--r-- | spec/models/concerns/cache_markdown_field_spec.rb | 183 |
3 files changed, 142 insertions, 73 deletions
diff --git a/app/models/concerns/cache_markdown_field.rb b/app/models/concerns/cache_markdown_field.rb index 4ae5dd8c677..db8cf322ef7 100644 --- a/app/models/concerns/cache_markdown_field.rb +++ b/app/models/concerns/cache_markdown_field.rb @@ -11,7 +11,9 @@ module CacheMarkdownField extend ActiveSupport::Concern # Increment this number every time the renderer changes its output - CACHE_VERSION = 3 + CACHE_REDCARPET_VERSION = 3 + CACHE_COMMONMARK_VERSION_START = 10 + CACHE_COMMONMARK_VERSION = 11 # changes to these attributes cause the cache to be invalidates INVALIDATED_BY = %w[author project].freeze @@ -49,12 +51,14 @@ module CacheMarkdownField # Always include a project key, or Banzai complains project = self.project if self.respond_to?(:project) - group = self.group if self.respond_to?(:group) + group = self.group if self.respond_to?(:group) context = cached_markdown_fields[field].merge(project: project, group: group) # Banzai is less strict about authors, so don't always have an author key context[:author] = self.author if self.respond_to?(:author) + context[:markdown_engine] = markdown_engine + context end @@ -69,7 +73,7 @@ module CacheMarkdownField Banzai::Renderer.cacheless_render_field(self, markdown_field, options) ] end.to_h - updates['cached_markdown_version'] = CacheMarkdownField::CACHE_VERSION + updates['cached_markdown_version'] = latest_cached_markdown_version updates.each {|html_field, data| write_attribute(html_field, data) } end @@ -90,7 +94,7 @@ module CacheMarkdownField markdown_changed = attribute_changed?(markdown_field) || false html_changed = attribute_changed?(html_field) || false - CacheMarkdownField::CACHE_VERSION == cached_markdown_version && + latest_cached_markdown_version == cached_markdown_version && (html_changed || markdown_changed == html_changed) end @@ -109,6 +113,24 @@ module CacheMarkdownField __send__(cached_markdown_fields.html_field(markdown_field)) # rubocop:disable GitlabSecurity/PublicSend end + def latest_cached_markdown_version + return CacheMarkdownField::CACHE_REDCARPET_VERSION unless cached_markdown_version + + if cached_markdown_version < CacheMarkdownField::CACHE_COMMONMARK_VERSION_START + CacheMarkdownField::CACHE_REDCARPET_VERSION + else + CacheMarkdownField::CACHE_COMMONMARK_VERSION + end + end + + def markdown_engine + if latest_cached_markdown_version < CacheMarkdownField::CACHE_COMMONMARK_VERSION_START + :redcarpet + else + :common_mark + end + end + included do cattr_reader :cached_markdown_fields do FieldData.new diff --git a/spec/lib/banzai/object_renderer_spec.rb b/spec/lib/banzai/object_renderer_spec.rb index 1fe034ae9a2..209a547c3b3 100644 --- a/spec/lib/banzai/object_renderer_spec.rb +++ b/spec/lib/banzai/object_renderer_spec.rb @@ -11,7 +11,7 @@ describe Banzai::ObjectRenderer do ) end - let(:object) { Note.new(note: 'hello', note_html: '<p dir="auto">hello</p>', cached_markdown_version: CacheMarkdownField::CACHE_VERSION) } + let(:object) { Note.new(note: 'hello', note_html: '<p dir="auto">hello</p>', cached_markdown_version: CacheMarkdownField::CACHE_COMMONMARK_VERSION) } describe '#render' do context 'with cache' do diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb index 3c7f578975b..b3797c1fb46 100644 --- a/spec/models/concerns/cache_markdown_field_spec.rb +++ b/spec/models/concerns/cache_markdown_field_spec.rb @@ -72,7 +72,7 @@ describe CacheMarkdownField do let(:updated_markdown) { '`Bar`' } let(:updated_html) { '<p dir="auto"><code>Bar</code></p>' } - let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: CacheMarkdownField::CACHE_VERSION) } + let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: CacheMarkdownField::CACHE_COMMONMARK_VERSION) } describe '.attributes' do it 'excludes cache attributes' do @@ -89,17 +89,24 @@ describe CacheMarkdownField do it { expect(thing.foo).to eq(markdown) } it { expect(thing.foo_html).to eq(html) } it { expect(thing.foo_html_changed?).not_to be_truthy } - it { expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_VERSION) } + it { expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_COMMONMARK_VERSION) } end context 'a changed markdown field' do - before do - thing.foo = updated_markdown - thing.save + shared_examples 'with cache version' do |cache_version| + let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version) } + + before do + thing.foo = updated_markdown + thing.save + end + + it { expect(thing.foo_html).to eq(updated_html) } + it { expect(thing.cached_markdown_version).to eq(cache_version) } end - it { expect(thing.foo_html).to eq(updated_html) } - it { expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_VERSION) } + it_behaves_like 'with cache version', CacheMarkdownField::CACHE_REDCARPET_VERSION + it_behaves_like 'with cache version', CacheMarkdownField::CACHE_COMMONMARK_VERSION end context 'when a markdown field is set repeatedly to an empty string' do @@ -123,15 +130,22 @@ describe CacheMarkdownField do end context 'a non-markdown field changed' do - before do - thing.bar = 'OK' - thing.save + shared_examples 'with cache version' do |cache_version| + let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version) } + + before do + thing.bar = 'OK' + thing.save + end + + it { expect(thing.bar).to eq('OK') } + it { expect(thing.foo).to eq(markdown) } + it { expect(thing.foo_html).to eq(html) } + it { expect(thing.cached_markdown_version).to eq(cache_version) } end - it { expect(thing.bar).to eq('OK') } - it { expect(thing.foo).to eq(markdown) } - it { expect(thing.foo_html).to eq(html) } - it { expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_VERSION) } + it_behaves_like 'with cache version', CacheMarkdownField::CACHE_REDCARPET_VERSION + it_behaves_like 'with cache version', CacheMarkdownField::CACHE_COMMONMARK_VERSION end context 'version is out of date' do @@ -142,59 +156,85 @@ describe CacheMarkdownField do end it { expect(thing.foo_html).to eq(updated_html) } - it { expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_VERSION) } + it { expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_REDCARPET_VERSION) } end describe '#cached_html_up_to_date?' do - subject { thing.cached_html_up_to_date?(:foo) } + shared_examples 'with cache version' do |cache_version| + let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version) } - it 'returns false when the version is absent' do - thing.cached_markdown_version = nil + subject { thing.cached_html_up_to_date?(:foo) } - is_expected.to be_falsy - end + it 'returns false when the version is absent' do + thing.cached_markdown_version = nil - it 'returns false when the version is too early' do - thing.cached_markdown_version -= 1 + is_expected.to be_falsy + end - is_expected.to be_falsy - end + it 'returns false when the version is too early' do + thing.cached_markdown_version -= 1 - it 'returns false when the version is too late' do - thing.cached_markdown_version += 1 + is_expected.to be_falsy + end - is_expected.to be_falsy - end + it 'returns false when the version is too late' do + thing.cached_markdown_version += 1 - it 'returns true when the version is just right' do - thing.cached_markdown_version = CacheMarkdownField::CACHE_VERSION + is_expected.to be_falsy + end - is_expected.to be_truthy - end + it 'returns true when the version is just right' do + thing.cached_markdown_version = cache_version - it 'returns false if markdown has been changed but html has not' do - thing.foo = updated_html + is_expected.to be_truthy + end - is_expected.to be_falsy - end + it 'returns false if markdown has been changed but html has not' do + thing.foo = updated_html - it 'returns true if markdown has not been changed but html has' do - thing.foo_html = updated_html + is_expected.to be_falsy + end + + it 'returns true if markdown has not been changed but html has' do + thing.foo_html = updated_html - is_expected.to be_truthy + is_expected.to be_truthy + end + + it 'returns true if markdown and html have both been changed' do + thing.foo = updated_markdown + thing.foo_html = updated_html + + is_expected.to be_truthy + end + + it 'returns false if the markdown field is set but the html is not' do + thing.foo_html = nil + + is_expected.to be_falsy + end end - it 'returns true if markdown and html have both been changed' do - thing.foo = updated_markdown - thing.foo_html = updated_html + it_behaves_like 'with cache version', CacheMarkdownField::CACHE_REDCARPET_VERSION + it_behaves_like 'with cache version', CacheMarkdownField::CACHE_COMMONMARK_VERSION + end + + describe '#latest_cached_markdown_version' do + subject { thing.latest_cached_markdown_version } - is_expected.to be_truthy + it 'returns redcarpet version' do + thing.cached_markdown_version = CacheMarkdownField::CACHE_COMMONMARK_VERSION_START - 1 + is_expected.to eq(CacheMarkdownField::CACHE_REDCARPET_VERSION) end - it 'returns false if the markdown field is set but the html is not' do - thing.foo_html = nil + it 'returns commonmark version' do + thing.cached_markdown_version = CacheMarkdownField::CACHE_COMMONMARK_VERSION_START + 1 + is_expected.to eq(CacheMarkdownField::CACHE_COMMONMARK_VERSION) + end - is_expected.to be_falsy + it 'returns default version when version is nil' do + thing.cached_markdown_version = nil + is_expected.to eq(CacheMarkdownField::CACHE_REDCARPET_VERSION) end end @@ -221,37 +261,44 @@ describe CacheMarkdownField do thing.cached_markdown_version = nil thing.refresh_markdown_cache - expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_VERSION) + expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_REDCARPET_VERSION) end end describe '#refresh_markdown_cache!' do - before do - thing.foo = updated_markdown - end + shared_examples 'with cache version' do |cache_version| + let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version) } - it 'fills all html fields' do - thing.refresh_markdown_cache! + 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! - 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! + 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" => cache_version) + + thing.refresh_markdown_cache! + end end + + it_behaves_like 'with cache version', CacheMarkdownField::CACHE_REDCARPET_VERSION + it_behaves_like 'with cache version', CacheMarkdownField::CACHE_COMMONMARK_VERSION end describe '#banzai_render_context' do @@ -299,7 +346,7 @@ describe CacheMarkdownField do expect(thing.foo_html).to eq(updated_html) expect(thing.baz_html).to eq(updated_html) - expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_VERSION) + expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_REDCARPET_VERSION) end end @@ -319,7 +366,7 @@ describe CacheMarkdownField do expect(thing.foo_html).to eq(updated_html) expect(thing.baz_html).to eq(updated_html) - expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_VERSION) + expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_REDCARPET_VERSION) end end end |