Add latest changes from gitlab-org/gitlab@master

10 files changed, 223 insertions, 58 deletions

diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb
index c83f60bf8b2..f3500301e22 100644
--- a/spec/factories/ci/builds.rb
+++ b/spec/factories/ci/builds.rb
@@ -237,8 +237,13 @@ FactoryBot.define do
         # to the job. If `build.deployment` has already been set, it doesn't
         # build a new instance.
         environment = Gitlab::Ci::Pipeline::Seed::Environment.new(build).to_resource
-        build.deployment =
-          Gitlab::Ci::Pipeline::Seed::Deployment.new(build, environment).to_resource
+
+        build.assign_attributes(
+          deployment: Gitlab::Ci::Pipeline::Seed::Deployment.new(build, environment).to_resource,
+          metadata_attributes: {
+            expanded_environment_name: environment.name
+          }
+        )
       end
     end
 
diff --git a/spec/finders/ci/pipelines_finder_spec.rb b/spec/finders/ci/pipelines_finder_spec.rb
index 16561aa65b6..c7bd52576e8 100644
--- a/spec/finders/ci/pipelines_finder_spec.rb
+++ b/spec/finders/ci/pipelines_finder_spec.rb
@@ -252,6 +252,29 @@ RSpec.describe Ci::PipelinesFinder do
       end
     end
 
+    context 'when source is specified' do
+      let(:params) { { source: 'web' } }
+      let!(:web_pipeline) { create(:ci_pipeline, project: project, source: 'web') }
+      let!(:push_pipeline) { create(:ci_pipeline, project: project, source: 'push') }
+      let!(:api_pipeline) { create(:ci_pipeline, project: project, source: 'api') }
+
+      context 'when `pipeline_source_filter` feature flag is disabled' do
+        before do
+          stub_feature_flags(pipeline_source_filter: false)
+        end
+
+        it 'returns all the pipelines' do
+          is_expected.to contain_exactly(web_pipeline, push_pipeline, api_pipeline)
+        end
+      end
+
+      context 'when `pipeline_source_filter` feature flag is enabled' do
+        it 'returns only the matched pipeline' do
+          is_expected.to eq([web_pipeline])
+        end
+      end
+    end
+
     describe 'ordering' do
       using RSpec::Parameterized::TableSyntax
 
diff --git a/spec/fixtures/api/schemas/pipeline_schedule.json b/spec/fixtures/api/schemas/pipeline_schedule.json
index 8a175ba081f..cdb4aea76da 100644
--- a/spec/fixtures/api/schemas/pipeline_schedule.json
+++ b/spec/fixtures/api/schemas/pipeline_schedule.json
@@ -18,6 +18,7 @@
         "sha": { "type": "string" },
         "ref": { "type": "string" },
         "status": { "type": "string" },
+        "source": { "type": "string" },
         "web_url": { "type": ["string", "null"] },
         "created_at": { "type": ["string", "null"], "format": "date-time" },
         "updated_at": { "type": ["string", "null"], "format": "date-time" }
diff --git a/spec/helpers/groups_helper_spec.rb b/spec/helpers/groups_helper_spec.rb
index 8447b92adbf..0b037a45945 100644
--- a/spec/helpers/groups_helper_spec.rb
+++ b/spec/helpers/groups_helper_spec.rb
@@ -19,11 +19,15 @@ RSpec.describe GroupsHelper do
     end
   end
 
-  describe '#group_dependency_proxy_url' do
+  describe '#group_dependency_proxy_image_prefix' do
+    let_it_be(:group) { build_stubbed(:group, path: 'GroupWithUPPERcaseLetters') }
+
     it 'converts uppercase letters to lowercase' do
-      group = build_stubbed(:group, path: 'GroupWithUPPERcaseLetters')
+      expect(group_dependency_proxy_image_prefix(group)).to end_with("/groupwithuppercaseletters#{DependencyProxy::URL_SUFFIX}")
+    end
 
-      expect(group_dependency_proxy_url(group)).to end_with("/groupwithuppercaseletters#{DependencyProxy::URL_SUFFIX}")
+    it 'removes the protocol' do
+      expect(group_dependency_proxy_image_prefix(group)).not_to include('http')
     end
   end
 
