Add latest changes from gitlab-org/gitlab@master

4 files changed, 628 insertions, 9 deletions

diff --git a/spec/services/design_management/delete_designs_service_spec.rb b/spec/services/design_management/delete_designs_service_spec.rb
new file mode 100644
index 00000000000..2c0c1570cb4
--- /dev/null
+++ b/spec/services/design_management/delete_designs_service_spec.rb
@@ -0,0 +1,195 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+describe DesignManagement::DeleteDesignsService do
+  include DesignManagementTestHelpers
+
+  let_it_be(:project) { create(:project) }
+  let_it_be(:issue) { create(:issue, project: project) }
+  let_it_be(:user) { create(:user) }
+  let(:designs) { create_designs }
+
+  subject(:service) { described_class.new(project, user, issue: issue, designs: designs) }
+
+  # Defined as a method so that the reponse is not cached. We also construct
+  # a new service executor each time to avoid the intermediate cached values
+  # it constructs during its execution.
+  def run_service(delenda = nil)
+    service = described_class.new(project, user, issue: issue, designs: delenda || designs)
+    service.execute
+  end
+
+  let(:response) { run_service }
+
+  shared_examples 'a service error' do
+    it 'returns an error', :aggregate_failures do
+      expect(response).to include(status: :error)
+    end
+  end
+
+  shared_examples 'a top-level error' do
+    let(:expected_error) { StandardError }
+    it 'raises an en expected error', :aggregate_failures do
+      expect { run_service }.to raise_error(expected_error)
+    end
+  end
+
+  shared_examples 'a success' do
+    it 'returns successfully', :aggregate_failures do
+      expect(response).to include(status: :success)
+    end
+
+    it 'saves the user as the author' do
+      version = response[:version]
+
+      expect(version.author).to eq(user)
+    end
+  end
+
+  before do
+    enable_design_management(enabled)
+    project.add_developer(user)
+  end
+
+  describe "#execute" do
+    context "when the feature is not available" do
+      let(:enabled) { false }
+
+      it_behaves_like "a service error"
+    end
+
+    context "when the feature is available" do
+      let(:enabled) { true }
+
+      it 'is able to delete designs' do
+        expect(service.send(:can_delete_designs?)).to be true
+      end
+
+      context 'no designs were passed' do
+        let(:designs) { [] }
+
+        it_behaves_like "a top-level error"
+
+        it 'does not log any events' do
+          counter = ::Gitlab::UsageDataCounters::DesignsCounter
+          expect { run_service rescue nil }.not_to change { counter.totals }
+        end
+      end
+
+      context 'one design is passed' do
+        before do
+          create_designs(2)
+        end
+
+        let!(:designs) { create_designs(1) }
+
+        it 'removes that design' do
+          expect { run_service }.to change { issue.designs.current.count }.from(3).to(2)
+        end
+
+        it 'logs a deletion event' do
+          counter = ::Gitlab::UsageDataCounters::DesignsCounter
+          expect { run_service }.to change { counter.read(:delete) }.by(1)
+        end
+
+        it 'informs the new-version-worker' do
+          expect(::DesignManagement::NewVersionWorker).to receive(:perform_async).with(Integer)
+
+          run_service
+        end
+
+        it 'creates a new version' do
+          expect { run_service }.to change { DesignManagement::Version.where(issue: issue).count }.by(1)
+        end
+
+        it 'returns the new version' do
+          version = response[:version]
+
+          expect(version).to eq(DesignManagement::Version.for_issue(issue).ordered.first)
+        end
+
+        it_behaves_like "a success"
+
+        it 'removes the design from the current design list' do
+          run_service
+
+          expect(issue.designs.current).not_to include(designs.first)
+        end
+
+        it 'marks the design as deleted' do
+          expect { run_service }
+            .to change { designs.first.deleted? }.from(false).to(true)
+        end
+      end
+
+      context 'more than one design is passed' do
+        before do
+          create_designs(1)
+        end
+
+        let!(:designs) { create_designs(2) }
+
+        it 'removes those designs' do
+          expect { run_service }
+            .to change { issue.designs.current.count }.from(3).to(1)
+        end
+
+        it 'logs the correct number of deletion events' do
+          counter = ::Gitlab::UsageDataCounters::DesignsCounter
+          expect { run_service }.to change { counter.read(:delete) }.by(2)
+        end
+
+        it_behaves_like "a success"
+
+        context 'after executing the service' do
+          let(:deleted_designs) { designs.map(&:reset) }
+
+          let!(:version) { run_service[:version] }
+
+          it 'removes the removed designs from the current design list' do
+            expect(issue.designs.current).not_to include(*deleted_designs)
+          end
+
+          it 'does not make the designs impossible to find' do
+            expect(issue.designs).to include(*deleted_designs)
+          end
+
+          it 'associates the new version with all the designs' do
+            current_versions = deleted_designs.map { |d| d.most_recent_action.version }
+            expect(current_versions).to all(eq version)
+          end
+
+          it 'marks all deleted designs as deleted' do
+            expect(deleted_designs).to all(be_deleted)
+          end
+
+          it 'marks all deleted designs with the same deletion version' do
+            expect(deleted_designs.map { |d| d.most_recent_action.version_id }.uniq)
+              .to have_attributes(size: 1)
+          end
+        end
+      end
+
+      describe 'scalability' do
+        before do
+          run_service(create_designs(1)) # ensure project, issue, etc are created
+        end
+
+        it 'makes the same number of DB requests for one design as for several' do
+          one = create_designs(1)
+          many = create_designs(5)
+
+          baseline = ActiveRecord::QueryRecorder.new { run_service(one) }
+
+          expect { run_service(many) }.not_to exceed_query_limit(baseline)
+        end
+      end
+    end
+  end
+
+  private
+
+  def create_designs(how_many = 2)
+    create_list(:design, how_many, :with_lfs_file, issue: issue)
+  end
+end
diff --git a/spec/services/design_management/design_user_notes_count_service_spec.rb b/spec/services/design_management/design_user_notes_count_service_spec.rb
index f4808542995..62211a4dd0f 100644
--- a/spec/services/design_management/design_user_notes_count_service_spec.rb
+++ b/spec/services/design_management/design_user_notes_count_service_spec.rb
@@ -23,15 +23,8 @@ describe DesignManagement::DesignUserNotesCountService, :use_clean_rails_memory_
     end
   end
 
