Merge remote-tracking branch 'upstream/master' into no-ivar-in-modules

* upstream/master: (1723 commits)
  Resolve "Editor icons"
  Refactor issuable destroy action
  Ignore routes matching legacy_*_redirect in route specs
  Gitlab::Git::RevList and LfsChanges use lazy popen
  Gitlab::Git::Popen can lazily hand output to a block
  Merge branch 'master-i18n' into 'master'
  Remove unique validation from external_url in Environment
  Expose `duration` in Job API entity
  Add TimeCop freeze for DST and Regular time
  Harcode project visibility
  update a changelog
  Put a condition to old migration that adds fast_forward column to MRs
  Expose project visibility as CI variable
  fix flaky tests by removing unneeded clicks and focus actions
  fix flaky test in gfm_autocomplete_spec.rb
  Use Gitlab::Git operations for repository mirroring
  Encapsulate git operations for mirroring in Gitlab::Git
  Create a Wiki Repository's raw_repository properly
  Add `Gitlab::Git::Repository#fetch` command
  Fix Gitlab::Metrics::System#real_time and #monotonic_time doc
  ...

15 files changed, 205 insertions, 63 deletions

diff --git a/app/models/concerns/avatarable.rb b/app/models/concerns/avatarable.rb
index 8fbfed11bdf..2ec70203710 100644
--- a/app/models/concerns/avatarable.rb
+++ b/app/models/concerns/avatarable.rb
@@ -11,7 +11,7 @@ module Avatarable
 
     # If asset_host is set then it is expected that assets are handled by a standalone host.
     # That means we do not want to get GitLab's relative_url_root option anymore.
-    host = asset_host.present? ? asset_host : gitlab_host
+    host = (asset_host.present? && (!respond_to?(:public?) || public?)) ? asset_host : gitlab_host
 
     [host, avatar.url].join
   end
diff --git a/app/models/concerns/cache_markdown_field.rb b/app/models/concerns/cache_markdown_field.rb
index 193e459977a..98776eab424 100644
--- a/app/models/concerns/cache_markdown_field.rb
+++ b/app/models/concerns/cache_markdown_field.rb
@@ -49,7 +49,8 @@ module CacheMarkdownField
 
     # Always include a project key, or Banzai complains
     project = self.project if self.respond_to?(:project)
-    context = cached_markdown_fields[field].merge(project: project)
+    group = self.group if self.respond_to?(:group)
+    context = cached_markdown_fields[field].merge(project: project, group: group)
 
     # Banzai is less strict about authors, so don't always have an author key
     context[:author] = self.author if self.respond_to?(:author)
@@ -59,7 +60,7 @@ module CacheMarkdownField
 
   # Update every column in a row if any one is invalidated, as we only store
   # one version per row
-  def refresh_markdown_cache!(do_update: false)
+  def refresh_markdown_cache
     options = { skip_project_check: skip_project_check? }
 
     updates = cached_markdown_fields.markdown_fields.map do |markdown_field|
@@ -71,8 +72,14 @@ module CacheMarkdownField
     updates['cached_markdown_version'] = CacheMarkdownField::CACHE_VERSION
 
     updates.each {|html_field, data| write_attribute(html_field, data) }
+  end
+
+  def refresh_markdown_cache!
+    updates = refresh_markdown_cache
+
+    return unless persisted? && Gitlab::Database.read_write?
 
-    update_columns(updates) if persisted? && do_update
+    update_columns(updates)
   end
 
   def cached_html_up_to_date?(markdown_field)
@@ -124,8 +131,8 @@ module CacheMarkdownField
     end
 
     # Using before_update here conflicts with elasticsearch-model somehow
-    before_create :refresh_markdown_cache!, if: :invalidated_markdown_cache?
-    before_update :refresh_markdown_cache!, if: :invalidated_markdown_cache?
+    before_create :refresh_markdown_cache, if: :invalidated_markdown_cache?
+    before_update :refresh_markdown_cache, if: :invalidated_markdown_cache?
   end
 
   class_methods do