diff --git a/spec/lib/gitlab/background_migration/backfill_integrations_type_new_spec.rb b/spec/lib/gitlab/background_migration/backfill_integrations_type_new_spec.rb
index eaad5f8158b..8f765a7a536 100644
--- a/spec/lib/gitlab/background_migration/backfill_integrations_type_new_spec.rb
+++ b/spec/lib/gitlab/background_migration/backfill_integrations_type_new_spec.rb
@@ -3,6 +3,7 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::BackgroundMigration::BackfillIntegrationsTypeNew do
+  let(:migration) { described_class.new }
   let(:integrations) { table(:integrations) }
   let(:namespaced_integrations) { Gitlab::Integrations::StiType.namespaced_integrations }
 
@@ -12,12 +13,31 @@ RSpec.describe Gitlab::BackgroundMigration::BackfillIntegrationsTypeNew do
     namespaced_integrations.each_with_index do |type, i|
       integrations.create!(id: i + 1, type: "#{type}Service")
     end
+
+    integrations.create!(id: namespaced_integrations.size + 1, type: 'LegacyService')
   ensure
     integrations.connection.execute 'ALTER TABLE integrations ENABLE TRIGGER "trigger_type_new_on_insert"'
   end
 
   it 'backfills `type_new` for the selected records' do
