Merge branch 'lm-follow-up-bridge-api-endpoint' into 'master'

Follow up api bridge endpoint

See merge request gitlab-org/gitlab!36622

app/finders/ci/jobs_finder.rb

@@ -4,31 +4,38 @@ module Ci
   class JobsFinder
     include Gitlab::Allowable
 
-    def initialize(current_user:, project: nil, params: {})
+    def initialize(current_user:, pipeline: nil, project: nil, params: {}, type: ::Ci::Build)
+      @pipeline = pipeline
       @current_user = current_user
       @project = project
       @params = params
+      @type = type
+      raise ArgumentError 'type must be a subclass of Ci::Processable' unless type < ::Ci::Processable
     end
 
     def execute
       builds = init_collection.order_id_desc
       filter_by_scope(builds)
     rescue Gitlab::Access::AccessDeniedError
-      Ci::Build.none
+      type.none
     end
 
     private
 
-    attr_reader :current_user, :project, :params
+    attr_reader :current_user, :pipeline, :project, :params, :type
 
     def init_collection
-      project ? project_builds : all_builds
+      if Feature.enabled?(:ci_jobs_finder_refactor)
+        pipeline_jobs || project_jobs || all_jobs
+      else
+        project ? project_builds : all_jobs
+      end
     end
 
-    def all_builds
+    def all_jobs
       raise Gitlab::Access::AccessDeniedError unless current_user&.admin?
 
-      Ci::Build.all
+      type.all
     end
 
     def project_builds
@@ -37,7 +44,25 @@ module Ci
       project.builds.relevant
     end
 
+    def project_jobs
+      return unless project
+      raise Gitlab::Access::AccessDeniedError unless can?(current_user, :read_build, project)
+
+      jobs_by_type(project, type).relevant
+    end
+
+    def pipeline_jobs
+      return unless pipeline
+      raise Gitlab::Access::AccessDeniedError unless can?(current_user, :read_build, pipeline)
+
+      jobs_by_type(pipeline, type).latest
+    end
+
     def filter_by_scope(builds)
+      if Feature.enabled?(:ci_jobs_finder_refactor)
+        return filter_by_statuses!(params[:scope], builds) if params[:scope].is_a?(Array)
+      end
+
       case params[:scope]
       when 'pending'
         builds.pending.reverse_order
@@ -49,5 +74,23 @@ module Ci
         builds
       end
     end
+
+    def filter_by_statuses!(statuses, builds)
+      unknown_statuses = params[:scope] - ::CommitStatus::AVAILABLE_STATUSES
+      raise ArgumentError, 'Scope contains invalid value(s)' unless unknown_statuses.empty?
+
+      builds.where(status: params[:scope]) # rubocop: disable CodeReuse/ActiveRecord
+    end
+
+    def jobs_by_type(relation, type)
+      case type.name
+      when ::Ci::Build.name
+        relation.builds
+      when ::Ci::Bridge.name
+        relation.bridges
+      else
+        raise ArgumentError, "finder does not support #{type} type"
+      end
+    end
   end
 end

app/models/ci/bridge.rb

@@ -48,6 +48,14 @@ module Ci
       raise NotImplementedError
     end
 
+    def self.with_preloads
+      preload(
+        :metadata,
+        downstream_pipeline: [project: [:route, { namespace: :route }]],
+        project: [:namespace]
+      )
+    end
+
     def schedule_downstream_pipeline!
       raise InvalidBridgeTypeError unless downstream_project
 

app/models/ci/build.rb

@@ -175,7 +175,6 @@ module Ci
     end
 
     scope :queued_before, ->(time) { where(arel_table[:queued_at].lt(time)) }