diff --git a/app/models/concerns/discussion_on_diff.rb b/app/models/concerns/discussion_on_diff.rb
index eee1a36ac6b..f5cbb3becad 100644
--- a/app/models/concerns/discussion_on_diff.rb
+++ b/app/models/concerns/discussion_on_diff.rb
@@ -28,6 +28,10 @@ module DiscussionOnDiff
     true
   end
 
+  def file_new_path
+    first_note.position.new_path
+  end
+
   # Returns an array of at most 16 highlighted lines above a diff note
   def truncated_diff_lines(highlight: true)
     lines = highlight ? highlighted_diff_lines : diff_lines
diff --git a/app/models/concerns/group_descendant.rb b/app/models/concerns/group_descendant.rb
new file mode 100644
index 00000000000..01957da0bf3
--- /dev/null
+++ b/app/models/concerns/group_descendant.rb
@@ -0,0 +1,56 @@
+module GroupDescendant
+  # Returns the hierarchy of a project or group in the from of a hash upto a
+  # given top.
+  #
+  # > project.hierarchy
+  # => { parent_group => { child_group => project } }
+  def hierarchy(hierarchy_top = nil, preloaded = nil)
+    preloaded ||= ancestors_upto(hierarchy_top)
+    expand_hierarchy_for_child(self, self, hierarchy_top, preloaded)
+  end
+
+  # Merges all hierarchies of the given groups or projects into an array of
+  # hashes. All ancestors need to be loaded into the given `descendants` to avoid
+  # queries down the line.
+  #
+  # > GroupDescendant.merge_hierarchy([project, child_group, child_group2, parent])
+  # => { parent => [{ child_group => project}, child_group2] }
+  def self.build_hierarchy(descendants, hierarchy_top = nil)
+    descendants = Array.wrap(descendants).uniq
+    return [] if descendants.empty?
+
+    unless descendants.all? { |hierarchy| hierarchy.is_a?(GroupDescendant) }
+      raise ArgumentError.new('element is not a hierarchy')
+    end
+
+    all_hierarchies = descendants.map do |descendant|
+      descendant.hierarchy(hierarchy_top, descendants)
+    end
+
+    Gitlab::Utils::MergeHash.merge(all_hierarchies)
+  end
+
+  private
+
+  def expand_hierarchy_for_child(child, hierarchy, hierarchy_top, preloaded)
+    parent = hierarchy_top if hierarchy_top && child.parent_id == hierarchy_top.id
+    parent ||= preloaded.detect { |possible_parent| possible_parent.is_a?(Group) && possible_parent.id == child.parent_id }
+
+    if parent.nil? && !child.parent_id.nil?
+      raise ArgumentError.new('parent was not preloaded')
+    end
+
+    if parent.nil? && hierarchy_top.present?
+      raise ArgumentError.new('specified top is not part of the tree')
+    end
+
+    if parent && parent != hierarchy_top
+      expand_hierarchy_for_child(parent,
+                                 { parent => hierarchy },
+                                 hierarchy_top,
+                                 preloaded)
+    else
+      hierarchy
+    end
+  end
+end
diff --git a/app/models/concerns/has_status.rb b/app/models/concerns/has_status.rb
index 3803e18a96e..7c3ed96bc28 100644
--- a/app/models/concerns/has_status.rb
+++ b/app/models/concerns/has_status.rb
@@ -81,6 +81,7 @@ module HasStatus
     scope :canceled, -> { where(status: 'canceled')  }
     scope :skipped, -> { where(status: 'skipped')  }
     scope :manual, -> { where(status: 'manual')  }
+    scope :alive, -> { where(status: [:created, :pending, :running]) }
     scope :created_or_pending, -> { where(status: [:created, :pending]) }
     scope :running_or_pending, -> { where(status: [:running, :pending]) }
     scope :finished, -> { where(status: [:success, :failed, :canceled]) }
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 265f6e48540..a928b9d6367 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -14,7 +14,6 @@ module Issuable
   include StripAttribute
   include Awardable
   include Taskable
-  include TimeTrackable
   include Importable
   include Editable
   include AfterCommitQueue
@@ -95,8 +94,6 @@ module Issuable
 
     strip_attributes :title
 
-    acts_as_paranoid
-
     after_save :record_metrics, unless: :imported?
 
     # We want to use optimistic lock for cases when only title or description are involved
