Add latest changes from gitlab-org/gitlab@master

db/schema.rb

@@ -4356,8 +4356,11 @@ ActiveRecord::Schema.define(version: 2020_02_13_204737) do
     t.datetime_with_timezone "resolved_at"
     t.integer "report_type", limit: 2, null: false
     t.integer "cached_markdown_version"
+    t.bigint "confirmed_by_id"
+    t.datetime_with_timezone "confirmed_at"
     t.index ["author_id"], name: "index_vulnerabilities_on_author_id"
     t.index ["closed_by_id"], name: "index_vulnerabilities_on_closed_by_id"
+    t.index ["confirmed_by_id"], name: "index_vulnerabilities_on_confirmed_by_id"
     t.index ["due_date_sourcing_milestone_id"], name: "index_vulnerabilities_on_due_date_sourcing_milestone_id"
     t.index ["epic_id"], name: "index_vulnerabilities_on_epic_id"
     t.index ["last_edited_by_id"], name: "index_vulnerabilities_on_last_edited_by_id"
@@ -5014,6 +5017,7 @@ ActiveRecord::Schema.define(version: 2020_02_13_204737) do
   add_foreign_key "vulnerabilities", "projects", name: "fk_efb96ab1e2", on_delete: :cascade
   add_foreign_key "vulnerabilities", "users", column: "author_id", name: "fk_b1de915a15", on_delete: :nullify
   add_foreign_key "vulnerabilities", "users", column: "closed_by_id", name: "fk_cf5c60acbf", on_delete: :nullify
+  add_foreign_key "vulnerabilities", "users", column: "confirmed_by_id", name: "fk_959d40ad0a", on_delete: :nullify
   add_foreign_key "vulnerabilities", "users", column: "last_edited_by_id", name: "fk_1302949740", on_delete: :nullify
   add_foreign_key "vulnerabilities", "users", column: "resolved_by_id", name: "fk_76bc5f5455", on_delete: :nullify
   add_foreign_key "vulnerabilities", "users", column: "updated_by_id", name: "fk_7ac31eacb9", on_delete: :nullify

doc/user/project/integrations/webhooks.md

