Merge branch 'master' into ci-tables-ui-improvementsci-tables-ui-improvements

* master: (196 commits)
  Add quotes to coverage pattern
  Update CHANGELOG.md
  Bump omniauth to 1.4.2
  Bump Hashie to 3.5.5 to eliminate warning noise
  use single backticks for inline code
  Add performance query regression fix for !9088 affecting #27267
  Fix spec
  API: Return 400 for all validation erros in the mebers API
  Adds confirmation for cancel button
  Add newline
  Corrected indentation on the template string
  Adds backoff algo from EE to CE
  We don't need these checks anymore
  Raise error when no content is provided
  Address review
  Update API v3 in line with v4
  Fix new offenses
  Fix spec
  Fix specs
  Rename commit_file, commit_dir and remove_file and update specs
  ...

48 files changed, 331 insertions, 401 deletions

diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 74b358d8c40..dc36c754438 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -5,7 +5,7 @@ class ApplicationSetting < ActiveRecord::Base
   add_authentication_token_field :runners_registration_token
   add_authentication_token_field :health_check_access_token
 
-  CACHE_KEY = 'application_setting.last'
+  CACHE_KEY = 'application_setting.last'.freeze
   DOMAIN_LIST_SEPARATOR = %r{\s*[,;]\s*     # comma or semicolon, optionally surrounded by whitespace
                             |               # or
                             \s              # any whitespace character
@@ -76,6 +76,12 @@ class ApplicationSetting < ActiveRecord::Base
             presence: true,
             numericality: { only_integer: true, greater_than: 0 }
 
+  validates :max_artifacts_size,
+            presence: true,
+            numericality: { only_integer: true, greater_than: 0 }
+
+  validates :default_artifacts_expire_in, presence: true, duration: true
+
   validates :container_registry_token_expire_delay,
             presence: true,
             numericality: { only_integer: true, greater_than: 0 }
@@ -168,6 +174,7 @@ class ApplicationSetting < ActiveRecord::Base
       after_sign_up_text: nil,
       akismet_enabled: false,
       container_registry_token_expire_delay: 5,
+      default_artifacts_expire_in: '30 days',
       default_branch_protection: Settings.gitlab['default_branch_protection'],
       default_project_visibility: Settings.gitlab.default_projects_features['visibility_level'],
       default_projects_limit: Settings.gitlab['default_projects_limit'],
@@ -201,9 +208,9 @@ class ApplicationSetting < ActiveRecord::Base
       sign_in_text: nil,
       signin_enabled: Settings.gitlab['signin_enabled'],
       signup_enabled: Settings.gitlab['signup_enabled'],
+      terminal_max_session_time: 0,
       two_factor_grace_period: 48,
-      user_default_external: false,
-      terminal_max_session_time: 0
+      user_default_external: false
     }
   end
 
@@ -215,6 +222,14 @@ class ApplicationSetting < ActiveRecord::Base
     create(defaults)
   end
 
+  def self.human_attribute_name(attr, _options = {})
+    if attr == :default_artifacts_expire_in
+      'Default artifacts expiration'
+    else
+      super
+    end
+  end
+
   def home_page_url_column_exist
     ActiveRecord::Base.connection.column_exists?(:application_settings, :home_page_url)
   end
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index e018f8e7c4e..77aba91f65c 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -22,8 +22,10 @@ module Ci
     serialize :options
     serialize :yaml_variables, Gitlab::Serializer::Ci::Variables
 
+    delegate :name, to: :project, prefix: true
+
     validates :coverage, numericality: true, allow_blank: true
-    validates_presence_of :ref
+    validates :ref, presence: true
 
     scope :unstarted, ->() { where(runner_id: nil) }
     scope :ignore_failures, ->() { where(allow_failure: false) }
@@ -233,10 +235,6 @@ module Ci
       gl_project_id
     end
 
-    def project_name
-      project.name
-    end
-
     def repo_url
       auth = "gitlab-ci-token:#{ensure_token!}@"
       project.http_url_to_repo.sub(/^https?:\/\//) do |prefix|
@@ -257,7 +255,7 @@ module Ci
       return unless regex
 
       matches = text.scan(Regexp.new(regex)).last
-      matches = matches.last if matches.kind_of?(Array)
+      matches = matches.last if matches.is_a?(Array)
       coverage = matches.gsub(/\d+(\.\d+)?/).first
 
       if coverage.present?
@@ -486,7 +484,7 @@ module Ci
     def artifacts_expire_in=(value)
       self.artifacts_expire_at =
         if value
-          Time.now + ChronicDuration.parse(value)
+          ChronicDuration.parse(value)&.seconds&.from_now
         end
     end
 
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index dc4590a9923..80e11a5b58f 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -14,9 +14,11 @@ module Ci
     has_many :builds, foreign_key: :commit_id
     has_many :trigger_requests, dependent: :destroy, foreign_key: :commit_id
 
-    validates_presence_of :sha, unless: :importing?
-    validates_presence_of :ref, unless: :importing?
-    validates_presence_of :status, unless: :importing?
+    delegate :id, to: :project, prefix: true
+
+    validates :sha, presence: { unless: :importing? }
+    validates :ref, presence: { unless: :importing? }
+    validates :status, presence: { unless: :importing? }
     validate :valid_commit_sha, unless: :importing?
 
     after_create :keep_around_commits, unless: :importing?
@@ -93,8 +95,11 @@ module Ci
         .select("max(#{quoted_table_name}.id)")
         .group(:ref, :sha)
 
-      relation = ref ? where(ref: ref) : self
-      relation.where(id: max_id)
+      if ref
+        where(ref: ref, id: max_id.where(ref: ref))
+      else
+        where(id: max_id)
+      end
     end
 
     def self.latest_status(ref = nil)
@@ -150,10 +155,6 @@ module Ci
       builds.latest.with_artifacts_not_expired.includes(project: [:namespace])
     end
 
-    def project_id
-      project.id
-    end
-
     # For now the only user who participates is the user who triggered
     def participants(_current_user = nil)
       Array(user)
diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb
index 07a086b0aca..4863c34a6a6 100644
--- a/app/models/ci/runner.rb
+++ b/app/models/ci/runner.rb
@@ -4,8 +4,8 @@ module Ci
 
     RUNNER_QUEUE_EXPIRY_TIME = 60.minutes
     LAST_CONTACT_TIME = 1.hour.ago
-    AVAILABLE_SCOPES = %w[specific shared active paused online]
-    FORM_EDITABLE = %i[description tag_list active run_untagged locked]
+    AVAILABLE_SCOPES = %w[specific shared active paused online].freeze
+    FORM_EDITABLE = %i[description tag_list active run_untagged locked].freeze
 
     has_many :builds
     has_many :runner_projects, dependent: :destroy
diff --git a/app/models/ci/runner_project.rb b/app/models/ci/runner_project.rb
index 1f9baeca5b1..234376a7e4c 100644
--- a/app/models/ci/runner_project.rb
+++ b/app/models/ci/runner_project.rb
@@ -5,6 +5,6 @@ module Ci
     belongs_to :runner
     belongs_to :project, foreign_key: :gl_project_id
 
-    validates_uniqueness_of :runner_id, scope: :gl_project_id
+    validates :runner_id, uniqueness: { scope: :gl_project_id }
   end
 end
diff --git a/app/models/ci/trigger.rb b/app/models/ci/trigger.rb
index 62889fe80d8..39a1dd86241 100644
--- a/app/models/ci/trigger.rb
+++ b/app/models/ci/trigger.rb
@@ -7,8 +7,8 @@ module Ci
     belongs_to :project, foreign_key: :gl_project_id
     has_many :trigger_requests, dependent: :destroy
 
-    validates_presence_of :token
-    validates_uniqueness_of :token
+    validates :token, presence: true
+    validates :token, uniqueness: true
 
     before_validation :set_default_values
 
diff --git a/app/models/commit.rb b/app/models/commit.rb
index 46f06733da1..0a18986ef26 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -22,12 +22,12 @@ class Commit
   DIFF_HARD_LIMIT_LINES = 50000
 
   # The SHA can be between 7 and 40 hex characters.
-  COMMIT_SHA_PATTERN = '\h{7,40}'
+  COMMIT_SHA_PATTERN = '\h{7,40}'.freeze
 
   class << self
     def decorate(commits, project)
       commits.map do |commit|
-        if commit.kind_of?(Commit)
+        if commit.is_a?(Commit)
           commit
         else
           self.new(commit, project)
@@ -105,7 +105,7 @@ class Commit
   end
 
   def diff_line_count
-    @diff_line_count ||= Commit::diff_line_count(raw_diffs)
+    @diff_line_count ||= Commit.diff_line_count(raw_diffs)
     @diff_line_count
   end
 
@@ -122,11 +122,12 @@ class Commit
   def full_title
     return @full_title if @full_title
 
-    if safe_message.blank?
-      @full_title = no_commit_message
-    else
-      @full_title = safe_message.split("\n", 2).first
-    end
+    @full_title =
+      if safe_message.blank?
+        no_commit_message
+      else
+        safe_message.split("\n", 2).first
+      end
   end
 
   # Returns the commits description
diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb
index 99a6326309d..fc750a3e5e9 100644
--- a/app/models/commit_status.rb
+++ b/app/models/commit_status.rb
@@ -10,10 +10,11 @@ class CommitStatus < ActiveRecord::Base
   belongs_to :user
 
   delegate :commit, to: :pipeline
+  delegate :sha, :short_sha, to: :pipeline
 
   validates :pipeline, presence: true, unless: :importing?
 
-  validates_presence_of :name
+  validates :name, presence: true
 
   alias_attribute :author, :user
 
@@ -102,8 +103,6 @@ class CommitStatus < ActiveRecord::Base
     end
   end
 
-  delegate :sha, :short_sha, to: :pipeline
-
   def before_sha
     pipeline.before_sha || Gitlab::Git::BLANK_SHA
   end
diff --git a/app/models/concerns/cache_markdown_field.rb b/app/models/concerns/cache_markdown_field.rb
index a600f9c14c5..8ea95beed79 100644
--- a/app/models/concerns/cache_markdown_field.rb
+++ b/app/models/concerns/cache_markdown_field.rb
@@ -11,14 +11,15 @@ module CacheMarkdownField
   # Knows about the relationship between markdown and html field names, and
   # stores the rendering contexts for the latter
   class FieldData
-    extend Forwardable
-
     def initialize
       @data = {}
     end
 
-    def_delegators :@data, :[], :[]=
-    def_delegator :@data, :keys, :markdown_fields
+    delegate :[], :[]=, to: :@data
+
+    def markdown_fields
+      @data.keys
+    end
 
     def html_field(markdown_field)
       "#{markdown_field}_html"
@@ -45,7 +46,7 @@ module CacheMarkdownField
     Project
     Release
     Snippet
-  ]
+  ].freeze
 
   def self.caching_classes
     CACHING_CLASSES.map(&:constantize)