@@ -143,16 +140,18 @@ module Issuable
     end
 
     def sort(method, excluded_labels: [])
-      sorted = case method.to_s
-               when 'milestone_due_asc' then order_milestone_due_asc
-               when 'milestone_due_desc' then order_milestone_due_desc
-               when 'downvotes_desc' then order_downvotes_desc
-               when 'upvotes_desc' then order_upvotes_desc
-               when 'label_priority' then order_labels_priority(excluded_labels: excluded_labels)
-               when 'priority' then order_due_date_and_labels_priority(excluded_labels: excluded_labels)
-               else
-                 order_by(method)
-               end
+      sorted =
+        case method.to_s
+        when 'downvotes_desc'     then order_downvotes_desc
+        when 'label_priority'     then order_labels_priority(excluded_labels: excluded_labels)
+        when 'milestone'          then order_milestone_due_asc
+        when 'milestone_due_asc'  then order_milestone_due_asc
+        when 'milestone_due_desc' then order_milestone_due_desc
+        when 'popularity'         then order_upvotes_desc
+        when 'priority'           then order_due_date_and_labels_priority(excluded_labels: excluded_labels)
+        when 'upvotes_desc'       then order_upvotes_desc
+        else order_by(method)
+        end
 
       # Break ties with the ID column for pagination
       sorted.order(id: :desc)
@@ -214,7 +213,7 @@ module Issuable
     def grouping_columns(sort)
       grouping_columns = [arel_table[:id]]
 
-      if %w(milestone_due_desc milestone_due_asc).include?(sort)
+      if %w(milestone_due_desc milestone_due_asc milestone).include?(sort)
         milestone_table = Milestone.arel_table
         grouping_columns << milestone_table[:id]
         grouping_columns << milestone_table[:due_date]
@@ -254,23 +253,22 @@ module Issuable
     participants(user).include?(user)
   end
 
-  def to_hook_data(user)
-    hook_data = {
-      object_kind: self.class.name.underscore,
-      user: user.hook_attrs,
-      project: project.hook_attrs,
-      object_attributes: hook_attrs,
-      labels: labels.map(&:hook_attrs),
-      # DEPRECATED
-      repository: project.hook_attrs.slice(:name, :url, :description, :homepage)
-    }
-    if self.is_a?(Issue)
-      hook_data[:assignees] = assignees.map(&:hook_attrs) if assignees.any?
-    else
-      hook_data[:assignee] = assignee.hook_attrs if assignee
+  def to_hook_data(user, old_labels: [], old_assignees: [])
+    changes = previous_changes
+
+    if old_labels != labels
+      changes[:labels] = [old_labels.map(&:hook_attrs), labels.map(&:hook_attrs)]
+    end
+
+    if old_assignees != assignees
+      if self.is_a?(Issue)
+        changes[:assignees] = [old_assignees.map(&:hook_attrs), assignees.map(&:hook_attrs)]
+      else
+        changes[:assignee] = [old_assignees&.first&.hook_attrs, assignee&.hook_attrs]
+      end
     end
 
-    hook_data
+    Gitlab::HookData::IssuableBuilder.new(self).build(user: user, changes: changes)
   end
 
   def labels_array
