Merge remote-tracking branch 'origin/master' into ce-to-ee

Signed-off-by: Rémy Coutable <remy@rymai.me>

.rubocop.yml

@@ -963,7 +963,7 @@ RSpec/DescribeSymbol:
 # Checks that the second argument to top level describe is the tested method
 # name.
 RSpec/DescribedClass:
-  Enabled: false
+  Enabled: true
 
 # Checks for long example.
 RSpec/ExampleLength:

app/controllers/concerns/notes_actions.rb

+module NotesActions
+  include RendersNotes
+  extend ActiveSupport::Concern
+
+  included do
+    before_action :authorize_admin_note!, only: [:update, :destroy]
+  end
+
+  def index
+    current_fetched_at = Time.now.to_i
+
+    notes_json = { notes: [], last_fetched_at: current_fetched_at }
+
+    @notes = notes_finder.execute.inc_relations_for_view
+    @notes = prepare_notes_for_rendering(@notes)
+
+    @notes.each do |note|
+      next if note.cross_reference_not_visible_for?(current_user)
+
+      notes_json[:notes] << note_json(note)
+    end
+
+    render json: notes_json
+  end
+
+  def create
+    create_params = note_params.merge(
+      merge_request_diff_head_sha: params[:merge_request_diff_head_sha],
+      in_reply_to_discussion_id: params[:in_reply_to_discussion_id]
+    )
+    @note = Notes::CreateService.new(project, current_user, create_params).execute
+
+    if @note.is_a?(Note)
+      Banzai::NoteRenderer.render([@note], @project, current_user)
+    end
+
+    respond_to do |format|
+      format.json { render json: note_json(@note) }
+      format.html { redirect_back_or_default }
+    end
+  end
+
+  def update
+    @note = Notes::UpdateService.new(project, current_user, note_params).execute(note)
+
+    if @note.is_a?(Note)
+      Banzai::NoteRenderer.render([@note], @project, current_user)
+    end
+
+    respond_to do |format|
+      format.json { render json: note_json(@note) }
+      format.html { redirect_back_or_default }
+    end
+  end
+
+  def destroy
+    if note.editable?
+      Notes::DestroyService.new(project, current_user).execute(note)
+    end
+
+    respond_to do |format|
+      format.js { head :ok }
+    end
+  end
+
+  private
+
+  def note_json(note)
+    attrs = {
+      commands_changes: note.commands_changes
+    }
+
+    if note.persisted?
+      attrs.merge!(
+        valid: true,
+        id: note.id,
+        discussion_id: note.discussion_id(noteable),
+        html: note_html(note),
+        note: note.note
+      )
+
+      discussion = note.to_discussion(noteable)
+      unless discussion.individual_note?
+        attrs.merge!(
+          discussion_resolvable: discussion.resolvable?,
+
+          diff_discussion_html: diff_discussion_html(discussion),
+          discussion_html: discussion_html(discussion)
+        )
+      end
+    else
+      attrs.merge!(
+        valid: false,
+        errors: note.errors
+      )
+    end
+
+    attrs
+  end
+
+  def authorize_admin_note!
+    return access_denied! unless can?(current_user, :admin_note, note)
+  end
+
+  def note_params
+    params.require(:note).permit(
+      :project_id,
+      :noteable_type,
+      :noteable_id,
+      :commit_id,
+      :noteable,
+      :type,
+
+      :note,
+      :attachment,
+
+      # LegacyDiffNote
+      :line_code,
+
+      # DiffNote
+      :position
+    )
+  end
+
+  def noteable
+    @noteable ||= notes_finder.target
+  end
+
+  def last_fetched_at
+    request.headers['X-Last-Fetched-At']
+  end
+
+  def notes_finder
+    @notes_finder ||= NotesFinder.new(project, current_user, finder_params)
+  end
+end

app/controllers/concerns/renders_notes.rb

@@ -10,6 +10,8 @@ module RendersNotes
   private
 
   def preload_max_access_for_authors(notes, project)
+    return nil unless project
+
     user_ids = notes.map(&:author_id)
     project.team.max_member_access_for_user_ids(user_ids)
   end

app/controllers/concerns/toggle_award_emoji.rb

@@ -22,7 +22,8 @@ module ToggleAwardEmoji
   def to_todoable(awardable)
     case awardable
     when Note
-      awardable.noteable
+      # we don't create todos for personal snippet comments for now
+      awardable.for_personal_snippet? ? nil : awardable.noteable
     when MergeRequest, Issue
       awardable
     when Snippet

app/controllers/projects/mirrors_controller.rb

@@ -49,6 +49,6 @@ class Projects::MirrorsController < Projects::ApplicationController
   def mirror_params
     params.require(:project).permit(:mirror, :import_url, :mirror_user_id,
                                     :mirror_trigger_builds, :sync_time,
-                                    remote_mirrors_attributes: [:url, :id, :enabled, :sync_time])
+                                    remote_mirrors_attributes: [:url, :id, :enabled])
   end
 end

app/controllers/projects/notes_controller.rb

 class Projects::NotesController < Projects::ApplicationController
-  include RendersNotes
+  include NotesActions
   include ToggleAwardEmoji
 
-  # Authorize
   before_action :authorize_read_note!
   before_action :authorize_create_note!, only: [:create]
-  before_action :authorize_admin_note!, only: [:update, :destroy]
   before_action :authorize_resolve_note!, only: [:resolve, :unresolve]
 
-  def index
-    current_fetched_at = Time.now.to_i
-
-    notes_json = { notes: [], last_fetched_at: current_fetched_at }
-
-    @notes = notes_finder.execute.inc_relations_for_view
-    @notes = prepare_notes_for_rendering(@notes)
-
-    @notes.each do |note|
-      next if note.cross_reference_not_visible_for?(current_user)
-
-      notes_json[:notes] << note_json(note)
-    end
-
-    render json: notes_json
-  end
-
+  #
+  # This is a fix to make spinach feature tests passing:
+  # Controller actions are returned from AbstractController::Base and methods of parent classes are
+  #   excluded in order to return only specific controller related methods.
+  # That is ok for the app (no :create method in ancestors)
+  #   but fails for tests because there is a :create method on FactoryGirl (one of the ancestors)
+  #
+  # see https://github.com/rails/rails/blob/v4.2.7/actionpack/lib/abstract_controller/base.rb#L78
+  #
   def create
-    create_params = note_params.merge(
-      merge_request_diff_head_sha: params[:merge_request_diff_head_sha],
-      in_reply_to_discussion_id: params[:in_reply_to_discussion_id]
-    )
-    @note = Notes::CreateService.new(project, current_user, create_params).execute
-
-    if @note.is_a?(Note)
-      Banzai::NoteRenderer.render([@note], @project, current_user)
-    end
-
-    respond_to do |format|
-      format.json { render json: note_json(@note) }
-      format.html { redirect_back_or_default }
-    end
-  end
-
-  def update
-    @note = Notes::UpdateService.new(project, current_user, note_params).execute(note)
-
-    if @note.is_a?(Note)
-      Banzai::NoteRenderer.render([@note], @project, current_user)
-    end
-
-    respond_to do |format|
-      format.json { render json: note_json(@note) }
-      format.html { redirect_back_or_default }
-    end
-  end
-
-  def destroy
-    if note.editable?
-      Notes::DestroyService.new(project, current_user).execute(note)
-    end
-
-    respond_to do |format|
-      format.js { head :ok }
-    end
+    super
   end
 
   def delete_attachment
@@ -110,7 +64,7 @@ class Projects::NotesController < Projects::ApplicationController
 
   def note_html(note)
     render_to_string(
-      "projects/notes/_note",
+      "shared/notes/_note",
       layout: false,
       formats: [:html],
       locals: { note: note }
@@ -152,76 +106,11 @@ class Projects::NotesController < Projects::ApplicationController
     )
   end
 
-  def note_json(note)
-    attrs = {
-      commands_changes: note.commands_changes
-    }
-
-    if note.persisted?
-      attrs.merge!(
-        valid: true,
-        id: note.id,
-        discussion_id: note.discussion_id(noteable),
-        html: note_html(note),
-        note: note.note
-      )
-
-      discussion = note.to_discussion(noteable)
-      unless discussion.individual_note?
-        attrs.merge!(
-          discussion_resolvable: discussion.resolvable?,
-
-          diff_discussion_html: diff_discussion_html(discussion),
-          discussion_html: discussion_html(discussion)
-        )
-      end
-    else
-      attrs.merge!(
-        valid: false,
-        errors: note.errors
-      )
-    end
-
-    attrs
-  end
-
-  def authorize_admin_note!
-    return access_denied! unless can?(current_user, :admin_note, note)
+  def finder_params
+    params.merge(last_fetched_at: last_fetched_at)
   end
 
   def authorize_resolve_note!
     return access_denied! unless can?(current_user, :resolve_note, note)
   end
-
-  def note_params
-    params.require(:note).permit(
-      :project_id,
-      :noteable_type,
-      :noteable_id,
-      :commit_id,
-      :noteable,
-      :type,
-
-      :note,
-      :attachment,
-
-      # LegacyDiffNote
-      :line_code,
-
-      # DiffNote
-      :position
-    )
-  end
-
-  def notes_finder
-    @notes_finder ||= NotesFinder.new(project, current_user, params.merge(last_fetched_at: last_fetched_at))
-  end
-
-  def noteable
-    @noteable ||= notes_finder.target
-  end
-
-  def last_fetched_at
-    request.headers['X-Last-Fetched-At']
-  end
 end

app/controllers/snippets/notes_controller.rb

+class Snippets::NotesController < ApplicationController
+  include NotesActions
+  include ToggleAwardEmoji
+
+  skip_before_action :authenticate_user!, only: [:index]
+  before_action :snippet
+  before_action :authorize_read_snippet!, only: [:show, :index, :create]
+
+  private
+
+  def note
+    @note ||= snippet.notes.find(params[:id])
+  end
+  alias_method :awardable, :note
+
+  def note_html(note)
+    render_to_string(
+      "shared/notes/_note",
+      layout: false,
+      formats: [:html],
+      locals: { note: note }
+    )
+  end
+
+  def project
+    nil
+  end
+
+  def snippet
+    PersonalSnippet.find_by(id: params[:snippet_id])
+  end
+
+  def note_params
+    super.merge(noteable_id: params[:snippet_id])
+  end
+
+  def finder_params
+    params.merge(last_fetched_at: last_fetched_at, target_id: snippet.id, target_type: 'personal_snippet')
+  end
+
+  def authorize_read_snippet!
+    return render_404 unless can?(current_user, :read_personal_snippet, snippet)
+  end
+end

app/controllers/snippets_controller.rb

 class SnippetsController < ApplicationController
+  include RendersNotes
   include ToggleAwardEmoji
   include SpammableActions
   include SnippetsActions
@@ -61,6 +62,7 @@ class SnippetsController < ApplicationController
   end
 
   def show
+<<<<<<< HEAD
     blob = @snippet.blob
     override_max_blob_size(blob)
 
@@ -73,6 +75,12 @@ class SnippetsController < ApplicationController
         render_blob_json(blob)
       end
     end
+=======
+    @noteable = @snippet
+
+    @discussions = @snippet.discussions
+    @notes = prepare_notes_for_rendering(@discussions.flat_map(&:notes))
+>>>>>>> origin/master
   end
 
   def destroy

app/finders/notes_finder.rb

@@ -68,6 +68,8 @@ class NotesFinder
       MergeRequestsFinder.new(@current_user, project_id: @project.id).execute
     when "snippet", "project_snippet"
       SnippetsFinder.new.execute(@current_user, filter: :by_project, project: @project)
+    when "personal_snippet"
+      PersonalSnippet.all
     else
       raise 'invalid target_type'
     end

app/helpers/award_emoji_helper.rb

 module AwardEmojiHelper
   def toggle_award_url(awardable)
-    return url_for([:toggle_award_emoji, awardable]) unless @project
+    return url_for([:toggle_award_emoji, awardable]) unless @project || awardable.is_a?(Note)
 
     if awardable.is_a?(Note)
       # We render a list of notes very frequently and calling the specific method is a lot faster than the generic one (4.5x)
-      toggle_award_emoji_namespace_project_note_url(@project.namespace, @project, awardable.id)
+      if awardable.for_personal_snippet?
+        toggle_award_emoji_snippet_note_path(awardable.noteable, awardable)
+      else
+        toggle_award_emoji_namespace_project_note_path(@project.namespace, @project, awardable.id)
+      end
     else
       url_for([:toggle_award_emoji, @project.namespace.becomes(Namespace), @project, awardable])
     end

app/models/application_setting.rb

@@ -182,7 +182,7 @@ class ApplicationSetting < ActiveRecord::Base
   before_save :ensure_runners_registration_token
   before_save :ensure_health_check_access_token
 
-  after_update :update_mirror_cron_jobs, if: :minimum_mirror_sync_time_changed?
+  after_update :update_mirror_cron_job, if: :minimum_mirror_sync_time_changed?
 
   after_commit do
     Rails.cache.write(CACHE_KEY, self)
@@ -291,13 +291,11 @@ class ApplicationSetting < ActiveRecord::Base
     end
   end
 
-  def update_mirror_cron_jobs
+  def update_mirror_cron_job
     Project.mirror.where('sync_time < ?', minimum_mirror_sync_time)
       .update_all(sync_time: minimum_mirror_sync_time)
-    RemoteMirror.where('sync_time < ?', minimum_mirror_sync_time)
-      .update_all(sync_time: minimum_mirror_sync_time)
 
-    Gitlab::Mirror.configure_cron_jobs!
+    Gitlab::Mirror.configure_cron_job!
   end
 
   def elasticsearch_url

app/models/concerns/cache_markdown_field.rb

@@ -78,6 +78,9 @@ module CacheMarkdownField
   def cached_html_up_to_date?(markdown_field)
     html_field = cached_markdown_fields.html_field(markdown_field)
 
+    cached = !cached_html_for(markdown_field).nil? && !__send__(markdown_field).nil?
+    return false unless cached
+
     markdown_changed = attribute_changed?(markdown_field) || false
     html_changed = attribute_changed?(html_field) || false
 

app/models/namespace.rb

@@ -34,7 +34,7 @@ class Namespace < ActiveRecord::Base
   validates :path,
     presence: true,
     length: { maximum: 255 },
-    namespace: true
+    dynamic_path: true
 
   validate :nesting_level_allowed
 
@@ -225,6 +225,10 @@ class Namespace < ActiveRecord::Base
     Project.inside_path(full_path)
   end
 
+  def has_parent?
+    parent.present?
+  end
+
   private
 
   def repository_storage_paths

app/models/project.rb

@@ -205,13 +205,14 @@ class Project < ActiveRecord::Base
               message: Gitlab::Regex.project_name_regex_message }
   validates :path,
     presence: true,
-    project_path: true,
+    dynamic_path: true,
     length: { maximum: 255 },
     format: { with: Gitlab::Regex.project_path_regex,
-              message: Gitlab::Regex.project_path_regex_message }
+              message: Gitlab::Regex.project_path_regex_message },
+    uniqueness: { scope: :namespace_id }
+
   validates :namespace, presence: true
   validates :name, uniqueness: { scope: :namespace_id }
-  validates :path, uniqueness: { scope: :namespace_id }
   validates :import_url, addressable_url: true, if: :external_import?
   validates :import_url, importable_url: true, if: [:external_import?, :import_url_changed?]
   validates :star_count, numericality: { greater_than_or_equal_to: 0 }
@@ -639,6 +640,14 @@ class Project < ActiveRecord::Base
     remote_mirrors.each(&:sync)
   end
 
+  def mark_stuck_remote_mirrors_as_failed!
+    remote_mirrors.stuck.update_all(
+      update_status: :failed,
+      last_error: 'The remote mirror took to long to complete.',
+      last_update_at: Time.now
+    )
+  end
+
   def fetch_mirror
     return unless mirror?
 

app/models/remote_mirror.rb

 class RemoteMirror < ActiveRecord::Base
   include AfterCommitQueue
+  include IgnorableColumn
+
+  ignore_column :sync_time
+
+  BACKOFF_DELAY = 5.minutes
 
   attr_encrypted :credentials,
                  key: Gitlab::Application.secrets.db_key_base,
@@ -12,9 +17,6 @@ class RemoteMirror < ActiveRecord::Base
   belongs_to :project, inverse_of: :remote_mirrors
 
   validates :url, presence: true, url: { protocols: %w(ssh git http https), allow_blank: true }
-  validates :sync_time,
-    presence: true,
-    inclusion: { in: Gitlab::Mirror::SYNC_TIME_OPTIONS.values }
 
   validate  :url_availability, if: -> (mirror) { mirror.url_changed? || mirror.enabled? }
 