@@ -1054,7 +1054,8 @@ X-Gitlab-Event: Pipeline Hook
    "user":{
       "name": "Administrator",
       "username": "root",
-      "avatar_url": "http://www.gravatar.com/avatar/e32bd13e2add097461cb96824b7a829c?s=80\u0026d=identicon"
+      "avatar_url": "http://www.gravatar.com/avatar/e32bd13e2add097461cb96824b7a829c?s=80\u0026d=identicon",
+      "email": "user_email@gitlab.com"
    },
    "project":{
       "id": 1,
@@ -1243,7 +1244,8 @@ X-Gitlab-Event: Job Hook
   "user": {
     "id": 3,
     "name": "User",
-    "email": "user@gitlab.com"
+    "email": "user@gitlab.com",
+    "avatar_url": "http://www.gravatar.com/avatar/e32bd13e2add097461cb96824b7a829c?s=80\u0026d=identicon"
   },
   "commit": {
     "id": 2366,

lib/api/helpers/internal_helpers.rb

@@ -55,30 +55,6 @@ module API
         ::Users::ActivityService.new(actor).execute if commands.include?(params[:action])
       end
-      def merge_request_urls
-        ::MergeRequests::GetUrlsService.new(project).execute(params[:changes])
-      end
-
-      def process_mr_push_options(push_options, project, user, changes)
-        Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-foss/issues/61359')
-
-        service = ::MergeRequests::PushOptionsHandlerService.new(
-          project,
-          user,
-          changes,
-          push_options
-        ).execute
-
-        if service.errors.present?
-          push_options_warning(service.errors.join("\n\n"))
-        end
-      end
-
-      def push_options_warning(warning)
-        options = Array.wrap(params[:push_options]).map { |p| "'#{p}'" }.join(' ')
-        "WARNINGS:\nError encountered with push options #{options}: #{warning}"
-      end
-
       def redis_ping
         result = Gitlab::Redis::SharedState.with { |redis| redis.ping }

lib/api/internal/base.rb

@@ -212,40 +212,7 @@ module API
         post '/post_receive' do
           status 200
-          response = Gitlab::InternalPostReceive::Response.new
-
-          # Try to load the project and users so we have the application context
-          # available for logging before we schedule any jobs.
-          user = actor.user
-          project
-
-          push_options = Gitlab::PushOptions.new(params[:push_options])
-
-          response.reference_counter_decreased = Gitlab::ReferenceCounter.new(params[:gl_repository]).decrease
-
-          PostReceive.perform_async(params[:gl_repository], params[:identifier],
-                                    params[:changes], push_options.as_json)
-
-          mr_options = push_options.get(:merge_request)
-          if mr_options.present?
-            message = process_mr_push_options(mr_options, project, user, params[:changes])
-            response.add_alert_message(message)
-          end
-
-          broadcast_message = BroadcastMessage.current&.last&.message
-          response.add_alert_message(broadcast_message)
-
-          response.add_merge_request_urls(merge_request_urls)
-
-          # Neither User nor Project are guaranteed to be returned; an orphaned write deploy
-          # key could be used
-          if user && project
-            redirect_message = Gitlab::Checks::ProjectMoved.fetch_message(user.id, project.id)
-            project_created_message = Gitlab::Checks::ProjectCreated.fetch_message(user.id, project.id)
-
-            response.add_basic_message(redirect_message)
-            response.add_basic_message(project_created_message)
-          end
+          response = PostReceiveService.new(actor.user, project, params).execute
           ee_post_receive_response_hook(response)

lib/gitlab/data_builder/build.rb

@@ -38,11 +38,7 @@ module Gitlab
           project_id: project.id,
           project_name: project.full_name,
-          user: {
-            id: user.try(:id),
-            name: user.try(:name),
-            email: user.try(:email)
-          },
+          user: user.try(:hook_attrs),
           commit: {
             # note: commit.id is actually the pipeline id

lib/gitlab/grape_logging/formatters/lograge_with_timestamp.rb

@@ -25,9 +25,12 @@ module Gitlab
         def process_params(data)
           return [] unless data.has_key?(:params)
-          data[:params]
-            .each_pair
-            .map { |k, v| { key: k, value: utf8_encode_values(v) } }
+          params_array =
+            data[:params]
+              .each_pair
+              .map { |k, v| { key: k, value: utf8_encode_values(v) } }
+
+          Gitlab::Utils::LogLimitedArray.log_limited_array(params_array)
         end
         def utf8_encode_values(data)

lib/gitlab/sidekiq_logging/structured_logger.rb

@@ -6,8 +6,6 @@ require 'active_record/log_subscriber'
 module Gitlab
   module SidekiqLogging
     class StructuredLogger
-      MAXIMUM_JOB_ARGUMENTS_LENGTH = 10.kilobytes
-
       def call(job, queue)
         started_time = get_time
         base_payload = parse_job(job)
@@ -85,7 +83,7 @@ module Gitlab
         job['pid'] = ::Process.pid
         job.delete('args') unless ENV['SIDEKIQ_LOG_ARGUMENTS']
-        job['args'] = limited_job_args(job['args']) if job['args']
+        job['args'] = Gitlab::Utils::LogLimitedArray.log_limited_array(job['args']) if job['args']
         job
       end
@@ -108,21 +106,6 @@ module Gitlab
       def current_time
         Gitlab::Metrics::System.monotonic_time
       end
-
-      def limited_job_args(args)
-        return unless args.is_a?(Array)
-
-        total_length = 0
-        limited_args = args.take_while do |arg|
-          total_length += arg.to_json.length
-
-          total_length <= MAXIMUM_JOB_ARGUMENTS_LENGTH
-        end
-
-        limited_args.push('...') if total_length > MAXIMUM_JOB_ARGUMENTS_LENGTH
-
-        limited_args
-      end
     end
   end
 end

lib/gitlab/utils/log_limited_array.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Utils
+    module LogLimitedArray
+      MAXIMUM_ARRAY_LENGTH = 10.kilobytes
+
+      # Prepare an array for logging by limiting its JSON representation
+      # to around 10 kilobytes. Once we hit the limit, add "..." as the
+      # last item in the returned array.
+      def self.log_limited_array(array)
+        return [] unless array.is_a?(Array)
+
+        total_length = 0
+        limited_array = array.take_while do |arg|
+          total_length += arg.to_json.length
+
+          total_length <= MAXIMUM_ARRAY_LENGTH
+        end
+
+        limited_array.push('...') if total_length > MAXIMUM_ARRAY_LENGTH
+
+        limited_array
+      end
+    end
+  end
+end

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('')

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],

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

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) }

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

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) + ['...']

spec/lib/gitlab/utils/log_limited_array_spec.rb

+# 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

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) }

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

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

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

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