17 files changed, 204 insertions, 75 deletions

diff --git a/app/models/ability.rb b/app/models/ability.rb
index fa8f8bc3a5f..ad6c588202e 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -22,6 +22,17 @@ class Ability
       end
     end
 
+    # Given a list of users and a snippet this method returns the users that can
+    # read the given snippet.
+    def users_that_can_read_personal_snippet(users, snippet)
+      case snippet.visibility_level
+      when Snippet::INTERNAL, Snippet::PUBLIC
+        users
+      when Snippet::PRIVATE
+        users.include?(snippet.author) ? [snippet.author] : []
+      end
+    end
+
     # Returns an Array of Issues that can be read by the given user.
     #
     # issues - The issues to reduce down to those readable by the user.
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 8fab77cda0a..e33a58d3771 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -13,6 +13,49 @@ class ApplicationSetting < ActiveRecord::Base
                             [\r\n]          # any number of newline characters
                           }x
 
+  DEFAULTS_CE = {
+    after_sign_up_text: nil,
+    akismet_enabled: false,
+    container_registry_token_expire_delay: 5,
+    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'],
+    default_snippet_visibility: Settings.gitlab.default_projects_features['visibility_level'],
+    disabled_oauth_sign_in_sources: [],
+    domain_whitelist: Settings.gitlab['domain_whitelist'],
+    gravatar_enabled: Settings.gravatar['enabled'],
+    help_page_text: nil,
+    housekeeping_bitmaps_enabled: true,
+    housekeeping_enabled: true,
+    housekeeping_full_repack_period: 50,
+    housekeeping_gc_period: 200,
+    housekeeping_incremental_repack_period: 10,
+    import_sources: Gitlab::ImportSources.values,
+    koding_enabled: false,
+    koding_url: nil,
+    max_artifacts_size: Settings.artifacts['max_size'],
+    max_attachment_size: Settings.gitlab['max_attachment_size'],
+    plantuml_enabled: false,
+    plantuml_url: nil,
+    recaptcha_enabled: false,
+    repository_checks_enabled: true,
+    repository_storages: ['default'],
+    require_two_factor_authentication: false,
+    restricted_visibility_levels: Settings.gitlab['restricted_visibility_levels'],
+    session_expire_delay: Settings.gitlab['session_expire_delay'],
+    send_user_confirmation_email: false,
+    shared_runners_enabled: Settings.gitlab_ci['shared_runners_enabled'],
+    shared_runners_text: nil,
+    sidekiq_throttling_enabled: false,
+    sign_in_text: nil,
+    signin_enabled: Settings.gitlab['signin_enabled'],
+    signup_enabled: Settings.gitlab['signup_enabled'],
+    two_factor_grace_period: 48,
+    user_default_external: false
+  }
+
+  DEFAULTS = DEFAULTS_CE
+
   serialize :restricted_visibility_levels
   serialize :import_sources
   serialize :disabled_oauth_sign_in_sources, Array
@@ -163,46 +206,7 @@ class ApplicationSetting < ActiveRecord::Base
   end
 
   def self.create_from_defaults
