Merge branch 'sh-cache-ci-variables' into 'master'

Memoize loading of CI variables

See merge request gitlab-org/gitlab!26147

app/models/group.rb

@@ -406,11 +406,15 @@ class Group < Namespace
   end
 
   def ci_variables_for(ref, project)
-    list_of_ids = [self] + ancestors
-    variables = Ci::GroupVariable.where(group: list_of_ids)
-    variables = variables.unprotected unless project.protected_for?(ref)
-    variables = variables.group_by(&:group_id)
-    list_of_ids.reverse.flat_map { |group| variables[group.id] }.compact
+    cache_key = "ci_variables_for:group:#{self&.id}:project:#{project&.id}:ref:#{ref}"
+
+    ::Gitlab::SafeRequestStore.fetch(cache_key) do
+      list_of_ids = [self] + ancestors
+      variables = Ci::GroupVariable.where(group: list_of_ids)
+      variables = variables.unprotected unless project.protected_for?(ref)
+      variables = variables.group_by(&:group_id)
+      list_of_ids.reverse.flat_map { |group| variables[group.id] }.compact
+    end
   end
 
   def group_member(user)

app/models/project.rb

@@ -1963,6 +1963,14 @@ class Project < ApplicationRecord
   end
 
   def ci_variables_for(ref:, environment: nil)
+    cache_key = "ci_variables_for:project:#{self&.id}:ref:#{ref}:environment:#{environment}"
+
+    ::Gitlab::SafeRequestStore.fetch(cache_key) do
+      uncached_ci_variables_for(ref: ref, environment: environment)
+    end
+  end
+
+  def uncached_ci_variables_for(ref:, environment: nil)
     result = if protected_for?(ref)
                variables
              else

changelogs/unreleased/sh-cache-ci-variables.yml

+---
+title: Memoize loading of CI variables
+merge_request: 26147
+author:
+type: performance

spec/models/group_spec.rb

@@ -911,6 +911,16 @@ describe Group do
 
     subject { group.ci_variables_for('ref', project) }
 
+    it 'memoizes the result by ref', :request_store do
+      expect(project).to receive(:protected_for?).with('ref').once.and_return(true)
+      expect(project).to receive(:protected_for?).with('other').once.and_return(false)
+
+      2.times do
+        expect(group.ci_variables_for('ref', project)).to contain_exactly(ci_variable, protected_variable)
+        expect(group.ci_variables_for('other', project)).to contain_exactly(ci_variable)
+      end
+    end
+
     shared_examples 'ref is protected' do
       it 'contains all the variables' do
         is_expected.to contain_exactly(ci_variable, protected_variable)

spec/models/project_spec.rb

@@ -2930,6 +2930,19 @@ describe Project do
       end
     end
 
+    it 'memoizes the result by ref and environment', :request_store do
+      scoped_variable = create(:ci_variable, value: 'secret', project: project, environment_scope: 'scoped')
+
+      expect(project).to receive(:protected_for?).with('ref').once.and_return(true)
+      expect(project).to receive(:protected_for?).with('other').twice.and_return(false)
+
+      2.times do
+        expect(project.reload.ci_variables_for(ref: 'ref', environment: 'production')).to contain_exactly(ci_variable, protected_variable)
+        expect(project.reload.ci_variables_for(ref: 'other')).to contain_exactly(ci_variable)
+        expect(project.reload.ci_variables_for(ref: 'other', environment: 'scoped')).to contain_exactly(ci_variable, scoped_variable)
+      end
+    end
+
     context 'when the ref is not protected' do
       before do
         allow(project).to receive(:protected_for?).with('ref').and_return(false)