-    scope :order_id_desc, -> { order('ci_builds.id DESC') }
 
     scope :preload_project_and_pipeline_project, -> do
       preload(Ci::Pipeline::PROJECT_ROUTE_AND_NAMESPACE_ROUTE,
@@ -213,6 +212,10 @@ module Ci
           .execute(build)
         # rubocop: enable CodeReuse/ServiceClass
       end
+
+      def with_preloads
+        preload(:job_artifacts_archive, :job_artifacts, project: [:namespace])
+      end
     end
 
     state_machine :status do

app/models/commit_status.rb

@@ -32,6 +32,8 @@ class CommitStatus < ApplicationRecord
     where(allow_failure: true, status: [:failed, :canceled])
   end
 
+  scope :order_id_desc, -> { order('ci_builds.id DESC') }
+
   scope :exclude_ignored, -> do
     # We want to ignore failed but allowed to fail jobs.
     #

config/feature_flags/development/ci_jobs_finder_refactor.yml

+---
+name: ci_jobs_finder_refactor
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/36622
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/245183
+group: group::continuous integration
+type: development
+default_enabled: false

lib/api/ci/pipelines.rb

@@ -15,6 +15,24 @@ module API
           detail 'This feature was introduced in GitLab 8.11.'
           success Entities::Ci::PipelineBasic
         end
+
+        helpers do
+          params :optional_scope do
+            optional :scope, types: [String, Array[String]], desc: 'The scope of builds to show',
+                            values: ::CommitStatus::AVAILABLE_STATUSES,
+                           coerce_with: ->(scope) {
+                             case scope
+                             when String
+                               [scope]
+                             when ::Array
+                               scope
+                             else
+                               ['unknown']
+                             end
+                           }
+          end
+        end
+
         params do
           use :pagination
           optional :scope,    type: String, values: %w[running pending finished branches tags],
@@ -96,6 +114,64 @@ module API
           present pipeline, with: Entities::Ci::Pipeline
         end
 
+        desc 'Get pipeline jobs' do
+          success Entities::Ci::Job
+        end
+        params do
+          requires :pipeline_id, type: Integer, desc: 'The pipeline ID'
+          use :optional_scope
+          use :pagination
+        end
+
+        get ':id/pipelines/:pipeline_id/jobs' do
+          authorize!(:read_pipeline, user_project)
+
+          pipeline = user_project.all_pipelines.find(params[:pipeline_id])
+
+          if Feature.enabled?(:ci_jobs_finder_refactor)
+            builds = ::Ci::JobsFinder
+              .new(current_user: current_user, pipeline: pipeline, params: params)
+              .execute
+          else
+            authorize!(:read_build, pipeline)
+            builds = pipeline.builds
+            builds = filter_builds(builds, params[:scope])
+          end
+
+          builds = builds.with_preloads
+
+          present paginate(builds), with: Entities::Ci::Job
+        end
+
+        desc 'Get pipeline bridge jobs' do
+          success Entities::Ci::Bridge
+        end
+        params do
+          requires :pipeline_id, type: Integer, desc: 'The pipeline ID'
+          use :optional_scope
+          use :pagination
+        end
+
+        get ':id/pipelines/:pipeline_id/bridges' do
+          authorize!(:read_build, user_project)
+
+          pipeline = user_project.all_pipelines.find(params[:pipeline_id])
+
+          if Feature.enabled?(:ci_jobs_finder_refactor)
+            bridges = ::Ci::JobsFinder
+              .new(current_user: current_user, pipeline: pipeline, params: params, type: ::Ci::Bridge)
+              .execute
+          else
+            authorize!(:read_pipeline, pipeline)
+            bridges = pipeline.bridges
+            bridges = filter_builds(bridges, params[:scope])
+          end
+
+          bridges = bridges.with_preloads
+
+          present paginate(bridges), with: Entities::Ci::Bridge
+        end
+
         desc 'Gets the variables for a given pipeline' do
           detail 'This feature was introduced in GitLab 11.11'
           success Entities::Ci::Variable
@@ -170,6 +246,21 @@ module API
       end
 
       helpers do
+        # NOTE: This method should be removed once the ci_jobs_finder_refactor FF is
+        # removed. https://gitlab.com/gitlab-org/gitlab/-/issues/245183
+        # rubocop: disable CodeReuse/ActiveRecord
+        def filter_builds(builds, scope)
+          return builds if scope.nil? || scope.empty?
+
+          available_statuses = ::CommitStatus::AVAILABLE_STATUSES
+
+          unknown = scope - available_statuses
+          render_api_error!('Scope contains invalid value(s)', 400) unless unknown.empty?
+
+          builds.where(status: scope)
+        end
+        # rubocop: enable CodeReuse/ActiveRecord
+
         def pipeline
           strong_memoize(:pipeline) do
             user_project.all_pipelines.find(params[:pipeline_id])

lib/api/jobs.rb

@@ -48,54 +48,6 @@ module API
       end
       # rubocop: enable CodeReuse/ActiveRecord
 
-      desc 'Get pipeline jobs' do
-        success Entities::Ci::Job
-      end
-      params do
-        requires :pipeline_id, type: Integer, desc: 'The pipeline ID'
-        use :optional_scope
-        use :pagination
-      end
-      # rubocop: disable CodeReuse/ActiveRecord
-      get ':id/pipelines/:pipeline_id/jobs' do
-        authorize!(:read_pipeline, user_project)
-        pipeline = user_project.all_pipelines.find(params[:pipeline_id])
-        authorize!(:read_build, pipeline)
-
-        builds = pipeline.builds
-        builds = filter_builds(builds, params[:scope])
-        builds = builds.preload(:job_artifacts_archive, :job_artifacts, project: [:namespace])
-
-        present paginate(builds), with: Entities::Ci::Job
-      end
-      # rubocop: enable CodeReuse/ActiveRecord
-
-      desc 'Get pipeline bridge jobs' do
-        success ::API::Entities::Ci::Bridge
-      end
-      params do
-        requires :pipeline_id, type: Integer, desc: 'The pipeline ID'
-        use :optional_scope
-        use :pagination
-      end
-      # rubocop: disable CodeReuse/ActiveRecord
-      get ':id/pipelines/:pipeline_id/bridges' do
-        authorize!(:read_build, user_project)
-        pipeline = user_project.ci_pipelines.find(params[:pipeline_id])
-        authorize!(:read_pipeline, pipeline)
-
-        bridges = pipeline.bridges
-        bridges = filter_builds(bridges, params[:scope])
-        bridges = bridges.preload(
-          :metadata,
-          downstream_pipeline: [project: [:route, { namespace: :route }]],
-          project: [:namespace]
-        )
-
-        present paginate(bridges), with: ::API::Entities::Ci::Bridge
-      end
-      # rubocop: enable CodeReuse/ActiveRecord
-
       desc 'Get a specific job of a project' do
         success Entities::Ci::Job
       end

spec/finders/ci/jobs_finder_spec.rb

@@ -36,53 +36,135 @@ RSpec.describe Ci::JobsFinder, '#execute' do
       end
     end
 
-    context 'scope is present' do
-      let(:jobs) { [job_1, job_2, job_3] }
-
-      where(:scope, :index) do
-        [
-          ['pending',  0],
-          ['running',  1],
-          ['finished', 2]
-        ]
+    context 'with ci_jobs_finder_refactor ff enabled' do
+      before do
+        stub_feature_flags(ci_jobs_finder_refactor: true)
       end
 
-      with_them do
-        let(:params) { { scope: scope } }
+      context 'scope is present' do
+        let(:jobs) { [job_1, job_2, job_3] }
+
+        where(:scope, :index) do
+          [
+            ['pending',  0],
+            ['running',  1],
+            ['finished', 2]
+          ]
+        end
+
+        with_them do
+          let(:params) { { scope: scope } }
 
-        it { expect(subject).to match_array([jobs[index]]) }
+          it { expect(subject).to match_array([jobs[index]]) }
+        end
       end
-    end
-  end
 
-  context 'a project is present' do
-    subject { described_class.new(current_user: user, project: project, params: params).execute }
+      context 'scope is an array' do
+        let(:jobs) { [job_1, job_2, job_3] }
+        let(:params) {{ scope: ['running'] }}
 
-    context 'user has access to the project' do
+        it 'filters by the job statuses in the scope' do
+          expect(subject).to match_array([job_2])
+        end
+      end
+    end
+
+    context 'with ci_jobs_finder_refactor ff disabled' do
       before do
-        project.add_maintainer(user)
+        stub_feature_flags(ci_jobs_finder_refactor: false)
       end
 
-      it 'returns jobs for the specified project' do
-        expect(subject).to match_array([job_3])
+      context 'scope is present' do
+        let(:jobs) { [job_1, job_2, job_3] }
+
+        where(:scope, :index) do
+          [
+            ['pending',  0],
+            ['running',  1],
+            ['finished', 2]
+          ]
+        end
+
+        with_them do
+          let(:params) { { scope: scope } }
+
+          it { expect(subject).to match_array([jobs[index]]) }
+        end
       end
     end
+  end
 
-    context 'user has no access to project builds' do
-      before do
-        project.add_guest(user)
+  context 'with ci_jobs_finder_refactor ff enabled' do
+    before do
+      stub_feature_flags(ci_jobs_finder_refactor: true)
+    end
+
+    context 'a project is present' do
+      subject { described_class.new(current_user: user, project: project, params: params).execute }
+
+      context 'user has access to the project' do
+        before do
+          project.add_maintainer(user)
+        end
+
+        it 'returns jobs for the specified project' do
+          expect(subject).to match_array([job_3])
+        end
       end
 
-      it 'returns no jobs' do
-        expect(subject).to be_empty
+      context 'user has no access to project builds' do
+        before do
+          project.add_guest(user)
+        end
+
+        it 'returns no jobs' do
+          expect(subject).to be_empty
+        end
+      end
+
+      context 'without user' do
+        let(:user) { nil }
+
+        it 'returns no jobs' do
+          expect(subject).to be_empty
+        end
       end
     end
+  end
 
-    context 'without user' do
-      let(:user) { nil }
+  context 'with ci_jobs_finder_refactor ff disabled' do
+    before do
+      stub_feature_flags(ci_jobs_finder_refactor: false)
+    end
+    context 'a project is present' do
+      subject { described_class.new(current_user: user, project: project, params: params).execute }
 
-      it 'returns no jobs' do
-        expect(subject).to be_empty
+      context 'user has access to the project' do
+        before do
+          project.add_maintainer(user)
+        end
+
+        it 'returns jobs for the specified project' do
+          expect(subject).to match_array([job_3])
+        end
+      end
+
+      context 'user has no access to project builds' do
+        before do
+          project.add_guest(user)
+        end
+
+        it 'returns no jobs' do
+          expect(subject).to be_empty
+        end
+      end
+
+      context 'without user' do
+        let(:user) { nil }
+
+        it 'returns no jobs' do
+          expect(subject).to be_empty
+        end
       end
     end
   end

spec/support/shared_examples/ci/jobs_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'a job with artifacts and trace' do |result_is_array: true|
+  context 'with artifacts and trace' do
+    let!(:second_job) { create(:ci_build, :trace_artifact, :artifacts, :test_reports, pipeline: pipeline) }
+
+    it 'returns artifacts and trace data', :skip_before_request do
+      get api(api_endpoint, api_user)
+      json_job = json_response.is_a?(Array) ? json_response.find { |job| job['id'] == second_job.id } : json_response
+
+      expect(json_job['artifacts_file']).not_to be_nil
+      expect(json_job['artifacts_file']).not_to be_empty
+      expect(json_job['artifacts_file']['filename']).to eq(second_job.artifacts_file.filename)
+      expect(json_job['artifacts_file']['size']).to eq(second_job.artifacts_file.size)
+      expect(json_job['artifacts']).not_to be_nil
+      expect(json_job['artifacts']).to be_an Array
+      expect(json_job['artifacts'].size).to eq(second_job.job_artifacts.length)
+      json_job['artifacts'].each do |artifact|
+        expect(artifact).not_to be_nil
+        file_type = Ci::JobArtifact.file_types[artifact['file_type']]
+        expect(artifact['size']).to eq(second_job.job_artifacts.find_by(file_type: file_type).size)
+        expect(artifact['filename']).to eq(second_job.job_artifacts.find_by(file_type: file_type).filename)
+        expect(artifact['file_format']).to eq(second_job.job_artifacts.find_by(file_type: file_type).file_format)
+      end
+    end
+  end
+end