diff --git a/app/models/concerns/case_sensitivity.rb b/app/models/concerns/case_sensitivity.rb
index fe0cea8465f..034e9f40ff0 100644
--- a/app/models/concerns/case_sensitivity.rb
+++ b/app/models/concerns/case_sensitivity.rb
@@ -13,11 +13,12 @@ module CaseSensitivity
       params.each do |key, value|
         column = ActiveRecord::Base.connection.quote_table_name(key)
 
-        if cast_lower
-          condition = "LOWER(#{column}) = LOWER(:value)"
-        else
-          condition = "#{column} = :value"
-        end
+        condition =
+          if cast_lower
+            "LOWER(#{column}) = LOWER(:value)"
+          else
+            "#{column} = :value"
+          end
 
         criteria = criteria.where(condition, value: value)
       end
diff --git a/app/models/concerns/has_status.rb b/app/models/concerns/has_status.rb
index 431c0354969..aea359e70bb 100644
--- a/app/models/concerns/has_status.rb
+++ b/app/models/concerns/has_status.rb
@@ -1,12 +1,12 @@
 module HasStatus
   extend ActiveSupport::Concern
 
-  DEFAULT_STATUS = 'created'
-  AVAILABLE_STATUSES = %w[created pending running success failed canceled skipped]
-  STARTED_STATUSES = %w[running success failed skipped]
-  ACTIVE_STATUSES = %w[pending running]
-  COMPLETED_STATUSES = %w[success failed canceled skipped]
-  ORDERED_STATUSES = %w[failed pending running canceled success skipped]
+  DEFAULT_STATUS = 'created'.freeze
+  AVAILABLE_STATUSES = %w[created pending running success failed canceled skipped].freeze
+  STARTED_STATUSES = %w[running success failed skipped].freeze
+  ACTIVE_STATUSES = %w[pending running].freeze
+  COMPLETED_STATUSES = %w[success failed canceled skipped].freeze
+  ORDERED_STATUSES = %w[failed pending running canceled success skipped].freeze
 
   class_methods do
     def status_sql
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index c9c6bd24d75..37c727b5d9f 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -46,6 +46,17 @@ module Issuable
 
     has_one :metrics
 
+    delegate :name,
+             :email,
+             to: :author,
+             prefix: true
+
+    delegate :name,
+             :email,
+             to: :assignee,
+             allow_nil: true,
+             prefix: true
+
     validates :author, presence: true
     validates :title, presence: true, length: { maximum: 255 }
 
@@ -68,21 +79,10 @@ module Issuable
 
     scope :without_label, -> { joins("LEFT OUTER JOIN label_links ON label_links.target_type = '#{name}' AND label_links.target_id = #{table_name}.id").where(label_links: { id: nil }) }
     scope :join_project, -> { joins(:project) }
-    scope :inc_notes_with_associations, -> { includes(notes: [ :project, :author, :award_emoji ]) }
+    scope :inc_notes_with_associations, -> { includes(notes: [:project, :author, :award_emoji]) }
     scope :references_project, -> { references(:project) }
     scope :non_archived, -> { join_project.where(projects: { archived: false }) }
 
-    delegate :name,
-             :email,
-             to: :author,
-             prefix: true
-
-    delegate :name,
-             :email,
-             to: :assignee,
-             allow_nil: true,
-             prefix: true
-
     attr_mentionable :title, pipeline: :single_line
     attr_mentionable :description
 
@@ -182,7 +182,7 @@ module Issuable
     def grouping_columns(sort)
       grouping_columns = [arel_table[:id]]
 
-      if ["milestone_due_desc", "milestone_due_asc"].include?(sort)
+      if %w(milestone_due_desc milestone_due_asc).include?(sort)
         milestone_table = Milestone.arel_table
         grouping_columns << milestone_table[:id]
         grouping_columns << milestone_table[:due_date]
@@ -235,7 +235,7 @@ module Issuable
       # DEPRECATED
       repository: project.hook_attrs.slice(:name, :url, :description, :homepage)
     }
-    hook_data.merge!(assignee: assignee.hook_attrs) if assignee
+    hook_data[:assignee] = assignee.hook_attrs if assignee
 
     hook_data
   end
diff --git a/app/models/concerns/reactive_service.rb b/app/models/concerns/reactive_service.rb
index e1f868a299b..713246039c1 100644
--- a/app/models/concerns/reactive_service.rb
+++ b/app/models/concerns/reactive_service.rb
@@ -5,6 +5,6 @@ module ReactiveService
     include ReactiveCaching
 
     # Default cache key: class name + project_id
-    self.reactive_cache_key = ->(service) { [ service.class.model_name.singular, service.project_id ] }
+    self.reactive_cache_key = ->(service) { [service.class.model_name.singular, service.project_id] }
   end
 end
diff --git a/app/models/concerns/sortable.rb b/app/models/concerns/sortable.rb
index 7edb0acd56c..b9a2d812edd 100644
--- a/app/models/concerns/sortable.rb
+++ b/app/models/concerns/sortable.rb
@@ -46,11 +46,12 @@ module Sortable
         where("label_links.target_id = #{target_column}").
         reorder(nil)
 
-      if target_type_column
-        query = query.where("label_links.target_type = #{target_type_column}")
-      else
-        query = query.where(label_links: { target_type: target_type })
-      end
+      query =
+        if target_type_column
+          query.where("label_links.target_type = #{target_type_column}")
+        else
+          query.where(label_links: { target_type: target_type })
+        end
 
       query = query.where.not(title: excluded_labels) if excluded_labels.present?
 
