Add markdown cache columns to the database, but don't use them yet

This commit adds a number of _html columns and, with the exception of Note,
starts updating them whenever the content of their partner fields changes.

Note has a collision with the note_html attr_accessor; that will be fixed later

A background worker for clearing these cache columns is also introduced - use
`rake cache:clear` to set it off. You can clear the database or Redis caches
separately by running `rake cache:clear:db` or `rake cache:clear:redis`,
respectively.

12 files changed, 287 insertions, 16 deletions

diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb
index 873d3fcb5af..331172445e4 100644
--- a/spec/factories/projects.rb
+++ b/spec/factories/projects.rb
@@ -9,6 +9,9 @@ FactoryGirl.define do
     namespace
     creator
 
+    # Behaves differently to nil due to cache_has_external_issue_tracker
+    has_external_issue_tracker false
+
     trait :public do
       visibility_level Gitlab::VisibilityLevel::PUBLIC
     end
@@ -92,6 +95,8 @@ FactoryGirl.define do
   end
 
   factory :redmine_project, parent: :project do
+    has_external_issue_tracker true
+
     after :create do |project|
       project.create_redmine_service(
         active: true,
@@ -105,6 +110,8 @@ FactoryGirl.define do
   end
 
   factory :jira_project, parent: :project do
+    has_external_issue_tracker true
+
     after :create do |project|
       project.create_jira_service(
         active: true,
diff --git a/spec/lib/banzai/renderer_spec.rb b/spec/lib/banzai/renderer_spec.rb
new file mode 100644
index 00000000000..aaa6b12e67e
--- /dev/null
+++ b/spec/lib/banzai/renderer_spec.rb
@@ -0,0 +1,74 @@
+require 'spec_helper'
+
+describe Banzai::Renderer do
+  def expect_render(project = :project)
+    expected_context = { project: project }
+    expect(renderer).to receive(:cacheless_render) { :html }.with(:markdown, expected_context)
+  end
+
+  def expect_cache_update
+    expect(object).to receive(:update_column).with("field_html", :html)
+  end
+
+  def fake_object(*features)
+    markdown = :markdown if features.include?(:markdown)
+    html = :html if features.include?(:html)
+
+    object = double(
+      "object",
+      banzai_render_context: { project: :project },
+      field: markdown,
+      field_html: html
+    )
+
+    allow(object).to receive(:markdown_cache_field_for).with(:field).and_return("field_html")
+    allow(object).to receive(:new_record?).and_return(features.include?(:new))
+    allow(object).to receive(:destroyed?).and_return(features.include?(:destroyed))
+
+    object
+  end
+
+  describe "#render_field" do
+    let(:renderer) { Banzai::Renderer }
+    let(:subject) { renderer.render_field(object, :field) }
+
+    context "with an empty cache" do
+      let(:object) { fake_object(:markdown) }
+      it "caches and returns the result" do
+        expect_render
+        expect_cache_update
+        expect(subject).to eq(:html)
+      end
+    end
+
+    context "with a filled cache" do
+      let(:object) { fake_object(:markdown, :html) }
+
+      it "uses the cache" do
+        expect_render.never
+        expect_cache_update.never
+        should eq(:html)
+      end
+    end
+
+    context "new object" do
+      let(:object) { fake_object(:new, :markdown) }
+
+      it "doesn't cache the result" do
+        expect_render
+        expect_cache_update.never
+        expect(subject).to eq(:html)
+      end
+    end
+
+    context "destroyed object" do
+      let(:object) { fake_object(:destroyed, :markdown) }
+
+      it "doesn't cache the result" do
+        expect_render
+        expect_cache_update.never
+        expect(subject).to eq(:html)
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/import_export/attribute_configuration_spec.rb b/spec/lib/gitlab/import_export/attribute_configuration_spec.rb
index 2e19d590d83..ea65a5dfed1 100644
--- a/spec/lib/gitlab/import_export/attribute_configuration_spec.rb
+++ b/spec/lib/gitlab/import_export/attribute_configuration_spec.rb
@@ -26,10 +26,11 @@ describe 'Import/Export attribute configuration', lib: true do
   it 'has no new columns' do
     relation_names.each do |relation_name|
       relation_class = relation_class_for_name(relation_name)
+      relation_attributes = relation_class.new.attributes.keys
 
       expect(safe_model_attributes[relation_class.to_s]).not_to be_nil, "Expected exported class #{relation_class} to exist in safe_model_attributes"
 
-      current_attributes = parsed_attributes(relation_name, relation_class.attribute_names)
+      current_attributes = parsed_attributes(relation_name, relation_attributes)
       safe_attributes = safe_model_attributes[relation_class.to_s]
       new_attributes = current_attributes - safe_attributes
 
diff --git a/spec/models/abuse_report_spec.rb b/spec/models/abuse_report_spec.rb
index 305f8bc88cc..c4486a32082 100644
--- a/spec/models/abuse_report_spec.rb
+++ b/spec/models/abuse_report_spec.rb
@@ -9,6 +9,10 @@ RSpec.describe AbuseReport, type: :model do
   describe 'associations' do
     it { is_expected.to belong_to(:reporter).class_name('User') }
     it { is_expected.to belong_to(:user) }
+
+    it "aliases reporter to author" do
+      expect(subject.author).to be(subject.reporter)
+    end
   end
 
   describe 'validations' do
diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb
new file mode 100644
index 00000000000..15cd3a7ed70
--- /dev/null
+++ b/spec/models/concerns/cache_markdown_field_spec.rb
@@ -0,0 +1,181 @@
+require 'spec_helper'
+
+describe CacheMarkdownField do
+  CacheMarkdownField::CACHING_CLASSES << "ThingWithMarkdownFields"
+
+  # 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
+    end
+
+    extend ActiveModel::Callbacks
+    define_model_callbacks :save
+
+    include CacheMarkdownField
+    cache_markdown_field :foo
+    cache_markdown_field :baz, pipeline: :single_line
+
+    def self.add_attr(attr_name)
+      self.attribute_names += [attr_name]
+      define_attribute_methods(attr_name)
+      attr_reader(attr_name)
+      define_method("#{attr_name}=") do |val|
+        send("#{attr_name}_will_change!") unless val == send(attr_name)
+        instance_variable_set("@#{attr_name}", val)
+      end
+    end
+
+    [:foo, :foo_html, :bar, :baz, :baz_html].each do |attr_name|
+      add_attr(attr_name)
+    end
+
+    def initialize(*)
+      super
+
+      # Pretend new is load
+      clear_changes_information
+    end
+
+    def save
+      run_callbacks :save do
+        changes_applied
+      end
+    end
+  end
+
+  CacheMarkdownField::CACHING_CLASSES.delete("ThingWithMarkdownFields")
+
+  def thing_subclass(new_attr)
+    Class.new(ThingWithMarkdownFields) { add_attr(new_attr) }
+  end
+
+  let(:markdown) { "`Foo`" }
+  let(:html) { "<p><code>Foo</code></p>" }
+
+  let(:updated_markdown) { "`Bar`" }
+  let(:updated_html) { "<p><code>Bar</code></p>" }
+
+  subject { ThingWithMarkdownFields.new(foo: markdown, foo_html: html) }
+
+  describe ".attributes" do
+    it "excludes cache attributes" do
+      expect(thing_subclass(:qux).new.attributes.keys.sort).to eq(%w[bar baz foo qux])
+    end
+  end
+
+  describe ".cache_markdown_field" do
+    it "refuses to allow untracked classes" do
+      expect { thing_subclass(:qux).__send__(:cache_markdown_field, :qux) }.to raise_error(RuntimeError)
+    end
+  end
+
+  context "an unchanged markdown field" do
+    before do
+      subject.foo = subject.foo
+      subject.save
+    end
+
+    it { expect(subject.foo).to eq(markdown) }
+    it { expect(subject.foo_html).to eq(html) }
+    it { expect(subject.foo_html_changed?).not_to be_truthy }
+  end
+
+  context "a changed markdown field" do
+    before do
+      subject.foo = updated_markdown
+      subject.save
+    end
+
+    it { expect(subject.foo_html).to eq(updated_html) }
+  end
+
+  context "a non-markdown field changed" do
+    before do
+      subject.bar = "OK"
+      subject.save
+    end
+
+    it { expect(subject.bar).to eq("OK") }
+    it { expect(subject.foo).to eq(markdown) }
+    it { expect(subject.foo_html).to eq(html) }
+  end
+
+  describe '#banzai_render_context' do
+    it "sets project to nil if the object lacks a project" do
+      context = subject.banzai_render_context(:foo)
+      expect(context).to have_key(:project)
+      expect(context[:project]).to be_nil
+    end
+
+    it "excludes author if the object lacks an author" do
+      context = subject.banzai_render_context(:foo)
+      expect(context).not_to have_key(:author)
+    end
+
+    it "raises if the context for an unrecognised field is requested" do
+      expect{subject.banzai_render_context(:not_found)}.to raise_error(ArgumentError)
+    end
+
+    it "includes the pipeline" do
+      context = subject.banzai_render_context(:baz)
+      expect(context[:pipeline]).to eq(:single_line)
+    end
+
+    it "returns copies of the context template" do
+      template = subject.cached_markdown_fields[:baz]
+      copy = subject.banzai_render_context(:baz)
+      expect(copy).not_to be(template)
+    end
+
+    context "with a project" do
+      subject { thing_subclass(:project).new(foo: markdown, foo_html: html, project: :project) }
+
+      it "sets the project in the context" do
+        context = subject.banzai_render_context(:foo)
+        expect(context).to have_key(:project)
+        expect(context[:project]).to eq(:project)
+      end
+
+      it "invalidates the cache when project changes" do
+        subject.project = :new_project
+        allow(Banzai::Renderer).to receive(:cacheless_render_field).and_return(updated_html)
+
+        subject.save
+
+        expect(subject.foo_html).to eq(updated_html)
+        expect(subject.baz_html).to eq(updated_html)
+      end
+    end
+
+    context "with an author" do
+      subject { thing_subclass(:author).new(foo: markdown, foo_html: html, author: :author) }
+
+      it "sets the author in the context" do
+        context = subject.banzai_render_context(:foo)
+        expect(context).to have_key(:author)
+        expect(context[:author]).to eq(:author)
+      end
+
+      it "invalidates the cache when author changes" do
+        subject.author = :new_author
+        allow(Banzai::Renderer).to receive(:cacheless_render_field).and_return(updated_html)
+
+        subject.save
+
+        expect(subject.foo_html).to eq(updated_html)
+        expect(subject.baz_html).to eq(updated_html)
+      end
+    end
+  end
+end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index e52d4aaf884..381d14ed21a 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -518,7 +518,7 @@ describe Project, models: true do
   end
 
   describe '#cache_has_external_issue_tracker' do
-    let(:project) { create(:project) }
+    let(:project) { create(:project, has_external_issue_tracker: nil) }
 
     it 'stores true if there is any external_issue_tracker' do
       services = double(:service, external_issue_trackers: [RedmineService.new])
diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb
index ed1bc9271ae..43937a54b2c 100644
--- a/spec/models/service_spec.rb
+++ b/spec/models/service_spec.rb
@@ -238,7 +238,7 @@ describe Service, models: true do
       it "updates the has_external_issue_tracker boolean" do
         expect do
           service.save!
-        end.to change { service.project.has_external_issue_tracker }.from(nil).to(true)
+        end.to change { service.project.has_external_issue_tracker }.from(false).to(true)
       end
     end
 
diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb
index e6bc5296398..f62f6bacbaa 100644
--- a/spec/models/snippet_spec.rb
+++ b/spec/models/snippet_spec.rb
@@ -46,6 +46,13 @@ describe Snippet, models: true do
     end
   end
 
+  describe "#content_html_invalidated?" do
+    let(:snippet) { create(:snippet, content: "md", content_html: "html", file_name: "foo.md") }
+    it "invalidates the HTML cache of content when the filename changes" do
+      expect { snippet.file_name = "foo.rb" }.to change { snippet.content_html_invalidated? }.from(false).to(true)
+    end
+  end
+
   describe '.search' do
     let(:snippet) { create(:snippet) }
 
diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb
index 4a0d727faea..861eb12b94c 100644
--- a/spec/requests/api/projects_spec.rb
+++ b/spec/requests/api/projects_spec.rb
@@ -232,7 +232,7 @@ describe API::API, api: true  do
       post api('/projects', user), project
 
       project.each_pair do |k, v|
-        next if %i{ issues_enabled merge_requests_enabled wiki_enabled }.include?(k)
+        next if %i[has_external_issue_tracker issues_enabled merge_requests_enabled wiki_enabled].include?(k)
         expect(json_response[k.to_s]).to eq(v)
       end
 
@@ -360,7 +360,7 @@ describe API::API, api: true  do
       post api("/projects/user/#{user.id}", admin), project
 
       project.each_pair do |k, v|
-        next if k == :path
+        next if %i[has_external_issue_tracker path].include?(k)
         expect(json_response[k.to_s]).to eq(v)
       end
     end
diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb
index 22991c5bc86..8e3e12114f2 100644
--- a/spec/services/git_push_service_spec.rb
+++ b/spec/services/git_push_service_spec.rb
@@ -448,6 +448,8 @@ describe GitPushService, services: true do
       let(:jira_tracker) { project.create_jira_service if project.jira_service.nil? }
 
       before do
+        # project.create_jira_service doesn't seem to invalidate the cache here
+        project.has_external_issue_tracker = true
         jira_service_settings
 
         WebMock.stub_request(:post, jira_api_transition_url)
diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb
index e49a0d5e553..ee53e110aee 100644
--- a/spec/services/merge_requests/merge_service_spec.rb
+++ b/spec/services/merge_requests/merge_service_spec.rb
@@ -60,7 +60,10 @@ describe MergeRequests::MergeService, services: true do
 
         let(:jira_tracker) { project.create_jira_service }
 
-        before { jira_service_settings }
+        before do
+          project.update_attributes!(has_external_issue_tracker: true)
+          jira_service_settings
+        end
 
         it 'closes issues on JIRA issue tracker' do
           jira_issue = ExternalIssue.new('JIRA-123', project)
diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb
index d1a47ea9b6f..304d4e62396 100644
--- a/spec/services/system_note_service_spec.rb
+++ b/spec/services/system_note_service_spec.rb
@@ -531,12 +531,12 @@ describe SystemNoteService, services: true do
   include JiraServiceHelper
 
   describe 'JIRA integration' do
-    let(:project)    { create(:project) }
+    let(:project)    { create(:jira_project) }
     let(:author)     { create(:user) }
     let(:issue)      { create(:issue, project: project) }
     let(:mergereq)   { create(:merge_request, :simple, target_project: project, source_project: project) }
     let(:jira_issue) { ExternalIssue.new("JIRA-1", project)}
-    let(:jira_tracker) { project.create_jira_service if project.jira_service.nil? }
+    let(:jira_tracker) { project.jira_service }
     let(:commit)     { project.commit }
 
     context 'in JIRA issue tracker' do
@@ -545,10 +545,6 @@ describe SystemNoteService, services: true do
         WebMock.stub_request(:post, jira_api_comment_url)
       end
 
-      after do
-        jira_tracker.destroy!
-      end
-
       describe "new reference" do
         before do
           WebMock.stub_request(:get, jira_api_comment_url).to_return(body: jira_issue_comments)
@@ -578,10 +574,6 @@ describe SystemNoteService, services: true do
           WebMock.stub_request(:get, jira_api_comment_url).to_return(body: jira_issue_comments)
         end
 
-        after do
-          jira_tracker.destroy!
-        end
-
         subject { described_class.cross_reference(jira_issue, issue, author) }
 
         it { is_expected.to eq(jira_status_message) }