Add local markdown version

Cached markdown version is composed both from global and local
markdown version. This allows admins to bump version locally when
needed (e.g. when external URL is changed).

app/helpers/application_settings_helper.rb

@@ -231,7 +231,8 @@ module ApplicationSettingsHelper
       :web_ide_clientside_preview_enabled,
       :diff_max_patch_bytes,
       :commit_email_hostname,
-      :protected_ci_variables
+      :protected_ci_variables,
+      :local_markdown_version
     ]
   end
 

app/models/application_setting.rb

@@ -193,6 +193,10 @@ class ApplicationSetting < ActiveRecord::Base
             allow_nil: true,
             numericality: { only_integer: true, greater_than_or_equal_to: 1.day.seconds }
 
+  validates :local_markdown_version,
+            allow_nil: true,
+            numericality: { only_integer: true, greater_than_or_equal_to: 0, less_than: 65536 }
+
   SUPPORTED_KEY_TYPES.each do |type|
     validates :"#{type}_key_restriction", presence: true, key_restriction: { type: type }
   end
@@ -303,7 +307,8 @@ class ApplicationSetting < ActiveRecord::Base
       usage_stats_set_by_user_id: nil,
       diff_max_patch_bytes: Gitlab::Git::Diff::DEFAULT_MAX_PATCH_BYTES,
       commit_email_hostname: default_commit_email_hostname,
-      protected_ci_variables: false
+      protected_ci_variables: false,
+      local_markdown_version: 0
     }
   end
 

app/models/concerns/cache_markdown_field.rb

@@ -115,7 +115,28 @@ module CacheMarkdownField
   end
 
   def latest_cached_markdown_version
-    CacheMarkdownField::CACHE_COMMONMARK_VERSION
+    @latest_cached_markdown_version ||= (CacheMarkdownField::CACHE_COMMONMARK_VERSION << 16) | local_version
+  end
+
+  def local_version
+    # because local_markdown_version is stored in application_settings which
+    # uses cached_markdown_version too, we check explicitly to avoid
+    # endless loop
+    return local_markdown_version if has_attribute?(:local_markdown_version)
+
+    settings = Gitlab::CurrentSettings.current_application_settings
+
+    # Following migrations are not properly isolated and
+    # use real models (by calling .ghost method), in these migrations
+    # local_markdown_version attribute doesn't exist yet, so we
+    # use a default value:
+    # db/migrate/20170825104051_migrate_issues_to_ghost_user.rb
+    # db/migrate/20171114150259_merge_requests_author_id_foreign_key.rb
+    if settings.respond_to?(:local_markdown_version)
+      settings.local_markdown_version
+    else
+      0
+    end
   end
 
   included do

changelogs/unreleased/local-markdown-version-bkp3.yml

+---
+title: Allow admins to invalidate markdown texts by setting local markdown version.
+merge_request:
+author:
+type: added

db/migrate/20190130091630_add_local_cached_markdown_version.rb

