Add latest changes from gitlab-org/gitlab@master

13 files changed, 163 insertions, 35 deletions

diff --git a/spec/controllers/projects/deployments_controller_spec.rb b/spec/controllers/projects/deployments_controller_spec.rb
index bf80346c63a..37dcfa78772 100644
--- a/spec/controllers/projects/deployments_controller_spec.rb
+++ b/spec/controllers/projects/deployments_controller_spec.rb
@@ -59,7 +59,7 @@ describe Projects::DeploymentsController do
           end
         end
 
-        it 'returns a empty response 204 resposne' do
+        it 'returns an empty 204 response' do
           get :metrics, params: deployment_params(id: deployment.to_param)
           expect(response).to have_gitlab_http_status(:no_content)
           expect(response.body).to eq('')
diff --git a/spec/db/schema_spec.rb b/spec/db/schema_spec.rb
index 482e0fbe7ce..f2f7d6cbafc 100644
--- a/spec/db/schema_spec.rb
+++ b/spec/db/schema_spec.rb
@@ -55,7 +55,7 @@ describe 'Database schema' do
     members: %w[source_id created_by_id],
     merge_requests: %w[last_edited_by_id state_id],
     namespaces: %w[owner_id parent_id],
-    notes: %w[author_id commit_id noteable_id updated_by_id resolved_by_id discussion_id],
+    notes: %w[author_id commit_id noteable_id updated_by_id resolved_by_id confirmed_by_id discussion_id],
     notification_settings: %w[source_id],
     oauth_access_grants: %w[resource_owner_id application_id],
     oauth_access_tokens: %w[resource_owner_id application_id],
diff --git a/spec/initializers/lograge_spec.rb b/spec/initializers/lograge_spec.rb
index 7c61d034ac9..15165c6db98 100644
--- a/spec/initializers/lograge_spec.rb
+++ b/spec/initializers/lograge_spec.rb
@@ -5,16 +5,37 @@ require 'spec_helper'
 describe 'lograge', type: :request do
   let(:headers) { { 'X-Request-ID' => 'new-correlation-id' } }
 
-  context 'for API requests' do
-    subject { get("/api/v4/endpoint", params: {}, headers: headers) }
+  let(:large_params) do
+    half_limit = Gitlab::Utils::LogLimitedArray::MAXIMUM_ARRAY_LENGTH / 2
+
+    {
+      a: 'a',
+      b: 'b' * half_limit,
+      c: 'c' * half_limit,
+      d: 'd'
+    }
+  end
+
+  let(:limited_params) do
+    large_params.slice(:a, :b).map { |k, v| { key: k.to_s, value: v } } + ['...']
+  end
 
+  context 'for API requests' do
     it 'logs to api_json log' do
       # we assert receiving parameters by grape logger
       expect_any_instance_of(Gitlab::GrapeLogging::Formatters::LogrageWithTimestamp).to receive(:call)
         .with(anything, anything, anything, a_hash_including("correlation_id" => "new-correlation-id"))
         .and_call_original
 
-      subject
+      get("/api/v4/endpoint", params: {}, headers: headers)
+    end
+
+    it 'limits param size' do
+      expect(Lograge.formatter).to receive(:call)
+        .with(a_hash_including(params: limited_params))
+        .and_call_original
+
+      get("/api/v4/endpoint", params: large_params, headers: headers)
     end
   end
 
@@ -67,6 +88,14 @@ describe 'lograge', type: :request do
 
       subject
     end
+
+    it 'limits param size' do
+      expect(Lograge.formatter).to receive(:call)
+        .with(a_hash_including(params: limited_params))
+        .and_call_original
+
+      get("/", params: large_params, headers: headers)
+    end
   end
 
   context 'with a log subscriber' do
diff --git a/spec/lib/gitlab/data_builder/build_spec.rb b/spec/lib/gitlab/data_builder/build_spec.rb
index fdb855de786..da27125c9a6 100644
--- a/spec/lib/gitlab/data_builder/build_spec.rb
+++ b/spec/lib/gitlab/data_builder/build_spec.rb
@@ -4,7 +4,8 @@ require 'spec_helper'
 
 describe Gitlab::DataBuilder::Build do
   let(:runner) { create(:ci_runner, :instance) }
-  let(:build) { create(:ci_build, :running, runner: runner) }
+  let(:user) { create(:user) }
+  let(:build) { create(:ci_build, :running, runner: runner, user: user) }
 
   describe '.build' do
     let(:data) do
@@ -22,6 +23,15 @@ describe Gitlab::DataBuilder::Build do
     it { expect(data[:project_id]).to eq(build.project.id) }
     it { expect(data[:project_name]).to eq(build.project.full_name) }
     it { expect(data[:pipeline_id]).to eq(build.pipeline.id) }
