Commit c0e6dd6a authored by Nick Thomas's avatar Nick Thomas

Improve cache sharing between cached markdown fields and mentionables

This change makes an item of implicit coupling between the Mentionable
and CachedMarkdownField concerns. Mentionable needs to render the
fields it depends on to extract references; when those fields are also
cached markdown fields, it currently uses that cache to skip the render
step.

For the Commit model, we couldn't share the cache because Mentionable
was looking at safe_message, while CachedMarkdownField was looking at
`title`, `full_title`, and `description`. However, the combination of
the latter two is equivalent to `safe_message`, so we can use them
instead.

This opens up a couple of cans of worms - in particular, we now call
Commit#store_mentions!, which we've never done before. However, it all
seems to work out in the end.

Changelog: performance
parent 85e509c2
......@@ -15,8 +15,6 @@ class Commit
include ActsAsPaginatedDiff
include CacheMarkdownField
attr_mentionable :safe_message, pipeline: :single_line
participant :author
participant :committer
participant :notes_with_associations
......@@ -39,6 +37,10 @@ class Commit
cache_markdown_field :full_title, pipeline: :single_line, limit: 1.kilobyte
cache_markdown_field :description, pipeline: :commit_description, limit: 1.megabyte
# Share the cache used by the markdown fields
attr_mentionable :full_title, pipeline: :single_line, limit: 1.kilobyte
attr_mentionable :description, pipeline: :commit_description, limit: 1.megabyte
class << self
def decorate(commits, container)
commits.map do |commit|
......
......@@ -157,6 +157,9 @@ module CacheMarkdownField
end
def store_mentions!
# We can only store mentions if the mentionable is a database object
return unless self.is_a?(ApplicationRecord)
refs = all_references(self.author)
references = {}
......
......@@ -33,7 +33,7 @@ RSpec.shared_examples 'a mentionable with EE-specific mentions' do
expect(refs).to include(mentioned_epic)
end
it 'creates cross-reference notes' do
it 'creates cross-reference notes', :clean_gitlab_redis_cache do
mentioned_objects = [mentioned_epic]
mentioned_objects.each do |referenced|
......
......@@ -471,16 +471,25 @@ eos
end
it_behaves_like 'a mentionable' do
subject { create(:project, :repository).commit }
subject(:commit) { create(:project, :repository).commit }
let(:author) { create(:user, email: subject.author_email) }
let(:backref_text) { "commit #{subject.id}" }
let(:set_mentionable_text) do
->(txt) { allow(subject).to receive(:safe_message).and_return(txt) }
->(txt) { allow(commit).to receive(:safe_message).and_return(txt) }
end
# Include the subject in the repository stub.
let(:extra_commits) { [subject] }
let(:extra_commits) { [commit] }
it 'uses the CachedMarkdownField cache instead of the Mentionable cache', :use_clean_rails_redis_caching do
expect(commit.title_html).not_to be_present
commit.all_references(project.owner).all
expect(commit.title_html).to be_present
expect(Rails.cache.read("banzai/commit:#{commit.id}/safe_message/single_line")).to be_nil
end
end
describe '#hook_attrs' do
......
......@@ -66,7 +66,7 @@ RSpec.shared_examples 'a mentionable' do
expect(subject.gfm_reference).to eq(backref_text)
end
it "extracts references from its reference property" do
it "extracts references from its reference property", :clean_gitlab_redis_cache do
# De-duplicate and omit itself
refs = subject.referenced_mentionables
expect(refs.size).to eq(6)
......@@ -98,7 +98,7 @@ RSpec.shared_examples 'a mentionable' do
end
end
it 'creates cross-reference notes' do
it 'creates cross-reference notes', :clean_gitlab_redis_cache do
mentioned_objects = [mentioned_issue, mentioned_mr, mentioned_commit,
ext_issue, ext_mr, ext_commit]
......
......@@ -138,7 +138,7 @@ RSpec.describe ProcessCommitWorker do
end
end
describe '#update_issue_metrics' do
describe '#update_issue_metrics', :clean_gitlab_redis_cache do
context 'when commit has issue reference' do
subject(:update_metrics_and_reload) do
-> {
......
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