Merge branch '213014_instance_level_exportable_security_reports_export_logic' into 'master'

Instance level exportable security reports export logic

See merge request gitlab-org/gitlab!29920

ee/app/models/ee/user.rb

@@ -349,6 +349,10 @@ module EE
       gitlab_employee? ? 'GitLab' : super
     end
 
+    def security_dashboard
+      InstanceSecurityDashboard.new(self)
+    end
+
     protected
 
     override :password_required?

ee/app/models/instance_security_dashboard.rb

@@ -3,6 +3,8 @@
 class InstanceSecurityDashboard
   extend ActiveModel::Naming
 
+  delegate :full_path, to: :user
+
   def initialize(user, project_ids: [])
     @project_ids = project_ids
     @user = user

ee/app/models/vulnerabilities/export.rb

@@ -16,8 +16,6 @@ module Vulnerabilities
       csv: 0
     }
 
-    validates :project, presence: true, unless: :group
-    validates :group, presence: true, unless: :project
     validates :status, presence: true
     validates :format, presence: true
     validates :file, presence: true, if: :finished?
@@ -49,6 +47,21 @@ module Vulnerabilities
       end
     end
 
+    def exportable
+      project || author.security_dashboard
+    end
+
+    def exportable=(value)
+      case value
+      when Project
+        self.project = value
+      when InstanceSecurityDashboard
+        self.project = nil
+      else
+        raise "Can not assign #{value.class} as exportable"
+      end
+    end
+
     def completed?
       finished? || failed?
     end

ee/app/policies/instance_security_dashboard_policy.rb

 # frozen_string_literal: true
 
 class InstanceSecurityDashboardPolicy < BasePolicy
-  rule { ~anonymous }.enable :read_instance_security_dashboard
+  rule { ~anonymous }.policy do
+    enable :read_instance_security_dashboard
+    enable :create_vulnerability_export
+  end
 end

ee/app/policies/vulnerabilities/export_policy.rb

@@ -2,10 +2,10 @@
 
 module Vulnerabilities
   class ExportPolicy < BasePolicy
-    delegate { @subject.project }
+    delegate { @subject.exportable }
 
     condition(:is_author) { @user && @subject.author == @user }
-    condition(:exportable) { can?(:create_vulnerability_export, @subject.project) }
+    condition(:exportable) { can?(:create_vulnerability_export, @subject.exportable) }
 
     rule { exportable & is_author }.policy do
       enable :read_vulnerability_export

ee/app/services/vulnerability_exports/create_service.rb

@@ -4,19 +4,19 @@ module VulnerabilityExports
   class CreateService
     include Gitlab::Allowable
 
-    attr_reader :project, :author, :format
+    attr_reader :exportable, :author, :format
 
-    def initialize(project, author, format:)
-      @project = project
+    def initialize(exportable, author, format:)
+      @exportable = exportable
       @author = author
       @format = format
     end
 
     def execute
-      raise Gitlab::Access::AccessDeniedError unless can?(author, :create_vulnerability_export, project)
+      raise Gitlab::Access::AccessDeniedError unless can?(author, :create_vulnerability_export, exportable)
 
-      vulnerability_export = Vulnerabilities::Export.create(project: project, format: format, author: author)
-      ::VulnerabilityExports::ExportWorker.perform_async(project.id, vulnerability_export.id)
+      vulnerability_export = Vulnerabilities::Export.create(exportable: exportable, format: format, author: author)
+      ::VulnerabilityExports::ExportWorker.perform_async(vulnerability_export.id)
       vulnerability_export
     end
   end

ee/app/services/vulnerability_exports/export_csv_service.rb

-# frozen_string_literal: true
-
-module VulnerabilityExports
-  class ExportCsvService
-    attr_reader :vulnerabilities
-
-    def initialize(vulnerabilities_relation)
-      @vulnerabilities = vulnerabilities_relation
-    end
-
-    def csv_data(&block)
-      csv_builder.render(&block)
-    end
-
-    def csv_builder
-      @csv_builder ||= CsvBuilder.new(vulnerabilities.with_findings_and_scanner, header_to_value_hash)
-    end
-
-    private
-
-    def header_to_value_hash
-      {
-        'Scanner Type' => 'report_type',
-        'Scanner Name' => 'finding_scanner_name',
-        'Status' => 'state',
-        'Vulnerability' => 'title',
-        'Details' => 'description',
-        'Additional Info' => -> (vulnerability) { vulnerability.finding_metadata&.fetch('message', nil) },
-        'Severity' => 'severity',
-        'CVE' => -> (vulnerability) { vulnerability.finding_metadata&.fetch('cve', nil) }
-      }
-    end
-  end
-end