+    it {
+      expect(data[:user]).to eq(
+        {
+            name: user.name,
+            username: user.username,
+            avatar_url: user.avatar_url(only_path: false),
+            email: user.email
+            })
+    }
     it { expect(data[:commit][:id]).to eq(build.pipeline.id) }
     it { expect(data[:runner][:id]).to eq(build.runner.id) }
     it { expect(data[:runner][:description]).to eq(build.runner.description) }
diff --git a/spec/lib/gitlab/data_builder/pipeline_spec.rb b/spec/lib/gitlab/data_builder/pipeline_spec.rb
index 86ab7f888ca..da22da8de0f 100644
--- a/spec/lib/gitlab/data_builder/pipeline_spec.rb
+++ b/spec/lib/gitlab/data_builder/pipeline_spec.rb
@@ -11,7 +11,8 @@ describe Gitlab::DataBuilder::Pipeline do
           project: project,
           status: 'success',
           sha: project.commit.sha,
-          ref: project.default_branch)
+          ref: project.default_branch,
+          user: user)
   end
 
   let!(:build) { create(:ci_build, pipeline: pipeline) }
@@ -37,6 +38,12 @@ describe Gitlab::DataBuilder::Pipeline do
       expect(build_data[:allow_failure]).to eq(build.allow_failure)
       expect(project_data).to eq(project.hook_attrs(backward: false))
       expect(data[:merge_request]).to be_nil
+      expect(data[:user]).to eq({
+        name: user.name,
+        username: user.username,
+        avatar_url: user.avatar_url(only_path: false),
+        email: user.email
+        })
     end
 
     context 'pipeline without variables' do
diff --git a/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb b/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb
index 43cdb998091..ac4cf1734a5 100644
--- a/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb
+++ b/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb
@@ -99,13 +99,8 @@ describe Gitlab::SidekiqLogging::StructuredLogger do
       context 'when the job args are bigger than the maximum allowed' do
         it 'keeps args from the front until they exceed the limit' do
           Timecop.freeze(timestamp) do
-            job['args'] = [
-              1,
-              2,
-              'a' * (described_class::MAXIMUM_JOB_ARGUMENTS_LENGTH / 2),
-              'b' * (described_class::MAXIMUM_JOB_ARGUMENTS_LENGTH / 2),
-              3
-            ]
+            half_limit = Gitlab::Utils::LogLimitedArray::MAXIMUM_ARRAY_LENGTH / 2
+            job['args'] = [1, 2, 'a' * half_limit, 'b' * half_limit, 3]
 
             expected_args = job['args'].take(3) + ['...']
 
diff --git a/spec/lib/gitlab/utils/log_limited_array_spec.rb b/spec/lib/gitlab/utils/log_limited_array_spec.rb
new file mode 100644
index 00000000000..2729b2c7b6f
--- /dev/null
+++ b/spec/lib/gitlab/utils/log_limited_array_spec.rb
@@ -0,0 +1,49 @@
+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+
+describe Gitlab::Utils::LogLimitedArray do
+  describe '.log_limited_array' do
+    context 'when the argument is not an array' do
+      it 'returns an empty array' do
+        expect(described_class.log_limited_array('aa')).to eq([])
+      end
+    end
+
+    context 'when the argument is an array' do
+      context 'when the array is under the limit' do
+        it 'returns the array unchanged' do
+          expect(described_class.log_limited_array(%w(a b))).to eq(%w(a b))
+        end
+      end
+
+      context 'when the array exceeds the limit' do
+        it 'replaces arguments after the limit with an ellipsis string' do
+          half_limit = described_class::MAXIMUM_ARRAY_LENGTH / 2
+          long_array = ['a' * half_limit, 'b' * half_limit, 'c']
+
+          expect(described_class.log_limited_array(long_array))
+            .to eq(long_array.take(1) + ['...'])
+        end
+      end
+
+      context 'when the array contains arrays and hashes' do
+        it 'calculates the size based on the JSON representation' do
+          long_array = [
+            'a',
+            ['b'] * 10,
+            { c: 'c' * 10 },
+            # Each character in the array takes up four characters: the
+            # character itself, the two quotes, and the comma (closing
+            # square bracket for the last item)
+            ['d'] * (described_class::MAXIMUM_ARRAY_LENGTH / 4),
+            'e'
+          ]
+
+          expect(described_class.log_limited_array(long_array))
+            .to eq(long_array.take(3) + ['...'])
+        end
+      end
+    end
+  end
+end
diff --git a/spec/models/label_spec.rb b/spec/models/label_spec.rb
index baf2cfeab0c..dc878c2d3c0 100644
--- a/spec/models/label_spec.rb
+++ b/spec/models/label_spec.rb
@@ -183,6 +183,31 @@ describe Label do
     end
   end
 