diff --git a/app/models/concerns/uniquify.rb b/app/models/concerns/uniquify.rb
new file mode 100644
index 00000000000..a7fe5951b6e
--- /dev/null
+++ b/app/models/concerns/uniquify.rb
@@ -0,0 +1,30 @@
+class Uniquify
+  # Return a version of the given 'base' string that is unique
+  # by appending a counter to it. Uniqueness is determined by
+  # repeated calls to the passed block.
+  #
+  # If `base` is a function/proc, we expect that calling it with a
+  # candidate counter returns a string to test/return.
+  def string(base)
+    @base = base
+    @counter = nil
+
+    increment_counter! while yield(base_string)
+    base_string
+  end
+
+  private
+
+  def base_string
+    if @base.respond_to?(:call)
+      @base.call(@counter)
+    else
+      "#{@base}#{@counter}"
+    end
+  end
+
+  def increment_counter!
+    @counter ||= 0
+    @counter += 1
+  end
+end
diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb
index 559b3075905..895a91139c9 100644
--- a/app/models/diff_note.rb
+++ b/app/models/diff_note.rb
@@ -8,7 +8,7 @@ class DiffNote < Note
   validates :position, presence: true
   validates :diff_line, presence: true
   validates :line_code, presence: true, line_code: true
-  validates :noteable_type, inclusion: { in: ['Commit', 'MergeRequest'] }
+  validates :noteable_type, inclusion: { in: %w(Commit MergeRequest) }
   validates :resolved_by, presence: true, if: :resolved?
   validate :positions_complete
   validate :verify_supported
diff --git a/app/models/event.rb b/app/models/event.rb
index e5027df3f8a..d7ca8e3c599 100644
--- a/app/models/event.rb
+++ b/app/models/event.rb
@@ -36,7 +36,7 @@ class Event < ActiveRecord::Base
   scope :code_push, -> { where(action: PUSHED) }
 
   scope :in_projects, ->(projects) do
-    where(project_id: projects).recent
+    where(project_id: projects.pluck(:id)).recent
   end
 
   scope :with_associations, -> { includes(:author, :project, project: :namespace).preload(:target) }
@@ -47,7 +47,7 @@ class Event < ActiveRecord::Base
     def contributions
       where("action = ? OR (target_type IN (?) AND action IN (?)) OR (target_type = ? AND action = ?)",
             Event::PUSHED,
-            ["MergeRequest", "Issue"], [Event::CREATED, Event::CLOSED, Event::MERGED],
+            %w(MergeRequest Issue), [Event::CREATED, Event::CLOSED, Event::MERGED],
             "Note", Event::COMMENTED)
     end
 
diff --git a/app/models/external_issue.rb b/app/models/external_issue.rb
index 26712c19b5a..b973bbcd8da 100644
--- a/app/models/external_issue.rb
+++ b/app/models/external_issue.rb
@@ -43,7 +43,7 @@ class ExternalIssue
   end
 
   def reference_link_text(from_project = nil)
-    return "##{id}" if /^\d+$/.match(id)
+    return "##{id}" if id =~ /^\d+$/
 
     id
   end
diff --git a/app/models/issue.rb b/app/models/issue.rb
index d8826b65fcc..de90f19f854 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -15,8 +15,6 @@ class Issue < ActiveRecord::Base
   DueThisWeek   = DueDateStruct.new('Due This Week', 'week').freeze
   DueThisMonth  = DueDateStruct.new('Due This Month', 'month').freeze
 
-  ActsAsTaggableOn.strict_case_match = true
-
   belongs_to :project
   belongs_to :moved_to, class_name: 'Issue'
 
diff --git a/app/models/label.rb b/app/models/label.rb
index 5b6b9a7a736..f68a8c9cff2 100644
--- a/app/models/label.rb
+++ b/app/models/label.rb
@@ -11,7 +11,7 @@ class Label < ActiveRecord::Base
 
   cache_markdown_field :description, pipeline: :single_line
 
-  DEFAULT_COLOR = '#428BCA'
+  DEFAULT_COLOR = '#428BCA'.freeze
 
   default_value_for :color, DEFAULT_COLOR
 
diff --git a/app/models/member.rb b/app/models/member.rb
index d07f270b757..0545bd4eedf 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -10,6 +10,8 @@ class Member < ActiveRecord::Base
   belongs_to :user
   belongs_to :source, polymorphic: true
 
+  delegate :name, :username, :email, to: :user, prefix: true
+
   validates :user, presence: true, unless: :invite?
   validates :source, presence: true
   validates :user_id, uniqueness: { scope: [:source_type, :source_id],
@@ -73,8 +75,6 @@ class Member < ActiveRecord::Base
   after_destroy :post_destroy_hook, unless: :pending?
   after_commit :refresh_member_authorized_projects
 
-  delegate :name, :username, :email, to: :user, prefix: true
-
   default_value_for :notification_level, NotificationSetting.levels[:global]
 
   class << self
diff --git a/app/models/members/group_member.rb b/app/models/members/group_member.rb
index 204f34f0269..446f9f8f8a7 100644
--- a/app/models/members/group_member.rb
+++ b/app/models/members/group_member.rb
@@ -1,11 +1,11 @@
 class GroupMember < Member
-  SOURCE_TYPE = 'Namespace'
+  SOURCE_TYPE = 'Namespace'.freeze
 
   belongs_to :group, foreign_key: 'source_id'
 
   # Make sure group member points only to group as it source
   default_value_for :source_type, SOURCE_TYPE
-  validates_format_of :source_type, with: /\ANamespace\z/
+  validates :source_type, format: { with: /\ANamespace\z/ }
   default_scope { where(source_type: SOURCE_TYPE) }
 
   def self.access_level_roles
diff --git a/app/models/members/project_member.rb b/app/models/members/project_member.rb
index 008fff0857c..912820b51ac 100644
--- a/app/models/members/project_member.rb
+++ b/app/models/members/project_member.rb
@@ -1,5 +1,5 @@
 class ProjectMember < Member
-  SOURCE_TYPE = 'Project'
+  SOURCE_TYPE = 'Project'.freeze
 
   include Gitlab::ShellAdapter
 
@@ -7,7 +7,7 @@ class ProjectMember < Member
 
   # Make sure project member points only to project as it source
   default_value_for :source_type, SOURCE_TYPE
-  validates_format_of :source_type, with: /\AProject\z/
+  validates :source_type, format: { with: /\AProject\z/ }
   validates :access_level, inclusion: { in: Gitlab::Access.values }
   default_scope { where(source_type: SOURCE_TYPE) }
 
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 204d2b153ad..7eb875f1ef5 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -203,7 +203,11 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def diff_size
-    opts = diff_options || {}
+    # The `#diffs` method ends up at an instance of a class inheriting from
+    # `Gitlab::Diff::FileCollection::Base`, so use those options as defaults
+    # here too, to get the same diff size without performing highlighting.
+    #
+    opts = Gitlab::Diff::FileCollection::Base.default_options.merge(diff_options || {})
 
     raw_diffs(opts).size
   end
@@ -527,7 +531,7 @@ class MergeRequest < ActiveRecord::Base
     }
 
     if diff_head_commit
-      attrs.merge!(last_commit: diff_head_commit.hook_attrs)
+      attrs[:last_commit] = diff_head_commit.hook_attrs
     end
 
     attributes.merge!(attrs)
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index 70bad2a4396..baee00b8fcd 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -7,7 +7,7 @@ class MergeRequestDiff < ActiveRecord::Base
   COMMITS_SAFE_SIZE = 100
 
   # Valid types of serialized diffs allowed by Gitlab::Git::Diff
-  VALID_CLASSES = [Hash, Rugged::Patch, Rugged::Diff::Delta]
+  VALID_CLASSES = [Hash, Rugged::Patch, Rugged::Diff::Delta].freeze
 
   belongs_to :merge_request
 
diff --git a/app/models/namespace.rb b/app/models/namespace.rb
index bd0336c984a..3137dd32f93 100644
--- a/app/models/namespace.rb
+++ b/app/models/namespace.rb
@@ -98,14 +98,8 @@ class Namespace < ActiveRecord::Base
       # Work around that by setting their username to "blank", followed by a counter.
       path = "blank" if path.blank?
 
