Merge branch '11759-dependency-list-response-statuses' into 'master'

Return dependency list with status

Closes #11759

See merge request gitlab-org/gitlab-ee!13918

ee/app/controllers/projects/security/dependencies_controller.rb

@@ -8,23 +8,30 @@ module Projects
       def index
         respond_to do |format|
           format.json do
-            render json: paginated_dependencies
+            render json: ::DependencyListSerializer.new(project: project)
+                           .represent(paginated_dependencies, build: build)
           end
         end
       end
 
       private
 
+      def build
+        @build ||= pipeline.builds.latest
+                  .with_reports(::Ci::JobArtifact.dependency_list_reports)
+                  .last
+      end
+
       def ensure_dependency_list_feature_available
         render_404 unless project.feature_available?(:dependency_list)
       end
 
-      def found_dependencies
-        @dependencies ||= pipeline ? service.execute : []
+      def dependencies
+        @dependencies ||= build&.success? ? service.execute : []
       end
 
       def paginated_dependencies
-        params[:page] ? Kaminari.paginate_array(found_dependencies).page(params[:page]) : found_dependencies
+        params[:page] ? Kaminari.paginate_array(dependencies).page(params[:page]) : dependencies
       end
 
       def pipeline

ee/app/serializers/dependency_entity.rb

+# frozen_string_literal: true
+
+class DependencyEntity < Grape::Entity
+  expose :name, :packager, :version
+  expose :location do
+    expose :blob_path, :path
+  end
+end

ee/app/serializers/dependency_list_entity.rb

+# frozen_string_literal: true
+
+class DependencyListEntity < Grape::Entity
+  include RequestAwareEntity
+
+  present_collection true, :dependencies
+
+  expose :dependencies, using: DependencyEntity
+  expose :report do
+    expose :status do |list, options|
+      status(list[:dependencies], options[:build])
+    end
+
+    expose :job_path, if: ->(_, options) { options[:build] } do |_, options|
+      project_build_path(project, options[:build].id)
+    end
+  end
+
+  private
+
+  def project
+    request.project
+  end
+
+  def status(dependencies, build)
+    if build&.success?
+      if dependencies.any?
+        :ok
+      else
+        :no_dependencies
+      end
+    elsif build&.failed?
+      :job_failed
+    else
+      :job_not_set_up
+    end
+  end
+end

ee/app/serializers/dependency_list_serializer.rb

+# frozen_string_literal: true
+
+class DependencyListSerializer < BaseSerializer
+  entity DependencyListEntity
+end

ee/app/services/security/dependency_list_service.rb

@@ -30,13 +30,13 @@ module Security
     end
 
     def sort(collection)
-      if @params[:sort_by] == 'packager'
+      if params[:sort_by] == 'packager'
         collection.sort_by! { |a| a[:packager] }
       else
         collection.sort_by! { |a| a[:name] }
       end
 
-      collection.reverse! if @params[:sort] == 'desc'
+      collection.reverse! if params[:sort] == 'desc'
 
       collection
     end

ee/changelogs/unreleased/11759-dependency-list-response-statuses.yml

+---
+title: Update response schema for DependencyList endpoint and add status
+merge_request: 13918
+author:
+type: changed

ee/spec/controllers/projects/security/dependencies_controller_spec.rb

@@ -6,8 +6,7 @@ describe Projects::Security::DependenciesController do
   describe 'GET index.json' do
     set(:project) { create(:project, :repository, :public) }
     set(:user) { create(:user) }
-
-    subject { get :index, params: { namespace_id: project.namespace, project_id: project }, format: :json }
+    let(:params) { { namespace_id: project.namespace, project_id: project } }
 
     before do
       project.add_developer(user)
@@ -26,47 +25,124 @@ describe Projects::Security::DependenciesController do
         context 'with existing report' do
           let!(:pipeline) { create(:ee_ci_pipeline, :with_dependency_list_report, project: project) }
 