+# frozen_string_literal: true
+
+class AddLocalCachedMarkdownVersion < ActiveRecord::Migration[5.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    add_column :application_settings, :local_markdown_version, :integer, default: 0, null: false
+  end
+end

db/schema.rb

@@ -214,6 +214,7 @@ ActiveRecord::Schema.define(version: 20190131122559) do
     t.string "commit_email_hostname"
     t.boolean "protected_ci_variables", default: false, null: false
     t.string "runners_registration_token_encrypted"
+    t.integer "local_markdown_version", default: 0, null: false
     t.index ["custom_project_templates_group_id"], name: "index_application_settings_on_custom_project_templates_group_id", using: :btree
     t.index ["file_template_project_id"], name: "index_application_settings_on_file_template_project_id", using: :btree
     t.index ["usage_stats_set_by_user_id"], name: "index_application_settings_on_usage_stats_set_by_user_id", using: :btree

doc/administration/index.md

@@ -75,6 +75,7 @@ Learn how to install, configure, update, and maintain your GitLab instance.
   - [Backup and restore](../raketasks/backup_restore.md): Backup and restore your GitLab instance.
 - [Operations](operations/index.md): Keeping GitLab up and running (clean up Redis sessions, moving repositories, Sidekiq MemoryKiller, Unicorn).
 - [Restart GitLab](restart_gitlab.md): Learn how to restart GitLab and its components.
+- [Invalidate markdown cache](invalidate_markdown_cache.md): Invalidate any cached markdown.
 
 #### Updating GitLab
 

doc/administration/invalidate_markdown_cache.md

+# Invalidate Markdown Cache
+
+For performance reasons, GitLab caches the HTML version of markdown text
+(e.g. issue and merge request descriptions, comments). It's possible
+that these cached versions become outdated, for example
+when the `external_url` configuration option is changed - causing links
+in the cached text to refer to the old URL.
+
+To avoid this problem, the administrator can invalidate the existing cache by
+increasing the `local_markdown_version` setting in application settings.  This can
+be done by [changing the application settings through
+the API](../api/settings.md#change-application-settings):
+
+```bash
+curl --request PUT --header "PRIVATE-TOKEN: <your_access_token>" https://gitlab.example.com/api/v4/application/settings?local_markdown_version=<increased_number>
+```

doc/api/settings.md

@@ -61,8 +61,9 @@ Example response:
    "terms": "Hello world!",
    "performance_bar_allowed_group_id": 42,
    "instance_statistics_visibility_private": false,
-   "user_show_add_ssh_key_message": true
-   "file_template_project_id": 1
+   "user_show_add_ssh_key_message": true,
+   "file_template_project_id": 1,
+   "local_markdown_version": 0
 }
 ```
 
@@ -122,8 +123,9 @@ Example response:
   "terms": "Hello world!",
   "performance_bar_allowed_group_id": 42,
   "instance_statistics_visibility_private": false,
-  "user_show_add_ssh_key_message": true
-  "file_template_project_id": 1
+  "user_show_add_ssh_key_message": true,
+  "file_template_project_id": 1,
+  "local_markdown_version": 0
 }
 ```
 
@@ -272,3 +274,4 @@ are listed in the descriptions of the relevant settings.
 | `user_oauth_applications`                | boolean          | no                                   | Allow users to register any application to use GitLab as an OAuth provider. |
 | `user_show_add_ssh_key_message`          | boolean          | no                                   | When set to `false` disable the "You won't be able to pull or push project code via SSH" warning shown to users with no uploaded SSH key. |
 | `version_check_enabled`                  | boolean          | no                                   | Let GitLab inform you when an update is available. |
+| `local_markdown_version`                 | integer          | no                                   | Increase this value when any cached markdown should be invalidated. |

lib/api/settings.rb

@@ -121,6 +121,7 @@ module API
       optional :terminal_max_session_time, type: Integer, desc: 'Maximum time for web terminal websocket connection (in seconds). Set to 0 for unlimited time.'
       optional :usage_ping_enabled, type: Boolean, desc: 'Every week GitLab will report license usage back to GitLab, Inc.'
       optional :instance_statistics_visibility_private, type: Boolean, desc: 'When set to `true` Instance statistics will only be available to admins'
+      optional :local_markdown_version, type: Integer, desc: "Local markdown version, increase this value when any cached markdown should be invalidated"
 
       ApplicationSetting::SUPPORTED_KEY_TYPES.each do |type|
         optional :"#{type}_key_restriction",

spec/features/snippets/show_spec.rb

@@ -79,14 +79,6 @@ describe 'Snippet', :js do
             expect(page).not_to have_xpath("//ol//li//ul")
           end
         end
-
-        context 'with cached CommonMark html' do
-          let(:snippet) { create(:personal_snippet, :public, file_name: file_name, content: content, cached_markdown_version: CacheMarkdownField::CACHE_COMMONMARK_VERSION) }
-
-          it 'renders correctly' do
-            expect(page).not_to have_xpath("//ol//li//ul")
-          end
-        end
       end
 
       context 'switching to the simple viewer' do

spec/javascripts/notes/mock_data.js

@@ -165,7 +165,6 @@ export const note = {
   report_abuse_path:
     '/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-ce%2Fissues%2F7%23note_546&user_id=1',
   path: '/gitlab-org/gitlab-ce/notes/546',
-  cached_markdown_version: 11,
 };
 
 export const discussionMock = {

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_COMMONMARK_VERSION) }
+  let(:object) { Note.new(note: 'hello', note_html: '<p dir="auto">hello</p>', cached_markdown_version: CacheMarkdownField::CACHE_COMMONMARK_VERSION << 16) }
 
   describe '#render' do
     context 'with cache' do

spec/models/application_setting_spec.rb

@@ -70,6 +70,13 @@ describe ApplicationSetting do
         .is_greater_than(0)
     end
 
+    it do
+      is_expected.to validate_numericality_of(:local_markdown_version)
+        .only_integer
+        .is_greater_than_or_equal_to(0)
+        .is_less_than(65536)
+    end
+
     context 'key restrictions' do
       it 'supports all key types' do
         expect(described_class::SUPPORTED_KEY_TYPES).to contain_exactly(:rsa, :dsa, :ecdsa, :ed25519)

spec/models/concerns/cache_markdown_field_spec.rb

@@ -60,6 +60,10 @@ describe CacheMarkdownField do
         changes_applied
       end
     end
+
+    def has_attribute?(attr_name)
+      attribute_names.include?(attr_name)
+    end
   end
 
   def thing_subclass(new_attr)