-      counter = 0
-      base = path
-      while Namespace.find_by_path_or_name(path)
-        counter += 1
-        path = "#{base}#{counter}"
-      end
-
-      path
+      uniquify = Uniquify.new
+      uniquify.string(path) { |s| Namespace.find_by_path_or_name(s) }
     end
   end
 
diff --git a/app/models/network/graph.rb b/app/models/network/graph.rb
index b524ca50ee8..0bbc9451ffd 100644
--- a/app/models/network/graph.rb
+++ b/app/models/network/graph.rb
@@ -188,11 +188,12 @@ module Network
       end
 
       # and mark it as reserved
-      if parent_time.nil?
-        min_time = leaves.first.time
-      else
-        min_time = parent_time + 1
-      end
+      min_time =
+        if parent_time.nil?
+          leaves.first.time
+        else
+          parent_time + 1
+        end
 
       max_time = leaves.last.time
       leaves.last.parents(@map).each do |parent|
diff --git a/app/models/note.rb b/app/models/note.rb
index 029fe667a45..d6d5396afa5 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -72,7 +72,7 @@ class Note < ActiveRecord::Base
   scope :inc_author, ->{ includes(:author) }
   scope :inc_relations_for_view, ->{ includes(:project, :author, :updated_by, :resolved_by, :award_emoji) }
 
-  scope :diff_notes, ->{ where(type: ['LegacyDiffNote', 'DiffNote']) }
+  scope :diff_notes, ->{ where(type: %w(LegacyDiffNote DiffNote)) }
   scope :non_diff_notes, ->{ where(type: ['Note', nil]) }
 
   scope :with_associations, -> do
diff --git a/app/models/notification_setting.rb b/app/models/notification_setting.rb
index 58f6214bea7..52577bd52ea 100644
--- a/app/models/notification_setting.rb
+++ b/app/models/notification_setting.rb
@@ -35,11 +35,11 @@ class NotificationSetting < ActiveRecord::Base
     :merge_merge_request,
     :failed_pipeline,
     :success_pipeline
-  ]
+  ].freeze
 
   EXCLUDED_WATCHER_EVENTS = [
     :success_pipeline
-  ]
+  ].freeze
 
   store :events, accessors: EMAIL_EVENTS, coder: JSON
 
diff --git a/app/models/pages_domain.rb b/app/models/pages_domain.rb
index 0b9ebf1ffe2..f2f2fc1e32a 100644
--- a/app/models/pages_domain.rb
+++ b/app/models/pages_domain.rb
@@ -2,7 +2,7 @@ class PagesDomain < ActiveRecord::Base
   belongs_to :project
 
   validates :domain, hostname: true
-  validates_uniqueness_of :domain, case_sensitive: false
+  validates :domain, uniqueness: { case_sensitive: false }
   validates :certificate, certificate: true, allow_nil: true, allow_blank: true
   validates :key, certificate_key: true, allow_nil: true, allow_blank: true
 
diff --git a/app/models/project.rb b/app/models/project.rb
index 411299eef63..e06fc30dc8a 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -22,7 +22,7 @@ class Project < ActiveRecord::Base
   class BoardLimitExceeded < StandardError; end
 
   NUMBER_OF_PERMITTED_BOARDS = 1
-  UNKNOWN_IMPORT_URL = 'http://unknown.git'
+  UNKNOWN_IMPORT_URL = 'http://unknown.git'.freeze
 
   cache_markdown_field :description, pipeline: :description
 
@@ -70,8 +70,7 @@ class Project < ActiveRecord::Base
 
   after_validation :check_pending_delete
 
-  ActsAsTaggableOn.strict_case_match = true
-  acts_as_taggable_on :tags
+  acts_as_taggable
 
   attr_accessor :new_default_branch
   attr_accessor :old_path_with_namespace
@@ -172,9 +171,11 @@ class Project < ActiveRecord::Base
   accepts_nested_attributes_for :project_feature
 
   delegate :name, to: :owner, allow_nil: true, prefix: true
+  delegate :count, to: :forks, prefix: true
   delegate :members, to: :team, prefix: true
   delegate :add_user, to: :team
   delegate :add_guest, :add_reporter, :add_developer, :add_master, to: :team
+  delegate :empty_repo?, to: :repository
 
   # Validations
   validates :creator, presence: true, on: :create
@@ -191,8 +192,8 @@ class Project < ActiveRecord::Base
     format: { with: Gitlab::Regex.project_path_regex,
               message: Gitlab::Regex.project_path_regex_message }
   validates :namespace, presence: true
-  validates_uniqueness_of :name, scope: :namespace_id
-  validates_uniqueness_of :path, scope: :namespace_id
+  validates :name, uniqueness: { scope: :namespace_id }
+  validates :path, uniqueness: { scope: :namespace_id }
   validates :import_url, addressable_url: true, if: :external_import?
   validates :star_count, numericality: { greater_than_or_equal_to: 0 }
   validate :check_limit, on: :create
@@ -358,7 +359,7 @@ class Project < ActiveRecord::Base
     end
 
     def reference_pattern
-      name_pattern = Gitlab::Regex::NAMESPACE_REGEX_STR
+      name_pattern = Gitlab::Regex::FULL_NAMESPACE_REGEX_STR
 
       %r{
         ((?<namespace>#{name_pattern})\/)?
@@ -453,13 +454,14 @@ class Project < ActiveRecord::Base
   end
 
   def add_import_job
-    if forked?
-      job_id = RepositoryForkWorker.perform_async(id, forked_from_project.repository_storage_path,
-                                                  forked_from_project.path_with_namespace,
-                                                  self.namespace.full_path)
-    else
-      job_id = RepositoryImportWorker.perform_async(self.id)
-    end
+    job_id =
+      if forked?
+        RepositoryForkWorker.perform_async(id, forked_from_project.repository_storage_path,
+          forked_from_project.path_with_namespace,
+          self.namespace.full_path)
+      else
+        RepositoryImportWorker.perform_async(self.id)
+      end
 
     if job_id
       Rails.logger.info "Import job started for #{path_with_namespace} with job ID #{job_id}"
@@ -837,10 +839,6 @@ class Project < ActiveRecord::Base
     false
   end
 
-  def empty_repo?
-    repository.empty_repo?
-  end
-
   def repo
     repository.raw
   end
@@ -849,10 +847,6 @@ class Project < ActiveRecord::Base
     gitlab_shell.url_to_repo(path_with_namespace)
   end
 
-  def namespace_dir
-    namespace.try(:path) || ''
-  end
-
   def repo_exists?
     @repo_exists ||= repository.exists?
   rescue
@@ -875,8 +869,14 @@ class Project < ActiveRecord::Base
     url_to_repo
   end
 
-  def http_url_to_repo
-    "#{web_url}.git"
+  def http_url_to_repo(user = nil)
+    url = web_url
+
+    if user
+      url.sub!(%r{\Ahttps?://}) { |protocol| "#{protocol}#{user.username}@" }
+    end
+
+    "#{url}.git"
   end
 
   # Check if current branch name is marked as protected in the system
@@ -901,8 +901,8 @@ class Project < ActiveRecord::Base
 
   def rename_repo
     path_was = previous_changes['path'].first
-    old_path_with_namespace = File.join(namespace_dir, path_was)
-    new_path_with_namespace = File.join(namespace_dir, path)
+    old_path_with_namespace = File.join(namespace.full_path, path_was)
+    new_path_with_namespace = File.join(namespace.full_path, path)
 
     Rails.logger.error "Attempting to rename #{old_path_with_namespace} -> #{new_path_with_namespace}"
 
@@ -1028,10 +1028,6 @@ class Project < ActiveRecord::Base
     forked? && project == forked_from_project
   end
 
-  def forks_count
-    forks.count
-  end
-
   def origin_merge_requests
     merge_requests.where(source_project_id: self.id)
   end
diff --git a/app/models/project_feature.rb b/app/models/project_feature.rb
index 03194fc2141..e3ef4919b28 100644
--- a/app/models/project_feature.rb
+++ b/app/models/project_feature.rb
@@ -18,7 +18,7 @@ class ProjectFeature < ActiveRecord::Base
   PRIVATE  = 10
   ENABLED  = 20
 
-  FEATURES = %i(issues merge_requests wiki snippets builds repository)
+  FEATURES = %i(issues merge_requests wiki snippets builds repository).freeze
 
   class << self
     def access_level_attribute(feature)
diff --git a/app/models/project_services/buildkite_service.rb b/app/models/project_services/buildkite_service.rb
index 0956c4a4ede..5fb95050b83 100644
--- a/app/models/project_services/buildkite_service.rb
+++ b/app/models/project_services/buildkite_service.rb
@@ -3,7 +3,7 @@ require "addressable/uri"
 class BuildkiteService < CiService
   include ReactiveService
 
-  ENDPOINT = "https://buildkite.com"
+  ENDPOINT = "https://buildkite.com".freeze
 
   prop_accessor :project_url, :token
   boolean_accessor :enable_ssl_verification
diff --git a/app/models/project_services/chat_message/issue_message.rb b/app/models/project_services/chat_message/issue_message.rb
index b96aca47e65..791e5b0cec7 100644
--- a/app/models/project_services/chat_message/issue_message.rb
+++ b/app/models/project_services/chat_message/issue_message.rb
@@ -51,7 +51,8 @@ module ChatMessage
         title: issue_title,
         title_link: issue_url,
         text: format(description),
-        color: "#C95823" }]
+        color: "#C95823"
+      }]
     end
 
     def project_link
diff --git a/app/models/project_services/drone_ci_service.rb b/app/models/project_services/drone_ci_service.rb
index 1ad9efac196..2717c240f05 100644
--- a/app/models/project_services/drone_ci_service.rb
+++ b/app/models/project_services/drone_ci_service.rb
@@ -39,7 +39,7 @@ class DroneCiService < CiService
   def commit_status_path(sha, ref)
     url = [drone_url,
            "gitlab/#{project.full_path}/commits/#{sha}",
-           "?branch=#{URI::encode(ref.to_s)}&access_token=#{token}"]
+           "?branch=#{URI.encode(ref.to_s)}&access_token=#{token}"]
 
     URI.join(*url).to_s
   end