@@ -28,7 +30,7 @@ class RemoteMirror < ActiveRecord::Base
 
   state_machine :update_status, initial: :none do
     event :update_start do
-      transition [:none, :finished] => :started
+      transition [:none, :finished, :failed] => :started
     end
 
     event :update_finish do
@@ -39,24 +41,22 @@ class RemoteMirror < ActiveRecord::Base
       transition started: :failed
     end
 
-    event :update_retry do
-      transition failed: :started
-    end
-
     state :started
     state :finished
     state :failed
 
-    after_transition any => :started, do: :schedule_update_job
+    after_transition any => :started do |remote_mirror, _|
+      remote_mirror.update(last_update_started_at: Time.now)
+    end
 
-    after_transition started: :finished do |remote_mirror, transaction|
+    after_transition started: :finished do |remote_mirror, _|
       timestamp = Time.now
       remote_mirror.update_attributes!(
         last_update_at: timestamp, last_successful_update_at: timestamp, last_error: nil
       )
     end
 
-    after_transition started: :failed do |remote_mirror, transaction|
+    after_transition started: :failed do |remote_mirror, _|
       remote_mirror.update(last_update_at: Time.now)
     end
   end
@@ -74,10 +74,13 @@ class RemoteMirror < ActiveRecord::Base
   end
 
   def sync
-    return unless project
-    return if !enabled || update_in_progress?
+    return unless project && enabled
 
-    update_failed? ? update_retry : update_start
+    RepositoryUpdateRemoteMirrorWorker.perform_in(BACKOFF_DELAY, self.id, Time.now) if project&.repository_exists?
+  end
+
+  def updated_since?(timestamp)
+    last_update_started_at && last_update_started_at > timestamp && !update_failed?
   end
 
   def mark_for_delete_if_blank_url
@@ -130,16 +133,6 @@ class RemoteMirror < ActiveRecord::Base
     )
   end
 
-  def schedule_update_job
-    run_after_commit(:add_update_job)
-  end
-
-  def add_update_job
-    if project && project.repository_exists?
-      RepositoryUpdateRemoteMirrorWorker.perform_async(self.id)
-    end
-  end
-
   def refresh_remote
     return unless project
 

app/models/user.rb

@@ -128,7 +128,7 @@ class User < ActiveRecord::Base
     presence: true,
     numericality: { greater_than_or_equal_to: 0, less_than_or_equal_to: Gitlab::Database::MAX_INT_VALUE }
   validates :username,
-    namespace: true,
+    dynamic_path: true,
     presence: true,
     uniqueness: { case_sensitive: false }
 

app/services/git_push_service.rb

@@ -59,6 +59,7 @@ class GitPushService < BaseService
     execute_related_hooks
     perform_housekeeping
 
+    update_remote_mirrors
     update_caches
   end
 
@@ -96,6 +97,13 @@ class GitPushService < BaseService
 
   protected
 
+  def update_remote_mirrors
+    return if @project.remote_mirrors.empty?
+
+    @project.mark_stuck_remote_mirrors_as_failed!
+    @project.update_remote_mirrors
+  end
+
   def execute_related_hooks
     # Update merge requests that may be affected by this push. A new branch
     # could cause the last commit of a merge request to change.

app/services/todo_service.rb