@@ -72,8 +76,8 @@ 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_COMMONMARK_VERSION) }
-  let(:cache_version) { CacheMarkdownField::CACHE_COMMONMARK_VERSION }
+  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
@@ -94,11 +98,11 @@ 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_COMMONMARK_VERSION) }
+    it { expect(thing.cached_markdown_version).to eq(cache_version) }
   end
 
   context 'a changed markdown field' do
-    let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version) }
+    let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version - 1) }
 
     before do
       thing.foo = updated_markdown
@@ -139,7 +143,7 @@ describe CacheMarkdownField do
   end
 
   context 'a non-markdown field changed' do
-    let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version) }
+    let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version - 1) }
 
     before do
       thing.bar = 'OK'
@@ -160,7 +164,7 @@ describe CacheMarkdownField do
     end
 
     it { expect(thing.foo_html).to eq(updated_html) }
-    it { expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_COMMONMARK_VERSION) }
+    it { expect(thing.cached_markdown_version).to eq(cache_version) }
   end
 
   describe '#cached_html_up_to_date?' do
@@ -174,24 +178,38 @@ describe CacheMarkdownField do
       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 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 version is too late' do
-      thing.cached_markdown_version += 1
+    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
 
-    it 'returns true when the version is just right' do
+    it 'returns true when the local version is default' do
       thing.cached_markdown_version = cache_version
 
       is_expected.to be_truthy
     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
+
+      is_expected.to be_truthy
+    end
+
     it 'returns false if markdown has been changed but html has not' do
       thing.foo = updated_html
 
@@ -221,14 +239,9 @@ describe CacheMarkdownField do
   describe '#latest_cached_markdown_version' do
     subject { thing.latest_cached_markdown_version }
 
-    it 'returns commonmark version' do
-      thing.cached_markdown_version = CacheMarkdownField::CACHE_COMMONMARK_VERSION_START + 1
-      is_expected.to eq(CacheMarkdownField::CACHE_COMMONMARK_VERSION)
-    end
-
-    it 'returns default version when version is nil' do
+    it 'returns default version' do
       thing.cached_markdown_version = nil
-      is_expected.to eq(CacheMarkdownField::CACHE_COMMONMARK_VERSION)
+      is_expected.to eq(cache_version)
     end
   end
 
@@ -255,7 +268,7 @@ describe CacheMarkdownField do
       thing.cached_markdown_version = nil
       thing.refresh_markdown_cache
 
-      expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_COMMONMARK_VERSION)
+      expect(thing.cached_markdown_version).to eq(cache_version)
     end
   end
 
@@ -336,7 +349,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_COMMONMARK_VERSION)
+        expect(thing.cached_markdown_version).to eq(cache_version)
       end
     end
 
@@ -356,7 +369,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_COMMONMARK_VERSION)
+        expect(thing.cached_markdown_version).to eq(cache_version)
       end
     end
   end

spec/models/resource_label_event_spec.rb

@@ -81,14 +81,14 @@ RSpec.describe ResourceLabelEvent, type: :model do
       expect(subject.outdated_markdown?).to be true
     end
 
-    it 'returns true markdown is outdated' do
-      subject.attributes = { cached_markdown_version: 0 }
+    it 'returns true if markdown is outdated' do
+      subject.attributes = { cached_markdown_version: ((CacheMarkdownField::CACHE_COMMONMARK_VERSION - 1) << 16) | 0 }
 
       expect(subject.outdated_markdown?).to be true
     end
 
     it 'returns false if label and reference are set' do
-      subject.attributes = { reference: 'whatever', cached_markdown_version: CacheMarkdownField::CACHE_COMMONMARK_VERSION }
+      subject.attributes = { reference: 'whatever', cached_markdown_version: CacheMarkdownField::CACHE_COMMONMARK_VERSION << 16 }
 
       expect(subject.outdated_markdown?).to be false
     end

spec/requests/api/settings_spec.rb

@@ -65,7 +65,8 @@ describe API::Settings, 'Settings' do
             performance_bar_allowed_group_path: group.full_path,
             instance_statistics_visibility_private: true,
             diff_max_patch_bytes: 150_000,
-            default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE
+            default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE,
+            local_markdown_version: 3
           }
 
         expect(response).to have_gitlab_http_status(200)
@@ -91,6 +92,7 @@ describe API::Settings, 'Settings' do
         expect(json_response['instance_statistics_visibility_private']).to be(true)
         expect(json_response['diff_max_patch_bytes']).to eq(150_000)
         expect(json_response['default_branch_protection']).to eq(Gitlab::Access::PROTECTION_DEV_CAN_MERGE)
+        expect(json_response['local_markdown_version']).to eq(3)
       end
     end