Commit 36a5a2c3 authored by Brett Walker's avatar Brett Walker

for cached markdown fields, select the correct engine

and also make sure that if a field is saved, then the existing cache version is maintained or only upgraded to the version with the same markdown engine.
parent 25cb70ba
...@@ -11,7 +11,9 @@ module CacheMarkdownField ...@@ -11,7 +11,9 @@ module CacheMarkdownField
extend ActiveSupport::Concern extend ActiveSupport::Concern
# Increment this number every time the renderer changes its output # 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 # changes to these attributes cause the cache to be invalidates
INVALIDATED_BY = %w[author project].freeze INVALIDATED_BY = %w[author project].freeze
...@@ -49,12 +51,14 @@ module CacheMarkdownField ...@@ -49,12 +51,14 @@ module CacheMarkdownField
# Always include a project key, or Banzai complains # Always include a project key, or Banzai complains
project = self.project if self.respond_to?(:project) 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) context = cached_markdown_fields[field].merge(project: project, group: group)
# Banzai is less strict about authors, so don't always have an author key # Banzai is less strict about authors, so don't always have an author key
context[:author] = self.author if self.respond_to?(:author) context[:author] = self.author if self.respond_to?(:author)
context[:markdown_engine] = markdown_engine
context context
end end
...@@ -69,7 +73,7 @@ module CacheMarkdownField ...@@ -69,7 +73,7 @@ module CacheMarkdownField
Banzai::Renderer.cacheless_render_field(self, markdown_field, options) Banzai::Renderer.cacheless_render_field(self, markdown_field, options)
] ]
end.to_h 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) } updates.each {|html_field, data| write_attribute(html_field, data) }
end end
...@@ -90,7 +94,7 @@ module CacheMarkdownField ...@@ -90,7 +94,7 @@ module CacheMarkdownField
markdown_changed = attribute_changed?(markdown_field) || false markdown_changed = attribute_changed?(markdown_field) || false
html_changed = attribute_changed?(html_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) (html_changed || markdown_changed == html_changed)
end end
...@@ -109,6 +113,24 @@ module CacheMarkdownField ...@@ -109,6 +113,24 @@ module CacheMarkdownField
__send__(cached_markdown_fields.html_field(markdown_field)) # rubocop:disable GitlabSecurity/PublicSend __send__(cached_markdown_fields.html_field(markdown_field)) # rubocop:disable GitlabSecurity/PublicSend
end 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 included do
cattr_reader :cached_markdown_fields do cattr_reader :cached_markdown_fields do
FieldData.new FieldData.new
......
...@@ -11,7 +11,7 @@ describe Banzai::ObjectRenderer do ...@@ -11,7 +11,7 @@ describe Banzai::ObjectRenderer do
) )
end 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 describe '#render' do
context 'with cache' do context 'with cache' do
......
...@@ -72,7 +72,7 @@ describe CacheMarkdownField do ...@@ -72,7 +72,7 @@ describe CacheMarkdownField do
let(:updated_markdown) { '`Bar`' } let(:updated_markdown) { '`Bar`' }
let(:updated_html) { '<p dir="auto"><code>Bar</code></p>' } 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 describe '.attributes' do
it 'excludes cache attributes' do it 'excludes cache attributes' do
...@@ -89,17 +89,24 @@ describe CacheMarkdownField do ...@@ -89,17 +89,24 @@ describe CacheMarkdownField do
it { expect(thing.foo).to eq(markdown) } it { expect(thing.foo).to eq(markdown) }
it { expect(thing.foo_html).to eq(html) } it { expect(thing.foo_html).to eq(html) }
it { expect(thing.foo_html_changed?).not_to be_truthy } 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 end
context 'a changed markdown field' do context 'a changed markdown field' do
before do shared_examples 'with cache version' do |cache_version|
thing.foo = updated_markdown let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version) }
thing.save
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 end
it { expect(thing.foo_html).to eq(updated_html) } it_behaves_like 'with cache version', CacheMarkdownField::CACHE_REDCARPET_VERSION
it { expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_VERSION) } it_behaves_like 'with cache version', CacheMarkdownField::CACHE_COMMONMARK_VERSION
end end
context 'when a markdown field is set repeatedly to an empty string' do context 'when a markdown field is set repeatedly to an empty string' do
...@@ -123,15 +130,22 @@ describe CacheMarkdownField do ...@@ -123,15 +130,22 @@ describe CacheMarkdownField do
end end
context 'a non-markdown field changed' do context 'a non-markdown field changed' do
before do shared_examples 'with cache version' do |cache_version|
thing.bar = 'OK' let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version) }
thing.save
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 end
it { expect(thing.bar).to eq('OK') } it_behaves_like 'with cache version', CacheMarkdownField::CACHE_REDCARPET_VERSION
it { expect(thing.foo).to eq(markdown) } it_behaves_like 'with cache version', CacheMarkdownField::CACHE_COMMONMARK_VERSION
it { expect(thing.foo_html).to eq(html) }
it { expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_VERSION) }
end end
context 'version is out of date' do context 'version is out of date' do
...@@ -142,59 +156,85 @@ describe CacheMarkdownField do ...@@ -142,59 +156,85 @@ describe CacheMarkdownField do
end end
it { expect(thing.foo_html).to eq(updated_html) } 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 end
describe '#cached_html_up_to_date?' do 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 subject { thing.cached_html_up_to_date?(:foo) }
thing.cached_markdown_version = nil
is_expected.to be_falsy it 'returns false when the version is absent' do
end thing.cached_markdown_version = nil
it 'returns false when the version is too early' do is_expected.to be_falsy
thing.cached_markdown_version -= 1 end
is_expected.to be_falsy it 'returns false when the version is too early' do
end thing.cached_markdown_version -= 1
it 'returns false when the version is too late' do is_expected.to be_falsy
thing.cached_markdown_version += 1 end
is_expected.to be_falsy it 'returns false when the version is too late' do
end thing.cached_markdown_version += 1
it 'returns true when the version is just right' do is_expected.to be_falsy
thing.cached_markdown_version = CacheMarkdownField::CACHE_VERSION end
is_expected.to be_truthy it 'returns true when the version is just right' do
end thing.cached_markdown_version = cache_version
it 'returns false if markdown has been changed but html has not' do is_expected.to be_truthy
thing.foo = updated_html end
is_expected.to be_falsy it 'returns false if markdown has been changed but html has not' do
end thing.foo = updated_html
it 'returns true if markdown has not been changed but html has' do is_expected.to be_falsy
thing.foo_html = updated_html 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 end
it 'returns true if markdown and html have both been changed' do it_behaves_like 'with cache version', CacheMarkdownField::CACHE_REDCARPET_VERSION
thing.foo = updated_markdown it_behaves_like 'with cache version', CacheMarkdownField::CACHE_COMMONMARK_VERSION
thing.foo_html = updated_html 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 end
it 'returns false if the markdown field is set but the html is not' do it 'returns commonmark version' do
thing.foo_html = nil 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
end end
...@@ -221,37 +261,44 @@ describe CacheMarkdownField do ...@@ -221,37 +261,44 @@ describe CacheMarkdownField do
thing.cached_markdown_version = nil thing.cached_markdown_version = nil
thing.refresh_markdown_cache 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
end end
describe '#refresh_markdown_cache!' do describe '#refresh_markdown_cache!' do
before do shared_examples 'with cache version' do |cache_version|
thing.foo = updated_markdown let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version) }
end
it 'fills all html fields' do before do
thing.refresh_markdown_cache! thing.foo = updated_markdown
end
expect(thing.foo_html).to eq(updated_html) it 'fills all html fields' do
expect(thing.foo_html_changed?).to be_truthy thing.refresh_markdown_cache!
expect(thing.baz_html_changed?).to be_truthy
end
it 'skips saving if not persisted' do expect(thing.foo_html).to eq(updated_html)
expect(thing).to receive(:persisted?).and_return(false) expect(thing.foo_html_changed?).to be_truthy
expect(thing).not_to receive(:update_columns) expect(thing.baz_html_changed?).to be_truthy
end
thing.refresh_markdown_cache! it 'skips saving if not persisted' do
end expect(thing).to receive(:persisted?).and_return(false)
expect(thing).not_to receive(:update_columns)
it 'saves the changes using #update_columns' do thing.refresh_markdown_cache!
expect(thing).to receive(:persisted?).and_return(true) end
expect(thing).to receive(:update_columns)
.with("foo_html" => updated_html, "baz_html" => "", "cached_markdown_version" => CacheMarkdownField::CACHE_VERSION)
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 end
it_behaves_like 'with cache version', CacheMarkdownField::CACHE_REDCARPET_VERSION
it_behaves_like 'with cache version', CacheMarkdownField::CACHE_COMMONMARK_VERSION
end end
describe '#banzai_render_context' do describe '#banzai_render_context' do
...@@ -299,7 +346,7 @@ describe CacheMarkdownField do ...@@ -299,7 +346,7 @@ describe CacheMarkdownField do
expect(thing.foo_html).to eq(updated_html) expect(thing.foo_html).to eq(updated_html)
expect(thing.baz_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 end
...@@ -319,7 +366,7 @@ describe CacheMarkdownField do ...@@ -319,7 +366,7 @@ describe CacheMarkdownField do
expect(thing.foo_html).to eq(updated_html) expect(thing.foo_html).to eq(updated_html)
expect(thing.baz_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 end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment