diff --git a/app/models/concerns/cache_markdown_field.rb b/app/models/concerns/cache_markdown_field.rb
index 7c7fd88222837fdee0252524c65cd47409f4b2e1..f7b29660e3e8e4e9726011beecdb99cd1ff7d994 100644
--- a/app/models/concerns/cache_markdown_field.rb
+++ b/app/models/concerns/cache_markdown_field.rb
@@ -129,28 +129,14 @@ def updated_cached_html_for(markdown_field)
end
def latest_cached_markdown_version
- @latest_cached_markdown_version ||= (Gitlab::MarkdownCache::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 respond_to?(:has_attribute?) && 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
+ # because local_markdown_version is stored in application_settings which uses
+ # cached_markdown_version too, we check explicitly to avoid an endless loop.
+ local_version = local_markdown_version if respond_to?(:has_attribute?) && has_attribute?(:local_markdown_version)
+
+ # rubocop:disable Gitlab/ModuleWithInstanceVariables -- acceptable use case
+ # See https://docs.gitlab.com/ee/development/module_with_instance_variables.html#acceptable-use
+ @latest_cached_markdown_version ||= Gitlab::MarkdownCache.latest_cached_markdown_version(local_version: local_version)
+ # rubocop:enable Gitlab/ModuleWithInstanceVariables
end
def parent_user
diff --git a/lib/banzai/renderer.rb b/lib/banzai/renderer.rb
index 22b83bfd516c1ca4646f939474a10b052b0a9f8e..2703c12a8b6d842657b316a263d14b4a53776368 100644
--- a/lib/banzai/renderer.rb
+++ b/lib/banzai/renderer.rb
@@ -175,7 +175,11 @@ def self.cacheless_render(text, context = {})
def self.full_cache_key(cache_key, pipeline_name)
return unless cache_key
- ["banzai", *cache_key, pipeline_name || :full]
+ [
+ "banzai",
+ *cache_key, pipeline_name || :full,
+ Gitlab::MarkdownCache.latest_cached_markdown_version(local_version: nil)
+ ]
end
# To map Rails.cache.read_multi results we need to know the Rails.cache.expanded_key.
diff --git a/lib/gitlab/markdown_cache.rb b/lib/gitlab/markdown_cache.rb
index b0b4a85128969bfa65ff49962402817f25c5ba45..e68e45668cc7e17e225ca290e2509d9bea705bae 100644
--- a/lib/gitlab/markdown_cache.rb
+++ b/lib/gitlab/markdown_cache.rb
@@ -13,8 +13,19 @@ module MarkdownCache
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/330313
CACHE_COMMONMARK_VERSION = 33
CACHE_COMMONMARK_VERSION_START = 10
+ CACHE_COMMONMARK_VERSION_SHIFTED = CACHE_COMMONMARK_VERSION << 16
BaseError = Class.new(StandardError)
UnsupportedClassError = Class.new(BaseError)
+
+ # We could be called by a method that is inside the Gitlab::CurrentSettings
+ # object. In this case we need to pass in the local_markdown_version in order
+ # to avoid an infinite loop. See usaage in `app/models/concerns/cache_markdown_field.rb`
+ # Otherwise pass in `nil`
+ def self.latest_cached_markdown_version(local_version:)
+ local_version ||= Gitlab::CurrentSettings.current_application_settings.local_markdown_version
+
+ CACHE_COMMONMARK_VERSION_SHIFTED | local_version
+ end
end
end
diff --git a/spec/lib/banzai/object_renderer_spec.rb b/spec/lib/banzai/object_renderer_spec.rb
index ba065f4a457391845068fc5a5a4eb0231ae3c151..d17855a88e9bf7822fe4e0003948fb4f6ea92be5 100644
--- a/spec/lib/banzai/object_renderer_spec.rb
+++ b/spec/lib/banzai/object_renderer_spec.rb
@@ -13,7 +13,7 @@
)
end
- let(:object) { Note.new(note: 'hello', note_html: '<p dir="auto">hello</p>', cached_markdown_version: Gitlab::MarkdownCache::CACHE_COMMONMARK_VERSION << 16) }
+ let(:object) { Note.new(note: 'hello', note_html: '<p dir="auto">hello</p>', cached_markdown_version: Gitlab::MarkdownCache::CACHE_COMMONMARK_VERSION_SHIFTED) }
describe '#render' do
context 'with cache' do
diff --git a/spec/lib/banzai/renderer_spec.rb b/spec/lib/banzai/renderer_spec.rb
index b34bdb0a2be1586e64bb0e90f9a13e4f1b4c5cde..c9eacb71507f05926bcaad6a7e5cc493614c2c55 100644
--- a/spec/lib/banzai/renderer_spec.rb
+++ b/spec/lib/banzai/renderer_spec.rb
@@ -151,6 +151,26 @@ def fake_cacheless_object
end
end
+ describe '#full_cache_key' do
+ it 'returns nil when no cache_key' do
+ expect(described_class.full_cache_key(nil, :full)).to be_nil
+ end
+
+ it 'returns a valid full cache key' do
+ cache_key = described_class.full_cache_key('my_cache_key', :emoji)
+ markdown_version = Gitlab::MarkdownCache.latest_cached_markdown_version(local_version: nil)
+
+ expect(cache_key).to eq ["banzai", "my_cache_key", :emoji, markdown_version]
+ end
+
+ it 'pipeline name defaults to :full' do
+ cache_key = described_class.full_cache_key('my_cache_key', nil)
+ markdown_version = Gitlab::MarkdownCache.latest_cached_markdown_version(local_version: nil)
+
+ expect(cache_key).to eq ["banzai", "my_cache_key", :full, markdown_version]
+ end
+ end
+
describe 'instrumentation in render_result' do
it 'calculates pipeline timing' do
expect(ActiveSupport::Notifications).to receive(:monotonic_subscribe).with('call_filter.html_pipeline')
diff --git a/spec/lib/gitlab/markdown_cache/active_record/extension_spec.rb b/spec/lib/gitlab/markdown_cache/active_record/extension_spec.rb
index 081f1f4057b706468e2dcfe738955944d5dcb7ef..3b078c5472061440d38d0b72de0e53c872279ba8 100644
--- a/spec/lib/gitlab/markdown_cache/active_record/extension_spec.rb
+++ b/spec/lib/gitlab/markdown_cache/active_record/extension_spec.rb
@@ -18,7 +18,7 @@
end
end
- let(:cache_version) { Gitlab::MarkdownCache::CACHE_COMMONMARK_VERSION << 16 }
+ let(:cache_version) { Gitlab::MarkdownCache::CACHE_COMMONMARK_VERSION_SHIFTED }
let(:thing) do
klass.create!(
project_id: project.id, namespace_id: project.project_namespace_id,
diff --git a/spec/lib/gitlab/markdown_cache/redis/extension_spec.rb b/spec/lib/gitlab/markdown_cache/redis/extension_spec.rb
index 529b8745ee8fd1aa35a06b9046279e395d4170d2..4da8933d75fff4adad60d6d523c8c6e07720f65e 100644
--- a/spec/lib/gitlab/markdown_cache/redis/extension_spec.rb
+++ b/spec/lib/gitlab/markdown_cache/redis/extension_spec.rb
@@ -22,7 +22,7 @@ def cache_key
end
end
- let(:cache_version) { Gitlab::MarkdownCache::CACHE_COMMONMARK_VERSION << 16 }
+ let(:cache_version) { Gitlab::MarkdownCache::CACHE_COMMONMARK_VERSION_SHIFTED }
let(:thing) { klass.new(title: "`Hello`", description: "`World`") }
let(:expected_cache_key) { "markdown_cache:cache-key" }
diff --git a/spec/lib/gitlab/markdown_cache_spec.rb b/spec/lib/gitlab/markdown_cache_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..ac2aa1ead85e064a3b32148b6481514e07ef6783
--- /dev/null
+++ b/spec/lib/gitlab/markdown_cache_spec.rb
@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::MarkdownCache, feature_category: :markdown do
+ it 'returns proper latest_cached_markdown_version' do
+ stub_application_setting(local_markdown_version: 2)
+
+ expect(described_class.latest_cached_markdown_version(local_version: nil))
+ .to eq described_class::CACHE_COMMONMARK_VERSION_SHIFTED | 2
+ end
+
+ it 'uses passed in local_version' do
+ stub_application_setting(local_markdown_version: 2)
+
+ expect(described_class.latest_cached_markdown_version(local_version: 5))
+ .to eq described_class::CACHE_COMMONMARK_VERSION_SHIFTED | 5
+ end
+end
diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb
index 97e43f3494c53f8bda23cecbfcdf1c8ca61c81df..90d166dbfb68d2ba5438f5a473d2a1312ecdc518 100644
--- a/spec/models/concerns/cache_markdown_field_spec.rb
+++ b/spec/models/concerns/cache_markdown_field_spec.rb
@@ -2,7 +2,7 @@
require 'spec_helper'
-RSpec.describe CacheMarkdownField, :clean_gitlab_redis_cache do
+RSpec.describe CacheMarkdownField, :clean_gitlab_redis_cache, feature_category: :markdown do
let(:ar_class) do
Class.new(ActiveRecord::Base) do
self.table_name = 'issues'
@@ -46,7 +46,7 @@ def cache_key
let(:updated_markdown) { '`Bar`' }
let(:updated_html) { '<p dir="auto"><code>Bar</code></p>' }
- let(:cache_version) { Gitlab::MarkdownCache::CACHE_COMMONMARK_VERSION << 16 }
+ let(:cache_version) { Gitlab::MarkdownCache::CACHE_COMMONMARK_VERSION_SHIFTED }
def thing_subclass(klass, *extra_attributes)
Class.new(klass) { attr_accessor(*extra_attributes) }
diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb
index 14994d90b6cebb8bff6a79e2172317d41f988a85..2c6ac42d26bc4873000df5396047587dd0d2b963 100644
--- a/spec/models/environment_spec.rb
+++ b/spec/models/environment_spec.rb
@@ -2196,7 +2196,9 @@ def create_deployment_with_stop_action(status, pipeline, stop_action_name)
end
it 'caches the freeze periods' do
- expect(Gitlab::SafeRequestStore).to receive(:fetch)
+ allow(Gitlab::SafeRequestStore).to receive(:fetch).and_call_original
+
+ expect(Gitlab::SafeRequestStore).to receive(:fetch).with("project:#{project.id}:freeze_periods_for_environments")
.at_least(:once)
.and_return([freeze_period])
diff --git a/spec/models/resource_label_event_spec.rb b/spec/models/resource_label_event_spec.rb
index 4c1a491c40bf4a9ae300dcb36fb2885718ca96b3..bfcb17626da2041c530ade5d8c5260fbecc0cc7c 100644
--- a/spec/models/resource_label_event_spec.rb
+++ b/spec/models/resource_label_event_spec.rb
@@ -85,13 +85,13 @@
end
it 'returns true if markdown is outdated' do
- subject.attributes = { cached_markdown_version: ((Gitlab::MarkdownCache::CACHE_COMMONMARK_VERSION - 1) << 16) | 0 }
+ subject.attributes = { cached_markdown_version: Gitlab::MarkdownCache::CACHE_COMMONMARK_VERSION_SHIFTED + 1 }
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: Gitlab::MarkdownCache::CACHE_COMMONMARK_VERSION << 16 }
+ subject.attributes = { reference: 'whatever', cached_markdown_version: Gitlab::MarkdownCache::CACHE_COMMONMARK_VERSION_SHIFTED }
expect(subject.outdated_markdown?).to be false
end
diff --git a/spec/support/shared_examples/models/mentionable_shared_examples.rb b/spec/support/shared_examples/models/mentionable_shared_examples.rb
index 5f66e2b31b92c33cd8c25cf177e7fba5c2ee1bd8..cecd9e7d02e40e74bbca4d586334a718639b7f78 100644
--- a/spec/support/shared_examples/models/mentionable_shared_examples.rb
+++ b/spec/support/shared_examples/models/mentionable_shared_examples.rb
@@ -146,7 +146,7 @@
context 'when the markdown cache is stale' do
before do
expect(subject).to receive(:latest_cached_markdown_version).at_least(:once) do
- (Gitlab::MarkdownCache::CACHE_COMMONMARK_VERSION + 1) << 16
+ Gitlab::MarkdownCache::CACHE_COMMONMARK_VERSION_SHIFTED + 1
end
end