-    described_class.new.perform(2, 10)
+    # We don't want to mock `Kernel.sleep`, so instead we mock it on the migration
+    # class before it gets forwarded.
+    expect(migration).to receive(:sleep).with(0.05).exactly(5).times
+
+    queries = ActiveRecord::QueryRecorder.new do
+      migration.perform(2, 10, :integrations, :id, 2, 50)
+    end
+
+    expect(queries.count).to be(16)
+    expect(queries.log.grep(/^SELECT/).size).to be(11)
+    expect(queries.log.grep(/^UPDATE/).size).to be(5)
+    expect(queries.log.grep(/^UPDATE/).join.scan(/WHERE .*/)).to eq([
+      'WHERE integrations.id BETWEEN 2 AND 3',
+      'WHERE integrations.id BETWEEN 4 AND 5',
+      'WHERE integrations.id BETWEEN 6 AND 7',
+      'WHERE integrations.id BETWEEN 8 AND 9',
+      'WHERE integrations.id BETWEEN 10 AND 10'
+    ])
 
     expect(integrations.where(id: 2..10).pluck(:type, :type_new)).to contain_exactly(
       ['AssemblaService',           'Integrations::Assembla'],
diff --git a/spec/lib/gitlab/content_security_policy/config_loader_spec.rb b/spec/lib/gitlab/content_security_policy/config_loader_spec.rb
index 436666c3011..239eff11bf3 100644
--- a/spec/lib/gitlab/content_security_policy/config_loader_spec.rb
+++ b/spec/lib/gitlab/content_security_policy/config_loader_spec.rb
@@ -4,7 +4,6 @@ require 'spec_helper'
 
 RSpec.describe Gitlab::ContentSecurityPolicy::ConfigLoader do
   let(:policy) { ActionDispatch::ContentSecurityPolicy.new }
-  let(:cdn_host) { nil }
   let(:csp_config) do
     {
       enabled: true,
@@ -20,14 +19,28 @@ RSpec.describe Gitlab::ContentSecurityPolicy::ConfigLoader do
     }
   end
 
-  describe '.default_settings_hash' do
-    let(:settings) { described_class.default_settings_hash(cdn_host) }
+  describe '.default_enabled' do
+    let(:enabled) { described_class.default_enabled }
 
-    it 'returns defaults for all keys' do
-      expect(settings['enabled']).to be_truthy
-      expect(settings['report_only']).to be_falsey
+    it 'is enabled' do
+      expect(enabled).to be_truthy
+    end
 
-      directives = settings['directives']
+    context 'when in production' do
+      before do
+        allow(Rails).to receive(:env).and_return(ActiveSupport::StringInquirer.new('production'))
+      end
+
+      it 'is disabled' do
+        expect(enabled).to be_falsey
+      end
+    end
+  end
+
+  describe '.default_directives' do
+    let(:directives) { described_class.default_directives }
+
+    it 'returns default directives' do
       directive_names = (described_class::DIRECTIVES - ['report_uri'])
       directive_names.each do |directive|
         expect(directives.has_key?(directive)).to be_truthy
@@ -39,22 +52,12 @@ RSpec.describe Gitlab::ContentSecurityPolicy::ConfigLoader do
       expect(directives['child_src']).to eq(directives['frame_src'])
     end
 
-    context 'when in production' do
+    context 'when CDN host is defined' do
       before do
-        allow(Rails).to receive(:env).and_return(ActiveSupport::StringInquirer.new('production'))
-      end
-
-      it 'is disabled' do
-        expect(settings['enabled']).to be_falsey
+        stub_config_setting(cdn_host: 'https://example.com')
       end
-    end
-
-    context 'when CDN host is defined' do
-      let(:cdn_host) { 'https://example.com' }
 
       it 'adds CDN host to CSP' do
-        directives = settings['directives']
-
         expect(directives['script_src']).to eq("'strict-dynamic' 'self' 'unsafe-inline' 'unsafe-eval' https://www.google.com/recaptcha/ https://www.recaptcha.net https://apis.google.com https://example.com")
         expect(directives['style_src']).to eq("'self' 'unsafe-inline' https://example.com")
         expect(directives['font_src']).to eq("'self' https://example.com")
@@ -67,8 +70,6 @@ RSpec.describe Gitlab::ContentSecurityPolicy::ConfigLoader do
       end
 
       it 'adds sentry path to CSP without user' do
-        directives = settings['directives']
-
         expect(directives['connect_src']).to eq("'self' dummy://example.com/43")
       end
     end
@@ -84,8 +85,6 @@ RSpec.describe Gitlab::ContentSecurityPolicy::ConfigLoader do
         end
 
         it 'does not add CUSTOMER_PORTAL_URL to CSP' do
-          directives = settings['directives']
-
           expect(directives['frame_src']).to eq("'self' https://www.google.com/recaptcha/ https://www.recaptcha.net/ https://content.googleapis.com https://content-compute.googleapis.com https://content-cloudbilling.googleapis.com https://content-cloudresourcemanager.googleapis.com")
         end
       end
@@ -96,8 +95,6 @@ RSpec.describe Gitlab::ContentSecurityPolicy::ConfigLoader do
         end
 
         it 'adds CUSTOMER_PORTAL_URL to CSP' do
-          directives = settings['directives']
-
           expect(directives['frame_src']).to eq("'self' https://www.google.com/recaptcha/ https://www.recaptcha.net/ https://content.googleapis.com https://content-compute.googleapis.com https://content-cloudbilling.googleapis.com https://content-cloudresourcemanager.googleapis.com https://customers.example.com")
         end
       end
diff --git a/spec/lib/gitlab/data_builder/pipeline_spec.rb b/spec/lib/gitlab/data_builder/pipeline_spec.rb
index 4c23b24aa31..0e574c7aa84 100644
--- a/spec/lib/gitlab/data_builder/pipeline_spec.rb
+++ b/spec/lib/gitlab/data_builder/pipeline_spec.rb
@@ -131,5 +131,41 @@ RSpec.describe Gitlab::DataBuilder::Pipeline do
         expect(build_environment_data[:deployment_tier]).to eq(build.persisted_environment.try(:tier))
       end
     end
+
+    context 'avoids N+1 database queries' do
+      it "with multiple builds" do
+        # Preparing the pipeline with the minimal builds
+        pipeline = create(:ci_pipeline, user: user, project: project)
+        create(:ci_build, user: user, project: project, pipeline: pipeline)
+        create(:ci_build, :deploy_to_production, :with_deployment, user: user, project: project, pipeline: pipeline)
+
+        # We need `.to_json` as the build hook data is wrapped within `Gitlab::Lazy`
+        control_count = ActiveRecord::QueryRecorder.new { described_class.build(pipeline.reload).to_json }.count
+
+        # Adding more builds to the pipeline and serializing the data again
+        create_list(:ci_build, 3, user: user, project: project, pipeline: pipeline)
+        create(:ci_build, :start_review_app, :with_deployment, user: user, project: project, pipeline: pipeline)
+        create(:ci_build, :stop_review_app, :with_deployment, user: user, project: project, pipeline: pipeline)
+
+        expect { described_class.build(pipeline.reload).to_json }.not_to exceed_query_limit(control_count)
+      end
+
+      it "with multiple retried builds" do
+        # Preparing the pipeline with the minimal builds
+        pipeline = create(:ci_pipeline, user: user, project: project)
+        create(:ci_build, :retried, user: user, project: project, pipeline: pipeline)
+        create(:ci_build, :deploy_to_production, :retried, :with_deployment, user: user, project: project, pipeline: pipeline)
+
+        # We need `.to_json` as the build hook data is wrapped within `Gitlab::Lazy`
+        control_count = ActiveRecord::QueryRecorder.new { described_class.build(pipeline.reload).with_retried_builds.to_json }.count
+
+        # Adding more builds to the pipeline and serializing the data again
+        create_list(:ci_build, 3, :retried, user: user, project: project, pipeline: pipeline)
+        create(:ci_build, :start_review_app, :retried, :with_deployment, user: user, project: project, pipeline: pipeline)
+        create(:ci_build, :stop_review_app, :retried, :with_deployment, user: user, project: project, pipeline: pipeline)
+
+        expect { described_class.build(pipeline.reload).with_retried_builds.to_json }.not_to exceed_query_limit(control_count)
+      end
+    end
   end
 end
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 74a476a6422..1d0b67c6902 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -263,6 +263,20 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
     end
   end
 
+  describe '.with_pipeline_source' do
+    subject { described_class.with_pipeline_source(source) }
+
+    let(:source) { 'web' }
+
+    let_it_be(:push_pipeline)   { create(:ci_pipeline, source: :push) }
+    let_it_be(:web_pipeline)    { create(:ci_pipeline, source: :web) }
+    let_it_be(:api_pipeline)    { create(:ci_pipeline, source: :api) }
+
+    it 'contains pipelines created due to specified source' do
+      expect(subject).to contain_exactly(web_pipeline)
+    end
+  end
+
   describe '.ci_sources' do
     subject { described_class.ci_sources }
 
diff --git a/spec/requests/api/ci/pipelines_spec.rb b/spec/requests/api/ci/pipelines_spec.rb
index 6c0a1b2502f..640e1ee6422 100644
--- a/spec/requests/api/ci/pipelines_spec.rb
+++ b/spec/requests/api/ci/pipelines_spec.rb
@@ -34,7 +34,28 @@ RSpec.describe API::Ci::Pipelines do
         expect(json_response.first['sha']).to match(/\A\h{40}\z/)
         expect(json_response.first['id']).to eq pipeline.id
         expect(json_response.first['web_url']).to be_present
-        expect(json_response.first.keys).to contain_exactly(*%w[id project_id sha ref status web_url created_at updated_at])
+      end
+
+      describe 'keys in the response' do
+        context 'when `pipeline_source_filter` feature flag is disabled' do
+          before do
+            stub_feature_flags(pipeline_source_filter: false)
+          end
+
+          it 'does not includes pipeline source' do
+            get api("/projects/#{project.id}/pipelines", user)
+
+            expect(json_response.first.keys).to contain_exactly(*%w[id project_id sha ref status web_url created_at updated_at])
+          end
+        end
+
+        context 'when `pipeline_source_filter` feature flag is disabled' do
+          it 'includes pipeline source' do
+            get api("/projects/#{project.id}/pipelines", user)
+
+            expect(json_response.first.keys).to contain_exactly(*%w[id project_id sha ref status web_url created_at updated_at source])
+          end
+        end
       end
 
       context 'when parameter is passed' do
@@ -294,6 +315,48 @@ RSpec.describe API::Ci::Pipelines do
             end
           end
         end
+
+        context 'when a source is specified' do
+          before do
+            create(:ci_pipeline, project: project, source: :push)
+            create(:ci_pipeline, project: project, source: :web)
+            create(:ci_pipeline, project: project, source: :api)
+          end
+
+          context 'when `pipeline_source_filter` feature flag is disabled' do
+            before do
+              stub_feature_flags(pipeline_source_filter: false)
+            end
+
+            it 'returns all pipelines' do
+              get api("/projects/#{project.id}/pipelines", user), params: { source: 'web' }
+
+              expect(response).to have_gitlab_http_status(:ok)
+              expect(response).to include_pagination_headers
+              expect(json_response).not_to be_empty
+              expect(json_response.length).to be >= 3
+            end
+          end
+
+          context 'when `pipeline_source_filter` feature flag is enabled' do
+            it 'returns matched pipelines' do
+              get api("/projects/#{project.id}/pipelines", user), params: { source: 'web' }
+
+              expect(response).to have_gitlab_http_status(:ok)
+              expect(response).to include_pagination_headers
+              expect(json_response).not_to be_empty
+              json_response.each { |r| expect(r['source']).to eq('web') }
+            end
+
+            context 'when source is invalid' do
+              it 'returns bad_request' do
+                get api("/projects/#{project.id}/pipelines", user), params: { source: 'invalid-source' }
+
+                expect(response).to have_gitlab_http_status(:bad_request)
+              end
+            end
+          end
+        end
       end
     end
 
diff --git a/spec/requests/projects/merge_requests_spec.rb b/spec/requests/projects/merge_requests_spec.rb
index 1c5104b39cb..59fde803560 100644
--- a/spec/requests/projects/merge_requests_spec.rb
+++ b/spec/requests/projects/merge_requests_spec.rb
@@ -5,10 +5,18 @@ require 'spec_helper'
 RSpec.describe 'merge requests actions' do
   let_it_be(:project) { create(:project, :repository) }
 
-  let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) }
+  let(:merge_request) do
+    create(:merge_request_with_diffs, target_project: project,
+                                      source_project: project,
+                                      assignees: [user],
+                                      reviewers: [user2])
+  end
+
   let(:user) { project.owner }
