[ci skip] Merge branch 'master' into 5845-extract-ee-environments-files

* master: (24 commits)
  Remove unneccessary imports
  fixed copy to cliboard button in embedded snippets
  Fix Error 500 viewing admin page due to statement timeouts
  Workhorse to send raw diff and patch for commits
  Refactor out duplication in runner_policy.rb
  Remove unnecessary runner.is_shared? checks in api because they are handled by policy
  Allow admin to assign shared runner to project through API
  Change policy list_runner_jobs -> read_runner
  Rename User#ci_authorized_runners -> ci_owned_runners
  Improve efficiency of authorized_runner policy query
  Use can? policies for lib/api/runners.rb
  Allow group runners to be viewed/edited in API
  Whitelisted query limits for group destroy API
  Fixed typo
  Changed order of include
  Use find_in_batches instead of destroy_all
  Delete remote uploads
  Use correct base width
  Refactor duplicate code
  Add changelog entry for contrib graphs width fix
  ...

app/assets/javascripts/notes/components/note_header.vue

@@ -93,10 +93,13 @@ export default {
           v-html="actionTextHtml"
           class="system-note-message">
         </span>
+        <span class="system-note-separator">
+          &middot;
+        </span>
         <a
           :href="noteTimestampLink"
           @click="updateTargetNoteHash"
-          class="note-timestamp">
+          class="note-timestamp system-note-separator">
           <time-ago-tooltip
             :time="createdAt"
             tooltip-placement="bottom"

app/assets/javascripts/pages/projects/graphs/show/stat_graph_contributors_graph.js

-/* eslint-disable func-names, space-before-function-paren, no-var, prefer-rest-params, max-len, no-restricted-syntax, vars-on-top, no-use-before-define, no-param-reassign, new-cap, no-underscore-dangle, wrap-iife, comma-dangle, no-return-assign, prefer-arrow-callback, quotes, prefer-template, newline-per-chained-call, no-else-return, no-shadow */
+/* eslint-disable func-names, space-before-function-paren, prefer-rest-params, max-len, no-restricted-syntax, vars-on-top, no-use-before-define, no-param-reassign, new-cap, no-underscore-dangle, wrap-iife, comma-dangle, no-return-assign, prefer-arrow-callback, quotes, prefer-template, newline-per-chained-call, no-else-return, no-shadow */
 
 import $ from 'jquery';
 import _ from 'underscore';
@@ -13,17 +13,17 @@ import { dateTickFormat } from '~/lib/utils/tick_formats';
 
 const d3 = { extent, max, select, scaleTime, scaleLinear, axisLeft, axisBottom, area, brushX, timeParse };
 
-const extend = function(child, parent) { for (var key in parent) { if (hasProp.call(parent, key)) child[key] = parent[key]; } function ctor() { this.constructor = child; } ctor.prototype = parent.prototype; child.prototype = new ctor(); child.__super__ = parent.prototype; return child; };
 const hasProp = {}.hasOwnProperty;
+const extend = function(child, parent) { for (const key in parent) { if (hasProp.call(parent, key)) child[key] = parent[key]; } function ctor() { this.constructor = child; } ctor.prototype = parent.prototype; child.prototype = new ctor(); child.__super__ = parent.prototype; return child; };
 
 export const ContributorsGraph = (function() {
   function ContributorsGraph() {}
 
   ContributorsGraph.prototype.MARGIN = {
     top: 20,
-    right: 20,
+    right: 10,
     bottom: 30,
-    left: 50
+    left: 40
   };
 
   ContributorsGraph.prototype.x_domain = null;
@@ -32,6 +32,12 @@ export const ContributorsGraph = (function() {
 
   ContributorsGraph.prototype.dates = [];
 
+  ContributorsGraph.prototype.determine_width = function(baseWidth, $parentElement) {
+    const parentPaddingWidth = parseFloat($parentElement.css('padding-left')) + parseFloat($parentElement.css('padding-right'));
+    const marginWidth = this.MARGIN.left + this.MARGIN.right;
+    return baseWidth - parentPaddingWidth - marginWidth;
+  };
+
   ContributorsGraph.set_x_domain = function(data) {
     return ContributorsGraph.prototype.x_domain = data;
   };
@@ -105,11 +111,10 @@ export const ContributorsMasterGraph = (function(superClass) {
 
   function ContributorsMasterGraph(data1) {
     const $parentElement = $('#contributors-master');
-    const parentPadding = parseFloat($parentElement.css('padding-left')) + parseFloat($parentElement.css('padding-right'));
 
     this.data = data1;
     this.update_content = this.update_content.bind(this);
-    this.width = $('.content').width() - parentPadding - (this.MARGIN.left + this.MARGIN.right);
+    this.width = this.determine_width($('.js-graphs-show').width(), $parentElement);
     this.height = 200;
     this.x = null;
     this.y = null;
@@ -122,8 +127,7 @@ export const ContributorsMasterGraph = (function(superClass) {
   }
 
   ContributorsMasterGraph.prototype.process_dates = function(data) {
-    var dates;
-    dates = this.get_dates(data);
+    const dates = this.get_dates(data);
     this.parse_dates(data);
     return ContributorsGraph.set_dates(dates);
   };
@@ -133,8 +137,7 @@ export const ContributorsMasterGraph = (function(superClass) {
   };
 
   ContributorsMasterGraph.prototype.parse_dates = function(data) {
-    var parseDate;
-    parseDate = d3.timeParse("%Y-%m-%d");
+    const parseDate = d3.timeParse("%Y-%m-%d");
     return data.forEach(function(d) {
       return d.date = parseDate(d.date);
     });
@@ -152,7 +155,14 @@ export const ContributorsMasterGraph = (function(superClass) {
   };
 
   ContributorsMasterGraph.prototype.create_svg = function() {
-    return this.svg = d3.select("#contributors-master").append("svg").attr("width", this.width + this.MARGIN.left + this.MARGIN.right).attr("height", this.height + this.MARGIN.top + this.MARGIN.bottom).attr("class", "tint-box").append("g").attr("transform", "translate(" + this.MARGIN.left + "," + this.MARGIN.top + ")");
+    this.svg = d3.select("#contributors-master")
+      .append("svg")
+      .attr("width", this.width + this.MARGIN.left + this.MARGIN.right)
+      .attr("height", this.height + this.MARGIN.top + this.MARGIN.bottom)
+      .attr("class", "tint-box")
+      .append("g")
+      .attr("transform", "translate(" + this.MARGIN.left + "," + this.MARGIN.top + ")");
+    return this.svg;
   };
 
   ContributorsMasterGraph.prototype.create_area = function(x, y) {
@@ -218,12 +228,14 @@ export const ContributorsAuthorGraph = (function(superClass) {
   extend(ContributorsAuthorGraph, superClass);
 
   function ContributorsAuthorGraph(data1) {
+    const $parentElements = $('.person');
+
     this.data = data1;
     // Don't split graph size in half for mobile devices.
-    if ($(window).width() < 768) {
-      this.width = $('.content').width() - 80;
+    if ($(window).width() < 790) {
+      this.width = this.determine_width($('.js-graphs-show').width(), $parentElements);
     } else {
-      this.width = ($('.content').width() / 2) - 100;
+      this.width = this.determine_width($('.js-graphs-show').width() / 2, $parentElements);
     }
     this.height = 200;
     this.x = null;
@@ -249,8 +261,7 @@ export const ContributorsAuthorGraph = (function(superClass) {
 
   ContributorsAuthorGraph.prototype.create_area = function(x, y) {
     return this.area = d3.area().x(function(d) {
-      var parseDate;
-      parseDate = d3.timeParse("%Y-%m-%d");
+      const parseDate = d3.timeParse("%Y-%m-%d");
       return x(parseDate(d));
     }).y0(this.height).y1((function(_this) {
       return function(d) {
@@ -264,9 +275,16 @@ export const ContributorsAuthorGraph = (function(superClass) {
   };
 
   ContributorsAuthorGraph.prototype.create_svg = function() {
-    var persons = document.querySelectorAll('.person');
+    const persons = document.querySelectorAll('.person');
     this.list_item = persons[persons.length - 1];
-    return this.svg = d3.select(this.list_item).append("svg").attr("width", this.width + this.MARGIN.left + this.MARGIN.right).attr("height", this.height + this.MARGIN.top + this.MARGIN.bottom).attr("class", "spark").append("g").attr("transform", "translate(" + this.MARGIN.left + "," + this.MARGIN.top + ")");
+    this.svg = d3.select(this.list_item)
+      .append("svg")
+      .attr("width", this.width + this.MARGIN.left + this.MARGIN.right)
+      .attr("height", this.height + this.MARGIN.top + this.MARGIN.bottom)
+      .attr("class", "spark")
+      .append("g")
+      .attr("transform", "translate(" + this.MARGIN.left + "," + this.MARGIN.top + ")");
+    return this.svg;
   };
 
   ContributorsAuthorGraph.prototype.draw_path = function(data) {

app/assets/javascripts/vue_shared/components/time_ago_tooltip.vue

@@ -40,7 +40,7 @@ export default {
     :class="cssClass"
     :title="tooltipTitle(time)"
     :data-placement="tooltipPlacement"
-    data-container="body">
-    {{ timeFormated(time) }}
+    data-container="body"
+    v-text="timeFormated(time)">
   </time>
 </template>

app/assets/stylesheets/framework/snippets.scss

@@ -67,7 +67,8 @@
     padding: 8px 40px;
   }
 
-  .embed-toggle {
+  .embed-toggle,
+  .snippet-clipboard-btn {
     height: 35px;
   }
 }

app/assets/stylesheets/pages/boards.scss

-@import './issues/issue_count_badge';
-
 [v-cloak] {
   display: none;
 }

app/assets/stylesheets/pages/issues.scss

-@import "./issues/issue_count_badge";
-
 .issues-list {
   .issue {
     padding: 10px 0 10px $gl-padding;

app/assets/stylesheets/pages/notes.scss

@@ -455,6 +455,10 @@ ul.notes {
     white-space: normal;
   }
 
+  .system-note-separator {
+    color: $gl-text-color-disabled;
+  }
+
   a:hover {
     text-decoration: underline;
   }

app/controllers/admin/dashboard_controller.rb

 class Admin::DashboardController < Admin::ApplicationController
+  include CountHelper
+
   def index
     @projects = Project.order_id_desc.without_deleted.with_route.limit(10)
     @users = User.order_id_desc.limit(10)

app/controllers/projects/commit_controller.rb

@@ -23,8 +23,12 @@ class Projects::CommitController < Projects::ApplicationController
 
     respond_to do |format|
       format.html  { render }
-      format.diff  { render text: @commit.to_diff }
-      format.patch { render text: @commit.to_patch }
+      format.diff  do
+        send_git_diff(@project.repository, @commit.diff_refs)
+      end
+      format.patch do
+        send_git_patch(@project.repository, @commit.diff_refs)
+      end
     end
   end
 

app/controllers/projects/settings/ci_cd_controller.rb

@@ -69,7 +69,7 @@ module Projects
         @project_runners = @project.runners.ordered
 
         @assignable_runners = current_user
-          .ci_authorized_runners
+          .ci_owned_runners
           .assignable_for(project)
           .ordered
           .page(params[:page]).per(20)

app/helpers/count_helper.rb

+module CountHelper
+  def approximate_count_with_delimiters(model)
+    number_with_delimiter(Gitlab::Database::Count.approximate_count(model))
+  end
+end

app/models/appearance.rb

@@ -2,6 +2,7 @@ class Appearance < ActiveRecord::Base
   include CacheMarkdownField
   include AfterCommitQueue
   include ObjectStorage::BackgroundMove
+  include WithUploads
 
   cache_markdown_field :description
   cache_markdown_field :new_project_guidelines
@@ -14,8 +15,6 @@ class Appearance < ActiveRecord::Base
   mount_uploader :logo,         AttachmentUploader
   mount_uploader :header_logo,  AttachmentUploader
 
-  has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
-
   CACHE_KEY = "current_appearance:#{Gitlab::VERSION}".freeze
 
   after_commit :flush_redis_cache

app/models/ci/runner.rb

@@ -52,7 +52,7 @@ module Ci
       #        Without that, placeholders would miss one and couldn't match.
       where(locked: false)
         .where.not("ci_runners.id IN (#{project.runners.select(:id).to_sql})")
-        .specific
+        .project_type
     end
 
     validate :tag_constraints

app/models/concerns/with_uploads.rb

+# Mounted uploaders are destroyed by carrierwave's after_commit
+# hook. This hook fetches upload location (local vs remote) from
+# Upload model. So it's neccessary to make sure that during that
+# after_commit hook model's associated uploads are not deleted yet.
+# IOW we can not use dependent: :destroy :
+# has_many :uploads, as: :model, dependent: :destroy
+#
+# And because not-mounted uploads require presence of upload's
+# object model when destroying them (FileUploader's `build_upload` method
+# references `model` on delete), we can not use after_commit hook for these
+# uploads.
+#
+# Instead FileUploads are destroyed in before_destroy hook and remaining uploads
+# are destroyed by the carrierwave's after_commit hook.
+
+module WithUploads
+  extend ActiveSupport::Concern
+
+  # Currently there is no simple way how to select only not-mounted
+  # uploads, it should be all FileUploaders so we select them by
+  # `uploader` class
+  FILE_UPLOADERS = %w(PersonalFileUploader NamespaceFileUploader FileUploader).freeze
+
+  included do
+    has_many :uploads, as: :model
+
+    before_destroy :destroy_file_uploads
+  end
+
+  # mounted uploads are deleted in carrierwave's after_commit hook,
+  # but FileUploaders which are not mounted must be deleted explicitly and
+  # it can not be done in after_commit because FileUploader requires loads
+  # associated model on destroy (which is already deleted in after_commit)
+  def destroy_file_uploads
+    self.uploads.where(uploader: FILE_UPLOADERS).find_each do |upload|
+      upload.destroy
+    end
+  end
+end

app/models/group.rb

@@ -10,6 +10,7 @@ class Group < Namespace
   include LoadedInGroupList
   include GroupDescendant
   include TokenAuthenticatable
+  include WithUploads
 
   has_many :group_members, -> { where(requested_at: nil) }, dependent: :destroy, as: :source # rubocop:disable Cop/ActiveRecordDependent
   alias_method :members, :group_members
@@ -30,8 +31,6 @@ class Group < Namespace
   has_many :variables, class_name: 'Ci::GroupVariable'
   has_many :custom_attributes, class_name: 'GroupCustomAttribute'
 
-  has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
-
   has_many :boards
   has_many :badges, class_name: 'GroupBadge'
 

app/models/list.rb

@@ -31,7 +31,8 @@ class List < ActiveRecord::Base
       if options.key?(:label)
         json[:label] = label.as_json(
           project: board.project,
-          only: [:id, :title, :description, :color]
+          only: [:id, :title, :description, :color],
+          methods: [:text_color]
         )
       end
     end

app/models/project.rb

@@ -23,6 +23,7 @@ class Project < ActiveRecord::Base
   include ::Gitlab::Utils::StrongMemoize
   include ChronicDurationAttribute
   include FastDestroyAll::Helpers
+  include WithUploads
 
   extend Gitlab::ConfigHelper
 
@@ -301,8 +302,6 @@ class Project < ActiveRecord::Base
     inclusion: { in: ->(_object) { Gitlab.config.repositories.storages.keys } }
   validates :variables, variable_duplicates: { scope: :environment_scope }
 
-  has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
-
   # Scopes
   scope :pending_delete, -> { where(pending_delete: true) }
   scope :without_deleted, -> { where(pending_delete: false) }

app/models/user.rb

@@ -17,6 +17,7 @@ class User < ActiveRecord::Base
   include IgnorableColumn
   include BulkMemberAccessLoad
   include BlocksJsonSerialization
+  include WithUploads
 
   DEFAULT_NOTIFICATION_LEVEL = :participating
 
@@ -137,7 +138,6 @@ class User < ActiveRecord::Base
 
   has_many :custom_attributes, class_name: 'UserCustomAttribute'
   has_many :callouts, class_name: 'UserCallout'
-  has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
   has_many :term_agreements
   belongs_to :accepted_term, class_name: 'ApplicationSetting::Term'
 
@@ -999,12 +999,19 @@ class User < ActiveRecord::Base
     !solo_owned_groups.present?
   end
 
-  def ci_authorized_runners
-    @ci_authorized_runners ||= begin
-      runner_ids = Ci::RunnerProject
+  def ci_owned_runners
+    @ci_owned_runners ||= begin
+      project_runner_ids = Ci::RunnerProject
         .where(project: authorized_projects(Gitlab::Access::MASTER))
         .select(:runner_id)
-      Ci::Runner.specific.where(id: runner_ids)
+
+      group_runner_ids = Ci::RunnerNamespace
+        .where(namespace_id: owned_or_masters_groups.select(:id))
+        .select(:runner_id)
+
+      union = Gitlab::SQL::Union.new([project_runner_ids, group_runner_ids])
+
+      Ci::Runner.specific.where("ci_runners.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection
     end
   end
 
@@ -1205,6 +1212,11 @@ class User < ActiveRecord::Base
       !terms_accepted?
   end
 
+  def owned_or_masters_groups
+    union = Gitlab::SQL::Union.new([owned_groups, masters_groups])
+    Group.from("(#{union.to_sql}) namespaces")
+  end
+
   protected
 
   # override, from Devise::Validatable

app/policies/ci/runner_policy.rb

 module Ci
   class RunnerPolicy < BasePolicy
-    with_options scope: :subject, score: 0
-    condition(:shared) { @subject.is_shared? }
-
     with_options scope: :subject, score: 0
     condition(:locked, scope: :subject) { @subject.locked? }
 
-    condition(:authorized_runner) { @user.ci_authorized_runners.include?(@subject) }
+    condition(:owned_runner) { @user.ci_owned_runners.exists?(@subject.id) }
 
     rule { anonymous }.prevent_all
-    rule { admin | authorized_runner }.enable :assign_runner
-    rule { ~admin & shared }.prevent :assign_runner
+
+    rule { admin | owned_runner }.policy do
+      enable :assign_runner
+      enable :read_runner
+      enable :update_runner
+      enable :delete_runner
+    end
+
     rule { ~admin & locked }.prevent :assign_runner
   end
 end

app/views/admin/dashboard/index.html.haml

@@ -10,7 +10,7 @@
             = link_to admin_projects_path do
               %h3.text-center
                 Projects:
-                = number_with_delimiter(Project.cached_count)
+                = approximate_count_with_delimiters(Project)
             %hr
             = link_to('New project', new_project_path, class: "btn btn-new")
       .col-sm-4
@@ -19,7 +19,7 @@
             = link_to admin_users_path do
               %h3.text-center
                 Users:
-                = number_with_delimiter(User.count)
+                = approximate_count_with_delimiters(User)
             %hr
             = link_to 'New user', new_admin_user_path, class: "btn btn-new"
       .col-sm-4
@@ -28,7 +28,7 @@
             = link_to admin_groups_path do
               %h3.text-center
                 Groups:
-                = number_with_delimiter(Group.count)
+                = approximate_count_with_delimiters(Group)
             %hr
             = link_to 'New group', new_admin_group_path, class: "btn btn-new"
     .row
@@ -39,31 +39,31 @@
             %p
               Forks
               %span.light.pull-right
-                = number_with_delimiter(ForkedProjectLink.count)
+                = approximate_count_with_delimiters(ForkedProjectLink)
             %p
               Issues
               %span.light.pull-right
-                = number_with_delimiter(Issue.count)
+                = approximate_count_with_delimiters(Issue)
             %p
               Merge Requests
               %span.light.pull-right
-                = number_with_delimiter(MergeRequest.count)
+                = approximate_count_with_delimiters(MergeRequest)
             %p
               Notes
               %span.light.pull-right
-                = number_with_delimiter(Note.count)
+                = approximate_count_with_delimiters(Note)
             %p
               Snippets
               %span.light.pull-right
-                = number_with_delimiter(Snippet.count)
+                = approximate_count_with_delimiters(Snippet)
             %p
               SSH Keys
               %span.light.pull-right
-                = number_with_delimiter(Key.count)
+                = approximate_count_with_delimiters(Key)
             %p
               Milestones
               %span.light.pull-right
-                = number_with_delimiter(Milestone.count)
+                = approximate_count_with_delimiters(Milestone)
             %p
               Active Users
               %span.light.pull-right

app/views/shared/boards/components/_board.html.haml

@@ -15,7 +15,7 @@
           ":title" => '(list.label ? list.label.description : "")',
           data: { container: "body", placement: "bottom" },
           class: "label color-label title board-title-text",
-          ":style" => "{ backgroundColor: (list.label && list.label.color ? list.label.color : null), color: (list.label && list.label.text_color ? list.label.text_color : \"#2e2e2e\") }" }
+          ":style" => "{ backgroundColor: (list.label && list.label.color ? list.label.color : null), color: (list.label && list.label.textColor ? list.label.textColor : \"#2e2e2e\") }" }
           {{ list.title }}
 
         - if can?(current_user, :admin_list, current_board_parent)

app/views/shared/notes/_note.html.haml

@@ -41,8 +41,9 @@
               - if note.system
                 %span.system-note-message
                   = markdown_field(note, :note)
-              %a{ href: "##{dom_id(note)}" }
-                = time_ago_with_tooltip(note.created_at, placement: 'bottom', html_class: 'note-created-ago')
+              %span.system-note-separator
+                ·
+              %a.system-note-separator{ href: "##{dom_id(note)}" }= time_ago_with_tooltip(note.created_at, placement: 'bottom', html_class: 'note-created-ago')
         - unless note.system?
           .note-actions
             - if note.for_personal_snippet?

app/views/shared/snippets/_header.html.haml

@@ -45,6 +45,6 @@
                 %strong.embed-toggle-list-item= _("Share")
         %input.js-snippet-url-area.snippet-embed-input.form-control{ type: "text", autocomplete: 'off', value: snippet_embed }
         .input-group-btn
-          %button.js-clipboard-btn.btn.btn-default.has-tooltip{ title: "Copy to clipboard", 'data-clipboard-target': '#snippet-url-area' }
+          %button.js-clipboard-btn.snippet-clipboard-btn.btn.btn-default.has-tooltip{ title: "Copy to clipboard", 'data-clipboard-target': '.js-snippet-url-area' }
             = sprite_icon('duplicate', size: 16)
     .clearfix

changelogs/unreleased/22647-width-contributors-graphs.yml

+---
+title: Fix width of contributors graphs
+merge_request: 18639
+author: Paul Vorbach
+type: fixed

changelogs/unreleased/46303_copy_button_fix_embedded_snippets.yml

+---
+title: fixed copy to blipboard button in embed bar of snippets
+merge_request: 18923
+author: haseebeqx
+type: fixed

changelogs/unreleased/jivl-add-dot-system-notes.yml

+---
+title: Add dot to separate system notes content
+merge_request: 18864
+author:
+type: changed

changelogs/unreleased/jprovazn-remote-upload-destroy.yml

+---
+title: Fix deletion of Object Store uploads
+merge_request:
+author:
+type: fixed

changelogs/unreleased/zj-workhorse-commit-patch-diff.yml

+---
+title: Workhorse to send raw diff and patch for commits
+merge_request:
+author:
+type: other

lib/api/groups.rb

@@ -165,6 +165,7 @@ module API
         group = find_group!(params[:id])
         authorize! :admin_group, group
 
+        Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/46285')
         destroy_conditionally!(group) do |group|
           ::Groups::DestroyService.new(group, current_user).execute
         end

lib/api/runners.rb

@@ -14,7 +14,7 @@ module API
         use :pagination
       end
       get do
-        runners = filter_runners(current_user.ci_authorized_runners, params[:scope], without: %w(specific shared))
+        runners = filter_runners(current_user.ci_owned_runners, params[:scope], without: %w(specific shared))
         present paginate(runners), with: Entities::Runner
       end
 
@@ -184,40 +184,35 @@ module API
       def authenticate_show_runner!(runner)
         return if runner.is_shared || current_user.admin?
 
-        forbidden!("No access granted") unless user_can_access_runner?(runner)
+        forbidden!("No access granted") unless can?(current_user, :read_runner, runner)
       end
 
       def authenticate_update_runner!(runner)
         return if current_user.admin?
 
-        forbidden!("Runner is shared") if runner.is_shared?
-        forbidden!("No access granted") unless user_can_access_runner?(runner)
+        forbidden!("No access granted") unless can?(current_user, :update_runner, runner)
       end
 
       def authenticate_delete_runner!(runner)
         return if current_user.admin?
 
-        forbidden!("Runner is shared") if runner.is_shared?
         forbidden!("Runner associated with more than one project") if runner.projects.count > 1
-        forbidden!("No access granted") unless user_can_access_runner?(runner)
+        forbidden!("No access granted") unless can?(current_user, :delete_runner, runner)
       end
 
       def authenticate_enable_runner!(runner)
-        forbidden!("Runner is shared") if runner.is_shared?
-        forbidden!("Runner is locked") if runner.locked?
+        forbidden!("Runner is a group runner") if runner.group_type?
+
         return if current_user.admin?
 
-        forbidden!("No access granted") unless user_can_access_runner?(runner)
+        forbidden!("Runner is locked") if runner.locked?
+        forbidden!("No access granted") unless can?(current_user, :assign_runner, runner)
       end
 
       def authenticate_list_runners_jobs!(runner)
         return if current_user.admin?
 
-        forbidden!("No access granted") unless user_can_access_runner?(runner)
-      end
-
-      def user_can_access_runner?(runner)
-        current_user.ci_authorized_runners.exists?(runner.id)
+        forbidden!("No access granted") unless can?(current_user, :read_runner, runner)
       end
     end
   end

lib/api/v3/groups.rb

@@ -131,6 +131,7 @@ module API
         delete ":id" do
           group = find_group!(params[:id])
           authorize! :admin_group, group
+          Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/46285')
           present ::Groups::DestroyService.new(group, current_user).execute, with: Entities::GroupDetail, current_user: current_user
         end
 

lib/api/v3/runners.rb

@@ -58,7 +58,7 @@ module API
         end
 
         def user_can_access_runner?(runner)
-          current_user.ci_authorized_runners.exists?(runner.id)
+          current_user.ci_owned_runners.exists?(runner.id)
         end
       end
     end

lib/gitlab/database/count.rb

+# For large tables, PostgreSQL can take a long time to count rows due to MVCC.
+# We can optimize this by using the reltuples count as described in https://wiki.postgresql.org/wiki/Slow_Counting.
+module Gitlab
+  module Database
+    module Count
+      CONNECTION_ERRORS =
+        if defined?(PG)
+          [
+            ActionView::Template::Error,
+            ActiveRecord::StatementInvalid,
+            PG::Error
+          ].freeze
+        else
+          [
+            ActionView::Template::Error,
+            ActiveRecord::StatementInvalid
+          ].freeze
+        end
+
+      def self.approximate_count(model)
+        return model.count unless Gitlab::Database.postgresql?
+
+        execute_estimate_if_updated_recently(model) || model.count
+      end
+
+      def self.execute_estimate_if_updated_recently(model)
+        ActiveRecord::Base.connection.select_value(postgresql_estimate_query(model)).to_i if reltuples_updated_recently?(model)
+      rescue *CONNECTION_ERRORS
+      end
+
+      def self.reltuples_updated_recently?(model)
+        time = "to_timestamp(#{1.hour.ago.to_i})"
+        query = <<~SQL
+          SELECT 1 FROM pg_stat_user_tables WHERE relname = '#{model.table_name}' AND
+          (last_vacuum > #{time} OR last_autovacuum > #{time} OR last_analyze > #{time} OR last_autoanalyze > #{time})
+        SQL
+
+        ActiveRecord::Base.connection.select_all(query).count > 0
+      rescue *CONNECTION_ERRORS
+        false
+      end
+
+      def self.postgresql_estimate_query(model)
+        "SELECT reltuples::bigint AS estimate FROM pg_class where relname = '#{model.table_name}'"
+      end
+    end
+  end
+end

lib/gitlab/git/commit.rb

@@ -342,21 +342,6 @@ module Gitlab
         parent_ids.first
       end
 
-      # Shows the diff between the commit's parent and the commit.
-      #
-      # Cuts out the header and stats from #to_patch and returns only the diff.
-      #
-      # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/324
-      def to_diff
-        Gitlab::GitalyClient.migrate(:commit_patch, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled|
-          if is_enabled
-            @repository.gitaly_commit_client.patch(id)
-          else
-            rugged_diff_from_parent.patch
-          end
-        end
-      end
-
       # Returns a diff object for the changes from this commit's first parent.
       # If there is no parent, then the diff is between this commit and an
       # empty repo. See Repository#diff for keys allowed in the +options+
@@ -432,16 +417,6 @@ module Gitlab
         Gitlab::Git::CommitStats.new(@repository, self)
       end
 
-      def to_patch(options = {})
-        begin
-          rugged_commit.to_mbox(options)
-        rescue Rugged::InvalidError => ex
-          if ex.message =~ /commit \w+ is a merge commit/i
-            'Patch format is not currently supported for merge commits.'
-          end
-        end
-      end
-
       # Get ref names collection
       #
       # Ex.

spec/controllers/projects/commit_controller_spec.rb

@@ -79,41 +79,18 @@ describe Projects::CommitController do
     end
 
     describe "as diff" do
-      include_examples "export as", :diff
-      let(:format) { :diff }
+      it "triggers workhorse to serve the request" do
+        go(id: commit.id, format: :diff)
 
-      it "should really only be a git diff" do
-        go(id: '66eceea0db202bb39c4e445e8ca28689645366c5', format: format)
-
-        expect(response.body).to start_with("diff --git")
-      end
-
-      it "is only be a git diff without whitespace changes" do
-        go(id: '66eceea0db202bb39c4e445e8ca28689645366c5', format: format, w: 1)
-
-        expect(response.body).to start_with("diff --git")
-
-        # without whitespace option, there are more than 2 diff_splits for other formats
-        diff_splits = assigns(:diffs).diff_files.first.diff.diff.split("\n")
-        expect(diff_splits.length).to be <= 2
+        expect(response.headers[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-diff:")
       end
     end
 
     describe "as patch" do
-      include_examples "export as", :patch
-      let(:format) { :patch }
-      let(:commit2) { project.commit('498214de67004b1da3d820901307bed2a68a8ef6') }
-
-      it "is a git email patch" do
-        go(id: commit2.id, format: format)
-
-        expect(response.body).to start_with("From #{commit2.id}")
-      end
-
       it "contains a git diff" do
-        go(id: commit2.id, format: format)
+        go(id: commit.id, format: :patch)
 
-        expect(response.body).to match(/^diff --git/)
+        expect(response.headers[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-format-patch:")
       end
     end
 

spec/controllers/projects/settings/ci_cd_controller_spec.rb

@@ -19,11 +19,11 @@ describe Projects::Settings::CiCdController do
     end
 
     context 'with group runners' do
-      let(:group_runner) { create(:ci_runner) }
+      let(:group_runner) { create(:ci_runner, runner_type: :group_type) }
       let(:parent_group) { create(:group) }
       let(:group) { create(:group, runners: [group_runner], parent: parent_group) }
       let(:other_project) { create(:project, group: group) }
-      let!(:project_runner) { create(:ci_runner, projects: [other_project]) }
+      let!(:project_runner) { create(:ci_runner, projects: [other_project], runner_type: :project_type) }
       let!(:shared_runner) { create(:ci_runner, :shared) }
 
       it 'sets assignable project runners only' do
@@ -31,7 +31,7 @@ describe Projects::Settings::CiCdController do
 
         get :show, namespace_id: project.namespace, project_id: project
 
-        expect(assigns(:assignable_runners)).to eq [project_runner]
+        expect(assigns(:assignable_runners)).to contain_exactly(project_runner)
       end
     end
   end

spec/fixtures/api/schemas/list.json

@@ -17,6 +17,7 @@
       "required": [
         "id",
         "color",
+        "text_color",
         "description",
         "title",
         "priority"
@@ -29,6 +30,7 @@
         },
         "description": { "type": ["string", "null"] },
         "title": { "type": "string" },
+        "title": { "text_color": "string" },
         "priority": { "type": ["integer", "null"] }
       }
     },

spec/lib/gitlab/database/count_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Database::Count do
+  before do
+    create_list(:project, 3)
+  end
+
+  describe '.execute_estimate_if_updated_recently', :postgresql do
+    context 'when reltuples have not been updated' do
+      before do
+        expect(described_class).to receive(:reltuples_updated_recently?).and_return(false)
+      end
+
+      it 'returns nil' do
+        expect(described_class.execute_estimate_if_updated_recently(Project)).to be nil
+      end
+    end
+
+    context 'when reltuples have been updated' do
+      before do
+        ActiveRecord::Base.connection.execute('ANALYZE projects')
+      end
+
+      it 'calls postgresql_estimate_query' do
+        expect(described_class).to receive(:postgresql_estimate_query).with(Project).and_call_original
+        expect(described_class.execute_estimate_if_updated_recently(Project)).to eq(3)
+      end
+    end
+  end
+
+  describe '.approximate_count' do
+    context 'when reltuples have not been updated' do
+      it 'counts all projects the normal way' do
+        allow(described_class).to receive(:reltuples_updated_recently?).and_return(false)
+
+        expect(Project).to receive(:count).and_call_original
+        expect(described_class.approximate_count(Project)).to eq(3)
+      end
+    end
+
+    context 'no permission' do
+      it 'falls back to standard query' do
+        allow(described_class).to receive(:reltuples_updated_recently?).and_raise(PG::InsufficientPrivilege)
+
+        expect(Project).to receive(:count).and_call_original
+        expect(described_class.approximate_count(Project)).to eq(3)
+      end
+    end
+
+    describe 'when reltuples have been updated', :postgresql do
+      before do
+        ActiveRecord::Base.connection.execute('ANALYZE projects')
+      end
+
+      it 'counts all projects in the fast way' do
+        expect(described_class).to receive(:postgresql_estimate_query).with(Project).and_call_original
+
+        expect(described_class.approximate_count(Project)).to eq(3)
+      end
+    end
+  end
+end

spec/lib/gitlab/git/commit_spec.rb

@@ -554,24 +554,10 @@ describe Gitlab::Git::Commit, seed_helper: true do
     it_should_behave_like '#stats'
   end
 
-  describe '#to_diff' do
-    subject { commit.to_diff }
-
-    it { is_expected.not_to include "From #{SeedRepo::Commit::ID}" }
-    it { is_expected.to include 'diff --git a/files/ruby/popen.rb b/files/ruby/popen.rb'}
-  end
-
   describe '#has_zero_stats?' do
     it { expect(commit.has_zero_stats?).to eq(false) }
   end
 
-  describe '#to_patch' do
-    subject { commit.to_patch }
-
-    it { is_expected.to include "From #{SeedRepo::Commit::ID}" }
-    it { is_expected.to include 'diff --git a/files/ruby/popen.rb b/files/ruby/popen.rb'}
-  end
-
   describe '#to_hash' do
     let(:hash) { commit.to_hash }
     subject { hash }

spec/models/appearance_spec.rb

@@ -5,7 +5,7 @@ describe Appearance do
 
   it { is_expected.to be_valid }
 
-  it { is_expected.to have_many(:uploads).dependent(:destroy) }
+  it { is_expected.to have_many(:uploads) }
 
   describe '.current', :use_clean_rails_memory_store_caching do
     let!(:appearance) { create(:appearance) }
@@ -41,4 +41,12 @@ describe Appearance do
       expect(new_row.valid?).to eq(false)
     end
   end
+
+  context 'with uploads' do
+    it_behaves_like 'model with mounted uploader', false do
+      let(:model_object) { create(:appearance, :with_logo) }
+      let(:upload_attribute) { :logo }
+      let(:uploader_class) { AttachmentUploader }
+    end
+  end
 end

spec/models/ci/runner_spec.rb

@@ -626,62 +626,26 @@ describe Ci::Runner do
   end
 
   describe '.assignable_for' do
-    let(:runner) { create(:ci_runner) }
+    let!(:unlocked_project_runner) { create(:ci_runner, runner_type: :project_type, projects: [project]) }
+    let!(:locked_project_runner) { create(:ci_runner, runner_type: :project_type, locked: true, projects: [project]) }
+    let!(:group_runner) { create(:ci_runner, runner_type: :group_type) }
+    let!(:instance_runner) { create(:ci_runner, :shared) }
     let(:project) { create(:project) }
     let(:another_project) { create(:project) }
 
-    before do
-      project.runners << runner
-    end
-
-    context 'with shared runners' do
-      before do
-        runner.update(is_shared: true)
-      end
-
-      context 'does not give owned runner' do
-        subject { described_class.assignable_for(project) }
-
-        it { is_expected.to be_empty }
-      end
-
-      context 'does not give shared runner' do
-        subject { described_class.assignable_for(another_project) }
-
-        it { is_expected.to be_empty }
-      end
-    end
-
-    context 'with unlocked runner' do
-      context 'does not give owned runner' do
+    context 'with already assigned project' do
       subject { described_class.assignable_for(project) }
 
       it { is_expected.to be_empty }
     end
 
-      context 'does give a specific runner' do
+    context 'with a different project' do
       subject { described_class.assignable_for(another_project) }
 
-        it { is_expected.to contain_exactly(runner) }
-      end
-    end
-
-    context 'with locked runner' do
-      before do
-        runner.update(locked: true)
-      end
-
-      context 'does not give owned runner' do
-        subject { described_class.assignable_for(project) }
-
-        it { is_expected.to be_empty }
-      end
-
-      context 'does not give a locked runner' do
-        subject { described_class.assignable_for(another_project) }
-
-        it { is_expected.to be_empty }
-      end
+      it { is_expected.to include(unlocked_project_runner) }
+      it { is_expected.not_to include(group_runner) }
+      it { is_expected.not_to include(locked_project_runner) }
+      it { is_expected.not_to include(instance_runner) }
     end
   end
 

spec/models/commit_spec.rb

@@ -182,7 +182,6 @@ eos
     it { is_expected.to respond_to(:date) }
     it { is_expected.to respond_to(:diffs) }
     it { is_expected.to respond_to(:id) }
-    it { is_expected.to respond_to(:to_patch) }
   end
 
   describe '#closes_issues' do

spec/models/group_spec.rb

@@ -15,7 +15,7 @@ describe Group do
     it { is_expected.to have_many(:notification_settings).dependent(:destroy) }
     it { is_expected.to have_many(:labels).class_name('GroupLabel') }
     it { is_expected.to have_many(:variables).class_name('Ci::GroupVariable') }
-    it { is_expected.to have_many(:uploads).dependent(:destroy) }
+    it { is_expected.to have_many(:uploads) }
     it { is_expected.to have_one(:chat_team) }
     it { is_expected.to have_many(:custom_attributes).class_name('GroupCustomAttribute') }
     it { is_expected.to have_many(:badges).class_name('GroupBadge') }
@@ -691,4 +691,12 @@ describe Group do
       end
     end
   end
+
+  context 'with uploads' do
+    it_behaves_like 'model with mounted uploader', true do
+      let(:model_object) { create(:group, :with_avatar) }
+      let(:upload_attribute) { :avatar }
+      let(:uploader_class) { AttachmentUploader }
+    end
+  end
 end

spec/models/project_spec.rb

@@ -76,7 +76,7 @@ describe Project do
     it { is_expected.to have_many(:project_group_links) }
     it { is_expected.to have_many(:notification_settings).dependent(:delete_all) }
     it { is_expected.to have_many(:forks).through(:forked_project_links) }
-    it { is_expected.to have_many(:uploads).dependent(:destroy) }
+    it { is_expected.to have_many(:uploads) }
     it { is_expected.to have_many(:pipeline_schedules) }
     it { is_expected.to have_many(:members_and_requesters) }
     it { is_expected.to have_many(:clusters) }
@@ -3739,4 +3739,12 @@ describe Project do
       it { is_expected.to be_nil }
     end
   end
+
+  context 'with uploads' do
+    it_behaves_like 'model with mounted uploader', true do
+      let(:model_object) { create(:project, :with_avatar) }
+      let(:upload_attribute) { :avatar }
+      let(:uploader_class) { AttachmentUploader }
+    end
+  end
 end

spec/models/user_spec.rb

@@ -39,7 +39,7 @@ describe User do
     it { is_expected.to have_many(:builds).dependent(:nullify) }
     it { is_expected.to have_many(:pipelines).dependent(:nullify) }
     it { is_expected.to have_many(:chat_names).dependent(:destroy) }
-    it { is_expected.to have_many(:uploads).dependent(:destroy) }
+    it { is_expected.to have_many(:uploads) }
     it { is_expected.to have_many(:reported_abuse_reports).dependent(:destroy).class_name('AbuseReport') }
     it { is_expected.to have_many(:custom_attributes).class_name('UserCustomAttribute') }
 
@@ -1786,28 +1786,54 @@ describe User do
     end
   end
 
-  describe '#ci_authorized_runners' do
+  describe '#ci_owned_runners' do
     let(:user) { create(:user) }
-    let(:runner) { create(:ci_runner) }
+    let(:runner_1) { create(:ci_runner) }
+    let(:runner_2) { create(:ci_runner) }
 
-    before do
-      project.runners << runner
-    end
-
-    context 'without any projects' do
-      let(:project) { create(:project) }
+    context 'without any projects nor groups' do
+      let!(:project) { create(:project, runners: [runner_1]) }
+      let!(:group) { create(:group) }
 
       it 'does not load' do
-        expect(user.ci_authorized_runners).to be_empty
+        expect(user.ci_owned_runners).to be_empty
       end
     end
 
     context 'with personal projects runners' do
       let(:namespace) { create(:namespace, owner: user) }
-      let(:project) { create(:project, namespace: namespace) }
+      let!(:project) { create(:project, namespace: namespace, runners: [runner_1]) }
+
+      it 'loads' do
+        expect(user.ci_owned_runners).to contain_exactly(runner_1)
+      end
+    end
+
+    context 'with personal group runner' do
+      let!(:project) { create(:project, runners: [runner_1]) }
+      let!(:group) do
+        create(:group, runners: [runner_2]).tap do |group|
+          group.add_owner(user)
+        end
+      end
 
       it 'loads' do
-        expect(user.ci_authorized_runners).to contain_exactly(runner)
+        expect(user.ci_owned_runners).to contain_exactly(runner_2)
+      end
+    end
+
+    context 'with personal project and group runner' do
+      let(:namespace) { create(:namespace, owner: user) }
+      let!(:project) { create(:project, namespace: namespace, runners: [runner_1]) }
+
+      let!(:group) do
+        create(:group, runners: [runner_2]).tap do |group|
+          group.add_owner(user)
+        end
+      end
+
+      it 'loads' do
+        expect(user.ci_owned_runners).to contain_exactly(runner_1, runner_2)
       end
     end
 
@@ -1818,7 +1844,7 @@ describe User do
         end
 
         it 'loads' do
-          expect(user.ci_authorized_runners).to contain_exactly(runner)
+          expect(user.ci_owned_runners).to contain_exactly(runner_1)
         end
       end
 
@@ -1828,14 +1854,28 @@ describe User do
         end
 
         it 'does not load' do
-          expect(user.ci_authorized_runners).to be_empty
+          expect(user.ci_owned_runners).to be_empty
         end
       end
     end
 
     context 'with groups projects runners' do
       let(:group) { create(:group) }
-      let(:project) { create(:project, group: group) }
+      let!(:project) { create(:project, group: group, runners: [runner_1]) }
+
+      def add_user(access)
+        group.add_user(user, access)
+      end
+
+      it_behaves_like :member
+    end
+
+    context 'with groups runners' do
+      let!(:group) do
+        create(:group, runners: [runner_1]).tap do |group|
+          group.add_owner(user)
+        end
+      end
 
       def add_user(access)
         group.add_user(user, access)
@@ -1845,7 +1885,7 @@ describe User do
     end
 
     context 'with other projects runners' do
-      let(:project) { create(:project) }
+      let!(:project) { create(:project, runners: [runner_1]) }
 
       def add_user(access)
         project.add_role(user, access)
@@ -1858,7 +1898,7 @@ describe User do
       let(:group) { create(:group) }
       let(:another_user) { create(:user) }
       let(:subgroup) { create(:group, parent: group) }
-      let(:project) { create(:project, group: subgroup) }
+      let!(:project) { create(:project, group: subgroup, runners: [runner_1]) }
 
       def add_user(access)
         group.add_user(user, access)
@@ -2769,4 +2809,12 @@ describe User do
       expect { user.increment_failed_attempts! }.not_to change(user, :failed_attempts)
     end
   end
+
+  context 'with uploads' do
+    it_behaves_like 'model with mounted uploader', false do
+      let(:model_object) { create(:user, :with_avatar) }
+      let(:upload_attribute) { :avatar }
+      let(:uploader_class) { AttachmentUploader }
+    end
+  end
 end

spec/requests/api/runners_spec.rb

@@ -27,7 +27,7 @@ describe API::Runners do
     end
   end
 
-  let!(:group_runner) { create(:ci_runner, description: 'Group runner', groups: [group]) }
+  let!(:group_runner) { create(:ci_runner, description: 'Group runner', groups: [group], runner_type: :group_type) }
 
   before do
     # Set project access for users
@@ -48,7 +48,7 @@ describe API::Runners do
         expect(json_response).to be_an Array
         expect(json_response[0]).to have_key('ip_address')
         expect(descriptions).to contain_exactly(
-          'Project runner', 'Two projects runner'
+          'Project runner', 'Two projects runner', 'Group runner'
         )
         expect(shared).to be_falsey
       end
@@ -592,6 +592,15 @@ describe API::Runners do
           end.to change { project.runners.count }.by(+1)
           expect(response).to have_gitlab_http_status(201)
         end
+
+        it 'enables a shared runner' do
+          expect do
+            post api("/projects/#{project.id}/runners", admin), runner_id: shared_runner.id
+          end.to change { project.runners.count }.by(1)
+
+          expect(shared_runner.reload).not_to be_shared
+          expect(response).to have_gitlab_http_status(201)
+        end
       end
 
       context 'user is not admin' do

spec/support/shared_examples/models/with_uploads_shared_examples.rb

+require 'spec_helper'
+
+shared_examples_for 'model with mounted uploader' do |supports_fileuploads|
+  describe '.destroy' do
+    before do
+      stub_uploads_object_storage(uploader_class)
+
+      model_object.public_send(upload_attribute).migrate!(ObjectStorage::Store::REMOTE)
+    end
+
+    it 'deletes remote uploads' do
+      expect_any_instance_of(CarrierWave::Storage::Fog::File).to receive(:delete).and_call_original
+
+      expect { model_object.destroy }.to change { Upload.count }.by(-1)
+    end
+
+    it 'deletes any FileUploader uploads which are not mounted', skip: !supports_fileuploads do
+      create(:upload, uploader: FileUploader, model: model_object)
+
+      expect { model_object.destroy }.to change { Upload.count }.by(-2)
+    end
+  end
+end