diff --git a/app/models/concerns/loaded_in_group_list.rb b/app/models/concerns/loaded_in_group_list.rb
new file mode 100644
index 00000000000..dcb3b2b5ff3
--- /dev/null
+++ b/app/models/concerns/loaded_in_group_list.rb
@@ -0,0 +1,72 @@
+module LoadedInGroupList
+  extend ActiveSupport::Concern
+
+  module ClassMethods
+    def with_counts(archived:)
+      selects_including_counts = [
+        'namespaces.*',
+        "(#{project_count_sql(archived).to_sql}) AS preloaded_project_count",
+        "(#{member_count_sql.to_sql}) AS preloaded_member_count",
+        "(#{subgroup_count_sql.to_sql}) AS preloaded_subgroup_count"
+      ]
+
+      select(selects_including_counts)
+    end
+
+    def with_selects_for_list(archived: nil)
+      with_route.with_counts(archived: archived)
+    end
+
+    private
+
+    def project_count_sql(archived = nil)
+      projects = Project.arel_table
+      namespaces = Namespace.arel_table
+
+      base_count = projects.project(Arel.star.count.as('preloaded_project_count'))
+                     .where(projects[:namespace_id].eq(namespaces[:id]))
+      if archived == 'only'
+        base_count.where(projects[:archived].eq(true))
+      elsif Gitlab::Utils.to_boolean(archived)
+        base_count
+      else
+        base_count.where(projects[:archived].not_eq(true))
+      end
+    end
+
+    def subgroup_count_sql
+      namespaces = Namespace.arel_table
+      children = namespaces.alias('children')
+
+      namespaces.project(Arel.star.count.as('preloaded_subgroup_count'))
+        .from(children)
+        .where(children[:parent_id].eq(namespaces[:id]))
+    end
+
+    def member_count_sql
+      members = Member.arel_table
+      namespaces = Namespace.arel_table
+
+      members.project(Arel.star.count.as('preloaded_member_count'))
+        .where(members[:source_type].eq(Namespace.name))
+        .where(members[:source_id].eq(namespaces[:id]))
+        .where(members[:requested_at].eq(nil))
+    end
+  end
+
+  def children_count
+    @children_count ||= project_count + subgroup_count
+  end
+
+  def project_count
+    @project_count ||= try(:preloaded_project_count) || projects.non_archived.count
+  end
+
+  def subgroup_count
+    @subgroup_count ||= try(:preloaded_subgroup_count) || children.count
+  end
+
+  def member_count
+    @member_count ||= try(:preloaded_member_count) || users.count
+  end
+end
diff --git a/app/models/concerns/noteable.rb b/app/models/concerns/noteable.rb
index 9d81a19cbb9..b44274f6145 100644
--- a/app/models/concerns/noteable.rb
+++ b/app/models/concerns/noteable.rb
@@ -75,4 +75,8 @@ module Noteable
   def discussions_can_be_resolved_by?(user)
     discussions_to_be_resolved.all? { |discussion| discussion.can_resolve?(user) }
   end
+
+  def lockable?
+    [MergeRequest, Issue].include?(self.class)
+  end
 end
diff --git a/app/models/concerns/repository_mirroring.rb b/app/models/concerns/repository_mirroring.rb
deleted file mode 100644
index fed336c29d6..00000000000
--- a/app/models/concerns/repository_mirroring.rb
+++ /dev/null
@@ -1,17 +0,0 @@
-module RepositoryMirroring
-  def set_remote_as_mirror(name)
-    config = raw_repository.rugged.config
-
-    # This is used to define repository as equivalent as "git clone --mirror"
-    config["remote.#{name}.fetch"] = 'refs/*:refs/*'
-    config["remote.#{name}.mirror"] = true
-    config["remote.#{name}.prune"] = true
-  end
-
-  def fetch_mirror(remote, url)
-    add_remote(remote, url)
-    set_remote_as_mirror(remote)
-    fetch_remote(remote, forced: true)
-    remove_remote(remote)
-  end
-end
diff --git a/app/models/concerns/routable.rb b/app/models/concerns/routable.rb
index 80a8f63514f..05ddae42d2d 100644
--- a/app/models/concerns/routable.rb
+++ b/app/models/concerns/routable.rb
@@ -107,7 +107,10 @@ module Routable
     RequestStore[full_path_key] ||= uncached_full_path
   end
 
-  # rubocop:disable Cop/ModuleWithInstanceVariables
+  def full_path_components
+    full_path.split('/')
+  end
+
   def expires_full_path_cache
     RequestStore.delete(full_path_key) if RequestStore.active?
     @full_path = nil
@@ -155,6 +158,8 @@ module Routable
   end
 
   def update_route
+    return if Gitlab::Database.read_only?
+
     prepare_route
     route.save
   end
diff --git a/app/models/concerns/sortable.rb b/app/models/concerns/sortable.rb
index db3cd257584..cefa5c13c5f 100644
--- a/app/models/concerns/sortable.rb
+++ b/app/models/concerns/sortable.rb
@@ -19,14 +19,15 @@ module Sortable
   module ClassMethods
     def order_by(method)
       case method.to_s
