Merge branch 'refactoring-build-dependencies' into 'master'

Refactor Ci::BuilDependencies methods

See merge request gitlab-org/gitlab!59893

app/models/ci/build_dependencies.rb

@@ -14,15 +14,33 @@ module Ci
       (local + cross_pipeline + cross_project).uniq
     end
 
+    def invalid_local
+      local.reject(&:valid_dependency?)
+    end
+
+    def valid?
+      valid_local? && valid_cross_pipeline? && valid_cross_project?
+    end
+
+    private
+
+    # Dependencies can only be of Ci::Build type because only builds
+    # can create artifacts
+    def model_class
+      ::Ci::Build
+    end
+
     # Dependencies local to the given pipeline
     def local
-      return [] if no_local_dependencies_specified?
-      return [] unless processable.pipeline_id # we don't have any dependency when creating the pipeline
-
-      deps = model_class.where(pipeline_id: processable.pipeline_id).latest
-      deps = from_previous_stages(deps)
-      deps = from_needs(deps)
-      from_dependencies(deps)
+      strong_memoize(:local) do
+        next [] if no_local_dependencies_specified?
+        next [] unless processable.pipeline_id # we don't have any dependency when creating the pipeline
+
+        deps = model_class.where(pipeline_id: processable.pipeline_id).latest
+        deps = from_previous_stages(deps)
+        deps = from_needs(deps)
+        from_dependencies(deps).to_a
+      end
     end
 
     # Dependencies from the same parent-pipeline hierarchy excluding
@@ -38,22 +56,6 @@ module Ci
       []
     end
 
-    def invalid_local
-      local.reject(&:valid_dependency?)
-    end
-
-    def valid?
-      valid_local? && valid_cross_pipeline? && valid_cross_project?
-    end
-
-    private
-
-    # Dependencies can only be of Ci::Build type because only builds
-    # can create artifacts
-    def model_class
-      ::Ci::Build
-    end
-
     def fetch_dependencies_in_hierarchy
       deps_specifications = specified_cross_pipeline_dependencies
       return [] if deps_specifications.empty?

ee/app/models/ee/ci/build_dependencies.rb

@@ -9,6 +9,8 @@ module EE
 
       CROSS_PROJECT_LIMIT = ::Gitlab::Ci::Config::Entry::Needs::NEEDS_CROSS_PROJECT_DEPENDENCIES_LIMIT
 
+      private
+
       override :cross_project
       def cross_project
         strong_memoize(:cross_project) do
@@ -16,8 +18,6 @@ module EE
         end
       end
 
-      private
-
       override :valid_cross_project?
       def valid_cross_project?
         cross_project.size == specified_cross_project_dependencies.size

ee/spec/models/ee/ci/build_dependencies_spec.rb

@@ -16,6 +16,14 @@ RSpec.describe Ci::BuildDependencies do
       status: 'success')
   end
 
+  let(:pipeline2) do
+    create(:ci_pipeline,
+      project: project,
+      sha: project.commit.id,
+      ref: project.default_branch,
+      status: 'success')
+  end
+
   let!(:job) do
     create(:ci_build,
       pipeline: pipeline,
@@ -32,8 +40,8 @@ RSpec.describe Ci::BuildDependencies do
     stub_licensed_features(cross_project_pipelines: true)
   end
 
-  describe '#cross_project' do
-    subject { described_class.new(job).cross_project }
+  context 'for cross_project dependencies' do
+    subject { described_class.new(job).all }
 
     context 'when cross_dependencies are not defined' do
       it { is_expected.to be_empty }
@@ -57,7 +65,7 @@ RSpec.describe Ci::BuildDependencies do
     context 'with cross_dependencies to the same project' do
       let!(:dependency) do
         create(:ci_build, :success,
-          pipeline: pipeline,
+          pipeline: pipeline2,
           name: 'dependency',
           stage_idx: 1,
           stage: 'build',
@@ -70,7 +78,7 @@ RSpec.describe Ci::BuildDependencies do
           {
             project: project.full_path,
             job: 'dependency',
-            ref: pipeline.ref,
+            ref: pipeline2.ref,
             artifacts: artifacts
           }
         ]
@@ -231,7 +239,7 @@ RSpec.describe Ci::BuildDependencies do
       before do
         cross_dependencies_limit.next.times do |index|
           create(:ci_build, :success,
-            pipeline: pipeline, name: "dependency-#{index}",
+            pipeline: pipeline2, name: "dependency-#{index}",
             stage_idx: 1, stage: 'build', user: user
           )
         end
@@ -242,7 +250,7 @@ RSpec.describe Ci::BuildDependencies do
           {
             project: project.full_path,
             job: "dependency-#{index}",
-            ref: pipeline.ref,
+            ref: pipeline2.ref,
             artifacts: true
           }
         end

spec/models/ci/build_dependencies_spec.rb

@@ -22,8 +22,8 @@ RSpec.describe Ci::BuildDependencies do
     stub_feature_flags(ci_validate_build_dependencies_override: false)
   end
 
-  describe '#local' do
-    subject { described_class.new(job).local }
+  context 'for local dependencies' do
+    subject { described_class.new(job).all }
 
     describe 'jobs from previous stages' do
       context 'when job is in the first stage' do
@@ -52,7 +52,7 @@ RSpec.describe Ci::BuildDependencies do
             project.add_developer(user)
           end
 
-          let(:retried_job) { Ci::Build.retry(rspec_test, user) }
+          let!(:retried_job) { Ci::Build.retry(rspec_test, user) }
 
           it 'contains the retried job instead of the original one' do
             is_expected.to contain_exactly(build, retried_job, rubocop_test)
@@ -150,7 +150,7 @@ RSpec.describe Ci::BuildDependencies do
     end
   end
 
-  describe '#cross_pipeline' do
+  context 'for cross_pipeline dependencies' do
     let!(:job) do
       create(:ci_build,
         pipeline: pipeline,
@@ -160,7 +160,7 @@ RSpec.describe Ci::BuildDependencies do
 
     subject { described_class.new(job) }
 
-    let(:cross_pipeline_deps) { subject.cross_pipeline }
+    let(:cross_pipeline_deps) { subject.all }
 
     context 'when dependency specifications are valid' do
       context 'when pipeline exists in the hierarchy' do

spec/requests/api/ci/runner/jobs_request_post_spec.rb

@@ -502,8 +502,8 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
                   expect { request_job }.to exceed_all_query_limit(1).for_model(::Ci::JobArtifact)
                 end
 
-                it 'queries the ci_builds table more than five times' do
-                  expect { request_job }.to exceed_all_query_limit(5).for_model(::Ci::Build)
+                it 'queries the ci_builds table more than three times' do
+                  expect { request_job }.to exceed_all_query_limit(3).for_model(::Ci::Build)
                 end
               end