1 files changed, 138 insertions, 308 deletions

diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb
index 78637ff10c6..0e5fb2b5153 100644
--- a/spec/models/concerns/cache_markdown_field_spec.rb
+++ b/spec/models/concerns/cache_markdown_field_spec.rb
@@ -2,383 +2,213 @@
 
 require 'spec_helper'
 
-describe CacheMarkdownField do
-  # The minimum necessary ActiveModel to test this concern
-  class ThingWithMarkdownFields
-    include ActiveModel::Model
-    include ActiveModel::Dirty
-
-    include ActiveModel::Serialization
-
-    class_attribute :attribute_names
-    self.attribute_names = []
-
-    def attributes
-      attribute_names.each_with_object({}) do |name, hsh|
-        hsh[name.to_s] = send(name)
-      end
+describe CacheMarkdownField, :clean_gitlab_redis_cache do
+  let(:ar_class) do
+    Class.new(ActiveRecord::Base) do
+      self.table_name = 'issues'
+      include CacheMarkdownField
+      cache_markdown_field :title, pipeline: :single_line
+      cache_markdown_field :description
     end
+  end
 
-    extend ActiveModel::Callbacks
-    define_model_callbacks :create, :update
-
-    include CacheMarkdownField
-    cache_markdown_field :foo
-    cache_markdown_field :baz, pipeline: :single_line
-    cache_markdown_field :zoo, whitelisted: true
+  let(:other_class) do
+    Class.new do
+      include CacheMarkdownField
 
-    def self.add_attr(name)
-      self.attribute_names += [name]
-      define_attribute_methods(name)
-      attr_reader(name)
-      define_method("#{name}=") do |value|
-        write_attribute(name, value)
+      def initialize(args = {})
+        @title, @description, @cached_markdown_version = args[:title], args[:description], args[:cached_markdown_version]
+        @title_html, @description_html = args[:title_html], args[:description_html]
+        @author, @project = args[:author], args[:project]
       end
-    end
 
-    add_attr :cached_markdown_version
+      attr_accessor :title, :description, :cached_markdown_version
 
-    [:foo, :foo_html, :bar, :baz, :baz_html, :zoo, :zoo_html].each do |name|
-      add_attr(name)
-    end
-
-    def initialize(*)
-      super
-
-      # Pretend new is load
-      clear_changes_information
-    end
-
-    def read_attribute(name)
-      instance_variable_get("@#{name}")
-    end
-
-    def write_attribute(name, value)
-      send("#{name}_will_change!") unless value == read_attribute(name)
-      instance_variable_set("@#{name}", value)
-    end
+      cache_markdown_field :title, pipeline: :single_line
+      cache_markdown_field :description
 
-    def save
-      run_callbacks :update do
-        changes_applied
+      def cache_key
+        "cache-key"
       end
     end
-
-    def has_attribute?(attr_name)
-      attribute_names.include?(attr_name)
-    end
-  end
-
-  def thing_subclass(new_attr)
-    Class.new(ThingWithMarkdownFields) { add_attr(new_attr) }
   end
 
   let(:markdown) { '`Foo`' }
-  let(:html)     { '<p dir="auto"><code>Foo</code></p>' }
+  let(:html) { '<p data-sourcepos="1:1-1:5" dir="auto"><code>Foo</code></p>' }
 
   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: cache_version) }
-  let(:cache_version) { CacheMarkdownField::CACHE_COMMONMARK_VERSION << 16 }
-
-  before do
-    stub_commonmark_sourcepos_disabled
-  end
+  let(:updated_html) { '<p data-sourcepos="1:1-1:5" dir="auto"><code>Bar</code></p>' }
 
-  describe '.attributes' do
-    it 'excludes cache attributes that is blacklisted by default' do
-      expect(thing.attributes.keys.sort).to eq(%w[bar baz cached_markdown_version foo zoo zoo_html])
-    end
-  end
-
-  context 'an unchanged markdown field' do
-    before do
-      thing.foo = thing.foo
-      thing.save
-    end
+  let(:cache_version) { Gitlab::MarkdownCache::CACHE_COMMONMARK_VERSION << 16 }
 
-    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(cache_version) }
+  def thing_subclass(klass, extra_attribute)
+    Class.new(klass) { attr_accessor(extra_attribute) }
   end
 
-  context 'a changed markdown field' do
-    let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version - 1) }
+  shared_examples 'a class with cached markdown fields' do
+    describe '#cached_html_up_to_date?' do
+      let(:thing) { klass.new(title: markdown, title_html: html, cached_markdown_version: cache_version) }
 
-    before do
-      thing.foo = updated_markdown
-      thing.save
-    end
+      subject { thing.cached_html_up_to_date?(:title) }
 
-    it { expect(thing.foo_html).to eq(updated_html) }
-    it { expect(thing.cached_markdown_version).to eq(cache_version) }
-  end
+      it 'returns false when the version is absent' do
+        thing.cached_markdown_version = nil
 
