Merge branch 'mc/feature/find-all-artifacts-for-sha' into 'master'

Find build by sha from ref

Closes #64534 and #45697

See merge request gitlab-org/gitlab-ce!30843

10 files changed, 136 insertions, 69 deletions

diff --git a/app/controllers/projects/artifacts_controller.rb b/app/controllers/projects/artifacts_controller.rb
index 2ef18d900f2..da8a371acaa 100644
--- a/app/controllers/projects/artifacts_controller.rb
+++ b/app/controllers/projects/artifacts_controller.rb
@@ -92,7 +92,10 @@ class Projects::ArtifactsController < Projects::ApplicationController
   def build_from_ref
     return unless @ref_name
 
-    project.latest_successful_build_for(params[:job], @ref_name)
+    commit = project.commit(@ref_name)
+    return unless commit
+
+    project.latest_successful_build_for_sha(params[:job], commit.id)
   end
 
   def artifacts_file
diff --git a/app/controllers/projects/build_artifacts_controller.rb b/app/controllers/projects/build_artifacts_controller.rb
index 4274c356227..99f4524eec5 100644
--- a/app/controllers/projects/build_artifacts_controller.rb
+++ b/app/controllers/projects/build_artifacts_controller.rb
@@ -51,6 +51,6 @@ class Projects::BuildArtifactsController < Projects::ApplicationController
   def job_from_ref
     return unless @ref_name
 
-    project.latest_successful_build_for(params[:job], @ref_name)
+    project.latest_successful_build_for_ref(params[:job], @ref_name)
   end
 end
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index c2eb51ba100..1c76f401690 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -229,10 +229,12 @@ module Ci
     #
     # ref - The name (or names) of the branch(es)/tag(s) to limit the list of
     #       pipelines to.
+    # sha - The commit SHA (or mutliple SHAs) to limit the list of pipelines to.
     # limit - This limits a backlog search, default to 100.
-    def self.newest_first(ref: nil, limit: 100)
+    def self.newest_first(ref: nil, sha: nil, limit: 100)
       relation = order(id: :desc)
       relation = relation.where(ref: ref) if ref
+      relation = relation.where(sha: sha) if sha
 
       if limit
         ids = relation.limit(limit).select(:id)
@@ -246,10 +248,14 @@ module Ci
       newest_first(ref: ref).pluck(:status).first
     end
 
-    def self.latest_successful_for(ref)
+    def self.latest_successful_for_ref(ref)
       newest_first(ref: ref).success.take
     end
 
+    def self.latest_successful_for_sha(sha)
+      newest_first(sha: sha).success.take
+    end
+
     def self.latest_successful_for_refs(refs)
       relation = newest_first(ref: refs).success
 
diff --git a/app/models/project.rb b/app/models/project.rb
index 0020e423628..cca7da8c49a 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -719,16 +719,27 @@ class Project < ApplicationRecord
     repository.commits_by(oids: oids)
   end
 
-  # ref can't be HEAD, can only be branch/tag name or SHA
-  def latest_successful_build_for(job_name, ref = default_branch)
-    latest_pipeline = ci_pipelines.latest_successful_for(ref)
+  # ref can't be HEAD, can only be branch/tag name
+  def latest_successful_build_for_ref(job_name, ref = default_branch)
+    return unless ref
+
+    latest_pipeline = ci_pipelines.latest_successful_for_ref(ref)
+    return unless latest_pipeline
+
+    latest_pipeline.builds.latest.with_artifacts_archive.find_by(name: job_name)
+  end
+
+  def latest_successful_build_for_sha(job_name, sha)
+    return unless sha
+
+    latest_pipeline = ci_pipelines.latest_successful_for_sha(sha)
     return unless latest_pipeline
 
     latest_pipeline.builds.latest.with_artifacts_archive.find_by(name: job_name)
   end
 