-  # TODO These tests are being temporarily skipped unless run in EE,
-  # as we are in the process of moving Design Management to FOSS in 13.0
-  # in steps. In the current step the services have not yet been moved.
-  #
-  # See https://gitlab.com/gitlab-org/gitlab/-/issues/212566#note_327724283.
   describe 'cache invalidation' do
     it 'changes when a new note is created' do
-      skip 'See https://gitlab.com/gitlab-org/gitlab/-/issues/212566#note_327724283' unless Gitlab.ee?
-
       new_note_attrs = attributes_for(:diff_note_on_design, noteable: design)
 
       expect do
@@ -40,8 +33,6 @@ describe DesignManagement::DesignUserNotesCountService, :use_clean_rails_memory_
     end
 
     it 'changes when a note is destroyed' do
-      skip 'See https://gitlab.com/gitlab-org/gitlab/-/issues/212566#note_327724283' unless Gitlab.ee?
-
       note = create(:diff_note_on_design, noteable: design)
 
       expect do
diff --git a/spec/services/design_management/generate_image_versions_service_spec.rb b/spec/services/design_management/generate_image_versions_service_spec.rb
new file mode 100644
index 00000000000..cd021c8d7d3
--- /dev/null
+++ b/spec/services/design_management/generate_image_versions_service_spec.rb
@@ -0,0 +1,77 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe DesignManagement::GenerateImageVersionsService do
+  let_it_be(:project) { create(:project) }
+  let_it_be(:issue) { create(:issue, project: project) }
+  let_it_be(:version) { create(:design, :with_lfs_file, issue: issue).versions.first }
+  let_it_be(:action) { version.actions.first }
+
+  describe '#execute' do
+    it 'generates the image' do
+      expect { described_class.new(version).execute }
+        .to change { action.reload.image_v432x230.file }
+        .from(nil).to(CarrierWave::SanitizedFile)
+    end
+
+    it 'skips generating image versions if the mime type is not whitelisted' do
+      stub_const('DesignManagement::DesignV432x230Uploader::MIME_TYPE_WHITELIST', [])
+
+      described_class.new(version).execute
+
+      expect(action.reload.image_v432x230.file).to eq(nil)
+    end
+
+    it 'skips generating image versions if the design file size is too large' do
+      stub_const("#{described_class.name}::MAX_DESIGN_SIZE", 1.byte)
+
+      described_class.new(version).execute
+
+      expect(action.reload.image_v432x230.file).to eq(nil)
+    end
+
+    it 'returns the status' do
+      result = described_class.new(version).execute
+
+      expect(result[:status]).to eq(:success)
+    end
+
+    it 'returns the version' do
+      result = described_class.new(version).execute
+
+      expect(result[:version]).to eq(version)
+    end
+
+    it 'logs if the raw image cannot be found' do
+      version.designs.first.update(filename: 'foo.png')
+
+      expect(Gitlab::AppLogger).to receive(:error).with("No design file found for Action: #{action.id}")
+
+      described_class.new(version).execute
+    end
+
+    context 'when an error is encountered when generating the image versions' do
+      before do
+        expect_next_instance_of(DesignManagement::DesignV432x230Uploader) do |uploader|
+          expect(uploader).to receive(:cache!).and_raise(CarrierWave::DownloadError, 'foo')
+        end
+      end
+
+      it 'logs the error' do
+        expect(Gitlab::AppLogger).to receive(:error).with('foo')
+
+        described_class.new(version).execute
+      end
+
+      it 'tracks the error' do
+        expect(Gitlab::ErrorTracking).to receive(:track_exception).with(
+          instance_of(CarrierWave::DownloadError),
+          project_id: project.id, version_id: version.id, design_id: version.designs.first.id
+        )
+
+        described_class.new(version).execute
+      end
+    end
+  end
+end
diff --git a/spec/services/design_management/save_designs_service_spec.rb b/spec/services/design_management/save_designs_service_spec.rb
new file mode 100644
index 00000000000..013d5473860
--- /dev/null
+++ b/spec/services/design_management/save_designs_service_spec.rb
@@ -0,0 +1,356 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+describe DesignManagement::SaveDesignsService do
+  include DesignManagementTestHelpers
+  include ConcurrentHelpers
+
+  let_it_be(:developer) { create(:user) }
+  let(:project) { issue.project }
+  let(:issue) { create(:issue) }
+  let(:user) { developer }
+  let(:files) { [rails_sample] }
+  let(:design_repository) { ::Gitlab::GlRepository::DESIGN.repository_resolver.call(project) }
+  let(:rails_sample_name) { 'rails_sample.jpg' }
+  let(:rails_sample) { sample_image(rails_sample_name) }
+  let(:dk_png) { sample_image('dk.png') }
+
+  def sample_image(filename)
+    fixture_file_upload("spec/fixtures/#{filename}")
+  end
+
+  before do
+    project.add_developer(developer)
+  end
+
+  def run_service(files_to_upload = nil)
+    design_files = files_to_upload || files
+    design_files.each(&:rewind)
+
+    service = described_class.new(project, user,
+                                  issue: issue,
+                                  files: design_files)
+    service.execute
+  end
+
+  # Randomly alter the content of files.
+  # This allows the files to be updated by the service, as unmodified
+  # files are rejected.
+  def touch_files(files_to_touch = nil)
+    design_files = files_to_touch || files
+
+    design_files.each do |f|
+      f.tempfile.write(SecureRandom.random_bytes)
+    end
+  end
+
+  let(:response) { run_service }
+
+  shared_examples 'a service error' do
+    it 'returns an error', :aggregate_failures do
+      expect(response).to match(a_hash_including(status: :error))
+    end
+  end
+
+  shared_examples 'an execution error' do
+    it 'returns an error', :aggregate_failures do
+      expect { service.execute }.to raise_error(some_error)
+    end
+  end
+
+  describe '#execute' do
+    context 'when the feature is not available' do
+      before do
+        enable_design_management(false)
+      end
+
+      it_behaves_like 'a service error'
+    end
+
+    context 'when the feature is available' do
+      before do
+        enable_design_management(true)
+      end
+
+      describe 'repository existence' do
+        def repository_exists
+          # Expire the memoized value as the service creates it's own instance
+          design_repository.expire_exists_cache
+          design_repository.exists?
+        end
+
+        it 'creates a design repository when it did not exist' do
+          expect { run_service }.to change { repository_exists }.from(false).to(true)
+        end
+      end
+
+      it 'updates the creation count' do
+        counter = Gitlab::UsageDataCounters::DesignsCounter
+        expect { run_service }.to change { counter.read(:create) }.by(1)
+      end
+
+      it 'creates a commit in the repository' do
+        run_service
+
+        expect(design_repository.commit).to have_attributes(
+          author: user,
+          message: include(rails_sample_name)
+        )
+      end
+
+      it 'can run the same command in parallel' do
+        blocks = Array.new(10).map do
+          unique_files = %w(rails_sample.jpg dk.png)
+                          .map { |name| RenameableUpload.unique_file(name) }
+
+          -> { run_service(unique_files) }
+        end
+
+        expect { run_parallel(blocks) }.to change(DesignManagement::Version, :count).by(10)
+      end
+
+      it 'causes diff_refs not to be nil' do
+        expect(response).to include(
+          designs: all(have_attributes(diff_refs: be_present))
+        )
+      end
+
+      it 'creates a design & a version for the filename if it did not exist' do
+        expect(issue.designs.size).to eq(0)
+
+        updated_designs = response[:designs]
+
+        expect(updated_designs.size).to eq(1)
+        expect(updated_designs.first.versions.size).to eq(1)
+      end
+
+      it 'saves the user as the author' do
+        updated_designs = response[:designs]
+
+        expect(updated_designs.first.versions.first.author).to eq(user)
+      end
+
+      describe 'saving the file to LFS' do
+        before do
+          expect_next_instance_of(Lfs::FileTransformer) do |transformer|
+            expect(transformer).to receive(:lfs_file?).and_return(true)
+          end
+        end
+
+        it 'saves the design to LFS' do
+          expect { run_service }.to change { LfsObject.count }.by(1)
+        end
+
+        it 'saves the repository_type of the LfsObjectsProject as design' do
+          expect do
+            run_service
+          end.to change { project.lfs_objects_projects.count }.from(0).to(1)
+
+          expect(project.lfs_objects_projects.first.repository_type).to eq('design')
+        end
+      end
+
+      context 'when a design is being updated' do
+        before do
+          run_service
+          touch_files
+        end
+
+        it 'creates a new version for the existing design and updates the file' do
+          expect(issue.designs.size).to eq(1)
+          expect(DesignManagement::Version.for_designs(issue.designs).size).to eq(1)
+
+          updated_designs = response[:designs]
+
+          expect(updated_designs.size).to eq(1)
+          expect(updated_designs.first.versions.size).to eq(2)
+        end
+
+        it 'increments the update counter' do
+          counter = Gitlab::UsageDataCounters::DesignsCounter
+          expect { run_service }.to change { counter.read(:update) }.by 1
+        end
+
+        context 'when uploading a new design' do
+          it 'does not link the new version to the existing design' do
+            existing_design = issue.designs.first
+
+            updated_designs = run_service([dk_png])[:designs]
+
+            expect(existing_design.versions.reload.size).to eq(1)
+            expect(updated_designs.size).to eq(1)
+            expect(updated_designs.first.versions.size).to eq(1)
+          end
+        end
+      end
+
+      context 'when a design has not changed since its previous version' do
+        before do
+          run_service
+        end
+
+        it 'does not create a new version' do
+          expect { run_service }.not_to change { issue.design_versions.count }
+        end
+
+        it 'returns the design in `skipped_designs` instead of `designs`' do
+          response = run_service
+
+          expect(response[:designs]).to be_empty
+          expect(response[:skipped_designs].size).to eq(1)
+        end
+      end
+
+      context 'when doing a mixture of updates and creations' do
+        let(:files) { [rails_sample, dk_png] }
+
+        before do
+          # Create just the first one, which we will later update.
+          run_service([files.first])
+          touch_files([files.first])
+        end
+
+        it 'counts one creation and one update' do
+          counter = Gitlab::UsageDataCounters::DesignsCounter
+          expect { run_service }
+            .to change { counter.read(:create) }.by(1)
+            .and change { counter.read(:update) }.by(1)
+        end
+
+        it 'creates a single commit' do
+          commit_count = -> do
+            design_repository.expire_all_method_caches
+            design_repository.commit_count
+          end
+
+          expect { run_service }.to change { commit_count.call }.by(1)
+        end
+
+        it 'enqueues just one new version worker' do
+          expect(::DesignManagement::NewVersionWorker)
+            .to receive(:perform_async).once.with(Integer)
+
+          run_service
+        end
+      end
+
+      context 'when uploading multiple files' do
+        let(:files) { [rails_sample, dk_png] }
+
+        it 'returns information about both designs in the response' do
+          expect(response).to include(designs: have_attributes(size: 2), status: :success)
+        end
+
+        it 'creates 2 designs with a single version' do
+          expect { run_service }.to change { issue.designs.count }.from(0).to(2)
+
+          expect(DesignManagement::Version.for_designs(issue.designs).size).to eq(1)
+        end
+
+        it 'increments the creation count by 2' do
+          counter = Gitlab::UsageDataCounters::DesignsCounter
+          expect { run_service }.to change { counter.read(:create) }.by 2
+        end
+
+        it 'enqueues a new version worker' do
+          expect(::DesignManagement::NewVersionWorker)
+            .to receive(:perform_async).once.with(Integer)
+
+          run_service
+        end
+
+        it 'creates a single commit' do
+          commit_count = -> do
+            design_repository.expire_all_method_caches
+            design_repository.commit_count
+          end
+
+          expect { run_service }.to change { commit_count.call }.by(1)
+        end
+
+        it 'only does 5 gitaly calls', :request_store, :sidekiq_might_not_need_inline do
+          allow(::DesignManagement::NewVersionWorker).to receive(:perform_async).with(Integer)
+          service = described_class.new(project, user, issue: issue, files: files)
+          # Some unrelated calls that are usually cached or happen only once
+          service.__send__(:repository).create_if_not_exists
+          service.__send__(:repository).has_visible_content?
+
+          request_count = -> { Gitlab::GitalyClient.get_request_count }
+
+          # An exists?, a check for existing blobs, default branch, an after_commit
+          # callback on LfsObjectsProject
+          expect { service.execute }.to change(&request_count).by(4)
+        end
+
+        context 'when uploading too many files' do
+          let(:files) { Array.new(DesignManagement::SaveDesignsService::MAX_FILES + 1) { dk_png } }
+
+          it 'returns the correct error' do
+            expect(response[:message]).to match(/only \d+ files are allowed simultaneously/i)
+          end
+        end
+      end
+
+      context 'when the user is not allowed to upload designs' do
+        let(:user) { create(:user) }
+
+        it_behaves_like 'a service error'
+      end
+
+      describe 'failure modes' do
+        let(:service) { described_class.new(project, user, issue: issue, files: files) }
+        let(:response) { service.execute }
+
+        before do
+          expect(service).to receive(:run_actions).and_raise(some_error)
+        end
+
+        context 'when creating the commit fails' do
+          let(:some_error) { Gitlab::Git::BaseError }
+
+          it_behaves_like 'an execution error'
+        end
+
+        context 'when creating the versions fails' do
+          let(:some_error) { ActiveRecord::RecordInvalid }
+
+          it_behaves_like 'a service error'
+        end
+      end
+
+      context "when a design already existed in the repo but we didn't know about it in the database" do
+        let(:filename) { rails_sample_name }
+
+        before do
+          path = File.join(build(:design, issue: issue, filename: filename).full_path)
+          design_repository.create_if_not_exists
+          design_repository.create_file(user, path, 'something fake',
+                                        branch_name: 'master',
+                                        message: 'Somehow created without being tracked in db')
+        end
+
+        it 'creates the design and a new version for it' do
+          first_updated_design = response[:designs].first
+
+          expect(first_updated_design.filename).to eq(filename)
+          expect(first_updated_design.versions.size).to eq(1)
+        end
+      end
+
+      describe 'scalability', skip: 'See: https://gitlab.com/gitlab-org/gitlab/-/issues/213169' do
+        before do
+          run_service([sample_image('banana_sample.gif')]) # ensure project, issue, etc are created
+        end
+
+        it 'runs the same queries for all requests, regardless of number of files' do
+          one = [dk_png]
+          two = [rails_sample, dk_png]
+
+          baseline = ActiveRecord::QueryRecorder.new { run_service(one) }
+
+          expect { run_service(two) }.not_to exceed_query_limit(baseline)
+        end
+      end
+    end
+  end
+end