@@ -74,7 +74,7 @@ class DroneCiService < CiService
   def build_page(sha, ref)
     url = [drone_url,
            "gitlab/#{project.full_path}/redirect/commits/#{sha}",
-           "?branch=#{URI::encode(ref.to_s)}"]
+           "?branch=#{URI.encode(ref.to_s)}"]
 
     URI.join(*url).to_s
   end
@@ -114,7 +114,7 @@ class DroneCiService < CiService
   end
 
   def merge_request_valid?(data)
-    ['opened', 'reopened'].include?(data[:object_attributes][:state]) &&
+    %w(opened reopened).include?(data[:object_attributes][:state]) &&
       data[:object_attributes][:merge_status] == 'unchecked'
   end
 end
diff --git a/app/models/project_services/hipchat_service.rb b/app/models/project_services/hipchat_service.rb
index 72da219df28..c4142c38b2f 100644
--- a/app/models/project_services/hipchat_service.rb
+++ b/app/models/project_services/hipchat_service.rb
@@ -6,7 +6,7 @@ class HipchatService < Service
     a b i strong em br img pre code
     table th tr td caption colgroup col thead tbody tfoot
     ul ol li dl dt dd
-  ]
+  ].freeze
 
   prop_accessor :token, :room, :server, :color, :api_version
   boolean_accessor :notify_only_broken_builds, :notify
@@ -36,7 +36,7 @@ class HipchatService < Service
       { type: 'text', name: 'token',     placeholder: 'Room token' },
       { type: 'text', name: 'room',      placeholder: 'Room name or ID' },
       { type: 'checkbox', name: 'notify' },
-      { type: 'select', name: 'color', choices: ['yellow', 'red', 'green', 'purple', 'gray', 'random'] },
+      { type: 'select', name: 'color', choices: %w(yellow red green purple gray random) },
       { type: 'text', name: 'api_version',
         placeholder: 'Leave blank for default (v2)' },
       { type: 'text', name: 'server',
diff --git a/app/models/project_services/irker_service.rb b/app/models/project_services/irker_service.rb
index 5d6862d9faa..c62bb4fa120 100644
--- a/app/models/project_services/irker_service.rb
+++ b/app/models/project_services/irker_service.rb
@@ -33,7 +33,8 @@ class IrkerService < Service
   end
 
   def settings
-    { server_host: server_host.present? ? server_host : 'localhost',
+    {
+      server_host: server_host.present? ? server_host : 'localhost',
       server_port: server_port.present? ? server_port : 6659
     }
   end
diff --git a/app/models/project_services/kubernetes_service.rb b/app/models/project_services/kubernetes_service.rb
index f2f019c43bb..9819e723fe8 100644
--- a/app/models/project_services/kubernetes_service.rb
+++ b/app/models/project_services/kubernetes_service.rb
@@ -3,7 +3,7 @@ class KubernetesService < DeploymentService
   include Gitlab::Kubernetes
   include ReactiveCaching
 
-  self.reactive_cache_key = ->(service) { [ service.class.model_name.singular, service.project_id ] }
+  self.reactive_cache_key = ->(service) { [service.class.model_name.singular, service.project_id] }
 
   # Namespace defaults to the project path, but can be overridden in case that
   # is an invalid or inappropriate name
@@ -62,23 +62,19 @@ class KubernetesService < DeploymentService
         { type: 'text',
           name: 'namespace',
           title: 'Kubernetes namespace',
-          placeholder: 'Kubernetes namespace',
-        },
+          placeholder: 'Kubernetes namespace' },
         { type: 'text',
           name: 'api_url',
           title: 'API URL',
-          placeholder: 'Kubernetes API URL, like https://kube.example.com/',
-        },
+          placeholder: 'Kubernetes API URL, like https://kube.example.com/' },
         { type: 'text',
           name: 'token',
           title: 'Service token',
-          placeholder: 'Service token',
-        },
+          placeholder: 'Service token' },
         { type: 'textarea',
           name: 'ca_pem',
           title: 'Custom CA bundle',
-          placeholder: 'Certificate Authority bundle (PEM format)',
-        },
+          placeholder: 'Certificate Authority bundle (PEM format)' },
     ]
   end
 
@@ -167,7 +163,7 @@ class KubernetesService < DeploymentService
     url = URI.parse(api_url)
     prefix = url.path.sub(%r{/+\z}, '')
 
-    url.path = [ prefix, *parts ].join("/")
+    url.path = [prefix, *parts].join("/")
 
     url.to_s
   end
diff --git a/app/models/project_services/mattermost_service.rb b/app/models/project_services/mattermost_service.rb
index 4ebc5318da1..c13538e9fea 100644
--- a/app/models/project_services/mattermost_service.rb
+++ b/app/models/project_services/mattermost_service.rb
@@ -15,10 +15,10 @@ class MattermostService < ChatNotificationService
     'This service sends notifications about projects events to Mattermost channels.<br />
     To set up this service:
     <ol>
-      <li><a href="https://docs.mattermost.com/developer/webhooks-incoming.html#enabling-incoming-webhooks">Enable incoming webhooks</a> in your Mattermost installation. </li>
-      <li><a href="https://docs.mattermost.com/developer/webhooks-incoming.html#creating-integrations-using-incoming-webhooks">Add an incoming webhook</a> in your Mattermost team. The default channel can be overridden for each event. </li>
-      <li>Paste the webhook <strong>URL</strong> into the field bellow. </li>
-      <li>Select events below to enable notifications. The channel and username are optional. </li>
+      <li><a href="https://docs.mattermost.com/developer/webhooks-incoming.html#enabling-incoming-webhooks">Enable incoming webhooks</a> in your Mattermost installation.</li>
+      <li><a href="https://docs.mattermost.com/developer/webhooks-incoming.html#creating-integrations-using-incoming-webhooks">Add an incoming webhook</a> in your Mattermost team. The default channel can be overridden for each event.</li>
+      <li>Paste the webhook <strong>URL</strong> into the field below.</li>
+      <li>Select events below to enable notifications. The <strong>Channel handle</strong> and <strong>Username</strong> fields are optional.</li>
     </ol>'
   end
 