+  let(:user2) { create(:user) }
 
   before do
+    project.add_maintainer(user2)
     sign_in(user)
   end
 
@@ -61,17 +69,11 @@ RSpec.describe 'merge requests actions' do
             end
 
             context 'when the merge request is updated' do
-              let(:user2) { create(:user) }
-
-              before do
-                project.add_maintainer(user2)
-              end
-
               def update_service(params)
                 MergeRequests::UpdateService.new(project: project, current_user: user, params: params).execute(merge_request)
               end
 
-              context 'when the user is different' do
+              context 'when the logged in user is different' do
                 before do
                   sign_in(user2)
                 end
@@ -79,61 +81,61 @@ RSpec.describe 'merge requests actions' do
                 it_behaves_like 'a non-cached request'
               end
 
-              context 'when the reviewer is changed' do
+              context 'when the assignee is changed' do
                 before do
-                  update_service(reviewer_ids: [user2.id])
+                  update_service( assignee_ids: [] )
                 end
 
                 it_behaves_like 'a non-cached request'
               end
 
-              context 'when the assignee is changed' do
+              context 'when the existing assignee gets updated' do
                 before do
-                  update_service( assignee_ids: [user2.id] )
+                  user.update_attribute(:avatar, 'uploads/avatar.png')
                 end
 
                 it_behaves_like 'a non-cached request'
               end
 
