Commit 433bcf9b authored by Jan Provaznik's avatar Jan Provaznik

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).
parent d0187de2
...@@ -231,7 +231,8 @@ module ApplicationSettingsHelper ...@@ -231,7 +231,8 @@ module ApplicationSettingsHelper
:web_ide_clientside_preview_enabled, :web_ide_clientside_preview_enabled,
:diff_max_patch_bytes, :diff_max_patch_bytes,
:commit_email_hostname, :commit_email_hostname,
:protected_ci_variables :protected_ci_variables,
:local_markdown_version
] ]
end end
......
...@@ -193,6 +193,10 @@ class ApplicationSetting < ActiveRecord::Base ...@@ -193,6 +193,10 @@ class ApplicationSetting < ActiveRecord::Base
allow_nil: true, allow_nil: true,
numericality: { only_integer: true, greater_than_or_equal_to: 1.day.seconds } 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| SUPPORTED_KEY_TYPES.each do |type|
validates :"#{type}_key_restriction", presence: true, key_restriction: { type: type } validates :"#{type}_key_restriction", presence: true, key_restriction: { type: type }
end end
...@@ -303,7 +307,8 @@ class ApplicationSetting < ActiveRecord::Base ...@@ -303,7 +307,8 @@ class ApplicationSetting < ActiveRecord::Base
usage_stats_set_by_user_id: nil, usage_stats_set_by_user_id: nil,
diff_max_patch_bytes: Gitlab::Git::Diff::DEFAULT_MAX_PATCH_BYTES, diff_max_patch_bytes: Gitlab::Git::Diff::DEFAULT_MAX_PATCH_BYTES,
commit_email_hostname: default_commit_email_hostname, commit_email_hostname: default_commit_email_hostname,
protected_ci_variables: false protected_ci_variables: false,
local_markdown_version: 0
} }
end end
......
...@@ -115,7 +115,28 @@ module CacheMarkdownField ...@@ -115,7 +115,28 @@ module CacheMarkdownField
end end
def latest_cached_markdown_version 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 end
included do included do
......
---
title: Allow admins to invalidate markdown texts by setting local markdown version.
merge_request:
author:
type: added
# 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
...@@ -168,6 +168,7 @@ ActiveRecord::Schema.define(version: 20190131122559) do ...@@ -168,6 +168,7 @@ ActiveRecord::Schema.define(version: 20190131122559) do
t.string "commit_email_hostname" t.string "commit_email_hostname"
t.boolean "protected_ci_variables", default: false, null: false t.boolean "protected_ci_variables", default: false, null: false
t.string "runners_registration_token_encrypted" t.string "runners_registration_token_encrypted"
t.integer "local_markdown_version", default: 0, null: false
t.index ["usage_stats_set_by_user_id"], name: "index_application_settings_on_usage_stats_set_by_user_id", using: :btree t.index ["usage_stats_set_by_user_id"], name: "index_application_settings_on_usage_stats_set_by_user_id", using: :btree
end end
......
...@@ -64,6 +64,7 @@ Learn how to install, configure, update, and maintain your GitLab instance. ...@@ -64,6 +64,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. - [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). - [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. - [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 #### Updating GitLab
......
# 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>
```
...@@ -61,7 +61,8 @@ Example response: ...@@ -61,7 +61,8 @@ Example response:
"terms": "Hello world!", "terms": "Hello world!",
"performance_bar_allowed_group_id": 42, "performance_bar_allowed_group_id": 42,
"instance_statistics_visibility_private": false, "instance_statistics_visibility_private": false,
"user_show_add_ssh_key_message": true "user_show_add_ssh_key_message": true,
"local_markdown_version": 0
} }
``` ```
...@@ -117,7 +118,8 @@ Example response: ...@@ -117,7 +118,8 @@ Example response:
"terms": "Hello world!", "terms": "Hello world!",
"performance_bar_allowed_group_id": 42, "performance_bar_allowed_group_id": 42,
"instance_statistics_visibility_private": false, "instance_statistics_visibility_private": false,
"user_show_add_ssh_key_message": true "user_show_add_ssh_key_message": true,
"local_markdown_version": 0
} }
``` ```
...@@ -235,3 +237,4 @@ are listed in the descriptions of the relevant settings. ...@@ -235,3 +237,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_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. | | `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. | | `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. |
...@@ -121,6 +121,7 @@ module API ...@@ -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 :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 :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 :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| ApplicationSetting::SUPPORTED_KEY_TYPES.each do |type|
optional :"#{type}_key_restriction", optional :"#{type}_key_restriction",
......
...@@ -79,14 +79,6 @@ describe 'Snippet', :js do ...@@ -79,14 +79,6 @@ describe 'Snippet', :js do
expect(page).not_to have_xpath("//ol//li//ul") expect(page).not_to have_xpath("//ol//li//ul")
end end
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 end
context 'switching to the simple viewer' do context 'switching to the simple viewer' do
......
...@@ -165,7 +165,6 @@ export const note = { ...@@ -165,7 +165,6 @@ export const note = {
report_abuse_path: report_abuse_path:
'/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-ce%2Fissues%2F7%23note_546&user_id=1', '/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', path: '/gitlab-org/gitlab-ce/notes/546',
cached_markdown_version: 11,
}; };
export const discussionMock = { export const discussionMock = {
......
...@@ -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_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 describe '#render' do
context 'with cache' do context 'with cache' do
......
...@@ -70,6 +70,13 @@ describe ApplicationSetting do ...@@ -70,6 +70,13 @@ describe ApplicationSetting do
.is_greater_than(0) .is_greater_than(0)
end 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 context 'key restrictions' do
it 'supports all key types' do it 'supports all key types' do
expect(described_class::SUPPORTED_KEY_TYPES).to contain_exactly(:rsa, :dsa, :ecdsa, :ed25519) expect(described_class::SUPPORTED_KEY_TYPES).to contain_exactly(:rsa, :dsa, :ecdsa, :ed25519)
......
...@@ -60,6 +60,10 @@ describe CacheMarkdownField do ...@@ -60,6 +60,10 @@ describe CacheMarkdownField do
changes_applied changes_applied
end end
end end
def has_attribute?(attr_name)
attribute_names.include?(attr_name)
end
end end
def thing_subclass(new_attr) def thing_subclass(new_attr)
...@@ -72,8 +76,8 @@ describe CacheMarkdownField do ...@@ -72,8 +76,8 @@ 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_COMMONMARK_VERSION) } let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version) }
let(:cache_version) { CacheMarkdownField::CACHE_COMMONMARK_VERSION } let(:cache_version) { CacheMarkdownField::CACHE_COMMONMARK_VERSION << 16 }
before do before do
stub_commonmark_sourcepos_disabled stub_commonmark_sourcepos_disabled
...@@ -94,11 +98,11 @@ describe CacheMarkdownField do ...@@ -94,11 +98,11 @@ 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_COMMONMARK_VERSION) } it { expect(thing.cached_markdown_version).to eq(cache_version) }
end end
context 'a changed markdown field' do 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 before do
thing.foo = updated_markdown thing.foo = updated_markdown
...@@ -139,7 +143,7 @@ describe CacheMarkdownField do ...@@ -139,7 +143,7 @@ describe CacheMarkdownField do
end end
context 'a non-markdown field changed' do 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 before do
thing.bar = 'OK' thing.bar = 'OK'
...@@ -160,7 +164,7 @@ describe CacheMarkdownField do ...@@ -160,7 +164,7 @@ 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_COMMONMARK_VERSION) } it { expect(thing.cached_markdown_version).to eq(cache_version) }
end end
describe '#cached_html_up_to_date?' do describe '#cached_html_up_to_date?' do
...@@ -174,21 +178,35 @@ describe CacheMarkdownField do ...@@ -174,21 +178,35 @@ describe CacheMarkdownField do
is_expected.to be_falsy is_expected.to be_falsy
end end
it 'returns false when the version is too early' do it 'returns false when the cached version is too old' do
thing.cached_markdown_version -= 1 thing.cached_markdown_version = cache_version - 1
is_expected.to be_falsy is_expected.to be_falsy
end end
it 'returns false when the version is too late' do it 'returns false when the cached version is in future' do
thing.cached_markdown_version += 1 thing.cached_markdown_version = cache_version + 1
is_expected.to be_falsy is_expected.to be_falsy
end end
it 'returns true when the version is just right' do 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 thing.cached_markdown_version = cache_version
is_expected.to be_falsy
end
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 is_expected.to be_truthy
end end
...@@ -221,14 +239,9 @@ describe CacheMarkdownField do ...@@ -221,14 +239,9 @@ describe CacheMarkdownField do
describe '#latest_cached_markdown_version' do describe '#latest_cached_markdown_version' do
subject { thing.latest_cached_markdown_version } subject { thing.latest_cached_markdown_version }
it 'returns commonmark version' do it 'returns default 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
thing.cached_markdown_version = nil thing.cached_markdown_version = nil
is_expected.to eq(CacheMarkdownField::CACHE_COMMONMARK_VERSION) is_expected.to eq(cache_version)
end end
end end
...@@ -255,7 +268,7 @@ describe CacheMarkdownField do ...@@ -255,7 +268,7 @@ 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_COMMONMARK_VERSION) expect(thing.cached_markdown_version).to eq(cache_version)
end end
end end
...@@ -336,7 +349,7 @@ describe CacheMarkdownField do ...@@ -336,7 +349,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_COMMONMARK_VERSION) expect(thing.cached_markdown_version).to eq(cache_version)
end end
end end
...@@ -356,7 +369,7 @@ describe CacheMarkdownField do ...@@ -356,7 +369,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_COMMONMARK_VERSION) expect(thing.cached_markdown_version).to eq(cache_version)
end end
end end
end end
......
...@@ -81,14 +81,14 @@ RSpec.describe ResourceLabelEvent, type: :model do ...@@ -81,14 +81,14 @@ RSpec.describe ResourceLabelEvent, type: :model do
expect(subject.outdated_markdown?).to be true expect(subject.outdated_markdown?).to be true
end end
it 'returns true markdown is outdated' do it 'returns true if markdown is outdated' do
subject.attributes = { cached_markdown_version: 0 } subject.attributes = { cached_markdown_version: ((CacheMarkdownField::CACHE_COMMONMARK_VERSION - 1) << 16) | 0 }
expect(subject.outdated_markdown?).to be true expect(subject.outdated_markdown?).to be true
end end
it 'returns false if label and reference are set' do 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 expect(subject.outdated_markdown?).to be false
end end
......
...@@ -64,7 +64,8 @@ describe API::Settings, 'Settings' do ...@@ -64,7 +64,8 @@ describe API::Settings, 'Settings' do
performance_bar_allowed_group_path: group.full_path, performance_bar_allowed_group_path: group.full_path,
instance_statistics_visibility_private: true, instance_statistics_visibility_private: true,
diff_max_patch_bytes: 150_000, 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) expect(response).to have_gitlab_http_status(200)
...@@ -90,6 +91,7 @@ describe API::Settings, 'Settings' do ...@@ -90,6 +91,7 @@ describe API::Settings, 'Settings' do
expect(json_response['instance_statistics_visibility_private']).to be(true) expect(json_response['instance_statistics_visibility_private']).to be(true)
expect(json_response['diff_max_patch_bytes']).to eq(150_000) 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['default_branch_protection']).to eq(Gitlab::Access::PROTECTION_DEV_CAN_MERGE)
expect(json_response['local_markdown_version']).to eq(3)
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