@@ -28,14 +28,14 @@ class MattermostService < ChatNotificationService
 
   def default_fields
     [
-      { type: 'text', name: 'webhook', placeholder: 'http://mattermost_host/hooks/...' },
-      { type: 'text', name: 'username', placeholder: 'username' },
+      { type: 'text', name: 'webhook', placeholder: 'e.g. http://mattermost_host/hooks/…' },
+      { type: 'text', name: 'username', placeholder: 'e.g. GitLab' },
       { type: 'checkbox', name: 'notify_only_broken_builds' },
       { type: 'checkbox', name: 'notify_only_broken_pipelines' },
     ]
   end
 
   def default_channel_placeholder
-    "town-square"
+    "Channel handle (e.g. town-square)"
   end
 end
diff --git a/app/models/project_services/pivotaltracker_service.rb b/app/models/project_services/pivotaltracker_service.rb
index 9cc642591f4..d86f4f6f448 100644
--- a/app/models/project_services/pivotaltracker_service.rb
+++ b/app/models/project_services/pivotaltracker_service.rb
@@ -1,7 +1,7 @@
 class PivotaltrackerService < Service
   include HTTParty
 
-  API_ENDPOINT = 'https://www.pivotaltracker.com/services/v5/source_commits'
+  API_ENDPOINT = 'https://www.pivotaltracker.com/services/v5/source_commits'.freeze
 
   prop_accessor :token, :restrict_to_branch
   validates :token, presence: true, if: :activated?
diff --git a/app/models/project_services/pushover_service.rb b/app/models/project_services/pushover_service.rb
index a963d27a376..3e618a8dbf1 100644
--- a/app/models/project_services/pushover_service.rb
+++ b/app/models/project_services/pushover_service.rb
@@ -29,25 +29,24 @@ class PushoverService < Service
           ['Normal Priority', 0],
           ['High Priority', 1]
         ],
-        default_choice: 0
-      },
+        default_choice: 0 },
       { type: 'select', name: 'sound', choices:
         [
           ['Device default sound', nil],
           ['Pushover (default)', 'pushover'],
-          ['Bike', 'bike'],
-          ['Bugle', 'bugle'],
+          %w(Bike bike),
+          %w(Bugle bugle),
           ['Cash Register', 'cashregister'],
-          ['Classical', 'classical'],
-          ['Cosmic', 'cosmic'],
-          ['Falling', 'falling'],
-          ['Gamelan', 'gamelan'],
-          ['Incoming', 'incoming'],
-          ['Intermission', 'intermission'],
-          ['Magic', 'magic'],
-          ['Mechanical', 'mechanical'],
+          %w(Classical classical),
+          %w(Cosmic cosmic),
+          %w(Falling falling),
+          %w(Gamelan gamelan),
+          %w(Incoming incoming),
+          %w(Intermission intermission),
+          %w(Magic magic),
+          %w(Mechanical mechanical),
           ['Piano Bar', 'pianobar'],
-          ['Siren', 'siren'],
+          %w(Siren siren),
           ['Space Alarm', 'spacealarm'],
           ['Tug Boat', 'tugboat'],
           ['Alien Alarm (long)', 'alien'],
@@ -56,8 +55,7 @@ class PushoverService < Service
           ['Pushover Echo (long)', 'echo'],
           ['Up Down (long)', 'updown'],
           ['None (silent)', 'none']
-        ]
-      },
+        ] },
     ]
   end
 
@@ -72,13 +70,14 @@ class PushoverService < Service
     before = data[:before]
     after = data[:after]
 
-    if Gitlab::Git.blank_ref?(before)
-      message = "#{data[:user_name]} pushed new branch \"#{ref}\"."
-    elsif Gitlab::Git.blank_ref?(after)
-      message = "#{data[:user_name]} deleted branch \"#{ref}\"."
-    else
-      message = "#{data[:user_name]} push to branch \"#{ref}\"."
-    end
+    message =
+      if Gitlab::Git.blank_ref?(before)
+        "#{data[:user_name]} pushed new branch \"#{ref}\"."
+      elsif Gitlab::Git.blank_ref?(after)
+        "#{data[:user_name]} deleted branch \"#{ref}\"."
+      else
+        "#{data[:user_name]} push to branch \"#{ref}\"."
+      end
 
     if data[:total_commits_count] > 0
       message << "\nTotal commits count: #{data[:total_commits_count]}"
@@ -97,7 +96,7 @@ class PushoverService < Service
 
     # Sound parameter MUST NOT be sent to API if not selected
     if sound
-      pushover_data.merge!(sound: sound)
+      pushover_data[:sound] = sound
     end
 
     PushoverService.post('/messages.json', body: pushover_data)
diff --git a/app/models/project_services/slack_service.rb b/app/models/project_services/slack_service.rb
index f77d2d7c60b..da7496573ef 100644
--- a/app/models/project_services/slack_service.rb
+++ b/app/models/project_services/slack_service.rb
@@ -13,11 +13,11 @@ class SlackService < ChatNotificationService
 
   def help
     'This service sends notifications about projects events to Slack channels.<br />
-    To setup this service:
+    To set up this service:
     <ol>
-      <li><a href="https://slack.com/apps/A0F7XDUAZ-incoming-webhooks">Add an incoming webhook</a> in your Slack team. The default channel can be overridden for each event. </li>
-      <li>Paste the <strong>Webhook URL</strong> into the field below. </li>
-      <li>Select events below to enable notifications. The channel and username are optional. </li>
+      <li><a href="https://slack.com/apps/A0F7XDUAZ-incoming-webhooks">Add an incoming webhook</a> in your Slack team. The default channel can be overridden for each event.</li>
+      <li>Paste the <strong>Webhook URL</strong> into the field below.</li>
+      <li>Select events below to enable notifications. The <strong>Channel name</strong> and <strong>Username</strong> fields are optional.</li>
     </ol>'
   end
 
@@ -27,14 +27,14 @@ class SlackService < ChatNotificationService
 
   def default_fields
     [
-      { type: 'text', name: 'webhook', placeholder: 'https://hooks.slack.com/services/...' },
-      { type: 'text', name: 'username', placeholder: 'username' },
+      { type: 'text', name: 'webhook', placeholder: 'e.g. https://hooks.slack.com/services/…' },
+      { type: 'text', name: 'username', placeholder: 'e.g. GitLab' },
       { type: 'checkbox', name: 'notify_only_broken_builds' },
       { type: 'checkbox', name: 'notify_only_broken_pipelines' },
     ]
   end
 
   def default_channel_placeholder
-    "#general"
+    "Channel name (e.g. general)"
   end
 end
diff --git a/app/models/project_statistics.rb b/app/models/project_statistics.rb
index 06abd406523..aeaf63abab9 100644
--- a/app/models/project_statistics.rb
+++ b/app/models/project_statistics.rb
@@ -4,7 +4,7 @@ class ProjectStatistics < ActiveRecord::Base
 
   before_save :update_storage_size
 
-  STORAGE_COLUMNS = [:repository_size, :lfs_objects_size, :build_artifacts_size]
+  STORAGE_COLUMNS = [:repository_size, :lfs_objects_size, :build_artifacts_size].freeze
   STATISTICS_COLUMNS = [:commit_count] + STORAGE_COLUMNS
 
   def total_repository_size
diff --git a/app/models/project_wiki.rb b/app/models/project_wiki.rb
index d0b991db112..9891f5edf41 100644
--- a/app/models/project_wiki.rb
+++ b/app/models/project_wiki.rb
@@ -5,7 +5,7 @@ class ProjectWiki
     'Markdown' => :markdown,
     'RDoc'     => :rdoc,
     'AsciiDoc' => :asciidoc
-  } unless defined?(MARKUPS)
+  }.freeze unless defined?(MARKUPS)
 
   class CouldNotCreateWikiError < StandardError; end
 
@@ -19,6 +19,9 @@ class ProjectWiki
     @user = user
   end
 
+  delegate :empty?, to: :pages
+  delegate :repository_storage_path, to: :project
+
   def path
     @project.path + '.wiki'
   end