+  describe '.top_labels_by_target' do
+    let(:label) { create(:label) }
+    let(:popular_label) { create(:label) }
+    let(:merge_request1) { create(:merge_request) }
+    let(:merge_request2) { create(:merge_request) }
+
+    before do
+      merge_request1.labels = [label, popular_label]
+      merge_request2.labels = [popular_label]
+    end
+
+    it 'returns distinct labels, ordered by usage in the given target relation' do
+      top_labels = described_class.top_labels_by_target(MergeRequest.all)
+
+      expect(top_labels).to match_array([popular_label, label])
+    end
+
+    it 'excludes labels that are not assigned to any records in the given target relation' do
+      merge_requests = MergeRequest.where(id: merge_request2.id)
+      top_labels = described_class.top_labels_by_target(merge_requests)
+
+      expect(top_labels).to match_array([popular_label])
+    end
+  end
+
   describe '.optionally_subscribed_by' do
     let!(:user)   { create(:user) }
     let!(:label)  { create(:label) }
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 90795e0a8e4..36ddb624cba 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -4200,4 +4200,17 @@ describe User, :do_not_mock_admin_mode do
       expect(described_class.bots).to match_array([bot])
     end
   end
+
+  describe '#hook_attrs' do
+    it 'includes name, username, avatar_url, and email' do
+      user = create(:user)
+      user_attributes = {
+        name: user.name,
+        username: user.username,
+        avatar_url: user.avatar_url(only_path: false),
+        email: user.email
+      }
+      expect(user.hook_attrs).to eq(user_attributes)
+    end
+  end
 end
diff --git a/spec/services/spam/ham_service_spec.rb b/spec/services/spam/ham_service_spec.rb
index a4583108dba..9848f48def2 100644
--- a/spec/services/spam/ham_service_spec.rb
+++ b/spec/services/spam/ham_service_spec.rb
@@ -4,10 +4,10 @@ require 'spec_helper'
 
 describe Spam::HamService do
   let_it_be(:user) { create(:user) }
-  let!(:target) { create(:spam_log, user: user, submitted_as_ham: false) }
+  let!(:spam_log) { create(:spam_log, user: user, submitted_as_ham: false) }
   let(:fake_akismet_service) { double(:akismet_service) }
 
-  subject { described_class.new(target) }
+  subject { described_class.new(spam_log) }
 
   before do
     allow(Spam::AkismetService).to receive(:new).and_return fake_akismet_service
@@ -24,23 +24,23 @@ describe Spam::HamService do
       end
 
       it 'does not update the record' do
-        expect { subject.execute }.not_to change { target.submitted_as_ham }
+        expect { subject.execute }.not_to change { spam_log.submitted_as_ham }
       end
 
       context 'if spam log record has already been marked as spam' do
         before do
-          target.update_attribute(:submitted_as_ham, true)
+          spam_log.update_attribute(:submitted_as_ham, true)
         end
 
         it 'does not update the record' do
-          expect { subject.execute }.not_to change { target.submitted_as_ham }
+          expect { subject.execute }.not_to change { spam_log.submitted_as_ham }
         end
       end
     end
 
     context 'Akismet ham submission is successful' do
       before do
-        target.update_attribute(:submitted_as_ham, false)
+        spam_log.update_attribute(:submitted_as_ham, false)
         allow(fake_akismet_service).to receive(:submit_ham).and_return true
       end
 
@@ -49,7 +49,7 @@ describe Spam::HamService do
       end
 
       it 'updates the record' do
-        expect { subject.execute }.to change { target.submitted_as_ham }.from(false).to(true)
+        expect { subject.execute }.to change { spam_log.submitted_as_ham }.from(false).to(true)
       end
     end
   end
diff --git a/spec/services/spam/mark_as_spam_service_spec.rb b/spec/services/spam/mark_as_spam_service_spec.rb
index 3824a8244bb..cba9d6a39cb 100644
--- a/spec/services/spam/mark_as_spam_service_spec.rb
+++ b/spec/services/spam/mark_as_spam_service_spec.rb
@@ -4,19 +4,19 @@ require 'spec_helper'
 
 describe Spam::MarkAsSpamService do
   let(:user_agent_detail) { build(:user_agent_detail) }