ee/app/services/vulnerability_exports/export_service.rb

+# frozen_string_literal: true
+
+module VulnerabilityExports
+  class ExportService
+    include ::Gitlab::ExclusiveLeaseHelpers
+
+    LEASE_TTL = 1.hour
+    LEASE_NAMESPACE = "vulnerability_exports_export"
+    EXPORTERS = {
+      'csv' => VulnerabilityExports::Exporters::CsvService
+    }.freeze
+
+    def self.export(vulnerability_export)
+      new(vulnerability_export).export
+    end
+
+    def initialize(vulnerability_export)
+      self.vulnerability_export = vulnerability_export
+    end
+
+    def export
+      in_lock(lease_key, ttl: LEASE_TTL) do
+        generate_export if vulnerability_export.created?
+      end
+    end
+
+    private
+
+    attr_accessor :vulnerability_export
+
+    delegate :exportable, to: :vulnerability_export, private: true
+    delegate :format, to: :vulnerability_export, private: true
+
+    def lease_key
+      "#{LEASE_NAMESPACE}:#{vulnerability_export.id}"
+    end
+
+    def generate_export
+      vulnerability_export.start!
+      generate_export_file
+      vulnerability_export.finish!
+    rescue => error
+      vulnerability_export.failed!
+      raise(error)
+    ensure
+      schedule_export_deletion
+    end
+
+    def generate_export_file
+      exporter.generate { |f| vulnerability_export.file = f }
+      vulnerability_export.file.filename = filename
+    end
+
+    def exporter
+      EXPORTERS[format].new(vulnerabilities)
+    end
+
+    def vulnerabilities
+      Security::VulnerabilitiesFinder.new(exportable).execute.with_findings_and_scanner
+    end
+
+    def schedule_export_deletion
+      VulnerabilityExports::ExportDeletionWorker.perform_in(1.hour, vulnerability_export.id)
+    end
+
+    def filename
+      [
+        exportable.full_path.parameterize,
+        '_vulnerabilities_',
+        Time.now.utc.strftime('%FT%H%M'),
+        '.',
+        format
+      ].join
+    end
+  end
+end

ee/app/services/vulnerability_exports/exporters/csv_service.rb

+# frozen_string_literal: true
+
+module VulnerabilityExports
+  module Exporters
+    class CsvService
+      attr_reader :vulnerabilities
+
+      def initialize(vulnerabilities)
+        @vulnerabilities = vulnerabilities
+      end
+
+      def generate(&block)
+        csv_builder.render(&block)
+      end
+
+      private
+
+      def csv_builder
+        @csv_builder ||= CsvBuilder.new(vulnerabilities, header_to_value_hash)
+      end
+
+      def header_to_value_hash
+        {
+          'Scanner Type' => 'report_type',
+          'Scanner Name' => 'finding_scanner_name',
+          'Status' => 'state',
+          'Vulnerability' => 'title',
+          'Details' => 'description',
+          'Additional Info' => -> (vulnerability) { vulnerability.finding_metadata&.fetch('message', nil) },
+          'Severity' => 'severity',
+          'CVE' => -> (vulnerability) { vulnerability.finding_metadata&.fetch('cve', nil) }
+        }
+      end
+    end
+  end
+end

ee/app/workers/vulnerability_exports/export_deletion_worker.rb

@@ -8,14 +8,9 @@ module VulnerabilityExports
 
     idempotent!
 
-    def perform(project_id, vulnerability_export_id)
-      project = Project.find_by_id(project_id)
-      return unless project
-
-      vulnerability_export = project.vulnerability_exports.find_by_id(vulnerability_export_id)
-      return unless vulnerability_export
-
-      vulnerability_export.destroy!
+    def perform(_exportable_id = nil, vulnerability_export_id) # rubocop:disable Style/OptionalArguments
+      vulnerability_export = Vulnerabilities::Export.find_by_id(vulnerability_export_id)
+      vulnerability_export&.destroy!
     end
   end
 end