@@ -60,10 +63,6 @@ class ProjectWiki
     !!repository.exists?
   end
 
-  def empty?
-    pages.empty?
-  end
-
   # Returns an Array of Gitlab WikiPage instances or an
   # empty Array if this Wiki has no pages.
   def pages
@@ -160,10 +159,6 @@ class ProjectWiki
     }
   end
 
-  def repository_storage_path
-    project.repository_storage_path
-  end
-
   private
 
   def init_repo(path_with_namespace)
diff --git a/app/models/protected_branch.rb b/app/models/protected_branch.rb
index 6240912a6e1..39e979ef15b 100644
--- a/app/models/protected_branch.rb
+++ b/app/models/protected_branch.rb
@@ -8,8 +8,8 @@ class ProtectedBranch < ActiveRecord::Base
   has_many :merge_access_levels, dependent: :destroy
   has_many :push_access_levels, dependent: :destroy
 
-  validates_length_of :merge_access_levels, is: 1, message: "are restricted to a single instance per protected branch."
-  validates_length_of :push_access_levels, is: 1, message: "are restricted to a single instance per protected branch."
+  validates :merge_access_levels, length: { is: 1, message: "are restricted to a single instance per protected branch." }
+  validates :push_access_levels, length: { is: 1, message: "are restricted to a single instance per protected branch." }
 
   accepts_nested_attributes_for :push_access_levels
   accepts_nested_attributes_for :merge_access_levels
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 56c582cd9be..0dbf246c3a4 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -18,7 +18,7 @@ class Repository
   CACHED_METHODS = %i(size commit_count readme version contribution_guide
                       changelog license_blob license_key gitignore koding_yml
                       gitlab_ci_yml branch_names tag_names branch_count
-                      tag_count avatar exists? empty? root_ref)
+                      tag_count avatar exists? empty? root_ref).freeze
 
   # Certain method caches should be refreshed when certain types of files are
   # changed. This Hash maps file types (as returned by Gitlab::FileDetector) to
@@ -33,7 +33,7 @@ class Repository
     koding: :koding_yml,
     gitlab_ci: :gitlab_ci_yml,
     avatar: :avatar
-  }
+  }.freeze
 
   # Wraps around the given method and caches its output in Redis and an instance
   # variable.
@@ -109,9 +109,7 @@ class Repository
       offset: offset,
       after: after,
       before: before,
-      # --follow doesn't play well with --skip. See:
-      # https://gitlab.com/gitlab-org/gitlab-ce/issues/3574#note_3040520
-      follow: false,
+      follow: path.present?,
       skip_merges: skip_merges
     }
 
@@ -487,9 +485,7 @@ class Repository
   end
   cache_method :exists?
 
-  def empty?
-    raw_repository.empty?
-  end
+  delegate :empty?, to: :raw_repository
   cache_method :empty?
 
   # The size of this repository in megabytes.
@@ -508,9 +504,7 @@ class Repository
   end
   cache_method :branch_names, fallback: []
 
-  def tag_names
-    raw_repository.tag_names
-  end
+  delegate :tag_names, to: :raw_repository
   cache_method :tag_names, fallback: []
 
   def branch_count
@@ -750,136 +744,63 @@ class Repository
     @tags ||= raw_repository.tags
   end
 
-  # rubocop:disable Metrics/ParameterLists
-  def commit_dir(
-    user, path,
-    message:, branch_name:,
-    author_email: nil, author_name: nil,
-    start_branch_name: nil, start_project: project)
-    check_tree_entry_for_dir(branch_name, path)
-
-    if start_branch_name
-      start_project.repository.
-        check_tree_entry_for_dir(start_branch_name, path)
-    end
+  def create_dir(user, path, **options)
+    options[:user] = user
+    options[:actions] = [{ action: :create_dir, file_path: path }]
 
-    commit_file(
-      user,
-      "#{path}/.gitkeep",
-      '',
-      message: message,
-      branch_name: branch_name,
-      update: false,
-      author_email: author_email,
-      author_name: author_name,
-      start_branch_name: start_branch_name,
-      start_project: start_project)
+    multi_action(**options)
   end
-  # rubocop:enable Metrics/ParameterLists
 
-  # rubocop:disable Metrics/ParameterLists
-  def commit_file(
-    user, path, content,
-    message:, branch_name:, update: true,
-    author_email: nil, author_name: nil,
-    start_branch_name: nil, start_project: project)
-    unless update
-      error_message = "Filename already exists; update not allowed"
+  def create_file(user, path, content, **options)
+    options[:user] = user
+    options[:actions] = [{ action: :create, file_path: path, content: content }]
 
-      if tree_entry_at(branch_name, path)
-        raise Gitlab::Git::Repository::InvalidBlobName.new(error_message)
-      end
+    multi_action(**options)
+  end
 
-      if start_branch_name &&
-          start_project.repository.tree_entry_at(start_branch_name, path)
-        raise Gitlab::Git::Repository::InvalidBlobName.new(error_message)
-      end
-    end
+  def update_file(user, path, content, **options)
+    previous_path = options.delete(:previous_path)
+    action = previous_path && previous_path != path ? :move : :update
 
-    multi_action(
-      user: user,
-      message: message,
-      branch_name: branch_name,
-      author_email: author_email,
-      author_name: author_name,
-      start_branch_name: start_branch_name,
-      start_project: start_project,
-      actions: [{ action: :create,
-                  file_path: path,
-                  content: content }])
-  end
-  # rubocop:enable Metrics/ParameterLists
+    options[:user] = user
+    options[:actions] = [{ action: action, file_path: path, previous_path: previous_path, content: content }]
 
-  # rubocop:disable Metrics/ParameterLists
-  def update_file(
-    user, path, content,
-    message:, branch_name:, previous_path:,
-    author_email: nil, author_name: nil,
-    start_branch_name: nil, start_project: project)
-    action = if previous_path && previous_path != path
-               :move
-             else
-               :update
-             end
-
-    multi_action(
-      user: user,
-      message: message,
-      branch_name: branch_name,
-      author_email: author_email,
-      author_name: author_name,
-      start_branch_name: start_branch_name,
-      start_project: start_project,
-      actions: [{ action: action,
-                  file_path: path,
-                  content: content,
-                  previous_path: previous_path }])
+    multi_action(**options)
   end
-  # rubocop:enable Metrics/ParameterLists
 
-  # rubocop:disable Metrics/ParameterLists
-  def remove_file(
-    user, path,
-    message:, branch_name:,
-    author_email: nil, author_name: nil,
-    start_branch_name: nil, start_project: project)
-    multi_action(
-      user: user,
-      message: message,
-      branch_name: branch_name,
-      author_email: author_email,
-      author_name: author_name,
-      start_branch_name: start_branch_name,
-      start_project: start_project,
-      actions: [{ action: :delete,
-                  file_path: path }])
+  def delete_file(user, path, **options)
+    options[:user] = user
+    options[:actions] = [{ action: :delete, file_path: path }]
+
+    multi_action(**options)
   end
-  # rubocop:enable Metrics/ParameterLists
 
   # rubocop:disable Metrics/ParameterLists
   def multi_action(
     user:, branch_name:, message:, actions:,
     author_email: nil, author_name: nil,
     start_branch_name: nil, start_project: project)
+
     GitOperationService.new(user, self).with_branch(
       branch_name,
       start_branch_name: start_branch_name,
       start_project: start_project) do |start_commit|
-      index = rugged.index
 
-      parents = if start_commit
-                  index.read_tree(start_commit.raw_commit.tree)
-                  [start_commit.sha]
-                else
-                  []
-                end
+      index = Gitlab::Git::Index.new(raw_repository)
 
-      actions.each do |act|
-        git_action(index, act)
+      if start_commit
+        index.read_tree(start_commit.raw_commit.tree)
+        parents = [start_commit.sha]
+      else
+        parents = []
+      end
+
+      actions.each do |options|
+        index.public_send(options.delete(:action), options)
       end
 
       options = {
-        tree: index.write_tree(rugged),
+        tree: index.write_tree,
         message: message,
         parents: parents
       }
@@ -892,7 +813,7 @@ class Repository
 
   def get_committer_and_author(user, email: nil, name: nil)
     committer = user_to_committer(user)