-  def latest_successful_build_for!(job_name, ref = default_branch)
-    latest_successful_build_for(job_name, ref) || raise(ActiveRecord::RecordNotFound.new("Couldn't find job #{job_name}"))
+  def latest_successful_build_for_ref!(job_name, ref = default_branch)
+    latest_successful_build_for_ref(job_name, ref) || raise(ActiveRecord::RecordNotFound.new("Couldn't find job #{job_name}"))
   end
 
   def merge_base_commit(first_commit_id, second_commit_id)
@@ -1503,12 +1514,12 @@ class Project < ApplicationRecord
     end
 
     @latest_successful_pipeline_for_default_branch =
-      ci_pipelines.latest_successful_for(default_branch)
+      ci_pipelines.latest_successful_for_ref(default_branch)
   end
 
   def latest_successful_pipeline_for(ref = nil)
     if ref && ref != default_branch
-      ci_pipelines.latest_successful_for(ref)
+      ci_pipelines.latest_successful_for_ref(ref)
     else
       latest_successful_pipeline_for_default_branch
     end
diff --git a/lib/api/job_artifacts.rb b/lib/api/job_artifacts.rb
index e7fed55170e..b35aa952f81 100644
--- a/lib/api/job_artifacts.rb
+++ b/lib/api/job_artifacts.rb
@@ -27,7 +27,7 @@ module API
         requirements: { ref_name: /.+/ } do
         authorize_download_artifacts!
 
-        latest_build = user_project.latest_successful_build_for!(params[:job], params[:ref_name])
+        latest_build = user_project.latest_successful_build_for_ref!(params[:job], params[:ref_name])
 
         present_carrierwave_file!(latest_build.artifacts_file)
       end
@@ -45,7 +45,7 @@ module API
           requirements: { ref_name: /.+/ } do
         authorize_download_artifacts!
 
-        build = user_project.latest_successful_build_for!(params[:job], params[:ref_name])
+        build = user_project.latest_successful_build_for_ref!(params[:job], params[:ref_name])
 
         path = Gitlab::Ci::Build::Artifacts::Path
                  .new(params[:artifact_path])
diff --git a/lib/gitlab/badge/coverage/report.rb b/lib/gitlab/badge/coverage/report.rb
index 7f7cc62c8ef..15cccc6f287 100644
--- a/lib/gitlab/badge/coverage/report.rb
+++ b/lib/gitlab/badge/coverage/report.rb
@@ -14,7 +14,7 @@ module Gitlab
           @ref = ref
           @job = job
 
-          @pipeline = @project.ci_pipelines.latest_successful_for(@ref)
+          @pipeline = @project.ci_pipelines.latest_successful_for_ref(@ref)
         end
 
         def entity
diff --git a/spec/features/projects/artifacts/user_downloads_artifacts_spec.rb b/spec/features/projects/artifacts/user_downloads_artifacts_spec.rb
index 5cb015e80be..69296ef00fd 100644
--- a/spec/features/projects/artifacts/user_downloads_artifacts_spec.rb
+++ b/spec/features/projects/artifacts/user_downloads_artifacts_spec.rb
@@ -1,8 +1,8 @@
 require "spec_helper"
 
 describe "User downloads artifacts" do
-  set(:project) { create(:project, :public) }
-  set(:pipeline) { create(:ci_empty_pipeline, status: :success, project: project) }
+  set(:project) { create(:project, :repository, :public) }
+  set(:pipeline) { create(:ci_empty_pipeline, status: :success, sha: project.commit.id, project: project) }
   set(:job) { create(:ci_build, :artifacts, :success, pipeline: pipeline) }
 
   shared_examples "downloading" do
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index e24bbc39761..1fb83fbb088 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -1799,7 +1799,7 @@ describe Ci::Pipeline, :mailer do
     end
   end
 
-  describe '.latest_successful_for' do
+  describe '.latest_successful_for_ref' do
     include_context 'with some outdated pipelines'
 
     let!(:latest_successful_pipeline) do
