summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJan Provaznik <jprovaznik@gitlab.com>2019-01-31 12:28:31 +0100
committerJan Provaznik <jprovaznik@gitlab.com>2019-02-06 19:46:59 +0100
commit433bcf9b0439771b1e2661a62fec115e44922232 (patch)
tree9587b4fb6cd4b87211b9f9a16d9e4c211a7d273f
parentd0187de202d94fa445a28c347b7a54dbf09a22a8 (diff)
downloadgitlab-ce-433bcf9b0439771b1e2661a62fec115e44922232.tar.gz
Add local markdown versionlocal-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).
-rw-r--r--app/helpers/application_settings_helper.rb3
-rw-r--r--app/models/application_setting.rb7
-rw-r--r--app/models/concerns/cache_markdown_field.rb23
-rw-r--r--changelogs/unreleased/local-markdown-version-bkp3.yml5
-rw-r--r--db/migrate/20190130091630_add_local_cached_markdown_version.rb11
-rw-r--r--db/schema.rb1
-rw-r--r--doc/administration/index.md1
-rw-r--r--doc/administration/invalidate_markdown_cache.md16
-rw-r--r--doc/api/settings.md7
-rw-r--r--lib/api/settings.rb1
-rw-r--r--spec/features/snippets/show_spec.rb8
-rw-r--r--spec/javascripts/notes/mock_data.js1
-rw-r--r--spec/lib/banzai/object_renderer_spec.rb2
-rw-r--r--spec/models/application_setting_spec.rb7
-rw-r--r--spec/models/concerns/cache_markdown_field_spec.rb55
-rw-r--r--spec/models/resource_label_event_spec.rb6
-rw-r--r--spec/requests/api/settings_spec.rb4
17 files changed, 118 insertions, 40 deletions
diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb
index c8e4e2e3df9..5ba90a272c9 100644
--- a/app/helpers/application_settings_helper.rb
+++ b/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
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 88746375c67..c658211d12d 100644
--- a/app/models/application_setting.rb
+++ b/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
diff --git a/app/models/concerns/cache_markdown_field.rb b/app/models/concerns/cache_markdown_field.rb
index 5fa6f79bdaa..1a8570b80c3 100644
--- a/app/models/concerns/cache_markdown_field.rb
+++ b/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
diff --git a/changelogs/unreleased/local-markdown-version-bkp3.yml b/changelogs/unreleased/local-markdown-version-bkp3.yml
new file mode 100644
index 00000000000..ce5bff6ae6b
--- /dev/null
+++ b/changelogs/unreleased/local-markdown-version-bkp3.yml
@@ -0,0 +1,5 @@
+---
+title: Allow admins to invalidate markdown texts by setting local markdown version.
+merge_request:
+author:
+type: added
diff --git a/db/migrate/20190130091630_add_local_cached_markdown_version.rb b/db/migrate/20190130091630_add_local_cached_markdown_version.rb
new file mode 100644
index 00000000000..00570e6458c
--- /dev/null
+++ b/db/migrate/20190130091630_add_local_cached_markdown_version.rb
@@ -0,0 +1,11 @@
+# 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
diff --git a/db/schema.rb b/db/schema.rb
index 4b6e4992056..095c2fbab4a 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -168,6 +168,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 ["usage_stats_set_by_user_id"], name: "index_application_settings_on_usage_stats_set_by_user_id", using: :btree
end
diff --git a/doc/administration/index.md b/doc/administration/index.md
index 0b673d61139..38f746ca5bc 100644
--- a/doc/administration/index.md
+++ b/doc/administration/index.md
@@ -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.
- [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
diff --git a/doc/administration/invalidate_markdown_cache.md b/doc/administration/invalidate_markdown_cache.md
new file mode 100644
index 00000000000..ad64cb077c1
--- /dev/null
+++ b/doc/administration/invalidate_markdown_cache.md
@@ -0,0 +1,16 @@
+# 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>
+```
diff --git a/doc/api/settings.md b/doc/api/settings.md
index c329e3cdf24..287e48c2a42 100644
--- a/doc/api/settings.md
+++ b/doc/api/settings.md
@@ -61,7 +61,8 @@ Example response:
"terms": "Hello world!",
"performance_bar_allowed_group_id": 42,
"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:
"terms": "Hello world!",
"performance_bar_allowed_group_id": 42,
"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.
| `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. |
diff --git a/lib/api/settings.rb b/lib/api/settings.rb
index 95371961398..b16faffe335 100644
--- a/lib/api/settings.rb
+++ b/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",
diff --git a/spec/features/snippets/show_spec.rb b/spec/features/snippets/show_spec.rb
index eb974c7c7fd..74fdfcf492e 100644
--- a/spec/features/snippets/show_spec.rb
+++ b/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
diff --git a/spec/javascripts/notes/mock_data.js b/spec/javascripts/notes/mock_data.js
index 7ae45c40c28..348743081eb 100644
--- a/spec/javascripts/notes/mock_data.js
+++ b/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 = {
diff --git a/spec/lib/banzai/object_renderer_spec.rb b/spec/lib/banzai/object_renderer_spec.rb
index 209a547c3b3..3b52f6666d0 100644
--- a/spec/lib/banzai/object_renderer_spec.rb
+++ b/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
diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb
index 96aa9a82b71..789e14e8a20 100644
--- a/spec/models/application_setting_spec.rb
+++ b/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)
diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb
index 29197ef372e..447279f19a8 100644
--- a/spec/models/concerns/cache_markdown_field_spec.rb
+++ b/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,21 +178,35 @@ 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 version is too late' do
- thing.cached_markdown_version += 1
+ 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 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
+ 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
end
@@ -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
diff --git a/spec/models/resource_label_event_spec.rb b/spec/models/resource_label_event_spec.rb
index 3797960ac3d..7eeb2fae57d 100644
--- a/spec/models/resource_label_event_spec.rb
+++ b/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
diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb
index 45fb1562e84..f33eb5b9e02 100644
--- a/spec/requests/api/settings_spec.rb
+++ b/spec/requests/api/settings_spec.rb
@@ -64,7 +64,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)
@@ -90,6 +91,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