-  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 'when a markdown field and html field are both changed' do
-    it do
-      expect(thing).not_to receive(:refresh_markdown_cache)
-      thing.foo = '_look over there!_'
-      thing.foo_html = '<em>look over there!</em>'
-      thing.save
-    end
-  end
-
-  context 'a non-markdown field changed' do
-    let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version - 1) }
-
-    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
-
-  context 'version is out of date' do
-    let(:thing) { ThingWithMarkdownFields.new(foo: updated_markdown, foo_html: html, cached_markdown_version: nil) }
-
-    before do
-      thing.save
-    end
-
-    it { expect(thing.foo_html).to eq(updated_html) }
-    it { expect(thing.cached_markdown_version).to eq(cache_version) }
-  end
-
-  describe '#cached_html_up_to_date?' do
-    let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version) }
-
-    subject { thing.cached_html_up_to_date?(:foo) }
-
-    it 'returns false when the version is absent' do
-      thing.cached_markdown_version = nil
-
-      is_expected.to be_falsy
-    end
-
-    it 'returns false when the cached version is too old' do
-      thing.cached_markdown_version = cache_version - 1
-
-      is_expected.to be_falsy
-    end
-
-    it 'returns false when the cached version is in future' do
-      thing.cached_markdown_version = cache_version + 1
-
-      is_expected.to be_falsy
-    end
-
-    it 'returns false when the local version was bumped' do
-      allow(Gitlab::CurrentSettings.current_application_settings).to receive(:local_markdown_version).and_return(2)
-      thing.cached_markdown_version = cache_version
-
-      is_expected.to be_falsy
-    end
+        is_expected.to be_falsy
+      end
 
-    it 'returns true when the local version is default' do
-      thing.cached_markdown_version = cache_version
+      it 'returns false when the version is too early' do
+        thing.cached_markdown_version -= 1
 
-      is_expected.to be_truthy
-    end
+        is_expected.to be_falsy
+      end
 
-    it 'returns true when the cached version is just right' do
-      allow(Gitlab::CurrentSettings.current_application_settings).to receive(:local_markdown_version).and_return(2)
-      thing.cached_markdown_version = cache_version + 2
+      it 'returns false when the version is too late' do
+        thing.cached_markdown_version += 1
 
-      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 false when the local version was bumped' do
+        allow(Gitlab::CurrentSettings.current_application_settings).to receive(:local_markdown_version).and_return(2)
+        thing.cached_markdown_version = cache_version
 
-      is_expected.to be_falsy
-    end
+        is_expected.to be_falsy
+      end
 
-    it 'returns true if markdown has not been changed but html has' do
-      thing.foo_html = updated_html
+      it 'returns true when the local version is default' do
+        thing.cached_markdown_version = cache_version
 
-      is_expected.to be_truthy
-    end
+        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
+      it 'returns true when the cached version is just right' do
+        allow(Gitlab::CurrentSettings.current_application_settings).to receive(:local_markdown_version).and_return(2)
+        thing.cached_markdown_version = cache_version + 2
 
-      is_expected.to be_truthy
+        is_expected.to be_truthy
+      end
     end
 
-    it 'returns false if the markdown field is set but the html is not' do
-      thing.foo_html = nil
+    describe '#latest_cached_markdown_version' do
+      let(:thing) { klass.new }
+      subject { thing.latest_cached_markdown_version }
 
-      is_expected.to be_falsy
+      it 'returns default version' do
+        thing.cached_markdown_version = nil
+        is_expected.to eq(cache_version)
+      end
     end
-  end
-
-  describe '#latest_cached_markdown_version' do
-    subject { thing.latest_cached_markdown_version }
 
-    it 'returns default version' do
-      thing.cached_markdown_version = nil
-      is_expected.to eq(cache_version)
-    end
-  end
+    describe '#refresh_markdown_cache' do
+      let(:thing) { klass.new(description: markdown, description_html: html, cached_markdown_version: cache_version) }
 
-  describe '#refresh_markdown_cache' do
-    before do
-      thing.foo = updated_markdown
-    end
+      before do
+        thing.description = updated_markdown
+      end
 
-    it 'fills all html fields' do
-      thing.refresh_markdown_cache
+      it 'fills all html fields' do
+        thing.refresh_markdown_cache
 
-      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
+        expect(thing.description_html).to eq(updated_html)
+      end
 
-    it 'does not save the result' do
-      expect(thing).not_to receive(:update_columns)
+      it 'does not save the result' do
+        expect(thing).not_to receive(:save_markdown)
 
-      thing.refresh_markdown_cache
-    end
+        thing.refresh_markdown_cache
+      end
 
-    it 'updates the markdown cache version' do
-      thing.cached_markdown_version = nil
-      thing.refresh_markdown_cache
+      it 'updates the markdown cache version' do
+        thing.cached_markdown_version = nil
+        thing.refresh_markdown_cache
 
-      expect(thing.cached_markdown_version).to eq(cache_version)
+        expect(thing.cached_markdown_version).to eq(cache_version)
+      end
     end
-  end
-
-  describe '#refresh_markdown_cache!' do
-    let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version) }
 