ee/app/workers/vulnerability_exports/export_worker.rb

@@ -12,52 +12,13 @@ module VulnerabilityExports
 
     idempotent!
 
-    def perform(project_id, vulnerability_export_id)
-      project = Project.find_by_id(project_id)
-      return unless project
+    def perform(_exportable_id = nil, vulnerability_export_id) # rubocop:disable Style/OptionalArguments
+      vulnerability_export = Vulnerabilities::Export.find_by_id(vulnerability_export_id)
+      return unless vulnerability_export
 
-      vulnerability_export = project.vulnerability_exports.find_by_id(vulnerability_export_id)
-
-      return unless vulnerability_export&.created?
-      return unless try_obtain_lease_for(project_id, vulnerability_export_id)
-
-      schedule_export_deletion(project_id, vulnerability_export_id)
-
-      vulnerability_export.start!
-
-      vulnerabilities = Security::VulnerabilitiesFinder.new(project).execute
-
-      generate_file_data(vulnerability_export.format, vulnerabilities) do |file|
-        vulnerability_export.file = file
-        vulnerability_export.file.filename = generate_filename(project, vulnerability_export.format)
-        vulnerability_export.finish!
-      end
+      ExportService.export(vulnerability_export)
     rescue => error
       logger.error class: self.class.name, message: error.message
-      vulnerability_export&.failed!
-    end
-
-    private
-
-    def try_obtain_lease_for(project_id, vulnerability_export_id)
-      Gitlab::ExclusiveLease
-        .new("vulnerability_exports_export:#{project_id}/#{vulnerability_export_id}", timeout: LEASE_TIMEOUT)
-        .try_obtain
-    end
-
-    def generate_file_data(format, vulnerabilities, &block)
-      case format
-      when 'csv'
-        VulnerabilityExports::ExportCsvService.new(vulnerabilities).csv_data(&block)
-      end
-    end
-
-    def schedule_export_deletion(project_id, vulnerability_export_id)
-      VulnerabilityExports::ExportDeletionWorker.perform_in(1.hour, project_id, vulnerability_export_id)
-    end
-
-    def generate_filename(project, format)
-      "#{project.full_path.parameterize}_vulnerabilities_#{Time.now.utc.strftime('%FT%H%M')}.#{format}"
     end
   end
 end

ee/spec/factories/vulnerabilities/exports.rb

@@ -38,5 +38,10 @@ FactoryBot.define do
       project { nil }
       group
     end
+
+    trait :user do
+      project { nil }
+      group { nil }
+    end
   end
 end

ee/spec/models/instance_security_dashboard_spec.rb

@@ -115,4 +115,12 @@ describe InstanceSecurityDashboard do
       end
     end
   end
+
+  describe '#full_path' do
+    let(:user) { create(:user) }
+
+    it 'returns the full_path of the user' do
+      expect(subject.full_path).to eql(user.full_path)
+    end
+  end
 end

ee/spec/models/user_spec.rb

@@ -1143,4 +1143,14 @@ describe User do
       it { is_expected.to eql(expected_result) }
     end
   end
+
+  describe '#security_dashboard' do
+    let(:user) { create(:user) }
+
+    subject(:security_dashboard) { user.security_dashboard }
+
+    it 'returns an instance of InstanceSecurityDashboard for the user' do
+      expect(security_dashboard).to be_a(InstanceSecurityDashboard)
+    end
+  end
 end

ee/spec/models/vulnerabilities/export_spec.rb

@@ -23,16 +23,6 @@ describe Vulnerabilities::Export do
 
       it { is_expected.to validate_presence_of(:file) }
     end
-
-    context 'when the group is not set' do
-      it { is_expected.to validate_presence_of(:project) }
-    end
-
-    context 'when the project is not set' do
-      subject { build(:vulnerability_export, :group) }
-
-      it { is_expected.to validate_presence_of(:group) }
-    end
   end
 
   describe '#status' do