@@ -299,7 +299,7 @@ class TodoService
 
   def attributes_for_target(target)
     attributes = {
-      project_id: target.project.id,
+      project_id: target&.project&.id,
       target_id: target.id,
       target_type: target.class.name,
       commit_id: nil

app/validators/dynamic_path_validator.rb

+# DynamicPathValidator
+#
+# Custom validator for GitLab path values.
+# These paths are assigned to `Namespace` (& `Group` as a subclass) & `Project`
+#
+# Values are checked for formatting and exclusion from a list of reserved path
+# names.
+class DynamicPathValidator < ActiveModel::EachValidator
+  # All routes that appear on the top level must be listed here.
+  # This will make sure that groups cannot be created with these names
+  # as these routes would be masked by the paths already in place.
+  #
+  # Example:
+  #   /api/api-project
+  #
+  #  the path `api` shouldn't be allowed because it would be masked by `api/*`
+  #
+  TOP_LEVEL_ROUTES = %w[
+    -
+    .well-known
+    abuse_reports
+    admin
+    all
+    api
+    assets
+    autocomplete
+    ci
+    dashboard
+    explore
+    files
+    groups
+    health_check
+    help
+    hooks
+    import
+    invites
+    issues
+    jwt
+    koding
+    member
+    merge_requests
+    new
+    notes
+    notification_settings
+    oauth
+    profile
+    projects
+    public
+    repository
+    robots.txt
+    s
+    search
+    sent_notifications
+    services
+    snippets
+    teams
+    u
+    unicorn_test
+    unsubscribes
+    uploads
+    users
+  ].freeze
+
+  # This list should contain all words following `/*namespace_id/:project_id` in
+  # routes that contain a second wildcard.
+  #
+  # Example:
+  #   /*namespace_id/:project_id/badges/*ref/build
+  #
+  # If `badges` was allowed as a project/group name, we would not be able to access the
+  # `badges` route for those projects:
+  #
+  # Consider a namespace with path `foo/bar` and a project called `badges`.
+  # The route to the build badge would then be `/foo/bar/badges/badges/master/build.svg`
+  #
+  # When accessing this path the route would be matched to the `badges` path
+  # with the following params:
+  #   - namespace_id: `foo`
+  #   - project_id: `bar`
+  #   - ref: `badges/master`
+  #
+  # Failing to find the project, this would result in a 404.
+  #
+  # By rejecting `badges` the router can _count_ on the fact that `badges` will
+  # be preceded by the `namespace/project`.
+  WILDCARD_ROUTES = %w[
+    badges
+    blame
+    blob
+    builds
+    commits
+    create
+    create_dir
+    edit
+    environments/folders
+    files
+    find_file
+    gitlab-lfs/objects
+    info/lfs/objects
+    new
+    preview
+    raw
+    refs
+    tree
+    update
+    wikis
+  ].freeze
+
+  # These are all the paths that follow `/groups/*id/ or `/groups/*group_id`
+  # We need to reject these because we have a `/groups/*id` page that is the same
+  # as the `/*id`.
+  #
+  # If we would allow a subgroup to be created with the name `activity` then
+  # this group would not be accessible through `/groups/parent/activity` since
+  # this would map to the activity-page of it's parent.
+  GROUP_ROUTES = %w[
+    activity
+    avatar
+    edit
+    group_members
+    issues
+    labels
+    merge_requests
+    milestones
+    projects
+    subgroups
+    analytics
+    audit_events
+    hooks
+    ldap
+    ldap_group_links
+    notification_setting
+    pipeline_quota
+  ].freeze
+
+  CHILD_ROUTES = (WILDCARD_ROUTES | GROUP_ROUTES).freeze
+
+  def self.without_reserved_wildcard_paths_regex
+    @without_reserved_wildcard_paths_regex ||= regex_excluding_child_paths(WILDCARD_ROUTES)
+  end
+
+  def self.without_reserved_child_paths_regex
+    @without_reserved_child_paths_regex ||= regex_excluding_child_paths(CHILD_ROUTES)
+  end
+
+  # This is used to validate a full path.
+  # It doesn't match paths
+  #   - Starting with one of the top level words
+  #   - Containing one of the child level words in the middle of a path
+  def self.regex_excluding_child_paths(child_routes)
+    reserved_top_level_words = Regexp.union(TOP_LEVEL_ROUTES)
+    not_starting_in_reserved_word = %r{\A/?(?!(#{reserved_top_level_words})(/|\z))}
+
+    reserved_child_level_words = Regexp.union(child_routes)
+    not_containing_reserved_child = %r{(?!\S+/(#{reserved_child_level_words})(/|\z))}
+
+    %r{#{not_starting_in_reserved_word}
+       #{not_containing_reserved_child}
+       #{Gitlab::Regex.full_namespace_regex}}x
+  end
+
+  def self.valid?(path)
+    path =~ Gitlab::Regex.full_namespace_regex && !full_path_reserved?(path)
+  end
+
+  def self.full_path_reserved?(path)
+    path = path.to_s.downcase
+    _project_part, namespace_parts = path.reverse.split('/', 2).map(&:reverse)
+
+    wildcard_reserved?(path) || child_reserved?(namespace_parts)
+  end
+
+  def self.child_reserved?(path)
+    return false unless path
+
+    path !~ without_reserved_child_paths_regex
+  end
+
+  def self.wildcard_reserved?(path)
+    return false unless path
+
+    path !~ without_reserved_wildcard_paths_regex
+  end
+
+  delegate :full_path_reserved?,
+           :child_reserved?,
+           to: :class
+
+  def path_reserved_for_record?(record, value)
+    full_path = record.respond_to?(:full_path) ? record.full_path : value
+
+    # For group paths the entire path cannot contain a reserved child word
+    # The path doesn't contain the last `_project_part` so we need to validate
+    # if the entire path.
+    # Example:
+    #   A *group* with full path `parent/activity` is reserved.
+    #   A *project* with full path `parent/activity` is allowed.
+    if record.is_a? Group
+      child_reserved?(full_path)
+    else
+      full_path_reserved?(full_path)
+    end
+  end
+
+  def validate_each(record, attribute, value)
+    unless value =~ Gitlab::Regex.namespace_regex
+      record.errors.add(attribute, Gitlab::Regex.namespace_regex_message)
+      return
+    end
+
+    if path_reserved_for_record?(record, value)
+      record.errors.add(attribute, "#{value} is a reserved name")
+    end
+  end
+end

app/validators/namespace_validator.rb

-# NamespaceValidator
-#
-# Custom validator for GitLab namespace values.
-#
-# Values are checked for formatting and exclusion from a list of reserved path
-# names.
-class NamespaceValidator < ActiveModel::EachValidator
-  RESERVED = %w[
-    .well-known
-    admin
-    all
-    assets
-    ci
-    dashboard
-    files
-    groups
-    help
-    hooks
-    issues
-    merge_requests
-    new
-    notes
-    profile
-    projects
-    public
-    repository
-    robots.txt
-    s
-    search
-    services
-    snippets
-    teams
-    u
-    unsubscribes
-    users
-  ].freeze
-
-  WILDCARD_ROUTES = %w[tree commits wikis new edit create update logs_tree
-                       preview blob blame raw files create_dir find_file
-                       artifacts graphs refs badges].freeze
-
-  STRICT_RESERVED = (RESERVED + WILDCARD_ROUTES).freeze
-
-  def self.valid?(value)
-    !reserved?(value) && follow_format?(value)
-  end
-
-  def self.reserved?(value, strict: false)
-    if strict
-      STRICT_RESERVED.include?(value)
-    else
-      RESERVED.include?(value)
-    end
-  end
-
-  def self.follow_format?(value)
-    value =~ Gitlab::Regex.namespace_regex
-  end
-
-  delegate :reserved?, :follow_format?, to: :class
-
-  def validate_each(record, attribute, value)
-    unless follow_format?(value)
-      record.errors.add(attribute, Gitlab::Regex.namespace_regex_message)
-    end
-
-    strict = record.is_a?(Group) && record.parent_id
-
-    if reserved?(value, strict: strict)
-      record.errors.add(attribute, "#{value} is a reserved name")
-    end
-  end
-end

app/validators/project_path_validator.rb

-# ProjectPathValidator
-#
-# Custom validator for GitLab project path values.
-#
-# Values are checked for formatting and exclusion from a list of reserved path
-# names.
-class ProjectPathValidator < ActiveModel::EachValidator
-  # All project routes with wildcard argument must be listed here.
-  # Otherwise it can lead to routing issues when route considered as project name.
-  #
-  # Example:
-  #  /group/project/tree/deploy_keys
-  #
-  #  without tree as reserved name routing can match 'group/project' as group name,
-  #  'tree' as project name and 'deploy_keys' as route.
-  #
-  RESERVED = (NamespaceValidator::STRICT_RESERVED -
-              %w[dashboard help ci admin search notes services assets profile public]).freeze
-
-  def self.valid?(value)
-    !reserved?(value)
-  end
-
-  def self.reserved?(value)
-    RESERVED.include?(value)
-  end
-
-  delegate :reserved?, to: :class
-
-  def validate_each(record, attribute, value)
-    if reserved?(value)
-      record.errors.add(attribute, "#{value} is a reserved name")
-    end
-  end
-end

app/views/discussions/_notes.html.haml

 .discussion-notes
   %ul.notes{ data: { discussion_id: discussion.id } }
-    = render partial: "projects/notes/note", collection: discussion.notes, as: :note
+    = render partial: "shared/notes/note", collection: discussion.notes, as: :note
 
   - if current_user
     .discussion-reply-holder

app/views/projects/mirrors/_show.html.haml

@@ -52,8 +52,8 @@
       %h4.prepend-top-0
         Push to a remote repository
       %p.light
-        Set up the remote repository that you want to update with the content
-        of the current repository every hour.
+        Set up the remote repository that you want to update with the content of the current repository
+        every time someone pushes to it.
         = link_to 'Read more', help_page_path('workflow/repository_mirroring', anchor: 'pushing-to-a-remote-repository'), target: '_blank'
     .col-lg-9
       = render "shared/remote_mirror_update_button", remote_mirror: @remote_mirror
@@ -73,13 +73,10 @@
           .prepend-left-20
             = rm_form.label :enabled, "Remote mirror repository", class: "label-light append-bottom-0"
             %p.light.append-bottom-0
-              Automatically update the remote mirror's branches, tags, and commits from this repository every hour.
+              Automatically update the remote mirror's branches, tags, and commits from this repository every time someone pushes to it.
         .form-group.has-feedback
           = rm_form.label :url, "Git repository URL", class: "label-light"
           = rm_form.text_field :url, class: "form-control", placeholder: 'https://username:password@gitlab.company.com/group/project.git'
           = render "projects/mirrors/instructions"
-        .form-group
-          = rm_form.label :sync_time, "Synchronization time", class: "label-light append-bottom-0"
-          = rm_form.select :sync_time, options_for_select(mirror_sync_time_options, @remote_mirror.sync_time), {}, class: 'form-control remote-mirror-sync-time'
         = f.submit 'Save changes', class: 'btn btn-create', name: 'update_remote_mirror'
 %hr

app/views/projects/notes/_actions.html.haml

+- access = note_max_access_for_user(note)
+- if access
+  %span.note-role= access
+
+- if note.resolvable?
+  - can_resolve = can?(current_user, :resolve_note, note)
+  %resolve-btn{ "project-path" => project_path(note.project),
+      "discussion-id" => note.discussion_id(@noteable),
+      ":note-id" => note.id,
+      ":resolved" => note.resolved?,
+      ":can-resolve" => can_resolve,
+      ":author-name" => "'#{j(note.author.name)}'",
+      "author-avatar" => note.author.avatar_url,
+      ":note-truncated" => "'#{j(truncate(note.note, length: 17))}'",
+      ":resolved-by" => "'#{j(note.resolved_by.try(:name))}'",
+      "v-show" => "#{can_resolve || note.resolved?}",
+      "inline-template" => true,
+      "ref" => "note_#{note.id}" }
+
+    %button.note-action-button.line-resolve-btn{ type: "button",
+        class: ("is-disabled" unless can_resolve),
+        ":class" => "{ 'is-active': isResolved }",
+        ":aria-label" => "buttonText",
+        "@click" => "resolve",
+        ":title" => "buttonText",
+        ":ref" => "'button'" }
+
+      = icon('spin spinner', 'v-show' => 'loading', class: 'loading', 'aria-hidden' => 'true', 'aria-label' => 'Loading')
+      %div{ 'v-show' => '!loading' }= render 'shared/icons/icon_status_success.svg'
+
+- if current_user
+  - if note.emoji_awardable?
+    - user_authored = note.user_authored?(current_user)
+    = link_to '#', title: 'Award Emoji', class: "note-action-button note-emoji-button js-add-award js-note-emoji #{'js-user-authored' if user_authored}", data: { position: 'right' } do
+      = icon('spinner spin')
+      %span{ class: 'link-highlight award-control-icon-neutral' }= custom_icon('emoji_slightly_smiling_face')
+      %span{ class: 'link-highlight award-control-icon-positive' }= custom_icon('emoji_smiley')
+      %span{ class: 'link-highlight award-control-icon-super-positive' }= custom_icon('emoji_smile')
+
+  - if note_editable
+    = link_to '#', title: 'Edit comment', class: 'note-action-button js-note-edit' do
+      = icon('pencil', class: 'link-highlight')
+    = link_to namespace_project_note_path(note.project.namespace, note.project, note), title: 'Remove comment', method: :delete, data: { confirm: 'Are you sure you want to remove this comment?' }, remote: true, class: 'note-action-button js-note-delete danger' do
+      = icon('trash-o', class: 'danger-highlight')

app/views/projects/notes/_edit.html.haml

+.original-note-content.hidden{ data: { post_url: namespace_project_note_path(@project.namespace, @project, note), target_id: note.noteable.id, target_type: note.noteable.class.name.underscore } }
+  #{note.note}
+%textarea.hidden.js-task-list-field.original-task-list{ data: {update_url: namespace_project_note_path(@project.namespace, @project, note) } }= note.note

app/views/projects/notes/_notes_with_form.html.haml

 %ul#notes-list.notes.main-notes-list.timeline
-  = render "projects/notes/notes"
+  = render "shared/notes/notes"
 
 = render 'projects/notes/edit_form'
 

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

+- return unless note.author
+- return if note.cross_reference_not_visible_for?(current_user)
+
+- note_editable = note_editable?(note)
+%li.timeline-entry{ id: dom_id(note), class: ["note", "note-row-#{note.id}", ('system-note' if note.system)], data: {author_id: note.author.id, editable: note_editable, note_id: note.id} }
+  .timeline-entry-inner
+    .timeline-icon
+      - if note.system
+        = icon_for_system_note(note)
+      - else
+        %a{ href: user_path(note.author) }
+          = image_tag avatar_icon(note.author), alt: '', class: 'avatar s40'
+    .timeline-content
+      .note-header
+        .note-header-info
+          %a{ href: user_path(note.author) }
+            %span.hidden-xs
+              = sanitize(note.author.name)
+            %span.note-headline-light
+              = note.author.to_reference
+          %span.note-headline-light
+            %span.note-headline-meta
+              - unless note.system
+                commented
+              - if note.system
+                %span.system-note-message
+                  = note.redacted_note_html
+              %a{ 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?
+              = render 'snippets/notes/actions', note: note, note_editable: note_editable
+            - else
+              = render 'projects/notes/actions', note: note, note_editable: note_editable
+      .note-body{ class: note_editable ? 'js-task-list-container' : '' }
+        .note-text.md
+          = preserve do
+            = note.redacted_note_html
+
+        = edited_time_ago_with_tooltip(note, placement: 'bottom', html_class: 'note_edited_ago', include_author: true)
+        - if note_editable
+          - if note.for_personal_snippet?
+            = render 'snippets/notes/edit', note: note
+          - else
+            = render 'projects/notes/edit', note: note
+        .note-awards
+          = render 'award_emoji/awards_block', awardable: note, inline: false
+        - if note.system
+          .system-note-commit-list-toggler
+            Toggle commit list
+            %i.fa.fa-angle-down
+      - if note.attachment.url
+        .note-attachment
+          - if note.attachment.image?
+            = link_to note.attachment.url, target: '_blank' do
+              = image_tag note.attachment.url, class: 'note-image-attach'
+          .attachment
+            = link_to note.attachment.url, target: '_blank' do
+              = icon('paperclip')
+              = note.attachment_identifier
+              = link_to delete_attachment_namespace_project_note_path(note.project.namespace, note.project, note),
+                title: 'Delete this attachment', method: :delete, remote: true, data: { confirm: 'Are you sure you want to remove the attachment?' }, class: 'danger js-note-attachment-delete' do
+                = icon('trash-o', class: 'cred')

app/views/projects/notes/_notes.html.haml → app/views/shared/notes/_notes.html.haml

 - if defined?(@discussions)
   - @discussions.each do |discussion|
     - if discussion.individual_note?
-      = render partial: "projects/notes/note", collection: discussion.notes, as: :note
+      = render partial: "shared/notes/note", collection: discussion.notes, as: :note
     - else
       = render 'discussions/discussion', discussion: discussion
 - else
-  = render partial: "projects/notes/note", collection: @notes, as: :note
+  = render partial: "shared/notes/note", collection: @notes, as: :note

app/views/snippets/notes/_actions.html.haml

+- if current_user
+  - if note.emoji_awardable?
+    - user_authored = note.user_authored?(current_user)
+    = link_to '#', title: 'Award Emoji', class: "note-action-button note-emoji-button js-add-award js-note-emoji #{'js-user-authored' if user_authored}", data: { position: 'right' } do
+      = icon('spinner spin')
+      %span{ class: 'link-highlight award-control-icon-neutral' }= custom_icon('emoji_slightly_smiling_face')
+      %span{ class: 'link-highlight award-control-icon-positive' }= custom_icon('emoji_smiley')
+      %span{ class: 'link-highlight award-control-icon-super-positive' }= custom_icon('emoji_smile')
+  - if note_editable
+    = link_to '#', title: 'Edit comment', class: 'note-action-button js-note-edit' do
+      = icon('pencil', class: 'link-highlight')
+    = link_to snippet_note_path(note.noteable, note), title: 'Remove comment', method: :delete, data: { confirm: 'Are you sure you want to remove this comment?' }, remote: true, class: 'note-action-button js-note-delete danger' do
+      = icon('trash-o', class: 'danger-highlight')

app/views/snippets/notes/_notes.html.haml

+%ul#notes-list.notes.main-notes-list.timeline
+  = render "projects/notes/notes"

app/views/snippets/show.html.haml

@@ -7,3 +7,6 @@
 
 .row-content-block.top-block.content-component-block
   = render 'award_emoji/awards_block', awardable: @snippet, inline: true
+
+%ul#notes-list.notes.main-notes-list.timeline
+  #notes= render 'shared/notes/notes'

app/workers/repository_update_remote_mirror_worker.rb

 class RepositoryUpdateRemoteMirrorWorker
-  UpdateRemoteMirrorError = Class.new(StandardError)
+  UpdateAlreadyInProgressError = Class.new(StandardError)
+  UpdateError = Class.new(StandardError)
 
   include Sidekiq::Worker
   include Gitlab::ShellAdapter
 
-  sidekiq_options queue: :project_mirror, retry: false
+  sidekiq_options queue: :project_mirror, retry: 3, dead: false
 
-  def perform(remote_mirror_id)
-    begin
+  sidekiq_retry_in { |count| 30 * count }
+
+  sidekiq_retries_exhausted do |msg, _|
+    Sidekiq.logger.warn "Failed #{msg['class']} with #{msg['args']}: #{msg['error_message']}"
+  end
+
+  def perform(remote_mirror_id, scheduled_time)
     remote_mirror = RemoteMirror.find(remote_mirror_id)
+    return if remote_mirror.updated_since?(scheduled_time)
+
+    raise UpdateAlreadyInProgressError if remote_mirror.update_in_progress?
+    remote_mirror.update_start
+
     project = remote_mirror.project
     current_user = project.creator
     result = Projects::UpdateRemoteMirrorService.new(project, current_user).execute(remote_mirror)
 
-      if result[:status] == :error
-        remote_mirror.mark_as_failed(result[:message])
-      else
+    raise UpdateError, result[:message] if result[:status] == :error
     remote_mirror.update_finish
-      end
+  rescue UpdateAlreadyInProgressError
+    raise
+  rescue UpdateError => ex
+    remote_mirror.mark_as_failed(Gitlab::UrlSanitizer.sanitize(ex.message))
+    raise
   rescue => ex
-      remote_mirror.mark_as_failed("We're sorry, a temporary error occurred, please try again.")
-
-      raise UpdateRemoteMirrorError, "#{ex.class}: #{Gitlab::UrlSanitizer.sanitize(ex.message)}"
-    end
+    raise UpdateError, "#{ex.class}: #{ex.message}"
   end
 end

app/workers/update_all_remote_mirrors_worker.rb

-class UpdateAllRemoteMirrorsWorker
-  include Sidekiq::Worker
-  include CronjobQueue
-
-  def perform
-    fail_stuck_mirrors!
-
-    remote_mirrors_to_sync.find_each(batch_size: 50).each(&:sync)
-  end
-
-  def fail_stuck_mirrors!
-    RemoteMirror.stuck.find_each(batch_size: 50) do |remote_mirror|
-      remote_mirror.mark_as_failed('The mirror update took too long to complete.')
-    end
-  end
-
-  private
-
-  def remote_mirrors_to_sync
-    RemoteMirror.where("last_successful_update_at + #{Gitlab::Database.minute_interval('sync_time')} <= ? OR sync_time IN (?)", DateTime.now, Gitlab::Mirror.sync_times)
-  end
-end

changelogs/unreleased-ee/stop-using-sidekiq-cron-for-push-mirrors.yml

+---
+title: Stop using sidekiq cron for push mirrors
+merge_request: 1616
+author:

changelogs/unreleased/12910-personal-snippets-notes-show.yml

+---
+title: Display comments for personal snippets
+merge_request:
+author:

changelogs/unreleased/30272-bvl-reject-more-namespaces.yml

+---
+title: Improve validation of namespace & project paths
+merge_request: 10413
+author:

config/initializers/sidekiq.rb

@@ -40,7 +40,7 @@ Sidekiq.configure_server do |config|
   end
   Sidekiq::Cron::Job.load_from_hash! cron_jobs
 
-  Gitlab::Mirror.configure_cron_jobs!
+  Gitlab::Mirror.configure_cron_job!
 
   Gitlab::Geo.configure_cron_jobs!
 

config/routes/snippets.rb

@@ -5,6 +5,14 @@ resources :snippets, concerns: :awardable do
     post :mark_as_spam
     post :preview_markdown
   end
+
+  scope module: :snippets do
+    resources :notes, only: [:index, :create, :destroy, :update], concerns: :awardable, constraints: { id: /\d+/ } do
+      member do
+        delete :delete_attachment
+      end
+    end
+  end
 end
 
 get '/s/:username', to: redirect('/u/%{username}/snippets'),

db/migrate/20170427180205_add_last_update_started_at_column_to_remote_mirrors.rb

+class AddLastUpdateStartedAtColumnToRemoteMirrors < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    add_column :remote_mirrors, :last_update_started_at, :datetime
+  end
+end

db/post_migrate/20170412174900_rename_reserved_dynamic_paths.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class RenameReservedDynamicPaths < ActiveRecord::Migration
+  include Gitlab::Database::RenameReservedPathsMigration::V1
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  DISALLOWED_ROOT_PATHS = %w[
+    -
+    abuse_reports
+    api
+    autocomplete
+    explore
+    health_check
+    import
+    invites
+    jwt
+    koding
+    member
+    notification_settings
+    oauth
+    sent_notifications
+    unicorn_test
+    uploads
+    users
+  ]
+
+  DISALLOWED_WILDCARD_PATHS = %w[
+    environments/folders
+    gitlab-lfs/objects
+    info/lfs/objects
+  ]
+
+  DISSALLOWED_GROUP_PATHS = %w[
+    activity
+    avatar
+    group_members
+    labels
+    milestones
+    subgroups
+    analytics
+    audit_events
+    hooks
+    ldap
+    ldap_group_links
+    notification_setting
+    pipeline_quota
+  ]
+
+  def up
+    rename_root_paths(DISALLOWED_ROOT_PATHS)
+    rename_wildcard_paths(DISALLOWED_WILDCARD_PATHS)
+    rename_child_paths(DISSALLOWED_GROUP_PATHS)
+  end
+
+  def down
+    # nothing to do
+  end
+end

db/post_migrate/20170427111108_remove_sync_time_column_from_remote_mirrors.rb

+class RemoveSyncTimeColumnFromRemoteMirrors < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    remove_concurrent_index :remote_mirrors, [:sync_time] if index_exists? :remote_mirrors, [:sync_time]
+    remove_column :remote_mirrors, :sync_time, :integer
+  end
+
+  def down
+    add_column :remote_mirrors, :sync_time, :integer
+    add_concurrent_index :remote_mirrors, [:sync_time]
+  end
+end

db/schema.rb

@@ -11,7 +11,11 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
+<<<<<<< HEAD
 ActiveRecord::Schema.define(version: 20170426181740) do
+=======
+ActiveRecord::Schema.define(version: 20170427180205) do
+>>>>>>> origin/master
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -1242,12 +1246,11 @@ ActiveRecord::Schema.define(version: 20170426181740) do
     t.string "encrypted_credentials_salt"
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
-    t.integer "sync_time", default: 60, null: false
+    t.datetime "last_update_started_at"
   end
 
   add_index "remote_mirrors", ["last_successful_update_at"], name: "index_remote_mirrors_on_last_successful_update_at", using: :btree
   add_index "remote_mirrors", ["project_id"], name: "index_remote_mirrors_on_project_id", using: :btree
-  add_index "remote_mirrors", ["sync_time"], name: "index_remote_mirrors_on_sync_time", using: :btree
 
   create_table "routes", force: :cascade do |t|
     t.integer "source_id", null: false

doc/development/migration_style_guide.md

@@ -270,3 +270,28 @@ end
 
 When doing so be sure to explicitly set the model's table name so it's not
 derived from the class name or namespace.
+
+### Renaming reserved paths
+
+When a new route for projects is introduced that could conflict with any
+existing records. The path for this records should be renamed, and the
+related data should be moved on disk.
+
+Since we had to do this a few times already, there are now some helpers to help
+with this.
+
+To use this you can include `Gitlab::Database::RenameReservedPathsMigration::V1`
+in your migration. This will provide 3 methods which you can pass one or more
+paths that need to be rejected.
+
+**`rename_root_paths`**: This will rename the path of all _namespaces_ with the
+given name that don't have a `parent_id`.
+
+**`rename_child_paths`**: This will rename the path of all _namespaces_ with the
+given name that have a `parent_id`.
+
+**`rename_wildcard_paths`**: This will rename the path of all _projects_, and all
+_namespaces_ that have a `project_id`.
+
+The `path` column for these rows will be renamed to their previous value followed
+by an integer. For example: `users` would turn into `users0`

doc/development/testing.md

@@ -188,7 +188,8 @@ Please consult the [dedicated "Frontend testing" guide](./fe_guide/testing.md).
 ### General Guidelines
 
 - Use a single, top-level `describe ClassName` block.
-- Use `described_class` instead of repeating the class name being described.
+- Use `described_class` instead of repeating the class name being described
+  (_this is enforced by RuboCop_).
 - Use `.method` to describe class methods and `#method` to describe instance
   methods.
 - Use `context` to test branching logic.
@@ -197,7 +198,7 @@ Please consult the [dedicated "Frontend testing" guide](./fe_guide/testing.md).
 - Don't `describe` symbols (see [Gotchas](gotchas.md#dont-describe-symbols)).
 - Don't assert against the absolute value of a sequence-generated attribute (see [Gotchas](gotchas.md#dont-assert-against-the-absolute-value-of-a-sequence-generated-attribute)).
 - Don't supply the `:each` argument to hooks since it's the default.
-- Prefer `not_to` to `to_not` (_this is enforced by Rubocop_).
+- Prefer `not_to` to `to_not` (_this is enforced by RuboCop_).
 - Try to match the ordering of tests to the ordering within the class.
 - Try to follow the [Four-Phase Test][four-phase-test] pattern, using newlines
   to separate phases.

doc/user/admin_area/settings/continuous_integration.md

@@ -45,6 +45,7 @@ If you have enabled shared Runners for your GitLab instance, you can limit their
 usage by setting a maximum number of pipeline minutes that a group can use on
 shared Runners per month. Set 0 to grant unlimited pipeline minutes.
 While build limits are stored as minutes, the counting is done in seconds.
+Usage resets on the first day of each month.
 
 1. Go to the **Admin area ➔ Settings** (`/admin/application_settings`).
 

doc/workflow/repository_mirroring.md

@@ -71,9 +71,10 @@ repository to push to. Hit **Save changes** for the changes to take effect.
 
 Similarly to the pull mirroring, since the upstream repository functions as a
 mirror to the repository in GitLab, you are advised not to push commits directly
-to the mirrored repository. Instead, any commits should be pushed to GitLab,
-and will end up in the mirrored repository automatically within the configured time,
-or when a [forced update](#forcing-an-update) is initiated.
+to the mirrored repository. Instead, all changes will end up in the mirrored repository
+whenever commits are pushed to GitLab, or when a [forced update](#forcing-an-update) is initiated.
+
+Pushes into GitLab are automatically pushed to the remote mirror 5 minutes after they come in.
 
 In case of a diverged branch, you will see an error indicated at the
 **Mirror repository** settings.

lib/constraints/group_url_constrainer.rb

@@ -2,16 +2,8 @@ class GroupUrlConstrainer
   def matches?(request)
     id = request.params[:id]
 
-    return false unless valid?(id)
+    return false unless DynamicPathValidator.valid?(id)
 
     Group.find_by_full_path(id).present?
   end
-
-  private
-
-  def valid?(id)
-    id.split('/').all? do |namespace|
-      NamespaceValidator.valid?(namespace)
-    end
-  end
 end

lib/constraints/project_url_constrainer.rb

@@ -4,9 +4,7 @@ class ProjectUrlConstrainer
     project_path = request.params[:project_id] || request.params[:id]
     full_path = namespace_path + '/' + project_path
 
-    unless ProjectPathValidator.valid?(project_path)
-      return false
-    end
+    return false unless DynamicPathValidator.valid?(full_path)
 
     Project.find_by_full_path(full_path).present?
   end

lib/gitlab/database/migration_helpers.rb

@@ -498,6 +498,29 @@ module Gitlab
 
         columns(table).find { |column| column.name == name }
       end
+
+      # This will replace the first occurance of a string in a column with
+      # the replacement
+      # On postgresql we can use `regexp_replace` for that.
+      # On mysql we find the location of the pattern, and overwrite it
+      # with the replacement
+      def replace_sql(column, pattern, replacement)
+        quoted_pattern = Arel::Nodes::Quoted.new(pattern.to_s)
+        quoted_replacement = Arel::Nodes::Quoted.new(replacement.to_s)
+
+        if Database.mysql?
+          locate = Arel::Nodes::NamedFunction.
+            new('locate', [quoted_pattern, column])
+          insert_in_place = Arel::Nodes::NamedFunction.
+            new('insert', [column, locate, pattern.size, quoted_replacement])
+
+          Arel::Nodes::SqlLiteral.new(insert_in_place.to_sql)
+        else
+          replace = Arel::Nodes::NamedFunction.
+            new("regexp_replace", [column, quoted_pattern, quoted_replacement])
+          Arel::Nodes::SqlLiteral.new(replace.to_sql)
+        end
+      end
     end
   end
 end

lib/gitlab/database/rename_reserved_paths_migration/v1.rb

+# This module can be included in migrations to make it easier to rename paths
+# of `Namespace` & `Project` models certain paths would become `reserved`.
+#
+# If the way things are stored on the filesystem related to namespaces and
+# projects ever changes. Don't update this module, or anything nested in `V1`,
+# since it needs to keep functioning for all migrations using it using the state
+# that the data is in at the time. Instead, create a `V2` module that implements
+# the new way of reserving paths.
+module Gitlab
+  module Database
+    module RenameReservedPathsMigration
+      module V1
+        def self.included(kls)
+          kls.include(MigrationHelpers)
+        end
+
+        def rename_wildcard_paths(one_or_more_paths)
+          rename_child_paths(one_or_more_paths)
+          paths = Array(one_or_more_paths)
+          RenameProjects.new(paths, self).rename_projects
+        end
+
+        def rename_child_paths(one_or_more_paths)
+          paths = Array(one_or_more_paths)
+          RenameNamespaces.new(paths, self).rename_namespaces(type: :child)
+        end
+
+        def rename_root_paths(paths)
+          paths = Array(paths)
+          RenameNamespaces.new(paths, self).rename_namespaces(type: :top_level)
+        end
+      end
+    end
+  end
+end

lib/gitlab/database/rename_reserved_paths_migration/v1/migration_classes.rb

+module Gitlab
+  module Database
+    module RenameReservedPathsMigration
+      module V1
+        module MigrationClasses
+          module Routable
+            def full_path
+              if route && route.path.present?
+                @full_path ||= route.path
+              else
+                update_route if persisted?
+
+                build_full_path
+              end
+            end
+
+            def build_full_path
+              if parent && path
+                parent.full_path + '/' + path
+              else
+                path
+              end
+            end
+
+            def update_route
+              prepare_route
+              route.save
+            end
+
+            def prepare_route
+              route || build_route(source: self)
+              route.path = build_full_path
+              @full_path = nil
+            end
+          end
+
+          class Namespace < ActiveRecord::Base
+            include MigrationClasses::Routable
+            self.table_name = 'namespaces'
+            belongs_to :parent,
+                       class_name: "#{MigrationClasses.name}::Namespace"
+            has_one :route, as: :source
+            has_many :children,
+                     class_name: "#{MigrationClasses.name}::Namespace",
+                     foreign_key: :parent_id
+
+            # Overridden to have the correct `source_type` for the `route` relation
+            def self.name
+              'Namespace'
+            end
+          end
+
+          class Route < ActiveRecord::Base
+            self.table_name = 'routes'
+            belongs_to :source, polymorphic: true
+          end
+
+          class Project < ActiveRecord::Base
+            include MigrationClasses::Routable
+            has_one :route, as: :source
+            self.table_name = 'projects'
+
+            def repository_storage_path
+              Gitlab.config.repositories.storages[repository_storage]['path']
+            end
+
+            # Overridden to have the correct `source_type` for the `route` relation
+            def self.name
+              'Project'
+            end
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/database/rename_reserved_paths_migration/v1/rename_base.rb

+module Gitlab
+  module Database
+    module RenameReservedPathsMigration
+      module V1
+        class RenameBase
+          attr_reader :paths, :migration
+
+          delegate :update_column_in_batches,
+                   :replace_sql,
+                   to: :migration
+
+          def initialize(paths, migration)
+            @paths = paths
+            @migration = migration
+          end
+
+          def path_patterns
+            @path_patterns ||= paths.map { |path| "%#{path}" }
+          end
+
+          def rename_path_for_routable(routable)
+            old_path = routable.path
+            old_full_path = routable.full_path
+            # Only remove the last occurrence of the path name to get the parent namespace path
+            namespace_path = remove_last_occurrence(old_full_path, old_path)
+            new_path = rename_path(namespace_path, old_path)
+            new_full_path = join_routable_path(namespace_path, new_path)
+
+            # skips callbacks & validations
+            routable.class.where(id: routable).
+              update_all(path: new_path)
+
+            rename_routes(old_full_path, new_full_path)
+
+            [old_full_path, new_full_path]
+          end
+
+          def rename_routes(old_full_path, new_full_path)
+            replace_statement = replace_sql(Route.arel_table[:path],
+                                            old_full_path,
+                                            new_full_path)
+
+            update_column_in_batches(:routes, :path, replace_statement)  do |table, query|
+              query.where(MigrationClasses::Route.arel_table[:path].matches("#{old_full_path}%"))
+            end
+          end
+
+          def rename_path(namespace_path, path_was)
+            counter = 0
+            path = "#{path_was}#{counter}"
+
+            while route_exists?(join_routable_path(namespace_path, path))
+              counter += 1
+              path = "#{path_was}#{counter}"
+            end
+
+            path
+          end
+
+          def remove_last_occurrence(string, pattern)
+            string.reverse.sub(pattern.reverse, "").reverse
+          end
+
+          def join_routable_path(namespace_path, top_level)
+            if namespace_path.present?
+              File.join(namespace_path, top_level)
+            else
+              top_level
+            end
+          end
+
+          def route_exists?(full_path)
+            MigrationClasses::Route.where(Route.arel_table[:path].matches(full_path)).any?
+          end
+
+          def move_pages(old_path, new_path)
+            move_folders(pages_dir, old_path, new_path)
+          end
+
+          def move_uploads(old_path, new_path)
+            return unless file_storage?
+
+            move_folders(uploads_dir, old_path, new_path)
+          end
+
+          def move_folders(directory, old_relative_path, new_relative_path)
+            old_path = File.join(directory, old_relative_path)
+            return unless File.directory?(old_path)
+
+            new_path = File.join(directory, new_relative_path)
+            FileUtils.mv(old_path, new_path)
+          end
+
+          def remove_cached_html_for_projects(project_ids)
+            update_column_in_batches(:projects, :description_html, nil) do |table, query|
+              query.where(table[:id].in(project_ids))
+            end
+
+            update_column_in_batches(:issues, :description_html, nil) do |table, query|
+              query.where(table[:project_id].in(project_ids))
+            end
+
+            update_column_in_batches(:merge_requests, :description_html, nil) do |table, query|
+              query.where(table[:target_project_id].in(project_ids))
+            end
+
+            update_column_in_batches(:notes, :note_html, nil) do |table, query|
+              query.where(table[:project_id].in(project_ids))
+            end
+
+            update_column_in_batches(:milestones, :description_html, nil) do |table, query|
+              query.where(table[:project_id].in(project_ids))
+            end
+          end
+
+          def file_storage?
+            CarrierWave::Uploader::Base.storage == CarrierWave::Storage::File
+          end
+
+          def uploads_dir
+            File.join(CarrierWave.root, "uploads")
+          end
+
+          def pages_dir
+            Settings.pages.path
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces.rb

+module Gitlab
+  module Database
+    module RenameReservedPathsMigration
+      module V1
+        class RenameNamespaces < RenameBase
+          include Gitlab::ShellAdapter
+
+          def rename_namespaces(type:)
+            namespaces_for_paths(type: type).each do |namespace|
+              rename_namespace(namespace)
+            end
+          end
+
+          def namespaces_for_paths(type:)
+            namespaces = case type
+                         when :child
+                           MigrationClasses::Namespace.where.not(parent_id: nil)
+                         when :top_level
+                           MigrationClasses::Namespace.where(parent_id: nil)
+                         end
+            with_paths = MigrationClasses::Route.arel_table[:path].
+                           matches_any(path_patterns)
+            namespaces.joins(:route).where(with_paths)
+          end
+
+          def rename_namespace(namespace)
+            old_full_path, new_full_path = rename_path_for_routable(namespace)
+
+            move_repositories(namespace, old_full_path, new_full_path)
+            move_uploads(old_full_path, new_full_path)
+            move_pages(old_full_path, new_full_path)
+            remove_cached_html_for_projects(projects_for_namespace(namespace).map(&:id))
+          end
+
+          def move_repositories(namespace, old_full_path, new_full_path)
+            repo_paths_for_namespace(namespace).each do |repository_storage_path|
+              # Ensure old directory exists before moving it
+              gitlab_shell.add_namespace(repository_storage_path, old_full_path)
+
+              unless gitlab_shell.mv_namespace(repository_storage_path, old_full_path, new_full_path)
+                message = "Exception moving path #{repository_storage_path} \
+                           from #{old_full_path} to #{new_full_path}"
+                Rails.logger.error message
+              end
+            end
+          end
+
+          def repo_paths_for_namespace(namespace)
+            projects_for_namespace(namespace).distinct.select(:repository_storage).
+              map(&:repository_storage_path)
+          end
+
+          def projects_for_namespace(namespace)
+            namespace_ids = child_ids_for_parent(namespace, ids: [namespace.id])
+            namespace_or_children = MigrationClasses::Project.
+                                      arel_table[:namespace_id].
+                                      in(namespace_ids)
+            MigrationClasses::Project.where(namespace_or_children)
+          end
+
+          def child_ids_for_parent(namespace, ids: [])
+            namespace.children.each do |child|
+              ids << child.id
+              child_ids_for_parent(child, ids: ids) if child.children.any?
+            end
+            ids
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects.rb

+module Gitlab
+  module Database
+    module RenameReservedPathsMigration
+      module V1
+        class RenameProjects < RenameBase
+          include Gitlab::ShellAdapter
+
+          def rename_projects
+            projects_for_paths.each do |project|
+              rename_project(project)
+            end
+
+            remove_cached_html_for_projects(projects_for_paths.map(&:id))
+          end
+
+          def rename_project(project)
+            old_full_path, new_full_path = rename_path_for_routable(project)
+
+            move_repository(project, old_full_path, new_full_path)
+            move_repository(project, "#{old_full_path}.wiki", "#{new_full_path}.wiki")
+            move_uploads(old_full_path, new_full_path)
+            move_pages(old_full_path, new_full_path)
+          end
+
+          def move_repository(project, old_path, new_path)
+            unless gitlab_shell.mv_repository(project.repository_storage_path,
+                                              old_path,
+                                              new_path)
+              Rails.logger.error "Error moving #{old_path} to #{new_path}"
+            end
+          end
+
+          def projects_for_paths
+            return @projects_for_paths if @projects_for_paths
+
+            with_paths = MigrationClasses::Route.arel_table[:path]
+                           .matches_any(path_patterns)
+
+            @projects_for_paths = MigrationClasses::Project.joins(:route).where(with_paths)
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/etag_caching/router.rb

@@ -2,31 +2,39 @@ module Gitlab
   module EtagCaching
     class Router
       Route = Struct.new(:regexp, :name)
-
-      RESERVED_WORDS = NamespaceValidator::WILDCARD_ROUTES.map { |word| "/#{word}/" }.join('|')
+      # We enable an ETag for every request matching the regex.
+      # To match a regex the path needs to match the following:
+      #   - Don't contain a reserved word (expect for the words used in the
+      #     regex itself)
+      #   - Ending in `noteable/issue/<id>/notes` for the `issue_notes` route
+      #   - Ending in `issues/id`/rendered_title` for the `issue_title` route
+      USED_IN_ROUTES = %w[noteable issue notes issues rendered_title
+                          commit pipelines merge_requests new].freeze
+      RESERVED_WORDS = DynamicPathValidator::WILDCARD_ROUTES - USED_IN_ROUTES
+      RESERVED_WORDS_REGEX = Regexp.union(*RESERVED_WORDS)
       ROUTES = [
         Gitlab::EtagCaching::Router::Route.new(
-          %r(^(?!.*(#{RESERVED_WORDS})).*/noteable/issue/\d+/notes\z),
+          %r(^(?!.*(#{RESERVED_WORDS_REGEX})).*/noteable/issue/\d+/notes\z),
           'issue_notes'
         ),
         Gitlab::EtagCaching::Router::Route.new(
-          %r(^(?!.*(#{RESERVED_WORDS})).*/issues/\d+/rendered_title\z),
+          %r(^(?!.*(#{RESERVED_WORDS_REGEX})).*/issues/\d+/rendered_title\z),
           'issue_title'
         ),
         Gitlab::EtagCaching::Router::Route.new(
-          %r(^(?!.*(#{RESERVED_WORDS})).*/commit/\S+/pipelines\.json\z),
+          %r(^(?!.*(#{RESERVED_WORDS_REGEX})).*/commit/\S+/pipelines\.json\z),
           'commit_pipelines'
         ),
         Gitlab::EtagCaching::Router::Route.new(
-          %r(^(?!.*(#{RESERVED_WORDS})).*/merge_requests/new\.json\z),
+          %r(^(?!.*(#{RESERVED_WORDS_REGEX})).*/merge_requests/new\.json\z),
           'new_merge_request_pipelines'
         ),
         Gitlab::EtagCaching::Router::Route.new(
-          %r(^(?!.*(#{RESERVED_WORDS})).*/merge_requests/\d+/pipelines\.json\z),
+          %r(^(?!.*(#{RESERVED_WORDS_REGEX})).*/merge_requests/\d+/pipelines\.json\z),
           'merge_request_pipelines'
         ),
         Gitlab::EtagCaching::Router::Route.new(
-          %r(^(?!.*(#{RESERVED_WORDS})).*/pipelines\.json\z),
+          %r(^(?!.*(#{RESERVED_WORDS_REGEX})).*/pipelines\.json\z),
           'project_pipelines'
         )
       ].freeze

lib/gitlab/git/repository.rb

@@ -45,17 +45,13 @@ module Gitlab
 
       # Default branch in the repository
       def root_ref
-        # NOTE: This feature is intentionally disabled until
-        # https://gitlab.com/gitlab-org/gitaly/issues/179 is resolved
-        # @root_ref ||= Gitlab::GitalyClient.migrate(:root_ref) do |is_enabled|
-        #   if is_enabled
-        #     gitaly_ref_client.default_branch_name
-        #   else
-        @root_ref ||= discover_default_branch
-        #   end
-        # end
-      rescue GRPC::BadStatus => e
-        raise CommandError.new(e)
+        @root_ref ||= gitaly_migrate(:root_ref) do |is_enabled|
+          if is_enabled
+            gitaly_ref_client.default_branch_name
+          else
+            discover_default_branch
+          end
+        end
       end
 
       # Alias to old method for compatibility
@@ -72,17 +68,13 @@ module Gitlab
       # Returns an Array of branch names
       # sorted by name ASC
       def branch_names
-        # Gitlab::GitalyClient.migrate(:branch_names) do |is_enabled|
-        #   NOTE: This feature is intentionally disabled until
-        #   https://gitlab.com/gitlab-org/gitaly/issues/179 is resolved
-        #   if is_enabled
-        #     gitaly_ref_client.branch_names
-        #   else
+        gitaly_migrate(:branch_names) do |is_enabled|
+          if is_enabled
+            gitaly_ref_client.branch_names
+          else
             branches.map(&:name)
-        #   end
-        # end
-      rescue GRPC::BadStatus => e
-        raise CommandError.new(e)
+          end
+        end
       end
 
       # Returns an Array of Branches
@@ -135,17 +127,13 @@ module Gitlab
 
       # Returns an Array of tag names
       def tag_names
-        # Gitlab::GitalyClient.migrate(:tag_names) do |is_enabled|
-        #   NOTE: This feature is intentionally disabled until
-        #   https://gitlab.com/gitlab-org/gitaly/issues/179 is resolved
-        #   if is_enabled
-        #     gitaly_ref_client.tag_names
-        #   else
+        gitaly_migrate(:tag_names) do |is_enabled|
+          if is_enabled
+            gitaly_ref_client.tag_names
+          else
             rugged.tags.map { |t| t.name }
-        #   end
-        # end
-      rescue GRPC::BadStatus => e
-        raise CommandError.new(e)
+          end
+        end
       end
 
       # Returns an Array of Tags
@@ -1273,8 +1261,17 @@ module Gitlab
         @gitaly_ref_client ||= Gitlab::GitalyClient::Ref.new(self)
       end
 
+<<<<<<< HEAD
       def gitaly_commit_client
         @gitaly_commit_client ||= Gitlab::GitalyClient::Commit.new(self)
+=======
+      def gitaly_migrate(method, &block)
+        Gitlab::GitalyClient.migrate(method, &block)
+      rescue GRPC::NotFound => e
+        raise NoRepository.new(e)
+      rescue GRPC::BadStatus => e
+        raise CommandError.new(e)
+>>>>>>> origin/master
       end
 
       # Returns the `Rugged` sorting type constant for a given

lib/gitlab/gitaly_client/ref.rb

@@ -11,7 +11,9 @@ module Gitlab
 
       def default_branch_name
         request = Gitaly::FindDefaultBranchNameRequest.new(repository: @gitaly_repo)
-        stub.find_default_branch_name(request).name.gsub(/^refs\/heads\//, '')
+        branch_name = stub.find_default_branch_name(request).name
+
+        Gitlab::Git.branch_name(branch_name)
       end
 
       def branch_names

lib/gitlab/mirror.rb

@@ -41,22 +41,12 @@ module Gitlab
         sync_times
       end
 
-      def configure_cron_jobs!
+      def configure_cron_job!
         minimum_mirror_sync_time = current_application_settings.minimum_mirror_sync_time rescue FIFTEEN
         sync_time = SYNC_TIME_TO_CRON[minimum_mirror_sync_time]
-        update_all_mirrors_worker_job = Sidekiq::Cron::Job.find("update_all_mirrors_worker")
-        update_all_remote_mirrors_worker_job = Sidekiq::Cron::Job.find("update_all_remote_mirrors_worker")
 
-        if update_all_mirrors_worker_job && update_all_remote_mirrors_worker_job
-          update_all_mirrors_worker_job.destroy
-          update_all_remote_mirrors_worker_job.destroy
-        end
+        Sidekiq::Cron::Job.find("update_all_mirrors_worker")&.destroy
 
-        Sidekiq::Cron::Job.create(
-          name: 'update_all_remote_mirrors_worker',
-          cron: sync_time,
-          class: 'UpdateAllRemoteMirrorsWorker'
-        )
         Sidekiq::Cron::Job.create(
           name: 'update_all_mirrors_worker',
           cron: sync_time,

lib/gitlab/regex.rb

@@ -22,6 +22,10 @@ module Gitlab
       @namespace_regex ||= /\A#{NAMESPACE_REGEX_STR}\z/.freeze
     end
 
+    def full_namespace_regex
+      @full_namespace_regex ||= %r{\A#{FULL_NAMESPACE_REGEX_STR}\z}
+    end
+
     def namespace_route_regex
       @namespace_route_regex ||= /#{NAMESPACE_REGEX_STR}/.freeze
     end

lib/tasks/geo.rake

@@ -100,7 +100,8 @@ namespace :geo do
   end
 
   def set_primary_geo_node(public_key)
-    params = { host: Gitlab.config.gitlab.host,
+    params = { schema: Gitlab.config.gitlab.protocol,
+               host: Gitlab.config.gitlab.host,
                port: Gitlab.config.gitlab.port,
                relative_url_root: Gitlab.config.gitlab.relative_url_root,
                primary: true,

spec/controllers/application_controller_spec.rb

@@ -4,7 +4,7 @@ describe ApplicationController do
   let(:user) { create(:user) }
 
   describe '#check_password_expiration' do
-    let(:controller) { ApplicationController.new }
+    let(:controller) { described_class.new }
 
     it 'redirects if the user is over their password expiry' do
       user.password_expires_at = Time.new(2002)
@@ -34,7 +34,7 @@ describe ApplicationController do
 
   describe "#authenticate_user_from_token!" do
     describe "authenticating a user from a private token" do
-      controller(ApplicationController) do
+      controller(described_class) do
         def index
           render text: "authenticated"
         end
@@ -66,7 +66,7 @@ describe ApplicationController do
     end
 
     describe "authenticating a user from a personal access token" do
-      controller(ApplicationController) do
+      controller(described_class) do
         def index
           render text: 'authenticated'
         end
@@ -115,7 +115,7 @@ describe ApplicationController do
   end
 
   context 'two-factor authentication' do
-    let(:controller) { ApplicationController.new }
+    let(:controller) { described_class.new }
 
     describe '#check_two_factor_requirement' do
       subject { controller.send :check_two_factor_requirement }

spec/controllers/projects/mirrors_controller_spec.rb

@@ -41,24 +41,6 @@ describe Projects::MirrorsController do
         end.to change { RemoteMirror.count }.to(1)
       end
 
-      context 'sync_time update' do
-        it 'allows sync_time update with valid time' do
-          sync_times.each do |sync_time|
-            expect do
-              do_put(@project, remote_mirrors_attributes: { '0' => { 'enabled' => 1, 'url' => 'http://foo.com', 'sync_time' => sync_time } })
-            end.to change { RemoteMirror.where(sync_time: sync_time).count }.by(1)
-          end
-        end
-
-        it 'fails to update sync_time with invalid time' do
-          expect(@project.remote_mirrors.count).to eq(0)
-
-          expect do
-            do_put(@project, remote_mirrors_attributes: { '0' => { 'enabled' => 1, 'url' => 'http://foo.com', 'sync_time' => 1000 } })
-          end.not_to change { @project.remote_mirrors.count }
-        end
-      end
-
       context 'when remote mirror has the same URL' do
         it 'does not allow to create the remote mirror' do
           expect do

spec/controllers/projects/notes_controller_spec.rb

@@ -167,6 +167,47 @@ describe Projects::NotesController do
     end
   end
 
+  describe 'DELETE destroy' do
+    let(:request_params) do
+      {
+        namespace_id: project.namespace,
+        project_id: project,
+        id: note,
+        format: :js
+      }
+    end
+
+    context 'user is the author of a note' do
+      before do
+        sign_in(note.author)
+        project.team << [note.author, :developer]
+      end
+
+      it "returns status 200 for html" do
+        delete :destroy, request_params
+
+        expect(response).to have_http_status(200)
+      end
+
+      it "deletes the note" do
+        expect { delete :destroy, request_params }.to change { Note.count }.from(1).to(0)
+      end
+    end
+
+    context 'user is not the author of a note' do
+      before do
+        sign_in(user)
+        project.team << [user, :developer]
+      end
+
+      it "returns status 404" do
+        delete :destroy, request_params
+
+        expect(response).to have_http_status(404)
+      end
+    end
+  end
+
   describe 'POST toggle_award_emoji' do
     before do
       sign_in(user)

spec/controllers/snippets/notes_controller_spec.rb

+require 'spec_helper'
+
+describe Snippets::NotesController do
+  let(:user) { create(:user) }
+
+  let(:private_snippet)  { create(:personal_snippet, :private) }
+  let(:internal_snippet) { create(:personal_snippet, :internal) }
+  let(:public_snippet)   { create(:personal_snippet, :public) }
+
+  let(:note_on_private)  { create(:note_on_personal_snippet, noteable: private_snippet) }
+  let(:note_on_internal) { create(:note_on_personal_snippet, noteable: internal_snippet) }
+  let(:note_on_public)   { create(:note_on_personal_snippet, noteable: public_snippet) }
+
+  describe 'GET index' do
+    context 'when a snippet is public' do
+      before do
+        note_on_public
+
+        get :index, { snippet_id: public_snippet }
+      end
+
+      it "returns status 200" do
+        expect(response).to have_http_status(200)
+      end
+
+      it "returns not empty array of notes" do
+        expect(JSON.parse(response.body)["notes"].empty?).to be_falsey
+      end
+    end
+
+    context 'when a snippet is internal' do
+      before do
+        note_on_internal
+      end
+
+      context 'when user not logged in' do
+        it "returns status 404" do
+          get :index, { snippet_id: internal_snippet }
+
+          expect(response).to have_http_status(404)
+        end
+      end
+
+      context 'when user logged in' do
+        before do
+          sign_in(user)
+        end
+
+        it "returns status 200" do
+          get :index, { snippet_id: internal_snippet }
+
+          expect(response).to have_http_status(200)
+        end
+      end
+    end
+
+    context 'when a snippet is private' do
+      before do
+        note_on_private
+      end
+
+      context 'when user not logged in' do
+        it "returns status 404" do
+          get :index, { snippet_id: private_snippet }
+
+          expect(response).to have_http_status(404)
+        end
+      end
+
+      context 'when user other than author logged in' do
+        before do
+          sign_in(user)
+        end
+
+        it "returns status 404" do
+          get :index, { snippet_id: private_snippet }
+
+          expect(response).to have_http_status(404)
+        end
+      end
+
+      context 'when author logged in' do
+        before do
+          note_on_private
+
+          sign_in(private_snippet.author)
+        end
+
+        it "returns status 200" do
+          get :index, { snippet_id: private_snippet }
+
+          expect(response).to have_http_status(200)
+        end
+
+        it "returns 1 note" do
+          get :index, { snippet_id: private_snippet }
+
+          expect(JSON.parse(response.body)['notes'].count).to eq(1)
+        end
+      end
+    end
+
+    context 'dont show non visible notes' do
+      before do
+        note_on_public
+
+        sign_in(user)
+
+        expect_any_instance_of(Note).to receive(:cross_reference_not_visible_for?).and_return(true)
+      end
+
+      it "does not return any note" do
+        get :index, { snippet_id: public_snippet }
+
+        expect(JSON.parse(response.body)['notes'].count).to eq(0)
+      end
+    end
+  end
+
+  describe 'DELETE destroy' do
+    let(:request_params) do
+      {
+        snippet_id: public_snippet,
+        id: note_on_public,
+        format: :js
+      }
+    end
+
+    context 'when user is the author of a note' do
+      before do
+        sign_in(note_on_public.author)
+      end
+
+      it "returns status 200" do
+        delete :destroy, request_params
+
+        expect(response).to have_http_status(200)
+      end
+
+      it "deletes the note" do
+        expect{ delete :destroy, request_params }.to change{ Note.count }.from(1).to(0)
+      end
+
+      context 'system note' do
+        before do
+          expect_any_instance_of(Note).to receive(:system?).and_return(true)
+        end
+
+        it "does not delete the note" do
+          expect{ delete :destroy, request_params }.not_to change{ Note.count }
+        end
+      end
+    end
+
+    context 'when user is not the author of a note' do
+      before do
+        sign_in(user)
+
+        note_on_public
+      end
+
+      it "returns status 404" do
+        delete :destroy, request_params
+
+        expect(response).to have_http_status(404)
+      end
+
+      it "does not update the note" do
+        expect{ delete :destroy, request_params }.not_to change{ Note.count }
+      end
+    end
+  end
+
+  describe 'POST toggle_award_emoji' do
+    let(:note) { create(:note_on_personal_snippet, noteable: public_snippet) }
+    before do
+      sign_in(user)
+    end
+
+    subject { post(:toggle_award_emoji, snippet_id: public_snippet, id: note.id, name: "thumbsup") }
+
+    it "toggles the award emoji" do
+      expect { subject }.to change { note.award_emoji.count }.by(1)
+
+      expect(response).to have_http_status(200)
+    end
+
+    it "removes the already awarded emoji when it exists" do
+      note.toggle_award_emoji('thumbsup', user) # create award emoji before
+
+      expect { subject }.to change { AwardEmoji.count }.by(-1)
+
+      expect(response).to have_http_status(200)
+    end
+  end
+end

spec/factories/notes.rb

@@ -5,7 +5,7 @@ include ActionDispatch::TestProcess
 FactoryGirl.define do
   factory :note do
     project factory: :empty_project
-    note "Note"
+    note { generate(:title) }
     author
     on_issue
 

spec/factories/projects.rb

@@ -69,13 +69,12 @@ FactoryGirl.define do
 
     trait :remote_mirror do
       transient do
-        sync_time Gitlab::Mirror::HOURLY
         url "http://foo.com"
         enabled true
       end
 
       after(:create) do |project, evaluator|
-        project.remote_mirrors.create!(url: evaluator.url, enabled: evaluator.enabled, sync_time: evaluator.sync_time)
+        project.remote_mirrors.create!(url: evaluator.url, enabled: evaluator.enabled)
       end
     end
 

spec/features/projects/mirror_spec.rb

@@ -30,7 +30,6 @@ feature 'Project mirror', feature: true do
 
           it 'shows the correct selector options' do
             expect(page).to have_selector('.project-mirror-sync-time > option', count: index + 1)
-            expect(page).to have_selector('.remote-mirror-sync-time > option', count: index + 1)
           end
         end
       end

spec/features/snippets/notes_on_personal_snippets_spec.rb

+require 'spec_helper'
+
+describe 'Comments on personal snippets', feature: true do
+  let!(:user)    { create(:user) }
+  let!(:snippet) { create(:personal_snippet, :public) }
+  let!(:snippet_notes) do
+    [
+      create(:note_on_personal_snippet, noteable: snippet, author: user),
+      create(:note_on_personal_snippet, noteable: snippet)
+    ]
+  end
+  let!(:other_note) { create(:note_on_personal_snippet) }
+
+  before do
+    login_as user
+    visit snippet_path(snippet)
+  end
+
+  subject { page }
+
+  context 'viewing the snippet detail page' do
+    it 'contains notes for a snippet with correct action icons' do
+      expect(page).to have_selector('#notes-list li', count: 2)
+
+      # comment authored by current user
+      page.within("#notes-list li#note_#{snippet_notes[0].id}") do
+        expect(page).to have_content(snippet_notes[0].note)
+        expect(page).to have_selector('.js-note-delete')
+        expect(page).to have_selector('.note-emoji-button')
+      end
+
+      page.within("#notes-list li#note_#{snippet_notes[1].id}") do
+        expect(page).to have_content(snippet_notes[1].note)
+        expect(page).not_to have_selector('.js-note-delete')
+        expect(page).to have_selector('.note-emoji-button')
+      end
+    end
+  end
+end

spec/finders/issues_finder_spec.rb

@@ -16,7 +16,7 @@ describe IssuesFinder do
     set(:label_link) { create(:label_link, label: label, target: issue2) }
     let(:search_user) { user }
     let(:params) { {} }
-    let(:issues) { IssuesFinder.new(search_user, params.reverse_merge(scope: scope, state: 'opened')).execute }
+    let(:issues) { described_class.new(search_user, params.reverse_merge(scope: scope, state: 'opened')).execute }
 
     before(:context) do
       project1.team << [user, :master]
@@ -300,23 +300,23 @@ describe IssuesFinder do
     let!(:confidential_issue) { create(:issue, project: project, confidential: true) }
 
     it 'returns non confidential issues for nil user' do
-      expect(IssuesFinder.send(:not_restricted_by_confidentiality, nil)).to include(public_issue)
+      expect(described_class.send(:not_restricted_by_confidentiality, nil)).to include(public_issue)
     end
 
     it 'returns non confidential issues for user not authorized for the issues projects' do
-      expect(IssuesFinder.send(:not_restricted_by_confidentiality, user)).to include(public_issue)
+      expect(described_class.send(:not_restricted_by_confidentiality, user)).to include(public_issue)
     end
 
     it 'returns all issues for user authorized for the issues projects' do
-      expect(IssuesFinder.send(:not_restricted_by_confidentiality, authorized_user)).to include(public_issue, confidential_issue)
+      expect(described_class.send(:not_restricted_by_confidentiality, authorized_user)).to include(public_issue, confidential_issue)
     end
 
     it 'returns all issues for an admin user' do
-      expect(IssuesFinder.send(:not_restricted_by_confidentiality, admin_user)).to include(public_issue, confidential_issue)
+      expect(described_class.send(:not_restricted_by_confidentiality, admin_user)).to include(public_issue, confidential_issue)
     end
 
     it 'returns all issues for an auditor user' do
-      expect(IssuesFinder.send(:not_restricted_by_confidentiality, auditor_user)).to include(public_issue, confidential_issue)
+      expect(described_class.send(:not_restricted_by_confidentiality, auditor_user)).to include(public_issue, confidential_issue)
     end
   end
 end

spec/finders/merge_requests_finder_spec.rb

@@ -23,32 +23,32 @@ describe MergeRequestsFinder do
   describe "#execute" do
     it 'filters by scope' do
       params = { scope: 'authored', state: 'opened' }
-      merge_requests = MergeRequestsFinder.new(user, params).execute
+      merge_requests = described_class.new(user, params).execute
       expect(merge_requests.size).to eq(3)
     end
 
     it 'filters by project' do
       params = { project_id: project1.id, scope: 'authored', state: 'opened' }
-      merge_requests = MergeRequestsFinder.new(user, params).execute
+      merge_requests = described_class.new(user, params).execute
       expect(merge_requests.size).to eq(1)
     end
 
     it 'ignores sorting by weight' do
       params = { project_id: project1.id, scope: 'authored', state: 'opened', weight: Issue::WEIGHT_ANY }
-      merge_requests = MergeRequestsFinder.new(user, params).execute
+      merge_requests = described_class.new(user, params).execute
       expect(merge_requests.size).to eq(1)
     end
 
     it 'filters by non_archived' do
       params = { non_archived: true }
-      merge_requests = MergeRequestsFinder.new(user, params).execute
+      merge_requests = described_class.new(user, params).execute
       expect(merge_requests.size).to eq(3)
     end
 
     it 'filters by iid' do
       params = { project_id: project1.id, iids: merge_request1.iid }
 
-      merge_requests = MergeRequestsFinder.new(user, params).execute
+      merge_requests = described_class.new(user, params).execute
 
       expect(merge_requests).to contain_exactly(merge_request1)
     end

spec/finders/notes_finder_spec.rb

@@ -110,6 +110,15 @@ describe NotesFinder do
         expect(notes.count).to eq(1)
       end
 
+      it 'finds notes on personal snippets' do
+        note = create(:note_on_personal_snippet)
+        params = { target_type: 'personal_snippet', target_id: note.noteable_id }
+
+        notes = described_class.new(project, user, params).execute
+
+        expect(notes.count).to eq(1)
+      end
+
       it 'raises an exception for an invalid target_type' do
         params[:target_type] = 'invalid'
         expect { described_class.new(project, user, params).execute }.to raise_error('invalid target_type')

spec/finders/snippets_finder_spec.rb

@@ -14,14 +14,14 @@ describe SnippetsFinder do
     let!(:snippet3) { create(:personal_snippet, :public) }
 
     it "returns all private and internal snippets" do
-      snippets = SnippetsFinder.new.execute(user, filter: :all)
+      snippets = described_class.new.execute(user, filter: :all)
 
       expect(snippets).to include(snippet2, snippet3)
       expect(snippets).not_to include(snippet1)
     end
 
     it "returns all public snippets" do
-      snippets = SnippetsFinder.new.execute(nil, filter: :all)
+      snippets = described_class.new.execute(nil, filter: :all)
 
       expect(snippets).to include(snippet3)
       expect(snippets).not_to include(snippet1, snippet2)
@@ -34,7 +34,7 @@ describe SnippetsFinder do
     let!(:snippet3) { create(:personal_snippet, :public) }
 
     it "returns public public snippets" do
-      snippets = SnippetsFinder.new.execute(nil, filter: :public)
+      snippets = described_class.new.execute(nil, filter: :public)
 
       expect(snippets).to include(snippet3)
       expect(snippets).not_to include(snippet1, snippet2)
@@ -47,40 +47,40 @@ describe SnippetsFinder do
     let!(:snippet3) { create(:personal_snippet, :public, author: user) }
 
     it "returns all public and internal snippets" do
-      snippets = SnippetsFinder.new.execute(user1, filter: :by_user, user: user)
+      snippets = described_class.new.execute(user1, filter: :by_user, user: user)
 
       expect(snippets).to include(snippet2, snippet3)
       expect(snippets).not_to include(snippet1)
     end
 
     it "returns internal snippets" do
-      snippets = SnippetsFinder.new.execute(user, filter: :by_user, user: user, scope: "are_internal")
+      snippets = described_class.new.execute(user, filter: :by_user, user: user, scope: "are_internal")
       expect(snippets).to include(snippet2)
       expect(snippets).not_to include(snippet1, snippet3)
     end
 
     it "returns private snippets" do
-      snippets = SnippetsFinder.new.execute(user, filter: :by_user, user: user, scope: "are_private")
+      snippets = described_class.new.execute(user, filter: :by_user, user: user, scope: "are_private")
 
       expect(snippets).to include(snippet1)
       expect(snippets).not_to include(snippet2, snippet3)
     end
 
     it "returns public snippets" do
-      snippets = SnippetsFinder.new.execute(user, filter: :by_user, user: user, scope: "are_public")
+      snippets = described_class.new.execute(user, filter: :by_user, user: user, scope: "are_public")
 
       expect(snippets).to include(snippet3)
       expect(snippets).not_to include(snippet1, snippet2)
     end
 
     it "returns all snippets" do
-      snippets = SnippetsFinder.new.execute(user, filter: :by_user, user: user)
+      snippets = described_class.new.execute(user, filter: :by_user, user: user)
 
       expect(snippets).to include(snippet1, snippet2, snippet3)
     end
 
     it "returns only public snippets if unauthenticated user" do
-      snippets = SnippetsFinder.new.execute(nil, filter: :by_user, user: user)
+      snippets = described_class.new.execute(nil, filter: :by_user, user: user)
 
       expect(snippets).to include(snippet3)
       expect(snippets).not_to include(snippet2, snippet1)
@@ -95,35 +95,35 @@ describe SnippetsFinder do
     end
 
     it "returns public snippets for unauthorized user" do
-      snippets = SnippetsFinder.new.execute(nil, filter: :by_project, project: project1)
+      snippets = described_class.new.execute(nil, filter: :by_project, project: project1)
 
       expect(snippets).to include(@snippet3)
       expect(snippets).not_to include(@snippet1, @snippet2)
     end
 
     it "returns public and internal snippets for non project members" do
-      snippets = SnippetsFinder.new.execute(user, filter: :by_project, project: project1)
+      snippets = described_class.new.execute(user, filter: :by_project, project: project1)
 
       expect(snippets).to include(@snippet2, @snippet3)
       expect(snippets).not_to include(@snippet1)
     end
 
     it "returns public snippets for non project members" do
-      snippets = SnippetsFinder.new.execute(user, filter: :by_project, project: project1, scope: "are_public")
+      snippets = described_class.new.execute(user, filter: :by_project, project: project1, scope: "are_public")
 
       expect(snippets).to include(@snippet3)
       expect(snippets).not_to include(@snippet1, @snippet2)
     end
 
     it "returns internal snippets for non project members" do
-      snippets = SnippetsFinder.new.execute(user, filter: :by_project, project: project1, scope: "are_internal")
+      snippets = described_class.new.execute(user, filter: :by_project, project: project1, scope: "are_internal")
 
       expect(snippets).to include(@snippet2)
       expect(snippets).not_to include(@snippet1, @snippet3)
     end
 
     it "does not return private snippets for non project members" do
-      snippets = SnippetsFinder.new.execute(user, filter: :by_project, project: project1, scope: "are_private")
+      snippets = described_class.new.execute(user, filter: :by_project, project: project1, scope: "are_private")
 
       expect(snippets).not_to include(@snippet1, @snippet2, @snippet3)
     end
@@ -131,7 +131,7 @@ describe SnippetsFinder do
     it "returns all snippets for project members" do
       project1.team << [user, :developer]
 
-      snippets = SnippetsFinder.new.execute(user, filter: :by_project, project: project1)
+      snippets = described_class.new.execute(user, filter: :by_project, project: project1)
 
       expect(snippets).to include(@snippet1, @snippet2, @snippet3)
     end
@@ -139,7 +139,7 @@ describe SnippetsFinder do
     it "returns private snippets for project members" do
       project1.team << [user, :developer]
 
-      snippets = SnippetsFinder.new.execute(user, filter: :by_project, project: project1, scope: "are_private")
+      snippets = described_class.new.execute(user, filter: :by_project, project: project1, scope: "are_private")
 
       expect(snippets).to include(@snippet1)
     end
@@ -147,7 +147,7 @@ describe SnippetsFinder do
     it "returns all snippets for admin users" do
       user = create(:user, :admin)
 
-      snippets = SnippetsFinder.new.execute(user, filter: :by_project, project: project1)
+      snippets = described_class.new.execute(user, filter: :by_project, project: project1)
 
       expect(snippets).to include(@snippet1, @snippet2, @snippet3)
     end
@@ -155,7 +155,7 @@ describe SnippetsFinder do
     it "returns all snippets for auditor users" do
       user = create(:user, :auditor)
 
-      snippets = SnippetsFinder.new.execute(user, filter: :by_project, project: project1)
+      snippets = described_class.new.execute(user, filter: :by_project, project: project1)
 
       expect(snippets).to include(@snippet1, @snippet2, @snippet3)
     end

spec/helpers/award_emoji_helper_spec.rb

+require 'spec_helper'
+
+describe AwardEmojiHelper do
+  describe '.toggle_award_url' do
+    context 'note on personal snippet' do
+      let(:note) { create(:note_on_personal_snippet) }
+
+      it 'returns correct url' do
+        expected_url = "/snippets/#{note.noteable.id}/notes/#{note.id}/toggle_award_emoji"
+
+        expect(helper.toggle_award_url(note)).to eq(expected_url)
+      end
+    end
+
+    context 'note on project item' do
+      let(:note) { create(:note_on_project_snippet) }
+
+      it 'returns correct url' do
+        @project = note.noteable.project
+
+        expected_url = "/#{@project.namespace.path}/#{@project.path}/notes/#{note.id}/toggle_award_emoji"
+
+        expect(helper.toggle_award_url(note)).to eq(expected_url)
+      end
+    end
+
+    context 'personal snippet' do
+      let(:snippet) { create(:personal_snippet) }
+
+      it 'returns correct url' do
+        expected_url = "/snippets/#{snippet.id}/toggle_award_emoji"
+
+        expect(helper.toggle_award_url(snippet)).to eq(expected_url)
+      end
+    end
+
+    context 'merge request' do
+      let(:merge_request) { create(:merge_request) }
+
+      it 'returns correct url' do
+        @project = merge_request.project
+
+        expected_url = "/#{@project.namespace.path}/#{@project.path}/merge_requests/#{merge_request.id}/toggle_award_emoji"
+
+        expect(helper.toggle_award_url(merge_request)).to eq(expected_url)
+      end
+    end
+
+    context 'issue' do
+      let(:issue) { create(:issue) }
+
+      it 'returns correct url' do
+        @project = issue.project
+
+        expected_url = "/#{@project.namespace.path}/#{@project.path}/issues/#{issue.id}/toggle_award_emoji"
+
+        expect(helper.toggle_award_url(issue)).to eq(expected_url)
+      end
+    end
+  end
+end

spec/lib/banzai/renderer_spec.rb

@@ -11,7 +11,7 @@ describe Banzai::Renderer do
   end
 
   describe '#render_field' do
-    let(:renderer) { Banzai::Renderer }
+    let(:renderer) { described_class }
     subject { renderer.render_field(object, :field) }
 
     context 'with a stale cache' do

spec/lib/constraints/group_url_constrainer_spec.rb

@@ -17,6 +17,13 @@ describe GroupUrlConstrainer, lib: true do
       it { expect(subject.matches?(request)).to be_truthy }
     end
 
+    context 'valid request for nested group with reserved top level name' do
+      let!(:nested_group) { create(:group, path: 'api', parent: group) }
+      let!(:request) { build_request('gitlab/api') }
+
+      it { expect(subject.matches?(request)).to be_truthy }
+    end
+
     context 'invalid request' do
       let(:request) { build_request('foo') }
 

spec/lib/ee/gitlab/ldap/person_spec.rb

@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe Gitlab::LDAP::Person do
   it 'includes the EE module' do
-    expect(Gitlab::LDAP::Person).to include(EE::Gitlab::LDAP::Person)
+    expect(described_class).to include(EE::Gitlab::LDAP::Person)
   end
 
   describe '#kerberos_principal' do
@@ -12,7 +12,7 @@ describe Gitlab::LDAP::Person do
       Net::LDAP::Entry.from_single_ldif_string(ldif)
     end
 
-    subject { Gitlab::LDAP::Person.new(entry, 'ldapmain') }
+    subject { described_class.new(entry, 'ldapmain') }
 
     context 'when sAMAccountName is not defined (non-AD LDAP server)' do
       let(:sam_account_name) { nil }
@@ -38,7 +38,7 @@ describe Gitlab::LDAP::Person do
       Net::LDAP::Entry.from_single_ldif_string("dn: cn=foo, dc=bar, dc=com\n#{keys}")
     end
 
-    subject { Gitlab::LDAP::Person.new(entry, 'ldapmain') }
+    subject { described_class.new(entry, 'ldapmain') }
 
     before do
       allow_any_instance_of(Gitlab::LDAP::Config).to receive_messages(sync_ssh_keys: ssh_key_attribute_name)

spec/lib/gitlab/changes_list_spec.rb

@@ -5,7 +5,7 @@ describe Gitlab::ChangesList do
   let(:invalid_changes) { 1 }
 
   context 'when changes is a valid string' do
-    let(:changes_list) { Gitlab::ChangesList.new(valid_changes_string) }
+    let(:changes_list) { described_class.new(valid_changes_string) }
 
     it 'splits elements by newline character' do
       expect(changes_list).to contain_exactly({

spec/lib/gitlab/ci/build/credentials/factory_spec.rb

@@ -3,14 +3,14 @@ require 'spec_helper'
 describe Gitlab::Ci::Build::Credentials::Factory do
   let(:build) { create(:ci_build, name: 'spinach', stage: 'test', stage_idx: 0) }
 
-  subject { Gitlab::Ci::Build::Credentials::Factory.new(build).create! }
+  subject { described_class.new(build).create! }
 
   class TestProvider
     def initialize(build); end
   end
 
   before do
-    allow_any_instance_of(Gitlab::Ci::Build::Credentials::Factory).to receive(:providers).and_return([TestProvider])
+    allow_any_instance_of(described_class).to receive(:providers).and_return([TestProvider])
   end
 
   context 'when provider is valid' do

spec/lib/gitlab/ci/build/credentials/registry_spec.rb

@@ -4,14 +4,14 @@ describe Gitlab::Ci::Build::Credentials::Registry do
   let(:build) { create(:ci_build, name: 'spinach', stage: 'test', stage_idx: 0) }
   let(:registry_url) { 'registry.example.com:5005' }
 
-  subject { Gitlab::Ci::Build::Credentials::Registry.new(build) }
+  subject { described_class.new(build) }
 
   before do
     stub_container_registry_config(host_port: registry_url)
   end
 
   it 'contains valid DockerRegistry credentials' do
-    expect(subject).to be_kind_of(Gitlab::Ci::Build::Credentials::Registry)
+    expect(subject).to be_kind_of(described_class)
 
     expect(subject.username).to eq 'gitlab-ci-token'
     expect(subject.password).to eq build.token
@@ -20,7 +20,7 @@ describe Gitlab::Ci::Build::Credentials::Registry do
   end
 
   describe '.valid?' do
-    subject { Gitlab::Ci::Build::Credentials::Registry.new(build).valid? }
+    subject { described_class.new(build).valid? }
 
     context 'when registry is enabled' do
       before do

spec/lib/gitlab/current_settings_spec.rb

@@ -10,7 +10,7 @@ describe Gitlab::CurrentSettings do
   describe '#current_application_settings' do
     context 'with DB available' do
       before do
-        allow_any_instance_of(Gitlab::CurrentSettings).to receive(:connect_to_db?).and_return(true)
+        allow_any_instance_of(described_class).to receive(:connect_to_db?).and_return(true)
       end
 
       it 'attempts to use cached values first' do
@@ -36,7 +36,7 @@ describe Gitlab::CurrentSettings do
 
     context 'with DB unavailable' do
       before do
-        allow_any_instance_of(Gitlab::CurrentSettings).to receive(:connect_to_db?).and_return(false)
+        allow_any_instance_of(described_class).to receive(:connect_to_db?).and_return(false)
       end
 
       it 'returns an in-memory ApplicationSetting object' do

spec/lib/gitlab/cycle_analytics/base_event_fetcher_spec.rb

@@ -20,7 +20,7 @@ describe Gitlab::CycleAnalytics::BaseEventFetcher do
 
   before do
     allow_any_instance_of(Gitlab::ReferenceExtractor).to receive(:issues).and_return(Issue.all)
-    allow_any_instance_of(Gitlab::CycleAnalytics::BaseEventFetcher).to receive(:serialize) do |event|
+    allow_any_instance_of(described_class).to receive(:serialize) do |event|
       event
     end
 

spec/lib/gitlab/database/migration_helpers_spec.rb

@@ -726,4 +726,37 @@ describe Gitlab::Database::MigrationHelpers, lib: true do
       expect(model.column_for(:users, :kittens)).to be_nil
     end
   end
+
+  describe '#replace_sql' do
+    context 'using postgres' do
+      before do
+        allow(Gitlab::Database).to receive(:mysql?).and_return(false)
+      end
+
+      it 'builds the sql with correct functions' do
+        expect(model.replace_sql(Arel::Table.new(:users)[:first_name], "Alice", "Eve").to_s).
+          to include('regexp_replace')
+      end
+    end
+
+    context 'using mysql' do
+      before do
+        allow(Gitlab::Database).to receive(:mysql?).and_return(true)
+      end
+
+      it 'builds the sql with the correct functions' do
+        expect(model.replace_sql(Arel::Table.new(:users)[:first_name], "Alice", "Eve").to_s).
+          to include('locate', 'insert')
+      end
+    end
+
+    describe 'results' do
+      let!(:user) { create(:user, name: 'Kathy Alice Aliceson') }
+
+      it 'replaces the correct part of the string' do
+        model.update_column_in_batches(:users, :name, model.replace_sql(Arel::Table.new(:users)[:name], 'Alice', 'Eve'))
+        expect(user.reload.name).to eq('Kathy Eve Aliceson')
+      end
+    end
+  end
 end

spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_base_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameBase do
+  let(:migration) { FakeRenameReservedPathMigrationV1.new }
+  let(:subject) { described_class.new(['the-path'], migration) }
+
+  before do
+    allow(migration).to receive(:say)
+  end
+
+  def migration_namespace(namespace)
+    Gitlab::Database::RenameReservedPathsMigration::V1::MigrationClasses::
+      Namespace.find(namespace.id)
+  end
+
+  def migration_project(project)
+    Gitlab::Database::RenameReservedPathsMigration::V1::MigrationClasses::
+      Project.find(project.id)
+  end
+
+  describe "#remove_last_ocurrence" do
+    it "removes only the last occurance of a string" do
+      input = "this/is/a-word-to-replace/namespace/with/a-word-to-replace"
+
+      expect(subject.remove_last_occurrence(input, "a-word-to-replace"))
+        .to eq("this/is/a-word-to-replace/namespace/with/")
+    end
+  end
+
+  describe '#remove_cached_html_for_projects' do
+    let(:project) { create(:empty_project, description_html: 'Project description') }
+
+    it 'removes description_html from projects' do
+      subject.remove_cached_html_for_projects([project.id])
+
+      expect(project.reload.description_html).to be_nil
+    end
+
+    it 'removes issue descriptions' do
+      issue = create(:issue, project: project, description_html: 'Issue description')
+
+      subject.remove_cached_html_for_projects([project.id])
+
+      expect(issue.reload.description_html).to be_nil
+    end
+
+    it 'removes merge request descriptions' do
+      merge_request = create(:merge_request,
+                             source_project: project,
+                             target_project: project,
+                             description_html: 'MergeRequest description')
+
+      subject.remove_cached_html_for_projects([project.id])
+
+      expect(merge_request.reload.description_html).to be_nil
+    end
+
+    it 'removes note html' do
+      note = create(:note,
+                    project: project,
+                    noteable: create(:issue, project: project),
+                    note_html: 'note description')
+
+      subject.remove_cached_html_for_projects([project.id])
+
+      expect(note.reload.note_html).to be_nil
+    end
+
+    it 'removes milestone description' do
+      milestone = create(:milestone,
+                    project: project,
+                    description_html: 'milestone description')
+
+      subject.remove_cached_html_for_projects([project.id])
+
+      expect(milestone.reload.description_html).to be_nil
+    end
+  end
+
+  describe '#rename_path_for_routable' do
+    context 'for namespaces' do
+      let(:namespace) { create(:namespace, path: 'the-path') }
+      it "renames namespaces called the-path" do
+        subject.rename_path_for_routable(migration_namespace(namespace))
+
+        expect(namespace.reload.path).to eq("the-path0")
+      end
+
+      it "renames the route to the namespace" do
+        subject.rename_path_for_routable(migration_namespace(namespace))
+
+        expect(Namespace.find(namespace.id).full_path).to eq("the-path0")
+      end
+
+      it "renames the route for projects of the namespace" do
+        project = create(:project, path: "project-path", namespace: namespace)
+
+        subject.rename_path_for_routable(migration_namespace(namespace))
+
+        expect(project.route.reload.path).to eq("the-path0/project-path")
+      end
+
+      it 'returns the old & the new path' do
+        old_path, new_path = subject.rename_path_for_routable(migration_namespace(namespace))
+
+        expect(old_path).to eq('the-path')
+        expect(new_path).to eq('the-path0')
+      end
+
+      context "the-path namespace -> subgroup -> the-path0 project" do
+        it "updates the route of the project correctly" do
+          subgroup = create(:group, path: "subgroup", parent: namespace)
+          project = create(:project, path: "the-path0", namespace: subgroup)
+
+          subject.rename_path_for_routable(migration_namespace(namespace))
+
+          expect(project.route.reload.path).to eq("the-path0/subgroup/the-path0")
+        end
+      end
+    end
+
+    context 'for projects' do
+      let(:parent) { create(:namespace, path: 'the-parent') }
+      let(:project) { create(:empty_project, path: 'the-path', namespace: parent) }
+
+      it 'renames the project called `the-path`' do
+        subject.rename_path_for_routable(migration_project(project))
+
+        expect(project.reload.path).to eq('the-path0')
+      end
+
+      it 'renames the route for the project' do
+        subject.rename_path_for_routable(project)
+
+        expect(project.reload.route.path).to eq('the-parent/the-path0')
+      end
+
+      it 'returns the old & new path' do
+        old_path, new_path = subject.rename_path_for_routable(migration_project(project))
+
+        expect(old_path).to eq('the-parent/the-path')
+        expect(new_path).to eq('the-parent/the-path0')
+      end
+    end
+  end
+
+  describe '#move_pages' do
+    it 'moves the pages directory' do
+      expect(subject).to receive(:move_folders)
+                           .with(TestEnv.pages_path, 'old-path', 'new-path')
+
+      subject.move_pages('old-path', 'new-path')
+    end
+  end
+
+  describe "#move_uploads" do
+    let(:test_dir) { File.join(Rails.root, 'tmp', 'tests', 'rename_reserved_paths') }
+    let(:uploads_dir) { File.join(test_dir, 'public', 'uploads') }
+
+    it 'moves subdirectories in the uploads folder' do
+      expect(subject).to receive(:uploads_dir).and_return(uploads_dir)
+      expect(subject).to receive(:move_folders).with(uploads_dir, 'old_path', 'new_path')
+
+      subject.move_uploads('old_path', 'new_path')
+    end
+
+    it "doesn't move uploads when they are stored in object storage" do
+      expect(subject).to receive(:file_storage?).and_return(false)
+      expect(subject).not_to receive(:move_folders)
+
+      subject.move_uploads('old_path', 'new_path')
+    end
+  end
+
+  describe '#move_folders' do
+    let(:test_dir) { File.join(Rails.root, 'tmp', 'tests', 'rename_reserved_paths') }
+    let(:uploads_dir) { File.join(test_dir, 'public', 'uploads') }
+
+    before do
+      FileUtils.remove_dir(test_dir) if File.directory?(test_dir)
+      FileUtils.mkdir_p(uploads_dir)
+      allow(subject).to receive(:uploads_dir).and_return(uploads_dir)
+    end
+
+    it 'moves a folder with files' do
+      source = File.join(uploads_dir, 'parent-group', 'sub-group')
+      FileUtils.mkdir_p(source)
+      destination = File.join(uploads_dir, 'parent-group', 'moved-group')
+      FileUtils.touch(File.join(source, 'test.txt'))
+      expected_file = File.join(destination, 'test.txt')
+
+      subject.move_folders(uploads_dir, File.join('parent-group', 'sub-group'), File.join('parent-group', 'moved-group'))
+
+      expect(File.exist?(expected_file)).to be(true)
+    end
+  end
+end

spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameNamespaces do
+  let(:migration) { FakeRenameReservedPathMigrationV1.new }
+  let(:subject) { described_class.new(['the-path'], migration) }
+
+  before do
+    allow(migration).to receive(:say)
+  end
+
+  def migration_namespace(namespace)
+    Gitlab::Database::RenameReservedPathsMigration::V1::MigrationClasses::
+      Namespace.find(namespace.id)
+  end
+
+  describe '#namespaces_for_paths' do
+    context 'nested namespaces' do
+      let(:subject) { described_class.new(['parent/the-Path'], migration) }
+
+      it 'includes the namespace' do
+        parent = create(:namespace, path: 'parent')
+        child = create(:namespace, path: 'the-path', parent: parent)
+
+        found_ids = subject.namespaces_for_paths(type: :child).
+                      map(&:id)
+        expect(found_ids).to contain_exactly(child.id)
+      end
+    end
+
+    context 'for child namespaces' do
+      it 'only returns child namespaces with the correct path' do
+        _root_namespace = create(:namespace, path: 'THE-path')
+        _other_path = create(:namespace,
+                             path: 'other',
+                             parent: create(:namespace))
+        namespace = create(:namespace,
+                           path: 'the-path',
+                           parent: create(:namespace))
+
+        found_ids = subject.namespaces_for_paths(type: :child).
+                      map(&:id)
+        expect(found_ids).to contain_exactly(namespace.id)
+      end
+    end
+
+    context 'for top levelnamespaces' do
+      it 'only returns child namespaces with the correct path' do
+        root_namespace = create(:namespace, path: 'the-path')
+        _other_path = create(:namespace, path: 'other')
+        _child_namespace = create(:namespace,
+                                  path: 'the-path',
+                                  parent: create(:namespace))
+
+        found_ids = subject.namespaces_for_paths(type: :top_level).
+                      map(&:id)
+        expect(found_ids).to contain_exactly(root_namespace.id)
+      end
+    end
+  end
+
+  describe '#move_repositories' do
+    let(:namespace) { create(:group, name: 'hello-group') }
+    it 'moves a project for a namespace' do
+      create(:project, namespace: namespace, path: 'hello-project')
+      expected_path = File.join(TestEnv.repos_path, 'bye-group', 'hello-project.git')
+
+      subject.move_repositories(namespace, 'hello-group', 'bye-group')
+
+      expect(File.directory?(expected_path)).to be(true)
+    end
+
+    it 'moves a namespace in a subdirectory correctly' do
+      child_namespace = create(:group, name: 'sub-group', parent: namespace)
+      create(:project, namespace: child_namespace, path: 'hello-project')
+
+      expected_path = File.join(TestEnv.repos_path, 'hello-group', 'renamed-sub-group', 'hello-project.git')
+
+      subject.move_repositories(child_namespace, 'hello-group/sub-group', 'hello-group/renamed-sub-group')
+
+      expect(File.directory?(expected_path)).to be(true)
+    end
+
+    it 'moves a parent namespace with subdirectories' do
+      child_namespace = create(:group, name: 'sub-group', parent: namespace)
+      create(:project, namespace: child_namespace, path: 'hello-project')
+      expected_path = File.join(TestEnv.repos_path, 'renamed-group', 'sub-group', 'hello-project.git')
+
+      subject.move_repositories(child_namespace, 'hello-group', 'renamed-group')
+
+      expect(File.directory?(expected_path)).to be(true)
+    end
+  end
+
+  describe "#child_ids_for_parent" do
+    it "collects child ids for all levels" do
+      parent = create(:namespace)
+      first_child = create(:namespace, parent: parent)
+      second_child = create(:namespace, parent: parent)
+      third_child = create(:namespace, parent: second_child)
+      all_ids = [parent.id, first_child.id, second_child.id, third_child.id]
+
+      collected_ids = subject.child_ids_for_parent(parent, ids: [parent.id])
+
+      expect(collected_ids).to contain_exactly(*all_ids)
+    end
+  end
+
+  describe "#rename_namespace" do
+    let(:namespace) { create(:namespace, path: 'the-path') }
+
+    it 'renames paths & routes for the namespace' do
+      expect(subject).to receive(:rename_path_for_routable).
+                           with(namespace).
+                           and_call_original
+
+      subject.rename_namespace(namespace)
+
+      expect(namespace.reload.path).to eq('the-path0')
+    end
+
+    it "moves the the repository for a project in the namespace" do
+      create(:project, namespace: namespace, path: "the-path-project")
+      expected_repo = File.join(TestEnv.repos_path, "the-path0", "the-path-project.git")
+
+      subject.rename_namespace(namespace)
+
+      expect(File.directory?(expected_repo)).to be(true)
+    end
+
+    it "moves the uploads for the namespace" do
+      expect(subject).to receive(:move_uploads).with("the-path", "the-path0")
+
+      subject.rename_namespace(namespace)
+    end
+
+    it "moves the pages for the namespace" do
+      expect(subject).to receive(:move_pages).with("the-path", "the-path0")
+
+      subject.rename_namespace(namespace)
+    end
+
+    it 'invalidates the markdown cache of related projects' do
+      project = create(:empty_project, namespace: namespace, path: "the-path-project")
+
+      expect(subject).to receive(:remove_cached_html_for_projects).with([project.id])
+
+      subject.rename_namespace(namespace)
+    end
+  end
+
+  describe '#rename_namespaces' do
+    let!(:top_level_namespace) { create(:namespace, path: 'the-path') }
+    let!(:child_namespace) do
+      create(:namespace, path: 'the-path', parent: create(:namespace))
+    end
+
+    it 'renames top level namespaces the namespace' do
+      expect(subject).to receive(:rename_namespace).
+                           with(migration_namespace(top_level_namespace))
+
+      subject.rename_namespaces(type: :top_level)
+    end
+
+    it 'renames child namespaces' do
+      expect(subject).to receive(:rename_namespace).
+                           with(migration_namespace(child_namespace))
+
+      subject.rename_namespaces(type: :child)
+    end
+  end
+end

spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameProjects do
+  let(:migration) { FakeRenameReservedPathMigrationV1.new }
+  let(:subject) { described_class.new(['the-path'], migration) }
+
+  before do
+    allow(migration).to receive(:say)
+  end
+
+  describe '#projects_for_paths' do
+    it 'searches using nested paths' do
+      namespace = create(:namespace, path: 'hello')
+      project = create(:empty_project, path: 'THE-path', namespace: namespace)
+
+      result_ids = described_class.new(['Hello/the-path'], migration).
+                     projects_for_paths.map(&:id)
+
+      expect(result_ids).to contain_exactly(project.id)
+    end
+
+    it 'includes the correct projects' do
+      project = create(:empty_project, path: 'THE-path')
+      _other_project = create(:empty_project)
+
+      result_ids = subject.projects_for_paths.map(&:id)
+
+      expect(result_ids).to contain_exactly(project.id)
+    end
+  end
+
+  describe '#rename_projects' do
+    let!(:projects) { create_list(:empty_project, 2, path: 'the-path') }
+
+    it 'renames each project' do
+      expect(subject).to receive(:rename_project).twice
+
+      subject.rename_projects
+    end
+
+    it 'invalidates the markdown cache of related projects' do
+      expect(subject).to receive(:remove_cached_html_for_projects).
+                           with(projects.map(&:id))
+
+      subject.rename_projects
+    end
+  end
+
+  describe '#rename_project' do
+    let(:project) do
+      create(:empty_project,
+             path: 'the-path',
+             namespace: create(:namespace, path: 'known-parent' ))
+    end
+
+    it 'renames path & route for the project' do
+      expect(subject).to receive(:rename_path_for_routable).
+                           with(project).
+                           and_call_original
+
+      subject.rename_project(project)
+
+      expect(project.reload.path).to eq('the-path0')
+    end
+
+    it 'moves the wiki & the repo' do
+      expect(subject).to receive(:move_repository).
+                           with(project, 'known-parent/the-path.wiki', 'known-parent/the-path0.wiki')
+      expect(subject).to receive(:move_repository).
+                           with(project, 'known-parent/the-path', 'known-parent/the-path0')
+
+      subject.rename_project(project)
+    end
+
+    it 'moves uploads' do
+      expect(subject).to receive(:move_uploads).
+                           with('known-parent/the-path', 'known-parent/the-path0')
+
+      subject.rename_project(project)
+    end
+
+    it 'moves pages' do
+      expect(subject).to receive(:move_pages).
+                           with('known-parent/the-path', 'known-parent/the-path0')
+
+      subject.rename_project(project)
+    end
+  end
+
+  describe '#move_repository' do
+    let(:known_parent) { create(:namespace, path: 'known-parent') }
+    let(:project) { create(:project, path: 'the-path', namespace: known_parent) }
+
+    it 'moves the repository for a project' do
+      expected_path = File.join(TestEnv.repos_path, 'known-parent', 'new-repo.git')
+
+      subject.move_repository(project, 'known-parent/the-path', 'known-parent/new-repo')
+
+      expect(File.directory?(expected_path)).to be(true)
+    end
+  end
+end

spec/lib/gitlab/database/rename_reserved_paths_migration/v1_spec.rb

+require 'spec_helper'
+
+shared_examples 'renames child namespaces' do |type|
+  it 'renames namespaces' do
+    rename_namespaces = double
+    expect(described_class::RenameNamespaces).
+      to receive(:new).with(['first-path', 'second-path'], subject).
+           and_return(rename_namespaces)
+    expect(rename_namespaces).to receive(:rename_namespaces).
+                                   with(type: :child)
+
+    subject.rename_wildcard_paths(['first-path', 'second-path'])
+  end
+end
+
+describe Gitlab::Database::RenameReservedPathsMigration::V1 do
+  let(:subject) { FakeRenameReservedPathMigrationV1.new }
+
+  before do
+    allow(subject).to receive(:say)
+  end
+
+  describe '#rename_child_paths' do
+    it_behaves_like 'renames child namespaces'
+  end
+
+  describe '#rename_wildcard_paths' do
+    it_behaves_like 'renames child namespaces'
+
+    it 'should rename projects' do
+      rename_projects = double
+      expect(described_class::RenameProjects).
+        to receive(:new).with(['the-path'], subject).
+             and_return(rename_projects)
+
+      expect(rename_projects).to receive(:rename_projects)
+
+      subject.rename_wildcard_paths(['the-path'])
+    end
+  end
+
+  describe '#rename_root_paths' do
+    it 'should rename namespaces' do
+      rename_namespaces = double
+      expect(described_class::RenameNamespaces).
+        to receive(:new).with(['the-path'], subject).
+             and_return(rename_namespaces)
+      expect(rename_namespaces).to receive(:rename_namespaces).
+                           with(type: :top_level)
+
+      subject.rename_root_paths('the-path')
+    end
+  end
+end

spec/lib/gitlab/git/repository_spec.rb

@@ -24,21 +24,26 @@ describe Gitlab::Git::Repository, seed_helper: true do
       end
     end
 
-    # TODO: Uncomment when feature is reenabled
-    # context 'with gitaly enabled' do
-    #   before { stub_gitaly }
-    #
-    #   it 'gets the branch name from GitalyClient' do
-    #     expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:default_branch_name)
-    #     repository.root_ref
-    #   end
-    #
-    #   it 'wraps GRPC exceptions' do
-    #     expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:default_branch_name).
-    #       and_raise(GRPC::Unknown)
-    #     expect { repository.root_ref }.to raise_error(Gitlab::Git::CommandError)
-    #   end
-    # end
+    context 'with gitaly enabled' do
+      before { stub_gitaly }
+
+      it 'gets the branch name from GitalyClient' do
+        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:default_branch_name)
+        repository.root_ref
+      end
+
+      it 'wraps GRPC not found' do
+        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:default_branch_name).
+          and_raise(GRPC::NotFound)
+        expect { repository.root_ref }.to raise_error(Gitlab::Git::Repository::NoRepository)
+      end
+
+      it 'wraps GRPC exceptions' do
+        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:default_branch_name).
+          and_raise(GRPC::Unknown)
+        expect { repository.root_ref }.to raise_error(Gitlab::Git::CommandError)
+      end
+    end
   end
 
   describe "#rugged" do
@@ -113,21 +118,26 @@ describe Gitlab::Git::Repository, seed_helper: true do
     it { is_expected.to include("master") }
     it { is_expected.not_to include("branch-from-space") }
 
-    # TODO: Uncomment when feature is reenabled
-    # context 'with gitaly enabled' do
-    #   before { stub_gitaly }
-    #
-    #   it 'gets the branch names from GitalyClient' do
-    #     expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:branch_names)
-    #     subject
-    #   end
-    #
-    #   it 'wraps GRPC exceptions' do
-    #     expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:branch_names).
-    #       and_raise(GRPC::Unknown)
-    #     expect { subject }.to raise_error(Gitlab::Git::CommandError)
-    #   end
-    # end
+    context 'with gitaly enabled' do
+      before { stub_gitaly }
+
+      it 'gets the branch names from GitalyClient' do
+        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:branch_names)
+        subject
+      end
+
+      it 'wraps GRPC not found' do
+        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:branch_names).
+          and_raise(GRPC::NotFound)
+        expect { subject }.to raise_error(Gitlab::Git::Repository::NoRepository)
+      end
+
+      it 'wraps GRPC other exceptions' do
+        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:branch_names).
+          and_raise(GRPC::Unknown)
+        expect { subject }.to raise_error(Gitlab::Git::CommandError)
+      end
+    end
   end
 
   describe '#tag_names' do
@@ -145,21 +155,26 @@ describe Gitlab::Git::Repository, seed_helper: true do
     it { is_expected.to include("v1.0.0") }
     it { is_expected.not_to include("v5.0.0") }
 
-    # TODO: Uncomment when feature is reenabled
-    # context 'with gitaly enabled' do
-    #   before { stub_gitaly }
-    #
-    #   it 'gets the tag names from GitalyClient' do
-    #     expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:tag_names)
-    #     subject
-    #   end
-    #
-    #   it 'wraps GRPC exceptions' do
-    #     expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:tag_names).
-    #       and_raise(GRPC::Unknown)
-    #     expect { subject }.to raise_error(Gitlab::Git::CommandError)
-    #   end
-    # end
+    context 'with gitaly enabled' do
+      before { stub_gitaly }
+
+      it 'gets the tag names from GitalyClient' do
+        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:tag_names)
+        subject
+      end
+
+      it 'wraps GRPC not found' do
+        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:tag_names).
+          and_raise(GRPC::NotFound)
+        expect { subject }.to raise_error(Gitlab::Git::Repository::NoRepository)
+      end
+
+      it 'wraps GRPC exceptions' do
+        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:tag_names).
+          and_raise(GRPC::Unknown)
+        expect { subject }.to raise_error(Gitlab::Git::CommandError)
+      end
+    end
   end
 
   shared_examples 'archive check' do |extenstion|

spec/lib/gitlab/git/util_spec.rb

@@ -9,7 +9,7 @@ describe Gitlab::Git::Util do
       ["foo\n\n", 2],
     ].each do |string, line_count|
       it "counts #{line_count} lines in #{string.inspect}" do
-        expect(Gitlab::Git::Util.count_lines(string)).to eq(line_count)
+        expect(described_class.count_lines(string)).to eq(line_count)
       end
     end
   end

spec/lib/gitlab/gitaly_client/ref_spec.rb

@@ -3,7 +3,7 @@ require 'spec_helper'
 describe Gitlab::GitalyClient::Ref do
   let(:project) { create(:empty_project) }
   let(:repo_path) { project.repository.path_to_repo }
-  let(:client) { Gitlab::GitalyClient::Ref.new(project.repository) }
+  let(:client) { described_class.new(project.repository) }
 
   before do
     allow(Gitlab.config.gitaly).to receive(:enabled).and_return(true)

spec/lib/gitlab/ldap/person_spec.rb

@@ -20,7 +20,7 @@ describe Gitlab::LDAP::Person do
     it 'uses the configured name attribute and handles values as an array' do
       name = 'John Doe'
       entry['cn'] = [name]
-      person = Gitlab::LDAP::Person.new(entry, 'ldapmain')
+      person = described_class.new(entry, 'ldapmain')
 
       expect(person.name).to eq(name)
     end
@@ -30,7 +30,7 @@ describe Gitlab::LDAP::Person do
     it 'returns the value of mail, if present' do
       mail = 'john@example.com'
       entry['mail'] = mail
-      person = Gitlab::LDAP::Person.new(entry, 'ldapmain')
+      person = described_class.new(entry, 'ldapmain')
 
       expect(person.email).to eq([mail])
     end
@@ -38,7 +38,7 @@ describe Gitlab::LDAP::Person do
     it 'returns the value of userPrincipalName, if mail and email are not present' do
       user_principal_name = 'john.doe@example.com'
       entry['userPrincipalName'] = user_principal_name
-      person = Gitlab::LDAP::Person.new(entry, 'ldapmain')
+      person = described_class.new(entry, 'ldapmain')
 
       expect(person.email).to eq([user_principal_name])
     end

spec/lib/gitlab/metrics_spec.rb

@@ -20,7 +20,7 @@ describe Gitlab::Metrics do
 
       expect(pool).to receive(:with).and_yield(connection)
       expect(connection).to receive(:write_points).with(an_instance_of(Array))
-      expect(Gitlab::Metrics).to receive(:pool).and_return(pool)
+      expect(described_class).to receive(:pool).and_return(pool)
 
       described_class.submit_metrics([{ 'series' => 'kittens', 'tags' => {} }])
     end
@@ -64,7 +64,7 @@ describe Gitlab::Metrics do
   describe '.measure' do
     context 'without a transaction' do
       it 'returns the return value of the block' do
-        val = Gitlab::Metrics.measure(:foo) { 10 }
+        val = described_class.measure(:foo) { 10 }
 
         expect(val).to eq(10)
       end
@@ -74,7 +74,7 @@ describe Gitlab::Metrics do
       let(:transaction) { Gitlab::Metrics::Transaction.new }
 
       before do
-        allow(Gitlab::Metrics).to receive(:current_transaction).
+        allow(described_class).to receive(:current_transaction).
           and_return(transaction)
       end
 
@@ -88,11 +88,11 @@ describe Gitlab::Metrics do
         expect(transaction).to receive(:increment).
           with('foo_call_count', 1)
 
-        Gitlab::Metrics.measure(:foo) { 10 }
+        described_class.measure(:foo) { 10 }
       end
 
       it 'returns the return value of the block' do
-        val = Gitlab::Metrics.measure(:foo) { 10 }
+        val = described_class.measure(:foo) { 10 }
 
         expect(val).to eq(10)
       end
@@ -105,7 +105,7 @@ describe Gitlab::Metrics do
         expect_any_instance_of(Gitlab::Metrics::Transaction).
           not_to receive(:add_tag)
 
-        Gitlab::Metrics.tag_transaction(:foo, 'bar')
+        described_class.tag_transaction(:foo, 'bar')
       end
     end
 
@@ -113,13 +113,13 @@ describe Gitlab::Metrics do
       let(:transaction) { Gitlab::Metrics::Transaction.new }
 
       it 'adds the tag to the transaction' do
-        expect(Gitlab::Metrics).to receive(:current_transaction).
+        expect(described_class).to receive(:current_transaction).
           and_return(transaction)
 
         expect(transaction).to receive(:add_tag).
           with(:foo, 'bar')
 
-        Gitlab::Metrics.tag_transaction(:foo, 'bar')
+        described_class.tag_transaction(:foo, 'bar')
       end
     end
   end
@@ -130,7 +130,7 @@ describe Gitlab::Metrics do
         expect_any_instance_of(Gitlab::Metrics::Transaction).
           not_to receive(:action=)
 
-        Gitlab::Metrics.action = 'foo'
+        described_class.action = 'foo'
       end
     end
 
@@ -138,12 +138,12 @@ describe Gitlab::Metrics do
       it 'sets the action of a transaction' do
         trans = Gitlab::Metrics::Transaction.new
 
-        expect(Gitlab::Metrics).to receive(:current_transaction).
+        expect(described_class).to receive(:current_transaction).
           and_return(trans)
 
         expect(trans).to receive(:action=).with('foo')
 
-        Gitlab::Metrics.action = 'foo'
+        described_class.action = 'foo'
       end
     end
   end
@@ -160,7 +160,7 @@ describe Gitlab::Metrics do
         expect_any_instance_of(Gitlab::Metrics::Transaction).
           not_to receive(:add_event)
 
-        Gitlab::Metrics.add_event(:meow)
+        described_class.add_event(:meow)
       end
     end
 
@@ -170,10 +170,10 @@ describe Gitlab::Metrics do
 
         expect(transaction).to receive(:add_event).with(:meow)
 
-        expect(Gitlab::Metrics).to receive(:current_transaction).
+        expect(described_class).to receive(:current_transaction).
           and_return(transaction)
 
-        Gitlab::Metrics.add_event(:meow)
+        described_class.add_event(:meow)
       end
     end
   end

spec/lib/gitlab/regex_spec.rb

@@ -45,8 +45,8 @@ describe Gitlab::Regex, lib: true do
     it { is_expected.not_to match('foo-') }
   end
 
-  describe 'FULL_NAMESPACE_REGEX_STR' do
-    subject { %r{\A#{Gitlab::Regex::FULL_NAMESPACE_REGEX_STR}\z} }
+  describe '.full_namespace_regex' do
+    subject { described_class.full_namespace_regex }
 
     it { is_expected.to match('gitlab.org') }
     it { is_expected.to match('gitlab.org/gitlab-git') }

spec/lib/gitlab/sidekiq_throttler_spec.rb

@@ -13,14 +13,14 @@ describe Gitlab::SidekiqThrottler do
 
   describe '#execute!' do
     it 'sets limits on the selected queues' do
-      Gitlab::SidekiqThrottler.execute!
+      described_class.execute!
 
       expect(Sidekiq::Queue['build'].limit).to eq 4
       expect(Sidekiq::Queue['project_cache'].limit).to eq 4
     end
 
     it 'does not set limits on other queues' do
-      Gitlab::SidekiqThrottler.execute!
+      described_class.execute!
 
       expect(Sidekiq::Queue['merge'].limit).to be_nil
     end

spec/lib/gitlab/slash_commands/dsl_spec.rb

@@ -3,7 +3,7 @@ require 'spec_helper'
 describe Gitlab::SlashCommands::Dsl do
   before :all do
     DummyClass = Struct.new(:project) do
-      include Gitlab::SlashCommands::Dsl
+      include Gitlab::SlashCommands::Dsl # rubocop:disable RSpec/DescribedClass
 
       desc 'A command with no args'
       command :no_args, :none do

spec/lib/gitlab/template/gitignore_template_spec.rb

@@ -24,7 +24,7 @@ describe Gitlab::Template::GitignoreTemplate do
     it 'returns the Gitignore object of a valid file' do
       ruby = subject.find('Ruby')
 
-      expect(ruby).to be_a Gitlab::Template::GitignoreTemplate
+      expect(ruby).to be_a described_class
       expect(ruby.name).to eq('Ruby')
     end
   end

spec/lib/gitlab/template/gitlab_ci_yml_template_spec.rb

@@ -25,7 +25,7 @@ describe Gitlab::Template::GitlabCiYmlTemplate do
     it 'returns the GitlabCiYml object of a valid file' do
       ruby = subject.find('Ruby')
 
-      expect(ruby).to be_a Gitlab::Template::GitlabCiYmlTemplate
+      expect(ruby).to be_a described_class
       expect(ruby.name).to eq('Ruby')
     end
   end

spec/lib/gitlab/template/issue_template_spec.rb

@@ -37,7 +37,7 @@ describe Gitlab::Template::IssueTemplate do
     it 'returns the issue object of a valid file' do
       ruby = subject.find('bug', project)
 
-      expect(ruby).to be_a Gitlab::Template::IssueTemplate
+      expect(ruby).to be_a described_class
       expect(ruby.name).to eq('bug')
     end
   end

spec/lib/gitlab/template/merge_request_template_spec.rb

@@ -37,7 +37,7 @@ describe Gitlab::Template::MergeRequestTemplate do
     it 'returns the merge request object of a valid file' do
       ruby = subject.find('bug', project)
 
-      expect(ruby).to be_a Gitlab::Template::MergeRequestTemplate
+      expect(ruby).to be_a described_class
       expect(ruby.name).to eq('bug')
     end
   end