-    create(
-      default_projects_limit: Settings.gitlab['default_projects_limit'],
-      default_branch_protection: Settings.gitlab['default_branch_protection'],
-      signup_enabled: Settings.gitlab['signup_enabled'],
-      signin_enabled: Settings.gitlab['signin_enabled'],
-      gravatar_enabled: Settings.gravatar['enabled'],
-      sign_in_text: nil,
-      after_sign_up_text: nil,
-      help_page_text: nil,
-      shared_runners_text: nil,
-      restricted_visibility_levels: Settings.gitlab['restricted_visibility_levels'],
-      max_attachment_size: Settings.gitlab['max_attachment_size'],
-      session_expire_delay: Settings.gitlab['session_expire_delay'],
-      default_project_visibility: Settings.gitlab.default_projects_features['visibility_level'],
-      default_snippet_visibility: Settings.gitlab.default_projects_features['visibility_level'],
-      domain_whitelist: Settings.gitlab['domain_whitelist'],
-      import_sources: Gitlab::ImportSources.values,
-      shared_runners_enabled: Settings.gitlab_ci['shared_runners_enabled'],
-      max_artifacts_size: Settings.artifacts['max_size'],
-      require_two_factor_authentication: false,
-      two_factor_grace_period: 48,
-      recaptcha_enabled: false,
-      akismet_enabled: false,
-      koding_enabled: false,
-      koding_url: nil,
-      plantuml_enabled: false,
-      plantuml_url: nil,
-      repository_checks_enabled: true,
-      disabled_oauth_sign_in_sources: [],
-      send_user_confirmation_email: false,
-      container_registry_token_expire_delay: 5,
-      repository_storages: ['default'],
-      user_default_external: false,
-      sidekiq_throttling_enabled: false,
-      housekeeping_enabled: true,
-      housekeeping_bitmaps_enabled: true,
-      housekeeping_incremental_repack_period: 10,
-      housekeeping_full_repack_period: 50,
-      housekeeping_gc_period: 200,
-    )
+    create(DEFAULTS)
   end
 
   def home_page_url_column_exist
diff --git a/app/models/commit.rb b/app/models/commit.rb
index 5d942cb0422..316bd2e512b 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -21,6 +21,9 @@ class Commit
   DIFF_HARD_LIMIT_FILES = 1000
   DIFF_HARD_LIMIT_LINES = 50000
 
+  # The SHA can be between 7 and 40 hex characters.
+  COMMIT_SHA_PATTERN = '\h{7,40}'
+
   class << self
     def decorate(commits, project)
       commits.map do |commit|
@@ -52,6 +55,10 @@ class Commit
     def from_hash(hash, project)
       new(Gitlab::Git::Commit.new(hash), project)
     end