@@ -77,6 +67,63 @@ describe Vulnerabilities::Export do
     end
   end
 
+  describe '#exportable' do
+    subject { vulnerability_export.exportable }
+
+    context 'when the export has project assigned' do
+      let(:project) { build(:project) }
+      let(:vulnerability_export) { build(:vulnerability_export, project: project) }
+
+      it { is_expected.to eql(project) }
+    end
+
+    context 'when the export does not have project assigned' do
+      let(:author) { build(:user) }
+      let(:vulnerability_export) { build(:vulnerability_export, :user, author: author) }
+      let(:mock_security_dashboard) { instance_double(InstanceSecurityDashboard) }
+
+      before do
+        allow(author).to receive(:security_dashboard).and_return(mock_security_dashboard)
+      end
+
+      it { is_expected.to eql(mock_security_dashboard) }
+    end
+  end
+
+  describe '#exportable=' do
+    let(:vulnerability_export) { build(:vulnerability_export) }
+
+    subject(:set_exportable) { vulnerability_export.exportable = exportable }
+
+    context 'when the exportable is a Project' do
+      let(:exportable) { build(:project) }
+
+      it 'changes the exportable of the export to given project' do
+        expect { set_exportable }.to change { vulnerability_export.exportable }.to(exportable)
+      end
+    end
+
+    context 'when the exportable is an InstanceSecurityDashboard' do
+      let(:exportable) { InstanceSecurityDashboard.new(vulnerability_export.author) }
+
+      before do
+        allow(vulnerability_export.author).to receive(:security_dashboard).and_return(exportable)
+      end
+
+      it 'changes the exportable of the export to security dashboard of the author' do
+        expect { set_exportable }.to change { vulnerability_export.exportable }.to(exportable)
+      end
+    end
+
+    context 'when the exportable is a String' do
+      let(:exportable) { 'Foo' }
+
+      it 'raises an exception' do
+        expect { set_exportable }.to raise_error(RuntimeError)
+      end
+    end
+  end
+
   describe '#completed?' do
     context 'when status is created' do
       subject { build(:vulnerability_export, :created) }

ee/spec/policies/instance_security_dashboard_policy_spec.rb

@@ -23,4 +23,16 @@ describe InstanceSecurityDashboardPolicy do
       it { is_expected.to be_allowed(:read_instance_security_dashboard) }
     end
   end
+
+  describe 'create_vulnerability_export' do
+    context 'when the user is not logged in' do
+      let(:current_user) { nil }
+
+      it { is_expected.not_to be_allowed(:create_vulnerability_export) }
+    end
+
+    context 'when the user is logged in' do
+      it { is_expected.to be_allowed(:create_vulnerability_export) }
+    end
+  end
 end

ee/spec/requests/api/vulnerability_exports_spec.rb

@@ -34,7 +34,7 @@ describe API::VulnerabilityExports do
         end
 
         it 'schedules job for export' do
-          expect(::VulnerabilityExports::ExportWorker).to receive(:perform_async).with(project.id, anything)
+          expect(::VulnerabilityExports::ExportWorker).to receive(:perform_async).with(anything)
 
           create_vulnerability_export
         end

ee/spec/services/vulnerability_exports/create_service_spec.rb

@@ -14,7 +14,7 @@ describe VulnerabilityExports::CreateService do
   let(:project) { create(:project, :public, group: group) }
   let(:format) { 'csv' }
 
-  subject { described_class.new(project, user, format: format).execute }
+  subject(:create_export) { described_class.new(project, user, format: format).execute }
 
   describe '#execute' do
     context 'when security dashboard feature is disabled' do
@@ -23,27 +23,33 @@ describe VulnerabilityExports::CreateService do
       end
 
       it 'raises an "access denied" error' do
-        expect { subject }.to raise_error(Gitlab::Access::AccessDeniedError)
+        expect { create_export }.to raise_error(Gitlab::Access::AccessDeniedError)
       end
     end
 
     context 'when security dashboard feature is enabled' do
+      let(:recent_vulnerability_export) { Vulnerabilities::Export.last }
+
+      before do
+        allow(::VulnerabilityExports::ExportWorker).to receive(:perform_async)
+      end
+
       it 'does not raise an "access denied" error' do