-  let(:target) { build(:issue, user_agent_detail: user_agent_detail) }
+  let(:spammable) { build(:issue, user_agent_detail: user_agent_detail) }
   let(:fake_akismet_service) { double(:akismet_service, submit_spam: true) }
 
-  subject { described_class.new(target: target) }
+  subject { described_class.new(spammable: spammable) }
 
   describe '#execute' do
     before do
       allow(subject).to receive(:akismet).and_return(fake_akismet_service)
     end
 
-    context 'when the target object is not submittable' do
+    context 'when the spammable object is not submittable' do
       before do
-        allow(target).to receive(:submittable_as_spam?).and_return false
+        allow(spammable).to receive(:submittable_as_spam?).and_return false
       end
 
       it 'does not submit as spam' do
@@ -26,7 +26,7 @@ describe Spam::MarkAsSpamService do
 
     context 'spam is submitted successfully' do
       before do
-        allow(target).to receive(:submittable_as_spam?).and_return true
+        allow(spammable).to receive(:submittable_as_spam?).and_return true
         allow(fake_akismet_service).to receive(:submit_spam).and_return true
       end
 
@@ -34,14 +34,14 @@ describe Spam::MarkAsSpamService do
         expect(subject.execute).to be_truthy
       end
 
-      it "updates the target object's user agent detail as being submitted as spam" do
+      it "updates the spammable object's user agent detail as being submitted as spam" do
         expect(user_agent_detail).to receive(:update_attribute)
 
         subject.execute
       end
 
       context 'when Akismet does not consider it spam' do
-        it 'does not update the target object as spam' do
+        it 'does not update the spammable object as spam' do
           allow(fake_akismet_service).to receive(:submit_spam).and_return false
 
           expect(subject.execute).to be_falsey
diff --git a/spec/services/spam/spam_check_service_spec.rb b/spec/services/spam/spam_check_service_spec.rb
index 376afc78b5f..732b64b52a0 100644
--- a/spec/services/spam/spam_check_service_spec.rb
+++ b/spec/services/spam/spam_check_service_spec.rb
@@ -22,12 +22,12 @@ describe Spam::SpamCheckService do
   end
 
   describe '#initialize' do
-    subject { described_class.new(target: issue, request: request) }
+    subject { described_class.new(spammable: issue, request: request) }
 
     context 'when the request is nil' do
       let(:request) { nil }
 
-      it 'assembles the options with information from the target' do
+      it 'assembles the options with information from the spammable' do
         aggregate_failures do
           expect(subject.options[:ip_address]).to eq(issue.ip_address)
           expect(subject.options[:user_agent]).to eq(issue.user_agent)
@@ -39,7 +39,7 @@ describe Spam::SpamCheckService do
     context 'when the request is present' do
       let(:request) { double(:request, env: env) }
 
-      it 'assembles the options with information from the target' do
+      it 'assembles the options with information from the spammable' do
         aggregate_failures do
           expect(subject.options[:ip_address]).to eq(fake_ip)
           expect(subject.options[:user_agent]).to eq(fake_user_agent)
@@ -55,7 +55,7 @@ describe Spam::SpamCheckService do
     let_it_be(:existing_spam_log) { create(:spam_log, user: user, recaptcha_verified: false) }
 
     subject do
-      described_service = described_class.new(target: issue, request: request)
+      described_service = described_class.new(spammable: issue, request: request)
       described_service.execute(user_id: user.id, api: nil, recaptcha_verified: recaptcha_verified, spam_log_id: existing_spam_log.id)
     end
 
@@ -81,7 +81,7 @@ describe Spam::SpamCheckService do
     context 'when recaptcha was not verified' do
       let(:recaptcha_verified) { false }
 
-      context 'when target attributes have not changed' do
+      context 'when spammable attributes have not changed' do
         before do
           issue.closed_at = Time.zone.now
 
@@ -98,7 +98,7 @@ describe Spam::SpamCheckService do
         end
       end
 
-      context 'when target attributes have changed' do
+      context 'when spammable attributes have changed' do
         before do
           issue.description = 'SPAM!'
         end
diff --git a/spec/support/shared_examples/spam_check_shared_examples.rb b/spec/support/shared_examples/spam_check_shared_examples.rb
index a16934b7d77..3ecae16db39 100644
--- a/spec/support/shared_examples/spam_check_shared_examples.rb
+++ b/spec/support/shared_examples/spam_check_shared_examples.rb
@@ -2,7 +2,7 @@
 
 shared_examples 'akismet spam' do
   context 'when request is missing' do
-    subject { described_class.new(target: issue, request: nil) }
+    subject { described_class.new(spammable: issue, request: nil) }
 
     it "doesn't check as spam" do
       subject