+          before do
+            get :index, params: params, format: :json
+          end
+
           context 'without pagination params' do
-            it "returns a list of dependencies" do
-              subject
+            it 'returns a hash with dependencies' do
+              expect(json_response).to be_a(Hash)
+              expect(json_response['dependencies'].length).to eq(21)
+            end
+
+            it 'returns status ok' do
+              expect(json_response['report']['status']).to eq('ok')
+            end
+
+            it 'returns job path' do
+              job_path = "/#{project.full_path}/builds/#{pipeline.builds.last.id}"
 
+              expect(json_response['report']['job_path']).to eq(job_path)
+            end
+
+            it 'returns success code' do
               expect(response).to have_gitlab_http_status(200)
-              expect(json_response).to be_a(Array)
-              expect(json_response.length).to eq(21)
             end
           end
 
           context 'with params' do
-            it 'returns paginated list' do
-              get :index, params: { namespace_id: project.namespace, project_id: project, page: 2 }, format: :json
-
-              expect(json_response.length).to eq(1)
+            context 'with sorting params' do
+              let(:params) do
+                {
+                  namespace_id: project.namespace,
+                  project_id: project,
+                  sort_by: 'packager',
+                  sort: 'desc',
+                  page: 1
+                }
+              end
+
+              it 'returns sorted list' do
+                expect(json_response['dependencies'].first['packager']).to eq('Ruby (Bundler)')
+                expect(json_response['dependencies'].last['packager']).to eq('JavaScript (Yarn)')
+              end
+
+              it 'return 20 dependencies' do
+                expect(json_response['dependencies'].length).to eq(20)
+              end
             end
 
-            it 'returns sorted list' do
-              get :index, params: { namespace_id: project.namespace, project_id: project, sort_by: 'packager', sort: 'desc', page: 1 }, format: :json
+            context 'with pagination params' do
+              let(:params) { { namespace_id: project.namespace, project_id: project, page: 2 } }
 
-              expect(json_response.length).to eq(20)
-              expect(json_response[0]['packager']).to eq('Ruby (Bundler)')
-              expect(json_response[19]['packager']).to eq('JavaScript (Yarn)')
+              it 'returns paginated list' do
+                expect(json_response['dependencies'].length).to eq(1)
+              end
             end
           end
         end
+
+        context 'without existing report' do
+          let!(:pipeline) { create(:ee_ci_pipeline, :with_sast_report, project: project) }
+
+          before do
+            get :index, params: params, format: :json
+          end
+
+          it 'returns job_not_set_up status' do
+            expect(json_response['report']['status']).to eq('job_not_set_up')
+          end
+
+          it 'returns a nil job_path' do
+            expect(json_response['report']['job_path']).to be_nil
+          end
+        end
+
+        context 'when report doesn\'t have dependency list' do
+          let!(:pipeline) { create(:ee_ci_pipeline, :with_dependency_scanning_report, project: project) }
+
+          before do
+            get :index, params: params, format: :json
+          end
+
+          it 'returns no_dependencies status' do
+            expect(json_response['report']['status']).to eq('no_dependencies')
+          end
+        end
+
+        context 'when job failed' do
+          let!(:pipeline) { create(:ee_ci_pipeline, :success, project: project) }
+          let!(:build) { create(:ee_ci_build, :dependency_list, :failed, :allowed_to_fail) }
+
+          before do
+            pipeline.builds << build
+
+            get :index, params: params, format: :json
+          end
+
+          it 'returns job_failed status' do
+            expect(json_response['report']['status']).to eq('job_failed')
+          end
+        end
       end
 
       context 'when feature is not available' do
-        it 'returns 404' do
-          subject
+        before do
+          get :index, params: params, format: :json
+        end
 
+        it 'returns 404' do
           expect(response).to have_gitlab_http_status(404)
         end
       end
     end
 
     context 'with unauthorized user' do