-        expect { subject }.not_to raise_error
+        expect { create_export }.not_to raise_error
       end
 
       it 'creates new Vulnerabilities::Export' do
-        expect { subject }.to change { Vulnerabilities::Export.count }.from(0).to(1)
+        expect { create_export }.to change { Vulnerabilities::Export.count }.from(0).to(1)
       end
 
       it 'schedules ::VulnerabilityExports::ExportWorker background job' do
-        expect(::VulnerabilityExports::ExportWorker).to receive(:perform_async).with(project.id, anything)
+        create_export
 
-        subject
+        expect(::VulnerabilityExports::ExportWorker).to have_received(:perform_async).with(recent_vulnerability_export.id)
       end
 
       it 'returns new Vulnerabilities::Export with project and format assigned' do
-        expect(subject).to have_attributes(project_id: project.id, format: format)
+        expect(create_export).to have_attributes(project_id: project.id, format: format)
       end
     end
   end

ee/spec/services/vulnerability_exports/export_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe VulnerabilityExports::ExportService do
+  describe '::export' do
+    let(:vulnerability_export) { create(:vulnerability_export) }
+    let(:mock_service_object) { instance_double(described_class, export: true) }
+
+    subject(:export) { described_class.export(vulnerability_export) }
+
+    before do
+      allow(described_class).to receive(:new).and_return(mock_service_object)
+    end
+
+    it 'instantiates a new instance of the service class and sends export message to it' do
+      export
+
+      expect(described_class).to have_received(:new).with(vulnerability_export)
+      expect(mock_service_object).to have_received(:export)
+    end
+  end
+
+  describe '#export' do
+    let(:vulnerability_export) { create(:vulnerability_export, :created) }
+    let(:service_object) { described_class.new(vulnerability_export) }
+
+    subject(:export) { service_object.export }
+
+    context 'generating the export file' do
+      let(:lease_name) { "vulnerability_exports_export:#{vulnerability_export.id}" }
+
+      before do
+        allow(service_object).to receive(:in_lock)
+      end
+
+      it 'runs synchronized with distributed semaphore' do
+        export
+
+        expect(service_object).to have_received(:in_lock).with(lease_name, ttl: 1.hour)
+      end
+    end
+
+    context 'when the vulnerability_export is not in `created` state' do
+      before do
+        allow(vulnerability_export).to receive(:created?).and_return(false)
+        allow(service_object).to receive(:generate_export)
+      end
+
+      it 'does not execute export file generation logic' do
+        export
+
+        expect(service_object).not_to have_received(:generate_export)
+      end
+    end
+
+    context 'when the vulnerability_export is in `created` state' do
+      before do
+        allow(VulnerabilityExports::ExportDeletionWorker).to receive(:perform_in)
+      end
+
+      context 'when the export generation fails' do
+        let(:error) { RuntimeError.new('foo') }
+
+        before do
+          allow(service_object).to receive(:generate_export_file).and_raise(error)
+        end
+
+        it 'marks the export object as `failed` and propagates the error to the caller' do
+          expect { export }.to raise_error(error)
+          expect(vulnerability_export.failed?).to be_truthy
+        end
+
+        it 'schedules the export deletion background job' do
+          expect { export }.to raise_error(error)
+          expect(VulnerabilityExports::ExportDeletionWorker).to have_received(:perform_in).with(1.hour, vulnerability_export.id)
+        end
+      end
+
+      context 'when the export generation succeeds' do
+        before do
+          allow(service_object).to receive(:generate_export_file)
+          allow(vulnerability_export).to receive(:start!)
+          allow(vulnerability_export).to receive(:finish!)
+        end
+
+        it 'marks the state of export object as `started` and then `finished`' do
+          export
+
+          expect(vulnerability_export).to have_received(:start!).ordered
+          expect(vulnerability_export).to have_received(:finish!).ordered
+        end
+
+        it 'schedules the export deletion background job' do
+          export
+
+          expect(VulnerabilityExports::ExportDeletionWorker).to have_received(:perform_in).with(1.hour, vulnerability_export.id)
+        end
+      end
+
+      context 'when the export format is csv' do
+        let(:vulnerabilities) { Vulnerability.none }
+        let(:mock_relation) { double(:relation, with_findings_and_scanner: vulnerabilities) }
+        let(:mock_vulnerability_finder_service_object) { instance_double(Security::VulnerabilitiesFinder, execute: mock_relation) }
+        let(:exportable_full_path) { 'foo' }
+        let(:time_suffix) { Time.now.utc.strftime('%FT%H%M') }
+        let(:expected_file_name) { "#{exportable_full_path}_vulnerabilities_#{time_suffix}.csv" }
+
+        before do
+          allow(Security::VulnerabilitiesFinder).to receive(:new).and_return(mock_vulnerability_finder_service_object)
+          allow(vulnerability_export.exportable).to receive(:full_path).and_return(exportable_full_path)
+        end
+
+        around do |example|
+          Timecop.freeze { example.run }
+        end
+
+        it 'calls the VulnerabilityExports::Exporters::CsvService which sets the file and filename' do
+          expect { export }.to change { vulnerability_export.file }
+                           .and change { vulnerability_export.file&.filename }.from(nil).to(expected_file_name)
+        end
+      end
+    end
+  end
+end