+
+    def valid_hash?(key)
+      !!(/\A#{COMMIT_SHA_PATTERN}\z/ =~ key)
+    end
   end
 
   attr_accessor :raw
@@ -77,8 +84,6 @@ class Commit
 
   # Pattern used to extract commit references from text
   #
-  # The SHA can be between 7 and 40 hex characters.
-  #
   # This pattern supports cross-project references.
   def self.reference_pattern
     @reference_pattern ||= %r{
@@ -88,7 +93,7 @@ class Commit
   end
 
   def self.link_reference_pattern
-    @link_reference_pattern ||= super("commit", /(?<commit>\h{7,40})/)
+    @link_reference_pattern ||= super("commit", /(?<commit>#{COMMIT_SHA_PATTERN})/)
   end
 
   def to_reference(from_project = nil, full: false)
diff --git a/app/models/concerns/cache_markdown_field.rb b/app/models/concerns/cache_markdown_field.rb
index 90bd6490a02..a600f9c14c5 100644
--- a/app/models/concerns/cache_markdown_field.rb
+++ b/app/models/concerns/cache_markdown_field.rb
@@ -51,6 +51,10 @@ module CacheMarkdownField
     CACHING_CLASSES.map(&:constantize)
   end
 
+  def skip_project_check?
+    false
+  end
+
   extend ActiveSupport::Concern
 
   included do
@@ -112,7 +116,8 @@ module CacheMarkdownField
       invalidation_method = "#{html_field}_invalidated?".to_sym
 
       define_method(cache_method) do
-        html = Banzai::Renderer.cacheless_render_field(self, markdown_field)
+        options = { skip_project_check: skip_project_check? }
+        html = Banzai::Renderer.cacheless_render_field(self, markdown_field, options)
         __send__("#{html_field}=", html)
         true
       end
diff --git a/app/models/concerns/mentionable.rb b/app/models/concerns/mentionable.rb
index 8ab0401d288..ef2c1e5d414 100644
--- a/app/models/concerns/mentionable.rb
+++ b/app/models/concerns/mentionable.rb
@@ -49,7 +49,11 @@ module Mentionable
 
     self.class.mentionable_attrs.each do |attr, options|
       text    = __send__(attr)
-      options = options.merge(cache_key: [self, attr], author: author)
+      options = options.merge(
+        cache_key: [self, attr],
+        author: author,
+        skip_project_check: skip_project_check?
+      )
 
       extractor.analyze(text, options)
     end
@@ -121,4 +125,8 @@ module Mentionable
   def cross_reference_exists?(target)
     SystemNoteService.cross_reference_exists?(target, local_reference)
   end
+
+  def skip_project_check?
+    false
+  end
 end
diff --git a/app/models/concerns/participable.rb b/app/models/concerns/participable.rb
index 70740c76e43..4865c0a14b1 100644
--- a/app/models/concerns/participable.rb
+++ b/app/models/concerns/participable.rb
@@ -96,6 +96,11 @@ module Participable
 
     participants.merge(ext.users)
 
-    Ability.users_that_can_read_project(participants.to_a, project)
+    case self
+    when PersonalSnippet
+      Ability.users_that_can_read_personal_snippet(participants.to_a, self)
+    else
+      Ability.users_that_can_read_project(participants.to_a, project)
+    end
   end
 end
diff --git a/app/models/concerns/routable.rb b/app/models/concerns/routable.rb
index 1108a64c59e..2b93aa30c0f 100644
--- a/app/models/concerns/routable.rb
+++ b/app/models/concerns/routable.rb
@@ -60,6 +60,21 @@ module Routable
         joins(:route).where(wheres.join(' OR '))
       end
     end
+
+    # Builds a relation to find multiple objects that are nested under user membership
+    #
+    # Usage:
+    #
+    #     Klass.member_descendants(1)
+    #
+    # Returns an ActiveRecord::Relation.
+    def member_descendants(user_id)
+      joins(:route).
+        joins("INNER JOIN routes r2 ON routes.path LIKE CONCAT(r2.path, '/%')
+               INNER JOIN members ON members.source_id = r2.source_id
+               AND members.source_type = r2.source_type").
+        where('members.user_id = ?', user_id)
+    end
   end
 
   private
diff --git a/app/models/concerns/taskable.rb b/app/models/concerns/taskable.rb
index ebc75100a54..68385dc47eb 100644
--- a/app/models/concerns/taskable.rb
+++ b/app/models/concerns/taskable.rb
@@ -11,7 +11,7 @@ module Taskable
   INCOMPLETE   = 'incomplete'.freeze
   ITEM_PATTERN = /
     ^
-    (?:\s*[-+*]|(?:\d+\.))? # optional list prefix
+    \s*(?:[-+*]|(?:\d+\.))? # optional list prefix
     \s*                     # optional whitespace prefix
     (\[\s\]|\[[xX]\])       # checkbox
     (\s.+)                  # followed by whitespace and some text.
diff --git a/app/models/group.rb b/app/models/group.rb
index 99675ddb366..4cdfd022094 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -201,7 +201,7 @@ class Group < Namespace
   end
 
   def members_with_parents
-    GroupMember.where(requested_at: nil, source_id: parents.map(&:id).push(id))
+    GroupMember.where(requested_at: nil, source_id: ancestors.map(&:id).push(id))
   end
 
   def users_with_parents
diff --git a/app/models/member.rb b/app/models/member.rb
index c585e0b450e..26a6054e00d 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -68,9 +68,9 @@ class Member < ActiveRecord::Base
   after_create :send_request, if: :request?, unless: :importing?
   after_create :create_notification_setting, unless: [:pending?, :importing?]
   after_create :post_create_hook, unless: [:pending?, :importing?]
-  after_create :refresh_member_authorized_projects, if: :importing?
   after_update :post_update_hook, unless: [:pending?, :importing?]
   after_destroy :post_destroy_hook, unless: :pending?
+  after_commit :refresh_member_authorized_projects
 
   delegate :name, :username, :email, to: :user, prefix: true
 
@@ -147,8 +147,6 @@ class Member < ActiveRecord::Base
         member.save
       end
 
-      UserProjectAccessChangedService.new(user.id).execute if user.is_a?(User)
-
       member
     end
 
@@ -275,23 +273,27 @@ class Member < ActiveRecord::Base
   end
 
   def post_create_hook
-    UserProjectAccessChangedService.new(user.id).execute
     system_hook_service.execute_hooks_for(self, :create)
   end
 
   def post_update_hook
-    UserProjectAccessChangedService.new(user.id).execute if access_level_changed?
+    # override in sub class
   end
 
   def post_destroy_hook
-    refresh_member_authorized_projects
     system_hook_service.execute_hooks_for(self, :destroy)
   end
 
+  # Refreshes authorizations of the current member.
+  #
+  # This method schedules a job using Sidekiq and as such **must not** be called
+  # in a transaction. Doing so can lead to the job running before the
+  # transaction has been committed, resulting in the job either throwing an
+  # error or not doing any meaningful work.
   def refresh_member_authorized_projects
-    # If user/source is being destroyed, project access are gonna be destroyed eventually
-    # because of DB foreign keys, so we shouldn't bother with refreshing after each
-    # member is destroyed through association
+    # If user/source is being destroyed, project access are going to be
+    # destroyed eventually because of DB foreign keys, so we shouldn't bother
+    # with refreshing after each member is destroyed through association
     return if destroyed_by_association.present?
 
     UserProjectAccessChangedService.new(user_id).execute
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index cd5b345bae5..6753504acff 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -865,9 +865,11 @@ class MergeRequest < ActiveRecord::Base
       paths: paths
     )
 
-    active_diff_notes.each do |note|
-      service.execute(note)
-      Gitlab::Timeless.timeless(note, &:save)
+    transaction do
+      active_diff_notes.each do |note|
+        service.execute(note)
+        Gitlab::Timeless.timeless(note, &:save)
+      end
     end
   end
 
diff --git a/app/models/namespace.rb b/app/models/namespace.rb
index d41833de66f..67d8c1c2e4c 100644
--- a/app/models/namespace.rb
+++ b/app/models/namespace.rb
@@ -4,6 +4,7 @@ class Namespace < ActiveRecord::Base
   include CacheMarkdownField
   include Sortable
   include Gitlab::ShellAdapter
+  include Gitlab::CurrentSettings
   include Routable
 
   cache_markdown_field :description, pipeline: :description
@@ -130,6 +131,8 @@ class Namespace < ActiveRecord::Base
 
     Gitlab::UploadsTransfer.new.rename_namespace(path_was, path)
 
+    remove_exports!
+
     # If repositories moved successfully we need to
     # send update instructions to users.
     # However we cannot allow rollback since we moved namespace dir
@@ -174,6 +177,10 @@ class Namespace < ActiveRecord::Base
     end
   end
 
+  def shared_runners_enabled?
+    projects.with_shared_runners.any?
+  end
+
   def full_name
     @full_name ||=
       if parent
@@ -183,8 +190,26 @@ class Namespace < ActiveRecord::Base
       end
   end
 
-  def parents
-    @parents ||= parent ? parent.parents + [parent] : []
+  # Scopes the model on ancestors of the record
+  def ancestors
+    if parent_id
+      path = route.path
+      paths = []
+
+      until path.blank?
+        path = path.rpartition('/').first
+        paths << path
+      end
+
+      self.class.joins(:route).where('routes.path IN (?)', paths).reorder('routes.path ASC')
+    else
+      self.class.none
+    end
+  end
+
+  # Scopes the model on direct and indirect children of the record
+  def descendants
+    self.class.joins(:route).where('routes.path LIKE ?', "#{route.path}/%").reorder('routes.path ASC')
   end
 
   private
@@ -214,6 +239,8 @@ class Namespace < ActiveRecord::Base
         GitlabShellWorker.perform_in(5.minutes, :rm_namespace, repository_storage_path, new_path)
       end
     end
+
+    remove_exports!
   end
 
   def refresh_access_of_projects_invited_groups
@@ -226,4 +253,20 @@ class Namespace < ActiveRecord::Base
   def full_path_changed?
     path_changed? || parent_id_changed?
   end
+
+  def remove_exports!
+    Gitlab::Popen.popen(%W(find #{export_path} -not -path #{export_path} -delete))
+  end
+
+  def export_path
+    File.join(Gitlab::ImportExport.storage_path, full_path_was)
+  end
+
+  def full_path_was
+    if parent
+      parent.full_path + '/' + path_was
+    else
+      path_was
+    end
+  end
 end
diff --git a/app/models/note.rb b/app/models/note.rb
index 0c1b05dabf2..bf090a0438c 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -43,7 +43,8 @@ class Note < ActiveRecord::Base
   delegate :name, :email, to: :author, prefix: true
   delegate :title, to: :noteable, allow_nil: true
 
-  validates :note, :project, presence: true
+  validates :note, presence: true
+  validates :project, presence: true, unless: :for_personal_snippet?
 
   # Attachments are deprecated and are handled by Markdown uploader
   validates :attachment, file_size: { maximum: :max_attachment_size }
@@ -53,7 +54,7 @@ class Note < ActiveRecord::Base
   validates :commit_id, presence: true, if: :for_commit?
   validates :author, presence: true
 
-  validate unless: [:for_commit?, :importing?] do |note|
+  validate unless: [:for_commit?, :importing?, :for_personal_snippet?] do |note|
     unless note.noteable.try(:project) == note.project
       errors.add(:invalid_project, 'Note and noteable project mismatch')
     end
@@ -83,7 +84,7 @@ class Note < ActiveRecord::Base
   after_initialize :ensure_discussion_id
   before_validation :nullify_blank_type, :nullify_blank_line_code
   before_validation :set_discussion_id
-  after_save :keep_around_commit
+  after_save :keep_around_commit, unless: :for_personal_snippet?
 
   class << self
     def model_name
@@ -165,6 +166,14 @@ class Note < ActiveRecord::Base
     noteable_type == "Snippet"
   end
 
+  def for_personal_snippet?
+    noteable.is_a?(PersonalSnippet)
+  end
+
+  def skip_project_check?
+    for_personal_snippet?
+  end
+
   # override to return commits, which are not active record
   def noteable
     if for_commit?
@@ -220,6 +229,10 @@ class Note < ActiveRecord::Base
     note.match(Banzai::Filter::EmojiFilter.emoji_pattern)[1]
   end
 
+  def to_ability_name
+    for_personal_snippet? ? 'personal_snippet' : noteable_type.underscore
+  end
+
   private
 
   def keep_around_commit
diff --git a/app/models/project.rb b/app/models/project.rb
index 1630975b0d3..59faf35e051 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -121,8 +121,6 @@ class Project < ActiveRecord::Base
 
   # Merge Requests for target project should be removed with it
   has_many :merge_requests,     dependent: :destroy, foreign_key: 'target_project_id'
-  # Merge requests from source project should be kept when source project was removed
-  has_many :fork_merge_requests, foreign_key: 'source_project_id', class_name: 'MergeRequest'
   has_many :issues,             dependent: :destroy
   has_many :labels,             dependent: :destroy, class_name: 'ProjectLabel'
   has_many :services,           dependent: :destroy
@@ -226,6 +224,7 @@ class Project < ActiveRecord::Base
 
   scope :with_project_feature, -> { joins('LEFT JOIN project_features ON projects.id = project_features.project_id') }
   scope :with_statistics, -> { includes(:statistics) }
+  scope :with_shared_runners, -> { where(shared_runners_enabled: true) }
 
   # "enabled" here means "not disabled". It includes private features!
   scope :with_feature_enabled, ->(feature) {
@@ -1098,12 +1097,20 @@ class Project < ActiveRecord::Base
     project_feature.update_attribute(:builds_access_level, ProjectFeature::ENABLED)
   end
 
+  def shared_runners_available?
+    shared_runners_enabled?
+  end
+
+  def shared_runners
+    shared_runners_available? ? Ci::Runner.shared : Ci::Runner.none
+  end
+
   def any_runners?(&block)
     if runners.active.any?(&block)
       return true
     end
 
-    shared_runners_enabled? && Ci::Runner.shared.active.any?(&block)
+    shared_runners.active.any?(&block)
   end
 
   def valid_runners_token?(token)
diff --git a/app/models/project_group_link.rb b/app/models/project_group_link.rb
index 6149c35cc61..5cb6b0c527d 100644
--- a/app/models/project_group_link.rb
+++ b/app/models/project_group_link.rb
@@ -16,8 +16,7 @@ class ProjectGroupLink < ActiveRecord::Base
   validates :group_access, inclusion: { in: Gitlab::Access.values }, presence: true
   validate :different_group
 
-  after_create :refresh_group_members_authorized_projects
-  after_destroy :refresh_group_members_authorized_projects
+  after_commit :refresh_group_members_authorized_projects
 
   def self.access_options
     Gitlab::Access.options
diff --git a/app/models/route.rb b/app/models/route.rb
index caf596efa79..dd171fdb069 100644
--- a/app/models/route.rb
+++ b/app/models/route.rb
@@ -8,15 +8,16 @@ class Route < ActiveRecord::Base
     presence: true,
     uniqueness: { case_sensitive: false }
 
-  after_update :rename_children, if: :path_changed?
+  after_update :rename_descendants, if: :path_changed?
 
-  def rename_children
+  def rename_descendants
     # We update each row separately because MySQL does not have regexp_replace.
     # rubocop:disable Rails/FindEach
     Route.where('path LIKE ?', "#{path_was}/%").each do |route|
       # Note that update column skips validation and callbacks.
-      # We need this to avoid recursive call of rename_children method
+      # We need this to avoid recursive call of rename_descendants method
       route.update_column(:path, route.path.sub(path_was, path))
     end
+    # rubocop:enable Rails/FindEach
   end
 end
diff --git a/app/models/user.rb b/app/models/user.rb
index 06dd98a3188..54f5388eb2c 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -179,8 +179,8 @@ class User < ActiveRecord::Base
   scope :not_in_project, ->(project) { project.users.present? ? where("id not in (:ids)", ids: project.users.map(&:id) ) : all }
   scope :without_projects, -> { where('id NOT IN (SELECT DISTINCT(user_id) FROM members WHERE user_id IS NOT NULL AND requested_at IS NULL)') }
   scope :todo_authors, ->(user_id, state) { where(id: Todo.where(user_id: user_id, state: state).select(:author_id)) }
-  scope :order_recent_sign_in, -> { reorder(last_sign_in_at: :desc) }
-  scope :order_oldest_sign_in, -> { reorder(last_sign_in_at: :asc) }
+  scope :order_recent_sign_in, -> { reorder(Gitlab::Database.nulls_last_order('last_sign_in_at', 'DESC')) }
+  scope :order_oldest_sign_in, -> { reorder(Gitlab::Database.nulls_last_order('last_sign_in_at', 'ASC')) }
 
   def self.with_two_factor
     joins("LEFT OUTER JOIN u2f_registrations AS u2f ON u2f.user_id = users.id").
@@ -439,6 +439,15 @@ class User < ActiveRecord::Base
     Group.where("namespaces.id IN (#{union.to_sql})")
   end
 
+  def nested_groups
+    Group.member_descendants(id)
+  end
+
+  def nested_projects
+    Project.joins(:namespace).where('namespaces.parent_id IS NOT NULL').
+      member_descendants(id)
+  end
+
   def refresh_authorized_projects
     Users::RefreshAuthorizedProjectsService.new(self).execute
   end