-      when 'name_asc' then order_name_asc
-      when 'name_desc' then order_name_desc
-      when 'updated_asc' then order_updated_asc
-      when 'updated_desc' then order_updated_desc
-      when 'created_asc' then order_created_asc
+      when 'created_asc'  then order_created_asc
+      when 'created_date' then order_created_desc
       when 'created_desc' then order_created_desc
-      when 'id_desc' then order_id_desc
-      when 'id_asc' then order_id_asc
+      when 'id_asc'       then order_id_asc
+      when 'id_desc'      then order_id_desc
+      when 'name_asc'     then order_name_asc
+      when 'name_desc'    then order_name_desc
+      when 'updated_asc'  then order_updated_asc
+      when 'updated_desc' then order_updated_desc
       else
         all
       end
diff --git a/app/models/concerns/storage/legacy_namespace.rb b/app/models/concerns/storage/legacy_namespace.rb
index 5ab5c80a2f5..b3020484738 100644
--- a/app/models/concerns/storage/legacy_namespace.rb
+++ b/app/models/concerns/storage/legacy_namespace.rb
@@ -7,6 +7,8 @@ module Storage
         raise Gitlab::UpdatePathError.new('Namespace cannot be moved, because at least one project has tags in container registry')
       end
 
+      expires_full_path_cache
+
       # Move the namespace directory in all storage paths used by member projects
       repository_storage_paths.each do |repository_storage_path|
         # Ensure old directory exists before moving it
diff --git a/app/models/concerns/subscribable.rb b/app/models/concerns/subscribable.rb
index 274b38a7708..f478c8ede18 100644
--- a/app/models/concerns/subscribable.rb
+++ b/app/models/concerns/subscribable.rb
@@ -13,6 +13,8 @@ module Subscribable
   end
 
   def subscribed?(user, project = nil)
+    return false unless user
+
     if subscription = subscriptions.find_by(user: user, project: project)
       subscription.subscribed
     else
diff --git a/app/models/concerns/time_trackable.rb b/app/models/concerns/time_trackable.rb
index 995fa98efac..49438908c36 100644
--- a/app/models/concerns/time_trackable.rb
+++ b/app/models/concerns/time_trackable.rb
@@ -8,7 +8,7 @@ module TimeTrackable
   extend ActiveSupport::Concern
 
   included do
-    attr_reader :time_spent, :time_spent_user
+    attr_reader :time_spent, :time_spent_user, :spent_at
 
     alias_method :time_spent?, :time_spent
 
@@ -24,6 +24,7 @@ module TimeTrackable
   def spend_time(options)
     @time_spent = options[:duration]
     @time_spent_user = options[:user]
+    @spent_at = options[:spent_at]
     @original_total_time_spent = nil
 
     return if @time_spent == 0
@@ -56,7 +57,11 @@ module TimeTrackable
   end
 
   def add_or_subtract_spent_time
-    timelogs.new(time_spent: time_spent, user: @time_spent_user)
+    timelogs.new(
+      time_spent: time_spent,
+      user: @time_spent_user,
+      spent_at: @spent_at
+    )
   end
 
   # rubocop:disable Cop/ModuleWithInstanceVariables
diff --git a/app/models/concerns/token_authenticatable.rb b/app/models/concerns/token_authenticatable.rb
index a7d5de48c66..ec3543f7053 100644
--- a/app/models/concerns/token_authenticatable.rb
+++ b/app/models/concerns/token_authenticatable.rb
@@ -43,15 +43,17 @@ module TokenAuthenticatable
         write_attribute(token_field, token) if token
       end
 
+      # Returns a token, but only saves when the database is in read & write mode
       define_method("ensure_#{token_field}!") do
         send("reset_#{token_field}!") if read_attribute(token_field).blank? # rubocop:disable GitlabSecurity/PublicSend
 
         read_attribute(token_field)
       end
 
+      # Resets the token, but only saves when the database is in read & write mode
       define_method("reset_#{token_field}!") do
         write_new_token(token_field)
-        save!
+        save! if Gitlab::Database.read_write?
       end
     end
   end