ee/spec/services/vulnerability_exports/export_csv_service_spec.rb → ee/spec/services/vulnerability_exports/exporters/csv_service_spec.rb

@@ -2,23 +2,23 @@
 
 require 'spec_helper'
 
-describe VulnerabilityExports::ExportCsvService do
+describe VulnerabilityExports::Exporters::CsvService do
   let_it_be(:project) { create(:project, :public) }
   let_it_be(:vulnerability) { create(:vulnerability, :with_findings, project: project) }
 
   let(:export_csv_service) { described_class.new(Vulnerability.all) }
 
-  subject(:csv) { CSV.parse(export_csv_service.csv_data, headers: true) }
+  subject(:csv) { CSV.parse(export_csv_service.generate, headers: true) }
 
   context 'when block is not given' do
     it 'renders csv to string' do
-      expect(export_csv_service.csv_data).to be_a String
+      expect(export_csv_service.generate).to be_a String
     end
   end
 
   context 'when block is given' do
     it 'returns handle to Tempfile' do
-      expect(export_csv_service.csv_data { |file| file }).to be_a Tempfile
+      expect(export_csv_service.generate { |file| file }).to be_a Tempfile
     end
   end
 

ee/spec/workers/vulnerability_exports/export_deletion_worker_spec.rb

@@ -4,40 +4,40 @@ require 'spec_helper'
 
 RSpec.describe VulnerabilityExports::ExportDeletionWorker, type: :worker do
   describe '#perform' do
-    let_it_be(:project) { create(:project) }
-    let_it_be(:vulnerability_export) { create(:vulnerability_export, :finished, :csv, :with_csv_file, project: project) }
-
     let(:worker) { described_class.new }
 
-    subject { worker.perform(project.id, vulnerability_export.id) }
+    subject(:delete_vulnerability_export) { worker.perform(vulnerability_export_id) }
 
     context 'when vulnerability export does not exist' do
-      subject { worker.perform(project.id, 9999) }
+      let(:vulnerability_export_id) { nil }
 
       it 'does not raise exception' do
-        expect { subject }.not_to raise_error
+        expect { delete_vulnerability_export }.not_to raise_error
       end
 
       it 'does not delete any vulnerability export from database' do
-        expect { subject }.not_to change { Vulnerabilities::Export.count }
+        expect { delete_vulnerability_export }.not_to change { Vulnerabilities::Export.count }
       end
     end
 
     context 'when vulnerability export exists' do
+      let_it_be(:vulnerability_export) { create(:vulnerability_export, :finished, :csv, :with_csv_file) }
+
+      let(:vulnerability_export_id) { vulnerability_export.id }
+
       context 'when destroy can be performed successfully' do
         it 'destroys vulnerability export' do
-          subject
-          expect(Vulnerabilities::Export.find_by_id(vulnerability_export.id)).to be_nil
+          expect { delete_vulnerability_export }.to change { Vulnerabilities::Export.find_by_id(vulnerability_export.id) }.to(nil)
         end
       end
 
       context 'when destroy fails' do
         before do