-    before do
-      thing.foo = updated_markdown
-    end
+    describe '#refresh_markdown_cache!' do
+      let(:thing) { klass.new(description: markdown, description_html: html, cached_markdown_version: cache_version) }
 
-    it 'fills all html fields' do
-      thing.refresh_markdown_cache!
+      before do
+        thing.description = 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.description_html).to eq(updated_html)
+      end
 
-      thing.refresh_markdown_cache!
-    end
+      it 'saves the changes' do
+        expect(thing)
+          .to receive(:save_markdown)
+          .with("description_html" => updated_html, "title_html" => "", "cached_markdown_version" => cache_version)
 
-    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" => "",
-          "zoo_html" => "",
-          "cached_markdown_version" => cache_version
-        )
-
-      thing.refresh_markdown_cache!
+        thing.refresh_markdown_cache!
+      end
     end
-  end
 
-  describe '#banzai_render_context' do
-    subject(:context) { thing.banzai_render_context(:foo) }
+    describe '#banzai_render_context' do
+      let(:thing) { klass.new(title: markdown, title_html: html, cached_markdown_version: cache_version) }
+      subject(:context) { thing.banzai_render_context(:title) }
 
-    it 'sets project to nil if the object lacks a project' do
-      is_expected.to have_key(:project)
-      expect(context[:project]).to be_nil
-    end
+      it 'sets project to nil if the object lacks a project' do
+        is_expected.to have_key(:project)
+        expect(context[:project]).to be_nil
+      end
 
-    it 'excludes author if the object lacks an author' do
-      is_expected.not_to have_key(:author)
-    end
+      it 'excludes author if the object lacks an author' do
+        is_expected.not_to have_key(:author)
+      end
 
-    it 'raises if the context for an unrecognised field is requested' do
-      expect { thing.banzai_render_context(:not_found) }.to raise_error(ArgumentError)
-    end
+      it 'raises if the context for an unrecognised field is requested' do
+        expect { thing.banzai_render_context(:not_found) }.to raise_error(ArgumentError)
+      end
 
-    it 'includes the pipeline' do
-      baz = thing.banzai_render_context(:baz)
+      it 'includes the pipeline' do
+        title_context = thing.banzai_render_context(:title)
 
-      expect(baz[:pipeline]).to eq(:single_line)
-    end
+        expect(title_context[:pipeline]).to eq(:single_line)
+      end
 
-    it 'returns copies of the context template' do
-      template = thing.cached_markdown_fields[:baz]
-      copy = thing.banzai_render_context(:baz)
+      it 'returns copies of the context template' do
+        template = thing.cached_markdown_fields[:description]
+        copy = thing.banzai_render_context(:description)
 
-      expect(copy).not_to be(template)
-    end
+        expect(copy).not_to be(template)
+      end
 
-    context 'with a project' do
-      let(:project) { create(:project, group: create(:group)) }
-      let(:thing) { thing_subclass(:project).new(foo: markdown, foo_html: html, project: project) }
+      context 'with a project' do
+        let(:project) { build(:project, group: create(:group)) }
+        let(:thing) { thing_subclass(klass, :project).new(title: markdown, title_html: html, project: project) }
 
-      it 'sets the project in the context' do
-        is_expected.to have_key(:project)
-        expect(context[:project]).to eq(project)
+        it 'sets the project in the context' do
+          is_expected.to have_key(:project)
+          expect(context[:project]).to eq(project)
+        end
       end
 
-      it 'invalidates the cache when project changes' do
-        thing.project = :new_project
-        allow(Banzai::Renderer).to receive(:cacheless_render_field).and_return(updated_html)
-
-        thing.save
+      context 'with an author' do
+        let(:thing) { thing_subclass(klass, :author).new(title: markdown, title_html: html, author: :author_value) }
 
-        expect(thing.foo_html).to eq(updated_html)
-        expect(thing.baz_html).to eq(updated_html)
-        expect(thing.cached_markdown_version).to eq(cache_version)
+        it 'sets the author in the context' do
+          is_expected.to have_key(:author)
+          expect(context[:author]).to eq(:author_value)
+        end
       end
     end
+  end
 
-    context 'with an author' do
-      let(:thing) { thing_subclass(:author).new(foo: markdown, foo_html: html, author: :author_value) }
-
-      it 'sets the author in the context' do
-        is_expected.to have_key(:author)
-        expect(context[:author]).to eq(:author_value)
-      end
+  context 'for Active record classes' do
+    let(:klass) { ar_class }
 
-      it 'invalidates the cache when author changes' do
-        thing.author = :new_author
-        allow(Banzai::Renderer).to receive(:cacheless_render_field).and_return(updated_html)
+    it_behaves_like 'a class with cached markdown fields'
+  end
 
-        thing.save
+  context 'for other classes' do
+    let(:klass) { other_class }
 
-        expect(thing.foo_html).to eq(updated_html)
-        expect(thing.baz_html).to eq(updated_html)
-        expect(thing.cached_markdown_version).to eq(cache_version)
-      end
-    end
+    it_behaves_like 'a class with cached markdown fields'
   end
 end