-    author = Gitlab::Git::committer_hash(email: email, name: name) || committer
+    author = Gitlab::Git.committer_hash(email: email, name: name) || committer
 
     {
       author: author,
@@ -1170,30 +1091,6 @@ class Repository
     blob_data_at(sha, '.gitlab-ci.yml')
   end
 
-  protected
-
-  def tree_entry_at(branch_name, path)
-    branch_exists?(branch_name) &&
-      # tree_entry is private
-      raw_repository.send(:tree_entry, commit(branch_name), path)
-  end
-
-  def check_tree_entry_for_dir(branch_name, path)
-    return unless branch_exists?(branch_name)
-
-    entry = tree_entry_at(branch_name, path)
-
-    return unless entry
-
-    if entry[:type] == :blob
-      raise Gitlab::Git::Repository::InvalidBlobName.new(
-        "Directory already exists as a file")
-    else
-      raise Gitlab::Git::Repository::InvalidBlobName.new(
-        "Directory already exists")
-    end
-  end
-
   private
 
   def blob_data_at(sha, path)
@@ -1204,58 +1101,6 @@ class Repository
     blob.data
   end
 
-  def git_action(index, action)
-    path = normalize_path(action[:file_path])
-
-    if action[:action] == :move
-      previous_path = normalize_path(action[:previous_path])
-    end
-
-    case action[:action]
-    when :create, :update, :move
-      mode =
-        case action[:action]
-        when :update
-          index.get(path)[:mode]
-        when :move
-          index.get(previous_path)[:mode]
-        end
-      mode ||= 0o100644
-
-      index.remove(previous_path) if action[:action] == :move
-
-      content = if action[:encoding] == 'base64'
-                  Base64.decode64(action[:content])
-                else
-                  action[:content]
-                end
-
-      detect = CharlockHolmes::EncodingDetector.new.detect(content) if content
-
-      unless detect && detect[:type] == :binary
-        # When writing to the repo directly as we are doing here,
-        # the `core.autocrlf` config isn't taken into account.
-        content.gsub!("\r\n", "\n") if self.autocrlf
-      end
-
-      oid = rugged.write(content, :blob)
-
-      index.add(path: path, oid: oid, mode: mode)
-    when :delete
-      index.remove(path)
-    end
-  end
-
-  def normalize_path(path)
-    pathname = Gitlab::Git::PathHelper.normalize_path(path)
-
-    if pathname.each_filename.include?('..')
-      raise Gitlab::Git::Repository::InvalidBlobName.new('Invalid path')
-    end
-
-    pathname.to_s
-  end
-
   def refs_directory_exists?
     return false unless path_with_namespace
 
diff --git a/app/models/todo.rb b/app/models/todo.rb
index 3dda7948d0b..47789a21133 100644
--- a/app/models/todo.rb
+++ b/app/models/todo.rb
@@ -17,7 +17,7 @@ class Todo < ActiveRecord::Base
     APPROVAL_REQUIRED => :approval_required,
     UNMERGEABLE => :unmergeable,
     DIRECTLY_ADDRESSED => :directly_addressed
-  }
+  }.freeze
 
   belongs_to :author, class_name: "User"
   belongs_to :note
diff --git a/app/models/user.rb b/app/models/user.rb
index f614eb66e1f..40264401b53 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -81,7 +81,6 @@ class User < ActiveRecord::Base
   has_many :authorized_projects, through: :project_authorizations, source: :project
 
   has_many :snippets,                 dependent: :destroy, foreign_key: :author_id
-  has_many :issues,                   dependent: :destroy, foreign_key: :author_id
   has_many :notes,                    dependent: :destroy, foreign_key: :author_id
   has_many :merge_requests,           dependent: :destroy, foreign_key: :author_id
   has_many :events,                   dependent: :destroy, foreign_key: :author_id
@@ -99,12 +98,17 @@ class User < ActiveRecord::Base
   has_many :assigned_issues,          dependent: :nullify, foreign_key: :assignee_id, class_name: "Issue"
   has_many :assigned_merge_requests,  dependent: :nullify, foreign_key: :assignee_id, class_name: "MergeRequest"
 
+  # Issues that a user owns are expected to be moved to the "ghost" user before
+  # the user is destroyed. If the user owns any issues during deletion, this
+  # should be treated as an exceptional condition.
+  has_many :issues,                   dependent: :restrict_with_exception, foreign_key: :author_id
+
   #
   # Validations
   #
   # Note: devise :validatable above adds validations for :email and :password
   validates :name, presence: true
-  validates_confirmation_of :email
+  validates :email, confirmation: true
   validates :notification_email, presence: true
   validates :notification_email, email: true, if: ->(user) { user.notification_email != user.email }
   validates :public_email, presence: true, uniqueness: true, email: true, allow_blank: true
@@ -120,6 +124,7 @@ class User < ActiveRecord::Base
   validate :unique_email, if: ->(user) { user.email_changed? }
   validate :owns_notification_email, if: ->(user) { user.notification_email_changed? }
   validate :owns_public_email, if: ->(user) { user.public_email_changed? }
+  validate :ghost_users_must_be_blocked
   validates :avatar, file_size: { maximum: 200.kilobytes.to_i }
 
   before_validation :generate_password, on: :create
@@ -334,9 +339,15 @@ class User < ActiveRecord::Base
     def reference_pattern
       %r{
         #{Regexp.escape(reference_prefix)}
-        (?<user>#{Gitlab::Regex::NAMESPACE_REF_REGEX_STR})
+        (?<user>#{Gitlab::Regex::FULL_NAMESPACE_REGEX_STR})
       }x
     end
+
+    # Return (create if necessary) the ghost user. The ghost user
+    # owns records previously belonging to deleted users.
+    def ghost
+      User.find_by_ghost(true) || create_ghost_user
+    end
   end
 
   #
@@ -435,6 +446,12 @@ class User < ActiveRecord::Base
     errors.add(:public_email, "is not an email you own") unless all_emails.include?(public_email)
   end
 
+  def ghost_users_must_be_blocked
+    if ghost? && !blocked?
+      errors.add(:ghost, 'cannot be enabled for a user who is not blocked.')
+    end
+  end
+
   def update_emails_with_primary_email
     primary_email_record = emails.find_by(email: email)
     if primary_email_record
@@ -580,8 +597,8 @@ class User < ActiveRecord::Base
 
       if project.repository.branch_exists?(event.branch_name)
         merge_requests = MergeRequest.where("created_at >= ?", event.created_at).
-            where(source_project_id: project.id,
-                  source_branch: event.branch_name)
+          where(source_project_id: project.id,
+                source_branch: event.branch_name)
         merge_requests.empty?
       end
     end
@@ -999,4 +1016,40 @@ class User < ActiveRecord::Base
       super
     end
   end
+
+  def self.create_ghost_user
+    # Since we only want a single ghost user in an instance, we use an
+    # exclusive lease to ensure than this block is never run concurrently.
+    lease_key = "ghost_user_creation"
+    lease = Gitlab::ExclusiveLease.new(lease_key, timeout: 1.minute.to_i)
+
+    until uuid = lease.try_obtain
+      # Keep trying until we obtain the lease. To prevent hammering Redis too
+      # much we'll wait for a bit between retries.
+      sleep(1)
+    end
+
+    # Recheck if a ghost user is already present. One might have been
+    # added between the time we last checked (first line of this method)
+    # and the time we acquired the lock.
+    ghost_user = User.find_by_ghost(true)
+    return ghost_user if ghost_user.present?
+
+    uniquify = Uniquify.new
+
+    username = uniquify.string("ghost") { |s| User.find_by_username(s) }
+
+    email = uniquify.string(-> (n) { "ghost#{n}@example.com" }) do |s|
+      User.find_by_email(s)
+    end
+
+    bio = 'This is a "Ghost User", created to hold all issues authored by users that have since been deleted. This user cannot be removed.'
+
+    User.create(
+      username: username, password: Devise.friendly_token, bio: bio,
+      email: email, name: "Ghost User", state: :blocked, ghost: true
+    )
+  ensure
+    Gitlab::ExclusiveLease.cancel(lease_key, uuid)
+  end
 end