-          allow_any_instance_of(Vulnerabilities::Export).to receive(:destroy!).and_raise(ActiveRecord::RecordNotFound)
+          allow_any_instance_of(Vulnerabilities::Export).to receive(:destroy!).and_raise(ActiveRecord::Rollback)
         end
 
         it 'raises exception' do
-          expect { subject }.to raise_error(ActiveRecord::RecordNotFound)
+          expect { delete_vulnerability_export }.to raise_error(ActiveRecord::Rollback)
         end
       end
     end

ee/spec/workers/vulnerability_exports/export_worker_spec.rb

@@ -4,67 +4,54 @@ require 'spec_helper'
 
 RSpec.describe VulnerabilityExports::ExportWorker, type: :worker do
   describe '#perform' do
-    let_it_be(:project) { create(:project, :with_vulnerability) }
-    let!(:vulnerability_export) { create(:vulnerability_export, :created, :csv, project: project) }
-
     let(:worker) { described_class.new }
 
+    subject(:export_vulnerabilities) { worker.perform(vulnerability_export_id) }
+
     before do
-      allow(VulnerabilityExports::ExportDeletionWorker).to receive(:perform_in)
+      allow(VulnerabilityExports::ExportService).to receive(:export)
       allow(Sidekiq.logger).to receive(:error)
     end
 
     context 'when vulnerability export does not exist' do
-      subject { worker.perform(project.id, 9999) }
+      let(:vulnerability_export_id) { nil }
 
       it 'does not raise any error' do
-        expect { subject }.not_to raise_error
-      end
+        expect { export_vulnerabilities }.not_to raise_error
       end
 
-    context 'when vulnerability export exists' do
-      include_examples 'an idempotent worker' do
-        let(:job_args) { [project.id, vulnerability_export.id] }
-
-        context 'when export can be performed successfully' do
-          it 'creates new export file' do
-            subject
-            vulnerability_export.reload
-            expect(vulnerability_export).to be_finished
-            expect(vulnerability_export.file.read).to include('Scanner Type,Scanner Name,Status,Vulnerability,Details,Additional Info,Severity,CVE')
-          end
+      it 'does not call VulnerabilityExports::ExportService::export' do
+        export_vulnerabilities
 
-          it 'schedules job to delete export in 1 hour' do
-            expect(VulnerabilityExports::ExportDeletionWorker).to receive(:perform_in).with(1.hour, project.id, vulnerability_export.id)
-            subject
+        expect(VulnerabilityExports::ExportService).not_to have_received(:export)
       end
     end
+
+    context 'when vulnerability export exists' do
+      let(:vulnerability_export) { create(:vulnerability_export, :created, :csv) }
+      let(:vulnerability_export_id) { vulnerability_export.id }
+
+      it 'calls VulnerabilityExports::ExportService::export with the vulnerability_export object' do
+        export_vulnerabilities
+
+        expect(VulnerabilityExports::ExportService).to have_received(:export).with(vulnerability_export)
       end
 
       context 'when export fails' do
-        subject { worker.perform(project.id, vulnerability_export.id) }
+        let(:error_message) { 'Foo' }
 
         before do
-          allow_any_instance_of(Vulnerabilities::Export).to receive(:finish!).and_raise(ActiveRecord::RecordInvalid)
+          allow(VulnerabilityExports::ExportService).to receive(:export).and_raise(error_message)
         end
 
         it 'does not raise exception' do
-          expect { subject }.not_to raise_error
+          expect { export_vulnerabilities }.not_to raise_error
         end
 
         it 'logs error' do
-          expect(Sidekiq.logger).to receive(:error).with(class: described_class.name, message: anything)
-          subject
-        end
-
-        it 'sets status of the export to failed' do
-          expect_any_instance_of(Vulnerabilities::Export).to receive(:failed!)
-          subject
-        end
+          export_vulnerabilities
 
-        it 'schedules job to delete export in 1 hour' do
-          expect(VulnerabilityExports::ExportDeletionWorker).to receive(:perform_in).with(1.hour, project.id, vulnerability_export.id)
-          subject
+          expect(Sidekiq.logger).to have_received(:error).with(class: described_class.name, message: error_message)
         end
       end
     end