-      it 'returns 404' do
-        subject
+      before do
+        get :index, params: params, format: :json
+      end
 
+      it 'returns 404' do
         expect(response).to have_gitlab_http_status(404)
       end
     end

ee/spec/factories/ci/builds.rb

@@ -29,7 +29,6 @@ FactoryBot.define do
     end
 
     trait :dependency_list do
-      success
       name :dependency_scanning
 
       after(:build) do |build|

ee/spec/factories/ci/pipelines.rb

@@ -7,12 +7,12 @@ FactoryBot.define do
       config_source :webide_source
     end
 
-    %i[license_management dependency_list].each do |report_type|
+    %i[license_management dependency_list dependency_scanning sast].each do |report_type|
       trait "with_#{report_type}_report".to_sym do
         status :success
 
         after(:build) do |pipeline, evaluator|
-          pipeline.builds << build(:ee_ci_build, report_type, pipeline: pipeline, project: pipeline.project)
+          pipeline.builds << build(:ee_ci_build, report_type, :success, pipeline: pipeline, project: pipeline.project)
         end
       end
     end

ee/spec/fixtures/api/schemas/dependency.json

+{
+  "type": "object",
+  "required": [
+    "name",
+    "packager",
+    "version",
+    "location"
+  ],
+  "properties": {
+    "name": {
+      "type": "string"
+    },
+    "packager": {
+      "type": "string"
+    },
+    "version": {
+      "type": "string"
+    },
+    "location": {
+      "blob_path": {
+        "type": "string"
+      },
+      "path": {
+        "type": "string"
+      }
+    }
+  },
+  "additionalProperties": false
+}

ee/spec/fixtures/api/schemas/dependency_list.json

+{
+  "type": "object",
+  "required": [
+    "dependencies",
+    "report"
+  ],
+  "properties": {
+    "dependencies": {
+      "type": "array",
+      "items": {
+        "$ref": "./dependency.json"
+      }
+    },
+    "report": {
+      "status": { "type": "string" },
+      "job_path": { "type": "string" }
+    }
+  },
+  "additionalProperties": false
+}

ee/spec/lib/gitlab/ci/parsers/security/dependency_list_spec.rb

@@ -20,7 +20,7 @@ describe Gitlab::Ci::Parsers::Security::DependencyList do
       let(:artifact) { create(:ee_ci_job_artifact, :dependency_list) }
 
       it 'parses all files' do
-        blob_path = "/#{project.namespace.name}/#{project.name}/blob/#{sha}/yarn/yarn.lock"
+        blob_path = "/#{project.full_path}/blob/#{sha}/yarn/yarn.lock"
 
         expect(report.dependencies.size).to eq(21)
         expect(report.dependencies[0][:name]).to eq('mini_portile2')

ee/spec/lib/gitlab/ci/parsers/security/formatters/dependency_list_spec.rb

@@ -22,7 +22,7 @@ describe Gitlab::Ci::Parsers::Security::Formatters::DependencyList do
 
     it 'format report into a right format' do
       data = formatter.format(dependency, package_manager, file_path)
-      blob_path = "/#{project.namespace.name}/#{project.name}/blob/#{sha}/rails/Gemfile.lock"
+      blob_path = "/#{project.full_path}/blob/#{sha}/rails/Gemfile.lock"
 
       expect(data[:name]).to eq('mini_portile2')
       expect(data[:packager]).to eq('Ruby (Bundler)')

ee/spec/models/ci/build_spec.rb

@@ -305,7 +305,7 @@ describe Ci::Build do
 
       it 'parses blobs and add the results to the report' do
         subject
-        blob_path = "/#{group.name}/#{project.name}/blob/#{job.sha}/yarn/yarn.lock"
+        blob_path = "/#{project.full_path}/blob/#{job.sha}/yarn/yarn.lock"
 
         expect(dependency_list_report.dependencies.count).to eq(21)
         expect(dependency_list_report.dependencies[0][:name]).to eq('mini_portile2')