@@ -1807,7 +1807,20 @@ describe Ci::Pipeline, :mailer do
     end
 
     it 'returns the latest successful pipeline' do
-      expect(described_class.latest_successful_for('ref'))
+      expect(described_class.latest_successful_for_ref('ref'))
+        .to eq(latest_successful_pipeline)
+    end
+  end
+
+  describe '.latest_successful_for_sha' do
+    include_context 'with some outdated pipelines'
+
+    let!(:latest_successful_pipeline) do
+      create_pipeline(:success, 'ref', 'awesomesha', project)
+    end
+
+    it 'returns the latest successful pipeline' do
+      expect(described_class.latest_successful_for_sha('awesomesha'))
         .to eq(latest_successful_pipeline)
     end
   end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 7d458324c20..8529e8ca820 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -2019,62 +2019,33 @@ describe Project do
     end
   end
 
-  describe '#latest_successful_build_for' do
+  describe '#latest_successful_build_for_ref' do
     let(:project) { create(:project, :repository) }
     let(:pipeline) { create_pipeline(project) }
 
-    context 'with many builds' do
-      it 'gives the latest builds from latest pipeline' do
-        pipeline1 = create_pipeline(project)
-        pipeline2 = create_pipeline(project)
-        create_build(pipeline1, 'test')
-        create_build(pipeline1, 'test2')
-        build1_p2 = create_build(pipeline2, 'test')
-        create_build(pipeline2, 'test2')
-
-        expect(project.latest_successful_build_for(build1_p2.name))
-          .to eq(build1_p2)
-      end
-    end
+    it_behaves_like 'latest successful build for sha or ref'
 
-    context 'with succeeded pipeline' do
-      let!(:build) { create_build }
+    subject { project.latest_successful_build_for_ref(build_name) }
 
-      context 'standalone pipeline' do
-        it 'returns builds for ref for default_branch' do
-          expect(project.latest_successful_build_for(build.name))
-            .to eq(build)
-        end
+    context 'with a specified ref' do
+      let(:build) { create_build }
 
-        it 'returns empty relation if the build cannot be found' do
-          expect(project.latest_successful_build_for('TAIL'))
-            .to be_nil
-        end
-      end
+      subject { project.latest_successful_build_for_ref(build.name, project.default_branch) }
 
-      context 'with some pending pipeline' do
-        before do
-          create_build(create_pipeline(project, 'pending'))
-        end
-
-        it 'gives the latest build from latest pipeline' do
-          expect(project.latest_successful_build_for(build.name))
-            .to eq(build)
-        end
-      end
+      it { is_expected.to eq(build) }
     end
+  end
 
-    context 'with pending pipeline' do
-      it 'returns empty relation' do
-        pipeline.update(status: 'pending')
-        pending_build = create_build(pipeline)
+  describe '#latest_successful_build_for_sha' do
+    let(:project) { create(:project, :repository) }
+    let(:pipeline) { create_pipeline(project) }
 
-        expect(project.latest_successful_build_for(pending_build.name)).to be_nil
-      end
-    end
+    it_behaves_like 'latest successful build for sha or ref'
+
+    subject { project.latest_successful_build_for_sha(build_name, project.commit.sha) }
   end
 
-  describe '#latest_successful_build_for!' do
+  describe '#latest_successful_build_for_ref!' do
     let(:project) { create(:project, :repository) }
     let(:pipeline) { create_pipeline(project) }
 
@@ -2087,7 +2058,7 @@ describe Project do
         build1_p2 = create_build(pipeline2, 'test')
         create_build(pipeline2, 'test2')
 
-        expect(project.latest_successful_build_for(build1_p2.name))
+        expect(project.latest_successful_build_for_ref!(build1_p2.name))
           .to eq(build1_p2)
       end
     end
@@ -2097,12 +2068,12 @@ describe Project do
 
       context 'standalone pipeline' do
         it 'returns builds for ref for default_branch' do
