Merge branch '271242_memoize_merge_request_policy' into 'master'

Optimize merge request permission check for references

See merge request gitlab-org/gitlab!61591

changelogs/unreleased/271242_memoize_merge_request_policy.yml

+---
+title: Optimize merge request permission check for references
+merge_request: 61591
+author:
+type: performance

lib/banzai/reference_parser/merge_request_parser.rb

@@ -3,6 +3,8 @@
 module Banzai
   module ReferenceParser
     class MergeRequestParser < IssuableParser
+      include Gitlab::Utils::StrongMemoize
+
       self.reference_type = :merge_request
 
       def records_for_nodes(nodes)
@@ -27,6 +29,16 @@ module Banzai
           self.class.data_attribute
         )
       end
+
+      def can_read_reference?(user, merge_request)
+        memo = strong_memoize(:can_read_reference) { {} }
+
+        project_id = merge_request.project_id
+
+        return memo[project_id] if memo.key?(project_id)
+
+        memo[project_id] = can?(user, :read_merge_request_iid, merge_request.project)
+      end
     end
   end
 end

spec/lib/banzai/reference_parser/merge_request_parser_spec.rb

@@ -8,7 +8,7 @@ RSpec.describe Banzai::ReferenceParser::MergeRequestParser do
   let(:user) { create(:user) }
   let(:project) { create(:project, :public) }
   let(:merge_request) { create(:merge_request, source_project: project) }
-  subject { described_class.new(Banzai::RenderContext.new(merge_request.target_project, user)) }
+  subject(:parser) { described_class.new(Banzai::RenderContext.new(merge_request.target_project, user)) }
 
   let(:link) { empty_html_link }
 
@@ -65,4 +65,49 @@ RSpec.describe Banzai::ReferenceParser::MergeRequestParser do
 
     it_behaves_like 'no N+1 queries'
   end
+
+  describe '#can_read_reference?' do
+    subject { parser.can_read_reference?(user, merge_request) }
+
+    it { is_expected.to be_truthy }
+
+    context 'when merge request belongs to the private project' do
+      let(:project) { create(:project, :private) }
+
+      it 'prevents user from reading merge request references' do
+        is_expected.to be_falsey
+      end
+
+      context 'when user has access to the project' do
+        before do
+          project.add_developer(user)
+        end
+
+        it { is_expected.to be_truthy }
+      end
+    end
+
+    context 'with memoization' do
+      context 'when project is the same' do
+        it 'calls #can? only once' do
+          expect(parser).to receive(:can?).once
+
+          2.times { parser.can_read_reference?(user, merge_request) }
+        end
+      end
+
+      context 'when merge requests belong to different projects' do
+        it 'calls #can? for each project' do
+          expect(parser).to receive(:can?).twice
+
+          another_merge_request = create(:merge_request)
+
+          2.times do
+            parser.can_read_reference?(user, merge_request)
+            parser.can_read_reference?(user, another_merge_request)
+          end
+        end
+      end
+    end
+  end
 end