ee/spec/serializers/dependency_entity_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe DependencyEntity do
+  describe '#as_json' do
+    let(:dependency) do
+      {
+        name:     'nokogiri',
+        packager: 'Ruby (Bundler)',
+        version:  '1.8.0',
+        location: {
+          blob_path: '/some_project/path/Gemfile.lock',
+          path:      'Gemfile.lock'
+        }
+      }
+    end
+
+    subject { described_class.represent(dependency).as_json }
+
+    it { is_expected.to include(:name) }
+    it { is_expected.to include(:version) }
+    it { is_expected.to include(:packager) }
+    it { expect(subject[:location]).to include(:blob_path) }
+    it { expect(subject[:location]).to include(:path) }
+  end
+end

ee/spec/serializers/dependency_list_entity_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe DependencyListEntity do
+  describe '#as_json' do
+    let(:entity) do
+      described_class.represent(dependencies, build: build, request: request)
+    end
+
+    let(:request) { double('request') }
+    let(:project) { create(:project) }
+
+    subject { entity.as_json }
+
+    before do
+      allow(request).to receive(:project).and_return(project)
+    end
+
+    context 'with success build' do
+      let(:build) { create(:ee_ci_build, :success) }
+
+      context 'with provided dependencies' do
+        let(:dependencies) do
+          [{
+             name:     'nokogiri',
+             packager: 'Ruby (Bundler)',
+             version:  '1.8.0',
+             location: {
+               blob_path: '/some_project/path/Gemfile.lock',
+               path:      'Gemfile.lock'
+             }
+           }]
+        end
+
+        it 'has array of dependencies with status ok' do
+          job_path = "/#{project.full_path}/builds/#{build.id}"
+
+          expect(subject[:dependencies][0][:name]).to eq('nokogiri')
+          expect(subject[:report][:status]).to eq(:ok)
+          expect(subject[:report][:job_path]).to eq(job_path)
+        end
+      end
+
+      context 'with no dependencies' do
+        let(:dependencies) { [] }
+
+        it 'has empty array of dependencies with status no_dependencies' do
+          job_path = "/#{project.full_path}/builds/#{build.id}"
+
+          expect(subject[:dependencies].length).to eq(0)
+          expect(subject[:report][:status]).to eq(:no_dependencies)
+          expect(subject[:report][:job_path]).to eq(job_path)
+        end
+      end
+    end
+
+    context 'with failed build' do
+      let(:build) { create(:ee_ci_build, :failed) }
+      let(:dependencies) { [] }
+
+      it 'has job_path with status failed_job' do
+        expect(subject[:report][:status]).to eq(:job_failed)
+        expect(subject[:report]).to include(:job_path)
+      end
+    end
+
+    context 'with no build' do
+      let(:build) { nil }
+      let(:dependencies) { [] }
+
+      it 'has status job_not_set_up and no job_path' do
+        expect(subject[:report][:status]).to eq(:job_not_set_up)
+        expect(subject[:report]).not_to include(:job_path)
+      end
+    end
+  end
+end

ee/spec/serializers/dependency_list_serializer_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe DependencyListSerializer do
+  let(:serializer) { described_class.new(project: project).represent(dependencies, build: build) }
+  let(:build) { create(:ee_ci_build, :success) }
+  let(:project) { create(:project) }
+
+  let(:dependencies) do
+    [{
+       name:     'nokogiri',
+       packager: 'Ruby (Bundler)',
+       version:  '1.8.0',
+       location: {
+         blob_path: '/some_project/path/Gemfile.lock',
+         path:      'Gemfile.lock'
+       }
+     }]
+  end
+
+  describe "#to_json" do
+    subject { serializer.to_json }
+
+    it 'matches the schema' do
+      is_expected.to match_schema('dependency_list', dir: 'ee')
+    end
+  end
+end