-              context 'when the time_estimate is changed' do
+              context 'when the reviewer is changed' do
                 before do
-                  update_service(time_estimate: 7200)
+                  update_service(reviewer_ids: [])
                 end
 
                 it_behaves_like 'a non-cached request'
               end
 
-              context 'when the spend_time is changed' do
+              context 'when the existing reviewer gets updated' do
                 before do
-                  update_service(spend_time: { duration: 7200, user_id: user.id, spent_at: Time.now, note_id: nil })
+                  user2.update_attribute(:avatar, 'uploads/avatar.png')
                 end
 
                 it_behaves_like 'a non-cached request'
               end
 
-              context 'when a user leaves a note' do
+              context 'when the time_estimate is changed' do
                 before do
-                  # We have 1 minute ThrottledTouch to account for.
-                  # It's not ideal as it means that our participants cache could be stale for about a day if a new note is created by another person or gets a mention.
-                  travel_to(Time.current + 61) do
-                    Notes::CreateService.new(project, user2, { note: 'Looks good', noteable_type: 'MergeRequest', noteable_id: merge_request.id }).execute
-                  end
+                  update_service(time_estimate: 7200)
                 end
 
                 it_behaves_like 'a non-cached request'
               end
 
-              context 'when the email setting has changed in project' do
+              context 'when the spend_time is changed' do
                 before do
-                  project.namespace.update_attribute(:emails_disabled, true)
+                  update_service(spend_time: { duration: 7200, user_id: user.id, spent_at: Time.now, note_id: nil })
                 end
 
                 it_behaves_like 'a non-cached request'
               end
 
-              context 'when the user changes unsubscribes' do
+              context 'when a user leaves a note' do
                 before do
-                  merge_request.set_subscription(user, false, project)
+                  # We have 1 minute ThrottledTouch to account for.
+                  # It's not ideal as it means that our participants cache could be stale for about a day if a new note is created by another person or gets a mention.
+                  travel_to(Time.current + 61) do
+                    Notes::CreateService.new(project, user2, { note: 'Looks good', noteable_type: 'MergeRequest', noteable_id: merge_request.id }).execute
+                  end
                 end
 
                 it_behaves_like 'a non-cached request'