-          expect(project.latest_successful_build_for!(build.name))
+          expect(project.latest_successful_build_for_ref!(build.name))
             .to eq(build)
         end
 
         it 'returns exception if the build cannot be found' do
-          expect { project.latest_successful_build_for!(build.name, 'TAIL') }
+          expect { project.latest_successful_build_for_ref!(build.name, 'TAIL') }
             .to raise_error(ActiveRecord::RecordNotFound)
         end
       end
@@ -2113,7 +2084,7 @@ describe Project do
         end
 
         it 'gives the latest build from latest pipeline' do
-          expect(project.latest_successful_build_for!(build.name))
+          expect(project.latest_successful_build_for_ref!(build.name))
             .to eq(build)
         end
       end
@@ -2124,7 +2095,7 @@ describe Project do
         pipeline.update(status: 'pending')
         pending_build = create_build(pipeline)
 
-        expect { project.latest_successful_build_for!(pending_build.name) }
+        expect { project.latest_successful_build_for_ref!(pending_build.name) }
           .to raise_error(ActiveRecord::RecordNotFound)
       end
     end
@@ -4033,7 +4004,7 @@ describe Project do
 
     context 'with a ref that is not the default branch' do
       it 'returns the latest successful pipeline for the given ref' do
-        expect(project.ci_pipelines).to receive(:latest_successful_for).with('foo')
+        expect(project.ci_pipelines).to receive(:latest_successful_for_ref).with('foo')
 
         project.latest_successful_pipeline_for('foo')
       end
@@ -4061,7 +4032,7 @@ describe Project do
     it 'memoizes and returns the latest successful pipeline for the default branch' do
       pipeline = double(:pipeline)
 
-      expect(project.ci_pipelines).to receive(:latest_successful_for)
+      expect(project.ci_pipelines).to receive(:latest_successful_for_ref)
         .with(project.default_branch)
         .and_return(pipeline)
         .once
diff --git a/spec/support/shared_examples/project_latest_successful_build_for_examples.rb b/spec/support/shared_examples/project_latest_successful_build_for_examples.rb
new file mode 100644
index 00000000000..a9bd23e9fc9
--- /dev/null
+++ b/spec/support/shared_examples/project_latest_successful_build_for_examples.rb
@@ -0,0 +1,63 @@
+# frozen_string_literal: true
+
+shared_examples 'latest successful build for sha or ref' do
+  context 'with many builds' do
+    let(:other_pipeline) { create_pipeline(project) }
+    let(:other_build) { create_build(other_pipeline, 'test') }
+    let(:build_name) { other_build.name }
+
+    before do
+      pipeline1 = create_pipeline(project)
+      pipeline2 = create_pipeline(project)
+      create_build(pipeline1, 'test')
+      create_build(pipeline1, 'test2')
+      create_build(pipeline2, 'test2')
+    end
+
+    it 'gives the latest builds from latest pipeline' do
+      expect(subject).to eq(other_build)
+    end
+  end
+
+  context 'with succeeded pipeline' do
+    let!(:build) { create_build }
+    let(:build_name) { build.name }
+
+    context 'standalone pipeline' do
+      it 'returns builds for ref for default_branch' do
+        expect(subject).to eq(build)
+      end
+
+      context 'with nonexistent build' do
+        let(:build_name) { 'TAIL' }
+
+        it 'returns empty relation if the build cannot be found' do
+          expect(subject).to be_nil
+        end
+      end
+    end
+
+    context 'with some pending pipeline' do
+      before do
+        create_build(create_pipeline(project, 'pending'))
+      end
+
+      it 'gives the latest build from latest pipeline' do
+        expect(subject).to eq(build)
+      end
+    end
+  end
+
+  context 'with pending pipeline' do
+    let!(:pending_build) { create_build(pipeline) }
+    let(:build_name) { pending_build.name }
+
+    before do
+      pipeline.update(status: 'pending')
+    end
+
+    it 'returns empty relation' do
+      expect(subject).to be_nil
+    end
+  end
+end