Merge remote-tracking branch 'origin/master' into pipeline-hooks-without-slack

# Conflicts:
#	app/models/ci/pipeline.rb
#	app/services/ci/create_pipeline_service.rb
#	spec/models/project_services/hipchat_service_spec.rb

CHANGELOG

 Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.11.0 (unreleased)
+  - Remove the http_parser.rb dependency by removing the tinder gem. !5758 (tbalthazar)
   - Fix don't pass a local variable called `i` to a partial. !20510 (herminiotorres)
   - Fix rename `add_users_into_project` and `projects_ids`. !20512 (herminiotorres)
   - Fix the title of the toggle dropdown button. !5515 (herminiotorres)
   - Improve diff performance by eliminating redundant checks for text blobs
+  - Ensure that branch names containing escapable characters (e.g. %20) aren't unescaped indiscriminately. !5770 (ewiltshi)
   - Convert switch icon into icon font (ClemMakesApps)
   - API: Endpoints for enabling and disabling deploy keys
+  - API: List access requests, request access, approve, and deny access requests to a project or a group. !4833
   - Use long options for curl examples in documentation !5703 (winniehell)
   - Remove magic comments (`# encoding: UTF-8`) from Ruby files. !5456 (winniehell)
   - Add support for relative links starting with ./ or / to RelativeLinkFilter (winniehell)
@@ -17,6 +20,7 @@ v 8.11.0 (unreleased)
   - Cache the commit author in RequestStore to avoid extra lookups in PostReceive
   - Expand commit message width in repo view (ClemMakesApps)
   - Cache highlighted diff lines for merge requests
+  - Pre-create all builds for a Pipeline when the new Pipeline is created !5295
   - Fix of 'Commits being passed to custom hooks are already reachable when using the UI'
   - Fix awardable button mutuality loading spinners (ClemMakesApps)
   - Add support for using RequestStore within Sidekiq tasks via SIDEKIQ_REQUEST_STORE env variable
@@ -24,7 +28,9 @@ v 8.11.0 (unreleased)
   - Add "No one can push" as an option for protected branches. !5081
   - Improve performance of AutolinkFilter#text_parse by using XPath
   - Add experimental Redis Sentinel support !1877
+  - Fix branches page dropdown sort initial state (ClemMakesApps)
   - Environments have an url to link to
+  - Various redundant database indexes have been removed
   - Update `timeago` plugin to use multiple string/locale settings
   - Remove unused images (ClemMakesApps)
   - Limit git rev-list output count to one in forced push check
@@ -38,6 +44,7 @@ v 8.11.0 (unreleased)
   - Retrieve rendered HTML from cache in one request
   - Fix renaming repository when name contains invalid chararacters under project settings
   - Upgrade Grape from 0.13.0 to 0.15.0. !4601
+  - Trigram indexes for the "ci_runners" table have been removed to speed up UPDATE queries
   - Fix devise deprecation warnings.
   - Update version_sorter and use new interface for faster tag sorting
   - Optimize checking if a user has read access to a list of issues !5370
@@ -70,6 +77,7 @@ v 8.11.0 (unreleased)
   - Add GitLab Workhorse version to admin dashboard (Katarzyna Kobierska Ula Budziszewska)
   - Allow branch names ending with .json for graph and network page !5579 (winniehell)
   - Add the `sprockets-es6` gem
+  - Improve OAuth2 client documentation (muteor)
   - Multiple trigger variables show in separate lines (Katarzyna Kobierska Ula Budziszewska)
   - Profile requests when a header is passed
   - Avoid calculation of line_code and position for _line partial when showing diff notes on discussion tab.
@@ -93,6 +101,7 @@ v 8.11.0 (unreleased)
   - Avoid to show the original password field when password is automatically set. !5712 (duduribeiro)
   - Fix importing GitLab projects with an invalid MR source project
   - Sort folders with submodules in Files view !5521
+  - Each `File::exists?` replaced to `File::exist?` because of deprecate since ruby version 2.2.0
 
 v 8.10.5
   - Add a data migration to fix some missing timestamps in the members table. !5670

Gemfile

@@ -163,9 +163,6 @@ gem 'redis-rails', '~> 4.0.0'
 gem 'redis', '~> 3.2'
 gem 'connection_pool', '~> 2.0'
 
-# Campfire integration
-gem 'tinder', '~> 1.10.0'
-
 # HipChat integration
 gem 'hipchat', '~> 1.5.0'
 

Gemfile.lock

@@ -335,7 +335,6 @@ GEM
       activesupport (>= 2)
       nokogiri (~> 1.4)
     htmlentities (4.3.4)
-    http_parser.rb (0.5.3)
     httparty (0.13.7)
       json (~> 1.8)
       multi_xml (>= 0.5.2)
@@ -672,7 +671,6 @@ GEM
       redis-namespace (>= 1.5.2)
       rufus-scheduler (>= 2.0.24)
       sidekiq (>= 4.0.0)
-    simple_oauth (0.1.9)
     simplecov (0.12.0)
       docile (~> 1.1.0)
       json (>= 1.8, < 3)
@@ -742,21 +740,8 @@ GEM
     tilt (2.0.5)
     timecop (0.8.1)
     timfel-krb5-auth (0.8.3)
-    tinder (1.10.1)
-      eventmachine (~> 1.0)
-      faraday (~> 0.9.0)
-      faraday_middleware (~> 0.9)
-      hashie (>= 1.0)
-      json (~> 1.8.0)
-      mime-types
-      multi_json (~> 1.7)
-      twitter-stream (~> 0.1)
     turbolinks (2.5.3)
       coffee-rails
-    twitter-stream (0.1.16)
-      eventmachine (>= 0.12.8)
-      http_parser.rb (~> 0.5.1)
-      simple_oauth (~> 0.1.4)
     tzinfo (1.2.2)
       thread_safe (~> 0.1)
     u2f (0.2.1)
@@ -981,7 +966,6 @@ DEPENDENCIES
   teaspoon-jasmine (~> 2.2.0)
   test_after_commit (~> 0.4.2)
   thin (~> 1.7.0)
-  tinder (~> 1.10.0)
   turbolinks (~> 2.5.0)
   u2f (~> 0.2.1)
   uglifier (~> 2.7.2)

app/controllers/projects/branches_controller.rb

 class Projects::BranchesController < Projects::ApplicationController
   include ActionView::Helpers::SanitizeHelper
+  include SortingHelper
   # Authorize
   before_action :require_non_empty_project
   before_action :authorize_download_code!
   before_action :authorize_push_code!, only: [:new, :create, :destroy]
 
   def index
-    @sort = params[:sort].presence || 'name'
+    @sort = params[:sort].presence || sort_value_name
     @branches = BranchesFinder.new(@repository, params).execute
     @branches = Kaminari.paginate_array(@branches).page(params[:page])
 

app/controllers/projects/builds_controller.rb

@@ -6,7 +6,7 @@ class Projects::BuildsController < Projects::ApplicationController
 
   def index
     @scope = params[:scope]
-    @all_builds = project.builds
+    @all_builds = project.builds.relevant
     @builds = @all_builds.order('created_at DESC')
     @builds =
       case @scope

app/controllers/projects/commit_controller.rb

@@ -134,8 +134,8 @@ class Projects::CommitController < Projects::ApplicationController
   end
 
   def define_status_vars
-    @statuses = CommitStatus.where(pipeline: pipelines)
-    @builds = Ci::Build.where(pipeline: pipelines)
+    @statuses = CommitStatus.where(pipeline: pipelines).relevant
+    @builds = Ci::Build.where(pipeline: pipelines).relevant
   end
 
   def assign_change_commit_vars(mr_source_branch)

app/controllers/projects/git_http_client_controller.rb

+# This file should be identical in GitLab Community Edition and Enterprise Edition
+
+class Projects::GitHttpClientController < Projects::ApplicationController
+  include ActionController::HttpAuthentication::Basic
+  include KerberosSpnegoHelper
+
+  attr_reader :user
+
+  # Git clients will not know what authenticity token to send along
+  skip_before_action :verify_authenticity_token
+  skip_before_action :repository
+  before_action :authenticate_user
+  before_action :ensure_project_found!
+
+  private
+
+  def authenticate_user
+    if project && project.public? && download_request?
+      return # Allow access
+    end
+
+    if allow_basic_auth? && basic_auth_provided?
+      login, password = user_name_and_password(request)
+      auth_result = Gitlab::Auth.find_for_git_client(login, password, project: project, ip: request.ip)
+
+      if auth_result.type == :ci && download_request?
+        @ci = true
+      elsif auth_result.type == :oauth && !download_request?
+        # Not allowed
+      else
+        @user = auth_result.user
+      end
+
+      if ci? || user
+        return # Allow access
+      end
+    elsif allow_kerberos_spnego_auth? && spnego_provided?
+      @user = find_kerberos_user
+
+      if user
+        send_final_spnego_response
+        return # Allow access
+      end
+    end
+
+    send_challenges
+    render plain: "HTTP Basic: Access denied\n", status: 401
+  end
+
+  def basic_auth_provided?
+    has_basic_credentials?(request)
+  end
+
+  def send_challenges
+    challenges = []
+    challenges << 'Basic realm="GitLab"' if allow_basic_auth?
+    challenges << spnego_challenge if allow_kerberos_spnego_auth?
+    headers['Www-Authenticate'] = challenges.join("\n") if challenges.any?
+  end
+
+  def ensure_project_found!
+    render_not_found if project.blank?
+  end
+
+  def project
+    return @project if defined?(@project)
+
+    project_id, _ = project_id_with_suffix
+    if project_id.blank?
+      @project = nil
+    else
+      @project = Project.find_with_namespace("#{params[:namespace_id]}/#{project_id}")
+    end
+  end
+
+  # This method returns two values so that we can parse
+  # params[:project_id] (untrusted input!) in exactly one place.
+  def project_id_with_suffix
+    id = params[:project_id] || ''
+
+    %w[.wiki.git .git].each do |suffix|
+      if id.end_with?(suffix)
+        # Be careful to only remove the suffix from the end of 'id'.
+        # Accidentally removing it from the middle is how security
+        # vulnerabilities happen!
+        return [id.slice(0, id.length - suffix.length), suffix]
+      end
+    end
+
+    # Something is wrong with params[:project_id]; do not pass it on.
+    [nil, nil]
+  end
+
+  def repository
+    _, suffix = project_id_with_suffix
+    if suffix == '.wiki.git'
+      project.wiki.repository
+    else
+      project.repository
+    end
+  end
+
+  def render_not_found
+    render plain: 'Not Found', status: :not_found
+  end
+
+  def ci?
+    @ci.present?
+  end
+end

app/controllers/projects/git_http_controller.rb

 # This file should be identical in GitLab Community Edition and Enterprise Edition
 
-class Projects::GitHttpController < Projects::ApplicationController
-  include ActionController::HttpAuthentication::Basic
-  include KerberosSpnegoHelper
-
-  attr_reader :user
-
-  # Git clients will not know what authenticity token to send along
-  skip_before_action :verify_authenticity_token
-  skip_before_action :repository
-  before_action :authenticate_user
-  before_action :ensure_project_found!
-
+class Projects::GitHttpController < Projects::GitHttpClientController
   # GET /foo/bar.git/info/refs?service=git-upload-pack (git pull)
   # GET /foo/bar.git/info/refs?service=git-receive-pack (git push)
   def info_refs
@@ -46,81 +35,8 @@ class Projects::GitHttpController < Projects::ApplicationController
 
   private
 
-  def authenticate_user
-    if project && project.public? && upload_pack?
-      return # Allow access
-    end
-
-    if allow_basic_auth? && basic_auth_provided?
-      login, password = user_name_and_password(request)
-      auth_result = Gitlab::Auth.find_for_git_client(login, password, project: project, ip: request.ip)
-
-      if auth_result.type == :ci && upload_pack?
-        @ci = true
-      elsif auth_result.type == :oauth && !upload_pack?
-        # Not allowed
-      else
-        @user = auth_result.user
-      end
-
-      if ci? || user
-        return # Allow access
-      end
-    elsif allow_kerberos_spnego_auth? && spnego_provided?
-      @user = find_kerberos_user
-
-      if user
-        send_final_spnego_response
-        return # Allow access
-      end
-    end
-
-    send_challenges
-    render plain: "HTTP Basic: Access denied\n", status: 401
-  end
-
-  def basic_auth_provided?
-    has_basic_credentials?(request)
-  end
-
-  def send_challenges
-    challenges = []
-    challenges << 'Basic realm="GitLab"' if allow_basic_auth?
-    challenges << spnego_challenge if allow_kerberos_spnego_auth?
-    headers['Www-Authenticate'] = challenges.join("\n") if challenges.any?
-  end
-
-  def ensure_project_found!
-    render_not_found if project.blank?
-  end
-
-  def project
-    return @project if defined?(@project)
-
-    project_id, _ = project_id_with_suffix
-    if project_id.blank?
-      @project = nil
-    else
-      @project = Project.find_with_namespace("#{params[:namespace_id]}/#{project_id}")
-    end
-  end
-
-  # This method returns two values so that we can parse
-  # params[:project_id] (untrusted input!) in exactly one place.
-  def project_id_with_suffix
-    id = params[:project_id] || ''
-
-    %w[.wiki.git .git].each do |suffix|
-      if id.end_with?(suffix)
-        # Be careful to only remove the suffix from the end of 'id'.
-        # Accidentally removing it from the middle is how security
-        # vulnerabilities happen!
-        return [id.slice(0, id.length - suffix.length), suffix]
-      end
-    end
-
-    # Something is wrong with params[:project_id]; do not pass it on.
-    [nil, nil]
+  def download_request?
+    upload_pack?
   end
 
   def upload_pack?
@@ -143,19 +59,6 @@ class Projects::GitHttpController < Projects::ApplicationController
     render json: Gitlab::Workhorse.git_http_ok(repository, user)
   end
 
-  def repository
-    _, suffix = project_id_with_suffix
-    if suffix == '.wiki.git'
-      project.wiki.repository
-    else
-      project.repository
-    end
-  end
-
-  def render_not_found
-    render plain: 'Not Found', status: :not_found
-  end
-
   def render_http_not_allowed
     render plain: access_check.message, status: :forbidden
   end
@@ -169,10 +72,6 @@ class Projects::GitHttpController < Projects::ApplicationController
     end
   end
 
-  def ci?
-    @ci.present?
-  end
-
   def upload_pack_allowed?
     return false unless Gitlab.config.gitlab_shell.upload_pack
 

app/controllers/projects/lfs_api_controller.rb

+class Projects::LfsApiController < Projects::GitHttpClientController
+  include LfsHelper
+
+  before_action :require_lfs_enabled!
+  before_action :lfs_check_access!, except: [:deprecated]
+
+  def batch
+    unless objects.present?
+      render_lfs_not_found
+      return
+    end
+
+    if download_request?
+      render json: { objects: download_objects! }
+    elsif upload_request?
+      render json: { objects: upload_objects! }
+    else
+      raise "Never reached"
+    end
+  end
+
+  def deprecated
+    render(
+      json: {
+        message: 'Server supports batch API only, please update your Git LFS client to version 1.0.1 and up.',
+        documentation_url: "#{Gitlab.config.gitlab.url}/help",
+      },
+      status: 501
+    )
+  end
+
+  private
+
+  def objects
+    @objects ||= (params[:objects] || []).to_a
+  end
+
+  def existing_oids
+    @existing_oids ||= begin
+      storage_project.lfs_objects.where(oid: objects.map { |o| o['oid'].to_s }).pluck(:oid)
+    end
+  end
+
+  def download_objects!
+    objects.each do |object|
+      if existing_oids.include?(object[:oid])
+        object[:actions] = download_actions(object)
+      else
+        object[:error] = {
+          code: 404,
+          message: "Object does not exist on the server or you don't have permissions to access it",
+        }
+      end
+    end
+    objects
+  end
+
+  def upload_objects!
+    objects.each do |object|
+      object[:actions] = upload_actions(object) unless existing_oids.include?(object[:oid])
+    end
+    objects
+  end
+
+  def download_actions(object)
+    {
+      download: {
+        href: "#{project.http_url_to_repo}/gitlab-lfs/objects/#{object[:oid]}",
+        header: {
+          Authorization: request.headers['Authorization']
+        }.compact
+      }
+    }
+  end
+
+  def upload_actions(object)
+    {
+      upload: {
+        href: "#{project.http_url_to_repo}/gitlab-lfs/objects/#{object[:oid]}/#{object[:size]}",
+        header: {
+          Authorization: request.headers['Authorization']
+        }.compact
+      }
+    }
+  end
+
+  def download_request?
+    params[:operation] == 'download'
+  end
+
+  def upload_request?
+    params[:operation] == 'upload'
+  end
+end

app/controllers/projects/lfs_storage_controller.rb

+class Projects::LfsStorageController < Projects::GitHttpClientController
+  include LfsHelper
+
+  before_action :require_lfs_enabled!
+  before_action :lfs_check_access!
+
+  def download
+    lfs_object = LfsObject.find_by_oid(oid)
+    unless lfs_object && lfs_object.file.exists?
+      render_lfs_not_found
+      return
+    end
+
+    send_file lfs_object.file.path, content_type: "application/octet-stream"
+  end
+
+  def upload_authorize
+    render(
+      json: {
+        StoreLFSPath: "#{Gitlab.config.lfs.storage_path}/tmp/upload",
+        LfsOid: oid,
+        LfsSize: size,
+      },
+      content_type: 'application/json; charset=utf-8'
+    )
+  end
+
+  def upload_finalize
+    unless tmp_filename
+      render_lfs_forbidden
+      return
+    end
+
+    if store_file(oid, size, tmp_filename)
+      head 200
+    else
+      render plain: 'Unprocessable entity', status: 422
+    end
+  end
+
+  private
+
+  def download_request?
+    action_name == 'download'
+  end
+
+  def upload_request?
+    %w[upload_authorize upload_finalize].include? action_name
+  end
+
+  def oid
+    params[:oid].to_s
+  end
+
+  def size
+    params[:size].to_i
+  end
+
+  def tmp_filename
+    name = request.headers['X-Gitlab-Lfs-Tmp']
+    return if name.include?('/')
+    return unless oid.present? && name.start_with?(oid)
+    name
+  end
+
+  def store_file(oid, size, tmp_file)
+    # Define tmp_file_path early because we use it in "ensure"
+    tmp_file_path = File.join("#{Gitlab.config.lfs.storage_path}/tmp/upload", tmp_file)
+
+    object = LfsObject.find_or_create_by(oid: oid, size: size)
+    file_exists = object.file.exists? || move_tmp_file_to_storage(object, tmp_file_path)
+    file_exists && link_to_project(object)
+  ensure
+    FileUtils.rm_f(tmp_file_path)
+  end
+
+  def move_tmp_file_to_storage(object, path)
+    File.open(path) do |f|
+      object.file = f
+    end
+
+    object.file.store!
+    object.save
+  end
+
+  def link_to_project(object)
+    if object && !object.projects.exists?(storage_project.id)
+      object.projects << storage_project
+      object.save
+    end
+  end
+end

app/controllers/projects/merge_requests_controller.rb

@@ -160,7 +160,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     @diff_notes_disabled = true
 
     @pipeline = @merge_request.pipeline
-    @statuses = @pipeline.statuses if @pipeline
+    @statuses = @pipeline.statuses.relevant if @pipeline
 
     @note_counts = Note.where(commit_id: @commits.map(&:id)).
       group(:commit_id).count
@@ -362,7 +362,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     @commits_count = @merge_request.commits.count
 
     @pipeline = @merge_request.pipeline
-    @statuses = @pipeline.statuses if @pipeline
+    @statuses = @pipeline.statuses.relevant if @pipeline
 
     if @merge_request.locked_long_ago?
       @merge_request.unlock_mr

app/controllers/projects/pipelines_controller.rb

@@ -19,7 +19,7 @@ class Projects::PipelinesController < Projects::ApplicationController
   end
 
   def create
-    @pipeline = Ci::CreatePipelineService.new(project, current_user, create_params).execute
+    @pipeline = Ci::CreatePipelineService.new(project, current_user, create_params).execute(ignore_skip_ci: true, save_on_errors: false)
     unless @pipeline.persisted?
       render 'new'
       return

app/helpers/lfs_helper.rb

+module LfsHelper
+  def require_lfs_enabled!
+    return if Gitlab.config.lfs.enabled
+
+    render(
+      json: {
+        message: 'Git LFS is not enabled on this GitLab server, contact your admin.',
+        documentation_url: "#{Gitlab.config.gitlab.url}/help",
+      },
+      status: 501
+    )
+  end
+
+  def lfs_check_access!
+    return if download_request? && lfs_download_access?
+    return if upload_request? && lfs_upload_access?
+
+    if project.public? || (user && user.can?(:read_project, project))
+      render_lfs_forbidden
+    else
+      render_lfs_not_found
+    end
+  end
+
+  def lfs_download_access?
+    project.public? || ci? || (user && user.can?(:download_code, project))
+  end
+
+  def lfs_upload_access?
+    user && user.can?(:push_code, project)
+  end
+
+  def render_lfs_forbidden
+    render(
+      json: {
+        message: 'Access forbidden. Check your access level.',
+        documentation_url: "#{Gitlab.config.gitlab.url}/help",
+      },
+      content_type: "application/vnd.git-lfs+json",
+      status: 403
+    )
+  end
+
+  def render_lfs_not_found
+    render(
+      json: {
+        message: 'Not found.',
+        documentation_url: "#{Gitlab.config.gitlab.url}/help",
+      },
+      content_type: "application/vnd.git-lfs+json",
+      status: 404
+    )
+  end
+
+  def storage_project
+    @storage_project ||= begin
+      result = project
+
+      loop do
+        break unless result.forked?
+        result = result.forked_from_project
+      end
+
+      result
+    end
+  end
+end

app/models/ci/build.rb

@@ -16,7 +16,7 @@ module Ci
     scope :with_artifacts_not_expired, ->() { with_artifacts.where('artifacts_expire_at IS NULL OR artifacts_expire_at > ?', Time.now) }
     scope :with_expired_artifacts, ->() { with_artifacts.where('artifacts_expire_at < ?', Time.now) }
     scope :last_month, ->() { where('created_at > ?', Date.today - 1.month) }
-    scope :manual_actions, ->() { where(when: :manual) }
+    scope :manual_actions, ->() { where(when: :manual).relevant }
 
     mount_uploader :artifacts_file, ArtifactUploader
     mount_uploader :artifacts_metadata, ArtifactUploader
@@ -65,17 +65,11 @@ module Ci
       end
     end
 
-    state_machine :status, initial: :pending do
+    state_machine :status do
       after_transition pending: :running do |build|
         build.execute_hooks
       end
 
-      # We use around_transition to create builds for next stage as soon as possible, before the `after_*` is executed
-      around_transition any => [:success, :failed, :canceled] do |build, block|
-        block.call
-        build.pipeline.create_next_builds(build) if build.pipeline
-      end
-
       after_transition any => [:success, :failed, :canceled] do |build|
         build.update_coverage
         build.execute_hooks

app/models/ci/pipeline.rb

@@ -13,11 +13,10 @@ module Ci
     has_many :trigger_requests, dependent: :destroy, class_name: 'Ci::TriggerRequest', foreign_key: :commit_id
 
     validates_presence_of :sha
+    validates_presence_of :ref
     validates_presence_of :status
     validate :valid_commit_sha
 
-    # Invalidate object and save if when touched
-    after_touch :update_state
     after_save :keep_around_commits
 
     # ref can't be HEAD or SHA, can only be branch/tag name
@@ -90,12 +89,16 @@ module Ci
 
     def cancel_running
       builds.running_or_pending.each(&:cancel)
+
+      reload_status!
     end
 
     def retry_failed(user)
       builds.latest.failed.select(&:retryable?).each do |build|
         Ci::Build.retry(build, user)
       end
+
+      reload_status!
     end
 
     def latest?
@@ -109,37 +112,6 @@ module Ci
       trigger_requests.any?
     end
 
-    def create_builds(user, trigger_request = nil)
-      ##
-      # We persist pipeline only if there are builds available
-      #
-      return unless config_processor
-
-      build_builds_for_stages(config_processor.stages, user,
-                              'success', trigger_request) && save
-    end
-
-    def create_next_builds(build)
-      return unless config_processor
-
-      # don't create other builds if this one is retried
-      latest_builds = builds.latest
-      return unless latest_builds.exists?(build.id)
-
-      # get list of stages after this build
-      next_stages = config_processor.stages.drop_while { |stage| stage != build.stage }
-      next_stages.delete(build.stage)
-
-      # get status for all prior builds
-      prior_builds = latest_builds.where.not(stage: next_stages)
-      prior_status = prior_builds.status
-
-      # build builds for next stage that has builds available
-      # and save pipeline if we have builds
-      build_builds_for_stages(next_stages, build.user, prior_status,
-                              build.trigger_request) && save
-    end
-
     def retried
       @retried ||= (statuses.order(id: :desc) - statuses.latest)
     end
@@ -151,6 +123,14 @@ module Ci
       end
     end
 
+    def config_builds_attributes
+      return [] unless config_processor
+
+      config_processor.
+        builds_for_ref(ref, tag?, trigger_requests.first).
+        sort_by { |build| build[:stage_idx] }
+    end
+
     def has_warnings?
       builds.latest.ignored.any?
     end
@@ -182,10 +162,6 @@ module Ci
       end
     end
 
-    def skip_ci?
-      git_commit_message =~ /\[(ci skip|skip ci)\]/i if git_commit_message
-    end
-
     def environments
       builds.where.not(environment: nil).success.pluck(:environment).uniq
     end
@@ -207,50 +183,34 @@ module Ci
       Note.for_commit_id(sha)
     end
 
+    def process!
+      Ci::ProcessPipelineService.new(project, user).execute(self)
+      reload_status!
+    end
+
     def predefined_variables
       [
         { key: 'CI_PIPELINE_ID', value: id.to_s, public: true }
       ]
     end
 
-    private
-
-    def build_builds_for_stages(stages, user, status, trigger_request)
-      ##
-      # Note that `Array#any?` implements a short circuit evaluation, so we
-      # build builds only for the first stage that has builds available.
-      #
-      stages.any? do |stage|
-        CreateBuildsService.new(self).
-          execute(stage, user, status, trigger_request).
-          any?(&:active?)
-      end
-    end
-
-    def update_state
-      last_status = status
-
-      if update_state_from_commit_statuses
-        execute_hooks if last_status != status
-        true
-      else
-        false
-      end
-    end
-
-    def update_state_from_commit_statuses
+    def reload_status!
       statuses.reload
-      self.status = if yaml_errors.blank?
-                      statuses.latest.status || 'skipped'
-                    else
-                      'failed'
-                    end
+      self.status =
+        if yaml_errors.blank?
+          statuses.latest.status || 'skipped'
+        else
+          'failed'
+        end
       self.started_at = statuses.started_at
       self.finished_at = statuses.finished_at
       self.duration = statuses.latest.duration
       save
+      execute_hooks if status_changed?
     end
 
+    private
+
     def execute_hooks
       project.execute_hooks(pipeline_data, :pipeline_hooks)
       project.execute_services(pipeline_data, :pipeline_hooks)

app/models/commit_status.rb

@@ -5,7 +5,7 @@ class CommitStatus < ActiveRecord::Base
   self.table_name = 'ci_builds'
 
   belongs_to :project, class_name: '::Project', foreign_key: :gl_project_id
-  belongs_to :pipeline, class_name: 'Ci::Pipeline', foreign_key: :commit_id, touch: true
+  belongs_to :pipeline, class_name: 'Ci::Pipeline', foreign_key: :commit_id
   belongs_to :user
 
   delegate :commit, to: :pipeline
@@ -25,28 +25,36 @@ class CommitStatus < ActiveRecord::Base
   scope :ordered, -> { order(:name) }
   scope :ignored, -> { where(allow_failure: true, status: [:failed, :canceled]) }
 
-  state_machine :status, initial: :pending do
+  state_machine :status do
     event :queue do
-      transition skipped: :pending
+      transition [:created, :skipped] => :pending
     end
 
     event :run do
       transition pending: :running
     end
 
+    event :skip do
+      transition [:created, :pending] => :skipped
+    end
+
     event :drop do
-      transition [:pending, :running] => :failed
+      transition [:created, :pending, :running] => :failed
     end
 
     event :success do
-      transition [:pending, :running] => :success
+      transition [:created, :pending, :running] => :success
     end
 
     event :cancel do
-      transition [:pending, :running] => :canceled
+      transition [:created, :pending, :running] => :canceled
+    end
+
+    after_transition created: [:pending, :running] do |commit_status|
+      commit_status.update_attributes queued_at: Time.now
     end
 
-    after_transition pending: :running do |commit_status|
+    after_transition [:created, :pending] => :running do |commit_status|
       commit_status.update_attributes started_at: Time.now
     end
 
@@ -54,13 +62,20 @@ class CommitStatus < ActiveRecord::Base
       commit_status.update_attributes finished_at: Time.now
     end
 
-    after_transition [:pending, :running] => :success do |commit_status|
+    after_transition [:created, :pending, :running] => :success do |commit_status|
       MergeRequests::MergeWhenBuildSucceedsService.new(commit_status.pipeline.project, nil).trigger(commit_status)
     end
 
     after_transition any => :failed do |commit_status|
       MergeRequests::AddTodoWhenBuildFailsService.new(commit_status.pipeline.project, nil).execute(commit_status)
     end
+
+    # We use around_transition to process pipeline on next stages as soon as possible, before the `after_*` is executed
+    around_transition any => [:success, :failed, :canceled] do |commit_status, block|
+      block.call
+
+      commit_status.pipeline.process! if commit_status.pipeline
+    end
   end
 
   delegate :sha, :short_sha, to: :pipeline

app/models/concerns/statuseable.rb

 module Statuseable
   extend ActiveSupport::Concern
 
-  AVAILABLE_STATUSES = %w(pending running success failed canceled skipped)
+  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]
 
   class_methods do
     def status_sql
-      builds = all.select('count(*)').to_sql
-      success = all.success.select('count(*)').to_sql
-      ignored = all.ignored.select('count(*)').to_sql if all.respond_to?(:ignored)
+      scope = all.relevant
+      builds = scope.select('count(*)').to_sql
+      success = scope.success.select('count(*)').to_sql
+      ignored = scope.ignored.select('count(*)').to_sql if scope.respond_to?(:ignored)
       ignored ||= '0'
-      pending = all.pending.select('count(*)').to_sql
-      running = all.running.select('count(*)').to_sql
-      canceled = all.canceled.select('count(*)').to_sql
-      skipped = all.skipped.select('count(*)').to_sql
+      pending = scope.pending.select('count(*)').to_sql
+      running = scope.running.select('count(*)').to_sql
+      canceled = scope.canceled.select('count(*)').to_sql
+      skipped = scope.skipped.select('count(*)').to_sql
 
       deduce_status = "(CASE
         WHEN (#{builds})=0 THEN NULL
@@ -48,7 +52,8 @@ module Statuseable
   included do
     validates :status, inclusion: { in: AVAILABLE_STATUSES }
 
-    state_machine :status, initial: :pending do
+    state_machine :status, initial: :created do
+      state :created, value: 'created'
       state :pending, value: 'pending'
       state :running, value: 'running'
       state :failed, value: 'failed'
@@ -57,6 +62,8 @@ module Statuseable
       state :skipped, value: 'skipped'
     end
 
+    scope :created, -> { where(status: 'created') }
+    scope :relevant, -> { where.not(status: 'created') }
     scope :running, -> { where(status: 'running') }
     scope :pending, -> { where(status: 'pending') }
     scope :success, -> { where(status: 'success') }
@@ -68,14 +75,14 @@ module Statuseable
   end
 
   def started?
-    !pending? && !canceled? && started_at
+    STARTED_STATUSES.include?(status) && started_at
   end
 
   def active?
-    running? || pending?
+    ACTIVE_STATUSES.include?(status)
   end
 
   def complete?
-    canceled? || success? || failed?
+    COMPLETED_STATUSES.include?(status)
   end
 end

app/models/members/project_member.rb

@@ -8,6 +8,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 :access_level, inclusion: { in: Gitlab::Access.values }
   default_scope { where(source_type: SOURCE_TYPE) }
 
   scope :in_project, ->(project) { where(source_id: project.id) }

app/models/project.rb

@@ -999,6 +999,10 @@ class Project < ActiveRecord::Base
     project_members.find_by(user_id: user)
   end
 
+  def add_user(user, access_level, current_user = nil)
+    team.add_user(user, access_level, current_user)
+  end
+
   def default_branch
     @default_branch ||= repository.root_ref if repository.exists?
   end

app/models/project_services/campfire_service.rb

 class CampfireService < Service
+  include HTTParty
+
   prop_accessor :token, :subdomain, :room
   validates :token, presence: true, if: :activated?
 
@@ -29,18 +31,53 @@ class CampfireService < Service
   def execute(data)
     return unless supported_events.include?(data[:object_kind])
 
-    room = gate.find_room_by_name(self.room)
-    return true unless room
-
+    self.class.base_uri base_uri
     message = build_message(data)
-
-    room.speak(message)
+    speak(self.room, message, auth)
   end
 
   private
 
-  def gate
-    @gate ||= Tinder::Campfire.new(subdomain, token: token)
+  def base_uri
+    @base_uri ||= "https://#{subdomain}.campfirenow.com"
+  end
+
+  def auth
+    # use a dummy password, as explained in the Campfire API doc:
+    # https://github.com/basecamp/campfire-api#authentication
+    @auth ||= {
+      basic_auth: {
+        username: token,
+        password: 'X'
+      }
+    }
+  end
+
+  # Post a message into a room, returns the message Hash in case of success.
+  # Returns nil otherwise.
+  # https://github.com/basecamp/campfire-api/blob/master/sections/messages.md#create-message
+  def speak(room_name, message, auth)
+    room = rooms(auth).find { |r| r["name"] == room_name }
+    return nil unless room
+
+    path = "/room/#{room["id"]}/speak.json"
+    body = {
+      body: {
+        message: {
+          type: 'TextMessage',
+          body: message
+        }
+      }
+    }
+    res = self.class.post(path, auth.merge(body))
+    res.code == 201 ? res : nil
+  end
+
+  # Returns a list of rooms, or [].
+  # https://github.com/basecamp/campfire-api/blob/master/sections/rooms.md#get-rooms
+  def rooms(auth)
+    res = self.class.get("/rooms.json", auth) 
+    res.code == 200 ? res["rooms"] : []
   end
 
   def build_message(push)

app/services/ci/create_builds_service.rb

-module Ci
-  class CreateBuildsService
-    def initialize(pipeline)
-      @pipeline = pipeline
-      @config = pipeline.config_processor
-    end
-
-    def execute(stage, user, status, trigger_request = nil)
-      builds_attrs = @config.builds_for_stage_and_ref(stage, @pipeline.ref, @pipeline.tag, trigger_request)
-
-      # check when to create next build
-      builds_attrs = builds_attrs.select do |build_attrs|
-        case build_attrs[:when]
-        when 'on_success'
-          status == 'success'
-        when 'on_failure'
-          status == 'failed'
-        when 'always', 'manual'
-          %w(success failed).include?(status)
-        end
-      end
-
-      # don't create the same build twice
-      builds_attrs.reject! do |build_attrs|
-        @pipeline.builds.find_by(ref: @pipeline.ref,
-                                 tag: @pipeline.tag,
-                                 trigger_request: trigger_request,
-                                 name: build_attrs[:name])
-      end
-
-      builds_attrs.map do |build_attrs|
-        build_attrs.slice!(:name,
-                           :commands,
-                           :tag_list,
-                           :options,
-                           :allow_failure,
-                           :stage,
-                           :stage_idx,
-                           :environment,
-                           :when,
-                           :yaml_variables)
-
-        build_attrs.merge!(pipeline: @pipeline,
-                           ref: @pipeline.ref,
-                           tag: @pipeline.tag,
-                           trigger_request: trigger_request,
-                           user: user,
-                           project: @pipeline.project)
-
-        # TODO: The proper implementation for this is in
-        # https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5295
-        build_attrs[:status] = 'skipped' if build_attrs[:when] == 'manual'
-
-        ##
-        # We do not persist new builds here.
-        # Those will be persisted when @pipeline is saved.
-        #
-        @pipeline.builds.new(build_attrs)
-      end
-    end
-  end
-end

app/services/ci/create_pipeline_builds_service.rb

+module Ci
+  class CreatePipelineBuildsService < BaseService
+    attr_reader :pipeline
+
+    def execute(pipeline)
+      @pipeline = pipeline
+
+      new_builds.map do |build_attributes|
+        create_build(build_attributes)
+      end
+    end
+
+    private
+
+    def create_build(build_attributes)
+      build_attributes = build_attributes.merge(
+        pipeline: pipeline,
+        project: pipeline.project,
+        ref: pipeline.ref,
+        tag: pipeline.tag,
+        user: current_user,
+        trigger_request: trigger_request
+      )
+      pipeline.builds.create(build_attributes)
+    end
+
+    def new_builds
+      @new_builds ||= pipeline.config_builds_attributes.
+        reject { |build| existing_build_names.include?(build[:name]) }
+    end
+
+    def existing_build_names
+      @existing_build_names ||= pipeline.builds.pluck(:name)
+    end
+
+    def trigger_request
+      return @trigger_request if defined?(@trigger_request)
+
+      @trigger_request ||= pipeline.trigger_requests.first
+    end
+  end
+end

app/services/ci/create_pipeline_service.rb

 module Ci
   class CreatePipelineService < BaseService
-    def execute
-      pipeline = project.pipelines.new(params)
-      pipeline.user = current_user
+    attr_reader :pipeline
 
-      unless ref_names.include?(params[:ref])
-        pipeline.errors.add(:base, 'Reference not found')
-        return pipeline
+    def execute(ignore_skip_ci: false, save_on_errors: true, trigger_request: nil)
+      @pipeline = Ci::Pipeline.new(
+        project: project,
+        ref: ref,
+        sha: sha,
+        before_sha: before_sha,
+        tag: tag?,
+        trigger_requests: Array(trigger_request),
+        user: current_user
+      )
+
+      unless project.builds_enabled?
+        return error('Pipeline is disabled')
+      end
+
+      unless trigger_request || can?(current_user, :create_pipeline, project)
+        return error('Insufficient permissions to create a new pipeline')
       end
 
-      if commit
-        pipeline.sha = commit.id
-      else
-        pipeline.errors.add(:base, 'Commit not found')
-        return pipeline
+      unless branch? || tag?
+        return error('Reference not found')
       end
 
-      unless can?(current_user, :create_pipeline, project)
-        pipeline.errors.add(:base, 'Insufficient permissions to create a new pipeline')
-        return pipeline
+      unless commit
+        return error('Commit not found')
       end
 
       unless pipeline.config_processor
-        pipeline.errors.add(:base, pipeline.yaml_errors || 'Missing .gitlab-ci.yml file')
-        return pipeline
+        unless pipeline.ci_yaml_file
+          return error('Missing .gitlab-ci.yml file')
+        end
+        return error(pipeline.yaml_errors, save: save_on_errors)
       end
 
-      pipeline.save!
+      if !ignore_skip_ci && skip_ci?
+        return error('Creation of pipeline is skipped', save: save_on_errors)
+      end
 
-      unless pipeline.create_builds(current_user)
-        pipeline.errors.add(:base, 'No builds for this pipeline.')
+      unless pipeline.config_builds_attributes.present?
+        return error('No builds for this pipeline.')
       end
 
       pipeline.save
-      pipeline.touch
+      pipeline.process!
       pipeline
     end
 
     private
 
-    def ref_names
-      @ref_names ||= project.repository.ref_names
+    def skip_ci?
+      pipeline.git_commit_message =~ /\[(ci skip|skip ci)\]/i if pipeline.git_commit_message
     end
 
     def commit
-      @commit ||= project.commit(params[:ref])
+      @commit ||= project.commit(origin_sha || origin_ref)
+    end
+
+    def sha
+      commit.try(:id)
+    end
+
+    def before_sha
+      params[:checkout_sha] || params[:before] || Gitlab::Git::BLANK_SHA
+    end
+
+    def origin_sha
+      params[:checkout_sha] || params[:after]
+    end
+
+    def origin_ref
+      params[:ref]
+    end
+
+    def branch?
+      project.repository.ref_exists?(Gitlab::Git::BRANCH_REF_PREFIX + ref)
+    end
+
+    def tag?
+      project.repository.ref_exists?(Gitlab::Git::TAG_REF_PREFIX + ref)
+    end
+
+    def ref
+      Gitlab::Git.ref_name(origin_ref)
+    end
+
+    def valid_sha?
+      origin_sha && origin_sha != Gitlab::Git::BLANK_SHA
+    end
+
+    def error(message, save: false)
+      pipeline.errors.add(:base, message)
+      pipeline.reload_status! if save
+      pipeline
     end
   end
 end

app/services/ci/create_trigger_request_service.rb

 module Ci
   class CreateTriggerRequestService
     def execute(project, trigger, ref, variables = nil)
-      commit = project.commit(ref)
-      return unless commit
+      trigger_request = trigger.trigger_requests.create(variables: variables)
 
-      # check if ref is tag
-      tag = project.repository.find_tag(ref).present?
-
-      pipeline = project.pipelines.create(sha: commit.sha, ref: ref, tag: tag)
-
-      trigger_request = trigger.trigger_requests.create!(
-        variables: variables,
-        pipeline: pipeline,
-      )
-
-      if pipeline.create_builds(nil, trigger_request)
+      pipeline = Ci::CreatePipelineService.new(project, nil, ref: ref).
+        execute(ignore_skip_ci: true, trigger_request: trigger_request)
+      if pipeline.persisted?
         trigger_request
       end
     end

app/services/ci/process_pipeline_service.rb

+module Ci
+  class ProcessPipelineService < BaseService
+    attr_reader :pipeline
+
+    def execute(pipeline)
+      @pipeline = pipeline
+
+      # This method will ensure that our pipeline does have all builds for all stages created
+      if created_builds.empty?
+        create_builds!
+      end
+
+      new_builds =
+        stage_indexes_of_created_builds.map do |index|
+          process_stage(index)
+        end
+
+      # Return a flag if a when builds got enqueued
+      new_builds.flatten.any?
+    end
+
+    private
+
+    def create_builds!
+      Ci::CreatePipelineBuildsService.new(project, current_user).execute(pipeline)
+    end
+
+    def process_stage(index)
+      current_status = status_for_prior_stages(index)
+
+      created_builds_in_stage(index).select do |build|
+        process_build(build, current_status)
+      end
+    end
+
+    def process_build(build, current_status)
+      return false unless Statuseable::COMPLETED_STATUSES.include?(current_status)
+
+      if valid_statuses_for_when(build.when).include?(current_status)
+        build.queue
+        true
+      else
+        build.skip
+        false
+      end
+    end
+
+    def valid_statuses_for_when(value)
+      case value
+      when 'on_success'
+        %w[success]
+      when 'on_failure'
+        %w[failed]
+      when 'always'
+        %w[success failed]
+      else
+        []
+      end
+    end
+
+    def status_for_prior_stages(index)
+      pipeline.builds.where('stage_idx < ?', index).latest.status || 'success'
+    end
+
+    def stage_indexes_of_created_builds
+      created_builds.order(:stage_idx).pluck('distinct stage_idx')
+    end
+
+    def created_builds_in_stage(index)
+      created_builds.where(stage_idx: index)
+    end
+
+    def created_builds
+      pipeline.builds.created
+    end
+  end
+end

app/services/create_commit_builds_service.rb

-class CreateCommitBuildsService
-  def execute(project, user, params)
-    return unless project.builds_enabled?
-
-    before_sha = params[:checkout_sha] || params[:before]
-    sha = params[:checkout_sha] || params[:after]
-    origin_ref = params[:ref]
-
-    ref = Gitlab::Git.ref_name(origin_ref)
-    tag = Gitlab::Git.tag_ref?(origin_ref)
-
-    # Skip branch removal
-    if sha == Gitlab::Git::BLANK_SHA
-      return false
-    end
-
-    @pipeline = Ci::Pipeline.new(
-      project: project,
-      sha: sha,
-      ref: ref,
-      before_sha: before_sha,
-      tag: tag,
-      user: user)
-
-    ##
-    # Skip creating pipeline if no gitlab-ci.yml is found
-    #
-    unless @pipeline.ci_yaml_file
-      return false
-    end
-
-    ##
-    # Skip creating builds for commits that have [ci skip]
-    # but save pipeline object
-    #
-    if @pipeline.skip_ci?
-      return save_pipeline!
-    end
-
-    ##
-    # Skip creating builds when CI config is invalid
-    # but save pipeline object
-    #
-    unless @pipeline.config_processor
-      return save_pipeline!
-    end
-
-    ##
-    # Skip creating pipeline object if there are no builds for it.
-    #
-    unless @pipeline.create_builds(user)
-      @pipeline.errors.add(:base, 'No builds created')
-      return false
-    end
-
-    save_pipeline!
-  end
-
-  private
-
-  ##
-  # Create a new pipeline and touch object to calculate status
-  #
-  def save_pipeline!
-    @pipeline.save!
-    @pipeline.touch
-    @pipeline
-  end
-end

app/services/git_push_service.rb

@@ -69,7 +69,7 @@ class GitPushService < BaseService
     SystemHooksService.new.execute_hooks(build_push_data_system_hook.dup, :push_hooks)
     @project.execute_hooks(build_push_data.dup, :push_hooks)
     @project.execute_services(build_push_data.dup, :push_hooks)
-    CreateCommitBuildsService.new.execute(@project, current_user, build_push_data)
+    Ci::CreatePipelineService.new(project, current_user, build_push_data).execute
     ProjectCacheWorker.perform_async(@project.id)
   end
 

app/services/git_tag_push_service.rb

@@ -11,7 +11,7 @@ class GitTagPushService < BaseService
     SystemHooksService.new.execute_hooks(build_system_push_data.dup, :tag_push_hooks)
     project.execute_hooks(@push_data.dup, :tag_push_hooks)
     project.execute_services(@push_data.dup, :tag_push_hooks)
-    CreateCommitBuildsService.new.execute(project, current_user, @push_data)
+    Ci::CreatePipelineService.new(project, current_user, @push_data).execute
     ProjectCacheWorker.perform_async(project.id)
 
     true

app/services/members/destroy_service.rb

@@ -2,8 +2,9 @@ module Members
   class DestroyService < BaseService
     attr_accessor :member, :current_user
 
-    def initialize(member, user)
-      @member, @current_user = member, user
+    def initialize(member, current_user)
+      @member = member
+      @current_user = current_user
     end
 
     def execute

app/views/projects/ci/pipelines/_pipeline.html.haml

@@ -33,7 +33,7 @@
           Cant find HEAD commit for this branch
 
 
-    - stages_status = pipeline.statuses.latest.stages_status
+    - stages_status = pipeline.statuses.relevant.latest.stages_status
     - stages.each do |stage|
       %td.stage-cell
         - status = stages_status[stage]

app/views/projects/commit/_pipeline.html.haml

@@ -46,5 +46,5 @@
         - if pipeline.project.build_coverage_enabled?
           %th Coverage
         %th
-    - pipeline.statuses.stages.each do |stage|
-      = render 'projects/commit/ci_stage', stage: stage, statuses: pipeline.statuses.where(stage: stage)
+    - pipeline.statuses.relevant.stages.each do |stage|
+      = render 'projects/commit/ci_stage', stage: stage, statuses: pipeline.statuses.relevant.where(stage: stage)

config/initializers/5_backend.rb

-# GIT over HTTP
-require_dependency Rails.root.join('lib/gitlab/backend/grack_auth')
-
 # GIT over SSH
 require_dependency Rails.root.join('lib/gitlab/backend/shell')
 

config/initializers/mime_types.rb

@@ -12,3 +12,10 @@ Mime::Type.register_alias "text/html",  :md
 Mime::Type.register "video/mp4",  :mp4, [], [:m4v, :mov]
 Mime::Type.register "video/webm", :webm
 Mime::Type.register "video/ogg",  :ogv
+
+middlewares = Gitlab::Application.config.middleware
+middlewares.swap(ActionDispatch::ParamsParser, ActionDispatch::ParamsParser, {
+  Mime::Type.lookup('application/vnd.git-lfs+json') => lambda do |body|
+    ActiveSupport::JSON.decode(body)
+  end
+})

config/routes.rb

@@ -84,9 +84,6 @@ Rails.application.routes.draw do
   # Health check
   get 'health_check(/:checks)' => 'health_check#index', as: :health_check
 
-  # Enable Grack support (for LFS only)
-  mount Grack::AuthSpawner, at: '/', constraints: lambda { |request| /[-\/\w\.]+\.git\/(info\/lfs|gitlab-lfs)/.match(request.path_info) }, via: [:get, :post, :put]
-
   # Help
   get 'help'           => 'help#index'
   get 'help/shortcuts' => 'help#shortcuts'
@@ -482,11 +479,26 @@ Rails.application.routes.draw do
       end
 
       scope module: :projects do
-        # Git HTTP clients ('git clone' etc.)
         scope constraints: { id: /.+\.git/, format: nil } do
+          # Git HTTP clients ('git clone' etc.)
           get '/info/refs', to: 'git_http#info_refs'
           post '/git-upload-pack', to: 'git_http#git_upload_pack'
           post '/git-receive-pack', to: 'git_http#git_receive_pack'
+
+          # Git LFS API (metadata)
+          post '/info/lfs/objects/batch', to: 'lfs_api#batch'
+          post '/info/lfs/objects', to: 'lfs_api#deprecated'
+          get '/info/lfs/objects/*oid', to: 'lfs_api#deprecated'
+
+          # GitLab LFS object storage
+          scope constraints: { oid: /[a-f0-9]{64}/ } do
+            get '/gitlab-lfs/objects/*oid', to: 'lfs_storage#download'
+
+            scope constraints: { size: /[0-9]+/ } do
+              put '/gitlab-lfs/objects/*oid/*size/authorize', to: 'lfs_storage#upload_authorize'
+              put '/gitlab-lfs/objects/*oid/*size', to: 'lfs_storage#upload_finalize'
+            end
+          end
         end
 
         # Allow /info/refs, /info/refs?service=git-upload-pack, and

db/migrate/20160716115711_add_queued_at_to_ci_builds.rb

+class AddQueuedAtToCiBuilds < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    add_column :ci_builds, :queued_at, :timestamp
+  end
+end

db/migrate/20160810102349_remove_ci_runner_trigram_indexes.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class RemoveCiRunnerTrigramIndexes < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  # Disabled for the "down" method so the indexes can be re-created concurrently.
+  disable_ddl_transaction!
+
+  def up
+    return unless Gitlab::Database.postgresql?
+
+    transaction do
+      execute 'DROP INDEX IF EXISTS index_ci_runners_on_token_trigram;'
+      execute 'DROP INDEX IF EXISTS index_ci_runners_on_description_trigram;'
+    end
+  end
+
+  def down
+    return unless Gitlab::Database.postgresql?
+
+    execute 'CREATE INDEX CONCURRENTLY index_ci_runners_on_token_trigram ON ci_runners USING gin(token gin_trgm_ops);'
+    execute 'CREATE INDEX CONCURRENTLY index_ci_runners_on_description_trigram ON ci_runners USING gin(description gin_trgm_ops);'
+  end
+end

db/migrate/20160810142633_remove_redundant_indexes.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class RemoveRedundantIndexes < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    indexes = [
+      [:ci_taggings, 'ci_taggings_idx'],
+      [:audit_events, 'index_audit_events_on_author_id'],
+      [:audit_events, 'index_audit_events_on_type'],
+      [:ci_builds, 'index_ci_builds_on_erased_by_id'],
+      [:ci_builds, 'index_ci_builds_on_project_id_and_commit_id'],
+      [:ci_builds, 'index_ci_builds_on_type'],
+      [:ci_commits, 'index_ci_commits_on_project_id'],
+      [:ci_commits, 'index_ci_commits_on_project_id_and_committed_at'],
+      [:ci_commits, 'index_ci_commits_on_project_id_and_committed_at_and_id'],
+      [:ci_commits, 'index_ci_commits_on_project_id_and_sha'],
+      [:ci_commits, 'index_ci_commits_on_sha'],
+      [:ci_events, 'index_ci_events_on_created_at'],
+      [:ci_events, 'index_ci_events_on_is_admin'],
+      [:ci_events, 'index_ci_events_on_project_id'],
+      [:ci_jobs, 'index_ci_jobs_on_deleted_at'],
+      [:ci_jobs, 'index_ci_jobs_on_project_id'],
+      [:ci_projects, 'index_ci_projects_on_gitlab_id'],
+      [:ci_projects, 'index_ci_projects_on_shared_runners_enabled'],
+      [:ci_services, 'index_ci_services_on_project_id'],
+      [:ci_sessions, 'index_ci_sessions_on_session_id'],
+      [:ci_sessions, 'index_ci_sessions_on_updated_at'],
+      [:ci_tags, 'index_ci_tags_on_name'],
+      [:ci_triggers, 'index_ci_triggers_on_deleted_at'],
+      [:identities, 'index_identities_on_created_at_and_id'],
+      [:issues, 'index_issues_on_title'],
+      [:keys, 'index_keys_on_created_at_and_id'],
+      [:members, 'index_members_on_created_at_and_id'],
+      [:members, 'index_members_on_type'],
+      [:milestones, 'index_milestones_on_created_at_and_id'],
+      [:namespaces, 'index_namespaces_on_visibility_level'],
+      [:projects, 'index_projects_on_builds_enabled_and_shared_runners_enabled'],
+      [:services, 'index_services_on_category'],
+      [:services, 'index_services_on_created_at_and_id'],
+      [:services, 'index_services_on_default'],
+      [:snippets, 'index_snippets_on_created_at'],
+      [:snippets, 'index_snippets_on_created_at_and_id'],
+      [:todos, 'index_todos_on_state'],
+      [:web_hooks, 'index_web_hooks_on_created_at_and_id'],
+
+      # These indexes _may_ be used but they can be replaced by other existing
+      # indexes.
+
+      # There's already a composite index on (project_id, iid) which means that
+      # a separate index for _just_ project_id is not needed.
+      [:issues, 'index_issues_on_project_id'],
+
+      # These are all composite indexes for the columns (created_at, id). In all
+      # these cases there's already a standalone index for "created_at" which
+      # can be used instead.
+      #
+      # Because the "id" column of these composite indexes is never needed (due
+      # to "id" already being indexed as its a primary key) these composite
+      # indexes are useless.
+      [:issues, 'index_issues_on_created_at_and_id'],
+      [:merge_requests, 'index_merge_requests_on_created_at_and_id'],
+      [:namespaces, 'index_namespaces_on_created_at_and_id'],
+      [:notes, 'index_notes_on_created_at_and_id'],
+      [:projects, 'index_projects_on_created_at_and_id'],
+      [:users, 'index_users_on_created_at_and_id'],
+    ]
+
+    transaction do
+      indexes.each do |(table, index)|
+        remove_index(table, name: index) if index_exists_by_name?(table, index)
+      end
+    end
+
+    add_concurrent_index(:users, :created_at)
+    add_concurrent_index(:projects, :created_at)
+    add_concurrent_index(:namespaces, :created_at)
+  end
+
+  def down
+    # We're only restoring the composite indexes that could be replaced with
+    # individual ones, just in case somebody would ever want to revert.
+    transaction do
+      remove_index(:users, :created_at)
+      remove_index(:projects, :created_at)
+      remove_index(:namespaces, :created_at)
+    end
+
+    [:issues, :merge_requests, :namespaces, :notes, :projects, :users].each do |table|
+      add_concurrent_index(table, [:created_at, :id],
+                           name: "index_#{table}_on_created_at_and_id")
+    end
+  end
+
+  # Rails' index_exists? doesn't work when you only give it a table and index
+  # name. As such we have to use some extra code to check if an index exists for
+  # a given name.
+  def index_exists_by_name?(table, index)
+    indexes_for_table[table].include?(index)
+  end
+
+  def indexes_for_table
+    @indexes_for_table ||= Hash.new do |hash, table_name|
+      hash[table_name] = indexes(table_name).map(&:name)
+    end
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20160804150737) do
+ActiveRecord::Schema.define(version: 20160810142633) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -102,9 +102,7 @@ ActiveRecord::Schema.define(version: 20160804150737) do
     t.datetime "updated_at"
   end
 
-  add_index "audit_events", ["author_id"], name: "index_audit_events_on_author_id", using: :btree
   add_index "audit_events", ["entity_id", "entity_type"], name: "index_audit_events_on_entity_id_and_entity_type", using: :btree
-  add_index "audit_events", ["type"], name: "index_audit_events_on_type", using: :btree
 
   create_table "award_emoji", force: :cascade do |t|
     t.string   "name"
@@ -172,6 +170,7 @@ ActiveRecord::Schema.define(version: 20160804150737) do
     t.integer  "artifacts_size"
     t.string   "when"
     t.text     "yaml_variables"
+    t.datetime "queued_at"
   end
 
   add_index "ci_builds", ["commit_id", "stage_idx", "created_at"], name: "index_ci_builds_on_commit_id_and_stage_idx_and_created_at", using: :btree
@@ -179,13 +178,10 @@ ActiveRecord::Schema.define(version: 20160804150737) do
   add_index "ci_builds", ["commit_id", "type", "name", "ref"], name: "index_ci_builds_on_commit_id_and_type_and_name_and_ref", using: :btree
   add_index "ci_builds", ["commit_id", "type", "ref"], name: "index_ci_builds_on_commit_id_and_type_and_ref", using: :btree
   add_index "ci_builds", ["commit_id"], name: "index_ci_builds_on_commit_id", using: :btree
-  add_index "ci_builds", ["erased_by_id"], name: "index_ci_builds_on_erased_by_id", using: :btree
   add_index "ci_builds", ["gl_project_id"], name: "index_ci_builds_on_gl_project_id", using: :btree
-  add_index "ci_builds", ["project_id", "commit_id"], name: "index_ci_builds_on_project_id_and_commit_id", using: :btree
   add_index "ci_builds", ["project_id"], name: "index_ci_builds_on_project_id", using: :btree
   add_index "ci_builds", ["runner_id"], name: "index_ci_builds_on_runner_id", using: :btree
   add_index "ci_builds", ["status"], name: "index_ci_builds_on_status", using: :btree
-  add_index "ci_builds", ["type"], name: "index_ci_builds_on_type", using: :btree
 
   create_table "ci_commits", force: :cascade do |t|
     t.integer  "project_id"
@@ -209,11 +205,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do
   add_index "ci_commits", ["gl_project_id", "sha"], name: "index_ci_commits_on_gl_project_id_and_sha", using: :btree
   add_index "ci_commits", ["gl_project_id", "status"], name: "index_ci_commits_on_gl_project_id_and_status", using: :btree
   add_index "ci_commits", ["gl_project_id"], name: "index_ci_commits_on_gl_project_id", using: :btree
-  add_index "ci_commits", ["project_id", "committed_at", "id"], name: "index_ci_commits_on_project_id_and_committed_at_and_id", using: :btree
-  add_index "ci_commits", ["project_id", "committed_at"], name: "index_ci_commits_on_project_id_and_committed_at", using: :btree
-  add_index "ci_commits", ["project_id", "sha"], name: "index_ci_commits_on_project_id_and_sha", using: :btree
-  add_index "ci_commits", ["project_id"], name: "index_ci_commits_on_project_id", using: :btree
-  add_index "ci_commits", ["sha"], name: "index_ci_commits_on_sha", using: :btree
   add_index "ci_commits", ["status"], name: "index_ci_commits_on_status", using: :btree
   add_index "ci_commits", ["user_id"], name: "index_ci_commits_on_user_id", using: :btree
 
@@ -226,10 +217,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do
     t.datetime "updated_at"
   end
 
-  add_index "ci_events", ["created_at"], name: "index_ci_events_on_created_at", using: :btree
-  add_index "ci_events", ["is_admin"], name: "index_ci_events_on_is_admin", using: :btree
-  add_index "ci_events", ["project_id"], name: "index_ci_events_on_project_id", using: :btree
-
   create_table "ci_jobs", force: :cascade do |t|
     t.integer  "project_id",                          null: false
     t.text     "commands"
@@ -244,9 +231,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do
     t.datetime "deleted_at"
   end
 
-  add_index "ci_jobs", ["deleted_at"], name: "index_ci_jobs_on_deleted_at", using: :btree
-  add_index "ci_jobs", ["project_id"], name: "index_ci_jobs_on_project_id", using: :btree
-
   create_table "ci_projects", force: :cascade do |t|
     t.string   "name"
     t.integer  "timeout",                  default: 3600,  null: false
@@ -270,9 +254,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do
     t.text     "generated_yaml_config"
   end
 
-  add_index "ci_projects", ["gitlab_id"], name: "index_ci_projects_on_gitlab_id", using: :btree
-  add_index "ci_projects", ["shared_runners_enabled"], name: "index_ci_projects_on_shared_runners_enabled", using: :btree
-
   create_table "ci_runner_projects", force: :cascade do |t|
     t.integer  "runner_id",     null: false
     t.integer  "project_id"
@@ -301,10 +282,8 @@ ActiveRecord::Schema.define(version: 20160804150737) do
     t.boolean  "locked",       default: false, null: false
   end
 
-  add_index "ci_runners", ["description"], name: "index_ci_runners_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"}
   add_index "ci_runners", ["locked"], name: "index_ci_runners_on_locked", using: :btree
   add_index "ci_runners", ["token"], name: "index_ci_runners_on_token", using: :btree
-  add_index "ci_runners", ["token"], name: "index_ci_runners_on_token_trigram", using: :gin, opclasses: {"token"=>"gin_trgm_ops"}
 
   create_table "ci_services", force: :cascade do |t|
     t.string   "type"
@@ -316,8 +295,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do
     t.text     "properties"
   end
 
-  add_index "ci_services", ["project_id"], name: "index_ci_services_on_project_id", using: :btree
-
   create_table "ci_sessions", force: :cascade do |t|
     t.string   "session_id", null: false
     t.text     "data"
@@ -325,9 +302,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do
     t.datetime "updated_at"
   end
 
-  add_index "ci_sessions", ["session_id"], name: "index_ci_sessions_on_session_id", using: :btree
-  add_index "ci_sessions", ["updated_at"], name: "index_ci_sessions_on_updated_at", using: :btree
-
   create_table "ci_taggings", force: :cascade do |t|
     t.integer  "tag_id"
     t.integer  "taggable_id"
@@ -338,7 +312,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do
     t.datetime "created_at"
   end
 
-  add_index "ci_taggings", ["tag_id", "taggable_id", "taggable_type", "context", "tagger_id", "tagger_type"], name: "ci_taggings_idx", unique: true, using: :btree
   add_index "ci_taggings", ["taggable_id", "taggable_type", "context"], name: "index_ci_taggings_on_taggable_id_and_taggable_type_and_context", using: :btree
 
   create_table "ci_tags", force: :cascade do |t|
@@ -346,8 +319,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do
     t.integer "taggings_count", default: 0
   end
 
-  add_index "ci_tags", ["name"], name: "index_ci_tags_on_name", unique: true, using: :btree
-
   create_table "ci_trigger_requests", force: :cascade do |t|
     t.integer  "trigger_id", null: false
     t.text     "variables"
@@ -365,7 +336,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do
     t.integer  "gl_project_id"
   end
 
-  add_index "ci_triggers", ["deleted_at"], name: "index_ci_triggers_on_deleted_at", using: :btree
   add_index "ci_triggers", ["gl_project_id"], name: "index_ci_triggers_on_gl_project_id", using: :btree
 
   create_table "ci_variables", force: :cascade do |t|
@@ -471,7 +441,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do
     t.datetime "updated_at"
   end
 
-  add_index "identities", ["created_at", "id"], name: "index_identities_on_created_at_and_id", using: :btree
   add_index "identities", ["user_id"], name: "index_identities_on_user_id", using: :btree
 
   create_table "issues", force: :cascade do |t|
@@ -497,16 +466,13 @@ ActiveRecord::Schema.define(version: 20160804150737) do
   add_index "issues", ["assignee_id"], name: "index_issues_on_assignee_id", using: :btree
   add_index "issues", ["author_id"], name: "index_issues_on_author_id", using: :btree
   add_index "issues", ["confidential"], name: "index_issues_on_confidential", using: :btree
-  add_index "issues", ["created_at", "id"], name: "index_issues_on_created_at_and_id", using: :btree
   add_index "issues", ["created_at"], name: "index_issues_on_created_at", using: :btree
   add_index "issues", ["deleted_at"], name: "index_issues_on_deleted_at", using: :btree
   add_index "issues", ["description"], name: "index_issues_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"}
   add_index "issues", ["due_date"], name: "index_issues_on_due_date", using: :btree
   add_index "issues", ["milestone_id"], name: "index_issues_on_milestone_id", using: :btree
   add_index "issues", ["project_id", "iid"], name: "index_issues_on_project_id_and_iid", unique: true, using: :btree
-  add_index "issues", ["project_id"], name: "index_issues_on_project_id", using: :btree
   add_index "issues", ["state"], name: "index_issues_on_state", using: :btree
-  add_index "issues", ["title"], name: "index_issues_on_title", using: :btree
   add_index "issues", ["title"], name: "index_issues_on_title_trigram", using: :gin, opclasses: {"title"=>"gin_trgm_ops"}
 
   create_table "keys", force: :cascade do |t|
@@ -520,7 +486,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do
     t.boolean  "public",      default: false, null: false
   end
 
-  add_index "keys", ["created_at", "id"], name: "index_keys_on_created_at_and_id", using: :btree
   add_index "keys", ["fingerprint"], name: "index_keys_on_fingerprint", unique: true, using: :btree
   add_index "keys", ["user_id"], name: "index_keys_on_user_id", using: :btree
 
@@ -585,11 +550,9 @@ ActiveRecord::Schema.define(version: 20160804150737) do
   end
 
   add_index "members", ["access_level"], name: "index_members_on_access_level", using: :btree
-  add_index "members", ["created_at", "id"], name: "index_members_on_created_at_and_id", using: :btree
   add_index "members", ["invite_token"], name: "index_members_on_invite_token", unique: true, using: :btree
   add_index "members", ["requested_at"], name: "index_members_on_requested_at", using: :btree
   add_index "members", ["source_id", "source_type"], name: "index_members_on_source_id_and_source_type", using: :btree
-  add_index "members", ["type"], name: "index_members_on_type", using: :btree
   add_index "members", ["user_id"], name: "index_members_on_user_id", using: :btree
 
   create_table "merge_request_diffs", force: :cascade do |t|
@@ -636,7 +599,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do
 
   add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree
   add_index "merge_requests", ["author_id"], name: "index_merge_requests_on_author_id", using: :btree
-  add_index "merge_requests", ["created_at", "id"], name: "index_merge_requests_on_created_at_and_id", using: :btree
   add_index "merge_requests", ["created_at"], name: "index_merge_requests_on_created_at", using: :btree
   add_index "merge_requests", ["deleted_at"], name: "index_merge_requests_on_deleted_at", using: :btree
   add_index "merge_requests", ["description"], name: "index_merge_requests_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"}
@@ -659,7 +621,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do
     t.integer  "iid"
   end
 
-  add_index "milestones", ["created_at", "id"], name: "index_milestones_on_created_at_and_id", using: :btree
   add_index "milestones", ["description"], name: "index_milestones_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"}
   add_index "milestones", ["due_date"], name: "index_milestones_on_due_date", using: :btree
   add_index "milestones", ["project_id", "iid"], name: "index_milestones_on_project_id_and_iid", unique: true, using: :btree
@@ -681,14 +642,13 @@ ActiveRecord::Schema.define(version: 20160804150737) do
     t.boolean  "request_access_enabled", default: true,  null: false
   end
 
-  add_index "namespaces", ["created_at", "id"], name: "index_namespaces_on_created_at_and_id", using: :btree
+  add_index "namespaces", ["created_at"], name: "index_namespaces_on_created_at", using: :btree
   add_index "namespaces", ["name"], name: "index_namespaces_on_name", unique: true, using: :btree
   add_index "namespaces", ["name"], name: "index_namespaces_on_name_trigram", using: :gin, opclasses: {"name"=>"gin_trgm_ops"}
   add_index "namespaces", ["owner_id"], name: "index_namespaces_on_owner_id", using: :btree
   add_index "namespaces", ["path"], name: "index_namespaces_on_path", unique: true, using: :btree
   add_index "namespaces", ["path"], name: "index_namespaces_on_path_trigram", using: :gin, opclasses: {"path"=>"gin_trgm_ops"}
   add_index "namespaces", ["type"], name: "index_namespaces_on_type", using: :btree
-  add_index "namespaces", ["visibility_level"], name: "index_namespaces_on_visibility_level", using: :btree
 
   create_table "notes", force: :cascade do |t|
     t.text     "note"
@@ -711,7 +671,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do
 
   add_index "notes", ["author_id"], name: "index_notes_on_author_id", using: :btree
   add_index "notes", ["commit_id"], name: "index_notes_on_commit_id", using: :btree
-  add_index "notes", ["created_at", "id"], name: "index_notes_on_created_at_and_id", using: :btree
   add_index "notes", ["created_at"], name: "index_notes_on_created_at", using: :btree
   add_index "notes", ["line_code"], name: "index_notes_on_line_code", using: :btree
   add_index "notes", ["note"], name: "index_notes_on_note_trigram", using: :gin, opclasses: {"note"=>"gin_trgm_ops"}
@@ -851,9 +810,8 @@ ActiveRecord::Schema.define(version: 20160804150737) do
     t.boolean  "request_access_enabled",             default: true,      null: false
   end
 
-  add_index "projects", ["builds_enabled", "shared_runners_enabled"], name: "index_projects_on_builds_enabled_and_shared_runners_enabled", using: :btree
   add_index "projects", ["ci_id"], name: "index_projects_on_ci_id", using: :btree
-  add_index "projects", ["created_at", "id"], name: "index_projects_on_created_at_and_id", using: :btree
+  add_index "projects", ["created_at"], name: "index_projects_on_created_at", using: :btree
   add_index "projects", ["creator_id"], name: "index_projects_on_creator_id", using: :btree
   add_index "projects", ["description"], name: "index_projects_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"}
   add_index "projects", ["last_activity_at"], name: "index_projects_on_last_activity_at", using: :btree
@@ -939,9 +897,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do
     t.boolean  "wiki_page_events",      default: true
   end
 
-  add_index "services", ["category"], name: "index_services_on_category", using: :btree
-  add_index "services", ["created_at", "id"], name: "index_services_on_created_at_and_id", using: :btree
-  add_index "services", ["default"], name: "index_services_on_default", using: :btree
   add_index "services", ["project_id"], name: "index_services_on_project_id", using: :btree
   add_index "services", ["template"], name: "index_services_on_template", using: :btree
 
@@ -958,8 +913,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do
   end
 
   add_index "snippets", ["author_id"], name: "index_snippets_on_author_id", using: :btree
-  add_index "snippets", ["created_at", "id"], name: "index_snippets_on_created_at_and_id", using: :btree
-  add_index "snippets", ["created_at"], name: "index_snippets_on_created_at", using: :btree
   add_index "snippets", ["file_name"], name: "index_snippets_on_file_name_trigram", using: :gin, opclasses: {"file_name"=>"gin_trgm_ops"}
   add_index "snippets", ["project_id"], name: "index_snippets_on_project_id", using: :btree
   add_index "snippets", ["title"], name: "index_snippets_on_title_trigram", using: :gin, opclasses: {"title"=>"gin_trgm_ops"}
@@ -1028,7 +981,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do
   add_index "todos", ["commit_id"], name: "index_todos_on_commit_id", using: :btree
   add_index "todos", ["note_id"], name: "index_todos_on_note_id", using: :btree
   add_index "todos", ["project_id"], name: "index_todos_on_project_id", using: :btree
-  add_index "todos", ["state"], name: "index_todos_on_state", using: :btree
   add_index "todos", ["target_type", "target_id"], name: "index_todos_on_target_type_and_target_id", using: :btree
   add_index "todos", ["user_id"], name: "index_todos_on_user_id", using: :btree
 
@@ -1108,7 +1060,7 @@ ActiveRecord::Schema.define(version: 20160804150737) do
   add_index "users", ["admin"], name: "index_users_on_admin", using: :btree
   add_index "users", ["authentication_token"], name: "index_users_on_authentication_token", unique: true, using: :btree
   add_index "users", ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true, using: :btree
-  add_index "users", ["created_at", "id"], name: "index_users_on_created_at_and_id", using: :btree
+  add_index "users", ["created_at"], name: "index_users_on_created_at", using: :btree
   add_index "users", ["current_sign_in_at"], name: "index_users_on_current_sign_in_at", using: :btree
   add_index "users", ["email"], name: "index_users_on_email", unique: true, using: :btree
   add_index "users", ["email"], name: "index_users_on_email_trigram", using: :gin, opclasses: {"email"=>"gin_trgm_ops"}
@@ -1148,7 +1100,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do
     t.string   "token"
   end
 
-  add_index "web_hooks", ["created_at", "id"], name: "index_web_hooks_on_created_at_and_id", using: :btree
   add_index "web_hooks", ["project_id"], name: "index_web_hooks_on_project_id", using: :btree
 
   add_foreign_key "personal_access_tokens", "users"

doc/api/README.md

@@ -16,6 +16,8 @@ following locations:
 - [Commits](commits.md)
 - [Deploy Keys](deploy_keys.md)
 - [Groups](groups.md)
+- [Group Access Requests](access_requests.md)
+- [Group Members](members.md)
 - [Issues](issues.md)
 - [Keys](keys.md)
 - [Labels](labels.md)
@@ -25,6 +27,8 @@ following locations:
 - [Namespaces](namespaces.md)
 - [Notes](notes.md) (comments)
 - [Projects](projects.md) including setting Webhooks
+- [Project Access Requests](access_requests.md)
+- [Project Members](members.md)
 - [Project Snippets](project_snippets.md)
 - [Repositories](repositories.md)
 - [Repository Files](repository_files.md)
@@ -154,7 +158,7 @@ be returned with status code `403`:
 
 ```json
 {
-  "message": "403 Forbidden: Must be admin to use sudo"
+  "message": "403 Forbidden - Must be admin to use sudo"
 }
 ```
 

doc/api/access_requests.md

+# Group and project access requests
+
+ >**Note:** This feature was introduced in GitLab 8.11
+
+ **Valid access levels**
+
+ The access levels are defined in the `Gitlab::Access` module. Currently, these levels are recognized:
+
+```
+10 => Guest access
+20 => Reporter access
+30 => Developer access
+40 => Master access
+50 => Owner access # Only valid for groups
+```
+
+## List access requests for a group or project
+
+Gets a list of access requests viewable by the authenticated user.
+
+Returns `200` if the request succeeds.
+
+```
+GET /groups/:id/access_requests
+GET /projects/:id/access_requests
+```
+
+| Attribute | Type | Required | Description |
+| --------- | ---- | -------- | ----------- |
+| `id`      | integer/string | yes | The group/project ID or path |
+
+```bash
+curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/groups/:id/access_requests
+curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/:id/access_requests
+```
+
+Example response:
+
+```json
+[
+ {
+   "id": 1,
+   "username": "raymond_smith",
+   "name": "Raymond Smith",
+   "state": "active",
+   "created_at": "2012-10-22T14:13:35Z",
+   "requested_at": "2012-10-22T14:13:35Z"
+ },
+ {
+   "id": 2,
+   "username": "john_doe",
+   "name": "John Doe",
+   "state": "active",
+   "created_at": "2012-10-22T14:13:35Z",
+   "requested_at": "2012-10-22T14:13:35Z"
+ }
+]
+```
+
+## Request access to a group or project
+
+Requests access for the authenticated user to a group or project.
+
+Returns `201` if the request succeeds.
+
+```
+POST /groups/:id/access_requests
+POST /projects/:id/access_requests
+```
+
+| Attribute | Type | Required | Description |
+| --------- | ---- | -------- | ----------- |
+| `id`      | integer/string | yes | The group/project ID or path |
+
+```bash
+curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/groups/:id/access_requests
+curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/:id/access_requests
+```
+
+Example response:
+
+```json
+{
+  "id": 1,
+  "username": "raymond_smith",
+  "name": "Raymond Smith",
+  "state": "active",
+  "created_at": "2012-10-22T14:13:35Z",
+  "requested_at": "2012-10-22T14:13:35Z"
+}
+```
+
+## Approve an access request
+
+Approves an access request for the given user.
+
+Returns `201` if the request succeeds.
+
+```
+PUT /groups/:id/access_requests/:user_id/approve
+PUT /projects/:id/access_requests/:user_id/approve
+```
+
+| Attribute | Type | Required | Description |
+| --------- | ---- | -------- | ----------- |
+| `id`      | integer/string | yes | The group/project ID or path |
+| `user_id` | integer | yes   | The user ID of the access requester |
+| `access_level` | integer | no | A valid access level (defaults: `30`, developer access level) |
+
+```bash
+curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/groups/:id/access_requests/:user_id/approve?access_level=20
+curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/:id/access_requests/:user_id/approve?access_level=20
+```
+
+Example response:
+
+```json
+{
+  "id": 1,
+  "username": "raymond_smith",
+  "name": "Raymond Smith",
+  "state": "active",
+  "created_at": "2012-10-22T14:13:35Z",
+  "access_level": 20
+}
+```
+
+## Deny an access request
+
+Denies an access request for the given user.
+
+Returns `200` if the request succeeds.
+
+```
+DELETE /groups/:id/access_requests/:user_id
+DELETE /projects/:id/access_requests/:user_id
+```
+
+| Attribute | Type | Required | Description |
+| --------- | ---- | -------- | ----------- |
+| `id`      | integer/string | yes | The group/project ID or path |
+| `user_id` | integer | yes   | The user ID of the access requester |
+
+```bash
+curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/groups/:id/access_requests/:user_id
+curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/:id/access_requests/:user_id
+```

doc/api/groups.md

@@ -417,87 +417,7 @@ GET /groups?search=foobar
 
 ## Group members
 
-**Group access levels**
-
-The group access levels are defined in the `Gitlab::Access` module. Currently, these levels are recognized:
-
-```
-GUEST     = 10
-REPORTER  = 20
-DEVELOPER = 30
-MASTER    = 40
-OWNER     = 50
-```
-
-### List group members
-
-Get a list of group members viewable by the authenticated user.
-
-```
-GET /groups/:id/members
-```
-
-```json
-[
-  {
-    "id": 1,
-    "username": "raymond_smith",
-    "name": "Raymond Smith",
-    "state": "active",
-    "created_at": "2012-10-22T14:13:35Z",
-    "access_level": 30
-  },
-  {
-    "id": 2,
-    "username": "john_doe",
-    "name": "John Doe",
-    "state": "active",
-    "created_at": "2012-10-22T14:13:35Z",
-    "access_level": 30
-  }
-]
-```
-
-### Add group member
-
-Adds a user to the list of group members.
-
-```
-POST /groups/:id/members
-```
-
-Parameters:
-
-- `id` (required) - The ID or path of a group
-- `user_id` (required) - The ID of a user to add
-- `access_level` (required) - Project access level
-
-### Edit group team member
-
-Updates a group team member to a specified access level.
-
-```
-PUT /groups/:id/members/:user_id
-```
-
-Parameters:
-
-- `id` (required) - The ID of a group
-- `user_id` (required) - The ID of a group member
-- `access_level` (required) - Project access level
-
-### Remove user team member
-
-Removes user from user team.
-
-```
-DELETE /groups/:id/members/:user_id
-```
-
-Parameters:
-
-- `id` (required) - The ID or path of a user group
-- `user_id` (required) - The ID of a group member
+Please consult the [Group Members](members.md) documentation.
 
 ## Namespaces in groups
 

doc/api/members.md

+# Group and project members
+
+**Valid access levels**
+
+The access levels are defined in the `Gitlab::Access` module. Currently, these levels are recognized:
+
+```
+10 => Guest access
+20 => Reporter access
+30 => Developer access
+40 => Master access
+50 => Owner access # Only valid for groups
+```
+
+## List all members of a group or project
+
+Gets a list of group or project members viewable by the authenticated user.
+
+Returns `200` if the request succeeds.
+
+```
+GET /groups/:id/members
+GET /projects/:id/members
+```
+
+| Attribute | Type | Required | Description |
+| --------- | ---- | -------- | ----------- |
+| `id`      | integer/string | yes | The group/project ID or path |
+| `query`   | string | no     | A query string to search for members |
+
+```bash
+curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/groups/:id/members
+curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/:id/members
+```
+
+Example response:
+
+```json
+[
+  {
+    "id": 1,
+    "username": "raymond_smith",
+    "name": "Raymond Smith",
+    "state": "active",
+    "created_at": "2012-10-22T14:13:35Z",
+    "access_level": 30
+  },
+  {
+    "id": 2,
+    "username": "john_doe",
+    "name": "John Doe",
+    "state": "active",
+    "created_at": "2012-10-22T14:13:35Z",
+    "access_level": 30
+  }
+]
+```
+
+## Get a member of a group or project
+
+Gets a member of a group or project.
+
+Returns `200` if the request succeeds.
+
+```
+GET /groups/:id/members/:user_id
+GET /projects/:id/members/:user_id
+```
+
+| Attribute | Type | Required | Description |
+| --------- | ---- | -------- | ----------- |
+| `id`      | integer/string | yes | The group/project ID or path |
+| `user_id` | integer | yes   | The user ID of the member |
+
+```bash
+curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/groups/:id/members/:user_id
+curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/:id/members/:user_id
+```
+
+Example response:
+
+```json
+{
+  "id": 1,
+  "username": "raymond_smith",
+  "name": "Raymond Smith",
+  "state": "active",
+  "created_at": "2012-10-22T14:13:35Z",
+  "access_level": 30
+}
+```
+
+## Add a member to a group or project
+
+Adds a member to a group or project.
+
+Returns `201` if the request succeeds.
+
+```
+POST /groups/:id/members
+POST /projects/:id/members
+```
+
+| Attribute | Type | Required | Description |
+| --------- | ---- | -------- | ----------- |
+| `id`      | integer/string  | yes | The group/project ID or path |
+| `user_id` | integer         | yes | The user ID of the new member |
+| `access_level` | integer | yes | A valid access level |
+
+```bash
+curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/groups/:id/members/:user_id?access_level=30
+curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/:id/members/:user_id?access_level=30
+```
+
+Example response:
+
+```json
+{
+  "id": 1,
+  "username": "raymond_smith",
+  "name": "Raymond Smith",
+  "state": "active",
+  "created_at": "2012-10-22T14:13:35Z",
+  "access_level": 30
+}
+```
+
+## Edit a member of a group or project
+
+Updates a member of a group or project.
+
+Returns `200` if the request succeeds.
+
+```
+PUT /groups/:id/members/:user_id
+PUT /projects/:id/members/:user_id
+```
+
+| Attribute | Type | Required | Description |
+| --------- | ---- | -------- | ----------- |
+| `id`      | integer/string | yes | The group/project ID or path |
+| `user_id` | integer | yes   | The user ID of the member |
+| `access_level` | integer | yes | A valid access level |
+
+```bash
+curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/groups/:id/members/:user_id?access_level=40
+curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/:id/members/:user_id?access_level=40
+```
+
+Example response:
+
+```json
+{
+  "id": 1,
+  "username": "raymond_smith",
+  "name": "Raymond Smith",
+  "state": "active",
+  "created_at": "2012-10-22T14:13:35Z",
+  "access_level": 40
+}
+```
+
+## Remove a member from a group or project
+
+Removes a user from a group or project.
+
+Returns `200` if the request succeeds.
+
+```
+DELETE /groups/:id/members/:user_id
+DELETE /projects/:id/members/:user_id
+```
+
+| Attribute | Type | Required | Description |
+| --------- | ---- | -------- | ----------- |
+| `id`      | integer/string | yes | The group/project ID or path |
+| `user_id` | integer | yes   | The user ID of the member |
+
+```bash
+curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/groups/:id/members/:user_id
+curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/:id/members/:user_id
+```

doc/api/oauth2.md

 # GitLab as an OAuth2 client
 
-This document is about using other OAuth authentication service providers to sign into GitLab.
-If you want GitLab to be an OAuth authentication service provider to sign into other services please see the [Oauth2 provider documentation](../integration/oauth_provider.md).
+This document covers using the OAuth2 protocol to access GitLab.
 
-OAuth2 is a protocol that enables us to authenticate a user without requiring them to give their password. 
+If you want GitLab to be an OAuth authentication service provider to sign into other services please see the [Oauth2 provider documentation](../integration/oauth_provider.md).
 
-Before using the OAuth2 you should create an application in user's account. Each application gets a unique App ID and App Secret parameters. You should not share these.
+OAuth2 is a protocol that enables us to authenticate a user without requiring them to give their password to a third-party. 
 
 This functionality is based on [doorkeeper gem](https://github.com/doorkeeper-gem/doorkeeper)
 
 ## Web Application Flow
 
-This flow is using for authentication from third-party web sites and is probably used the most. 
-It basically consists of an exchange of an authorization token for an access token. For more detailed info, check out the [RFC spec here](http://tools.ietf.org/html/rfc6749#section-4.1)
+This is the most common type of flow and is used by server-side clients that wish to access GitLab on a user's behalf.
+
+>**Note:**
+This flow **should not** be used for client-side clients as you would leak your `client_secret`. Client-side clients should use the Implicit Grant (which is currently unsupported).
 
-This flow consists from 3 steps.
+For more detailed information, check out the [RFC spec](http://tools.ietf.org/html/rfc6749#section-4.1)
+
+In the following sections you will be introduced to the three steps needed for this flow.
 
 ### 1. Registering the client
 
-Create an application in user's account profile.
+First, you should create an application (`/profile/applications`) in your user's account.
+Each application gets a unique App ID and App Secret parameters. 
+
+>**Note:**
+**You should not share/leak your App ID or App Secret.**
 
 ### 2. Requesting authorization
 
-To request the authorization token, you should visit the `/oauth/authorize` endpoint. You can do that by visiting manually the URL:
+To request the authorization code, you should redirect the user to the `/oauth/authorize` endpoint:
+
+```
+https://gitlab.example.com/oauth/authorize?client_id=APP_ID&redirect_uri=REDIRECT_URI&response_type=code&state=your_unique_state_hash
+```
+
+This will ask the user to approve the applications access to their account and then redirect back to the `REDIRECT_URI` you provided.
+
+The redirect will include the GET `code` parameter, for example:
 
 ```
-http://localhost:3000/oauth/authorize?client_id=APP_ID&redirect_uri=REDIRECT_URI&response_type=code
+http://myapp.com/oauth/redirect?code=1234567890&state=your_unique_state_hash
 ```
 
-Where REDIRECT_URI is the URL in your app where users will be sent after authorization. 	
+You should then use the `code` to request an access token.
+
+>**Important:**
+It is highly recommended that you send a `state` value with the request to `/oauth/authorize` and 
+validate that value is returned and matches in the redirect request. 
+This is important to prevent [CSFR attacks](http://www.oauthsecurity.com/#user-content-authorization-code-flow), 
+`state` really should have been a requirement in the standard! 
 
 ### 3. Requesting the access token
 
-To request the access token, you should use the returned code and exchange it for an access token. To do that you can use any HTTP client. In this case, I used rest-client:
+Once you have the authorization code you can request an `access_token` using the code, to do that you can use any HTTP client. In the following example, we are using Ruby's `rest-client`:
 
 ```
 parameters = 'client_id=APP_ID&client_secret=APP_SECRET&code=RETURNED_CODE&grant_type=authorization_code&redirect_uri=REDIRECT_URI'
@@ -46,6 +67,8 @@ RestClient.post 'http://localhost:3000/oauth/token', parameters
  "refresh_token": "8257e65c97202ed1726cf9571600918f3bffb2544b26e00a61df9897668c33a1"
 }
 ```
+>**Note:**
+The `redirect_uri` must match the `redirect_uri` used in the original authorization request.
 
 You can now make requests to the API with the access token returned.
 
@@ -77,6 +100,9 @@ The credentials should only be used when there is a high degree of trust between
 client is part of the device operating system or a highly privileged application), and when other authorization grant types are not
 available (such as an authorization code).
 
+>**Important:**
+Never store the users credentials and only use this grant type when your client is deployed to a trusted environment, in 99% of cases [personal access tokens] are a better choice.
+
 Even though this grant type requires direct client access to the resource owner credentials, the resource owner credentials are used
 for a single request and are exchanged for an access token.  This grant type can eliminate the need for the client to store the
 resource owner credentials for future use, by exchanging the credentials with a long-lived access token or refresh token.

doc/api/projects.md

@@ -858,95 +858,9 @@ Parameters:
 In Markdown contexts, the link is automatically expanded when the format in `markdown` is used.
 
 
-## Team members
+## Project members
 
-### List project team members
-
-Get a list of a project's team members.
-
-```
-GET /projects/:id/members
-```
-
-Parameters:
-
-- `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project
-- `query` (optional) - Query string to search for members
-
-### Get project team member
-
-Gets a project team member.
-
-```
-GET /projects/:id/members/:user_id
-```
-
-Parameters:
-
-- `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project
-- `user_id` (required) - The ID of a user
-
-```json
-{
-  "id": 1,
-  "username": "john_smith",
-  "email": "john@example.com",
-  "name": "John Smith",
-  "state": "active",
-  "created_at": "2012-05-23T08:00:58Z",
-  "access_level": 40
-}
-```
-
-### Add project team member
-
-Adds a user to a project team. This is an idempotent method and can be called multiple times
-with the same parameters. Adding team membership to a user that is already a member does not
-affect the existing membership.
-
-```
-POST /projects/:id/members
-```
-
-Parameters:
-
-- `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project
-- `user_id` (required) - The ID of a user to add
-- `access_level` (required) - Project access level
-
-### Edit project team member
-
-Updates a project team member to a specified access level.
-
-```
-PUT /projects/:id/members/:user_id
-```
-
-Parameters:
-
-- `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project
-- `user_id` (required) - The ID of a team member
-- `access_level` (required) - Project access level
-
-### Remove project team member
-
-Removes a user from a project team.
-
-```
-DELETE /projects/:id/members/:user_id
-```
-
-Parameters:
-
-- `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project
-- `user_id` (required) - The ID of a team member
-
-This method removes the project member if the user has the proper access rights to do so.
-It returns a status code 403 if the member does not have the proper rights to perform this action.
-In all other cases this method is idempotent and revoking team membership for a user who is not
-currently a team member is considered success.
-Please note that the returned JSON currently differs slightly. Thus you should not
-rely on the returned JSON structure.
+Please consult the [Project Members](members.md) documentation.
 
 ### Share project with group
 

features/steps/shared/builds.rb

@@ -10,20 +10,22 @@ module SharedBuilds
   end
 
   step 'project has a recent build' do
-    @pipeline = create(:ci_pipeline, project: @project, sha: @project.commit.sha, ref: 'master')
+    @pipeline = create(:ci_empty_pipeline, project: @project, sha: @project.commit.sha, ref: 'master')
     @build = create(:ci_build_with_coverage, pipeline: @pipeline)
+    @pipeline.reload_status!
   end
 
   step 'recent build is successful' do
-    @build.update(status: 'success')
+    @build.success
   end
 
   step 'recent build failed' do
-    @build.update(status: 'failed')
+    @build.drop
   end
 
   step 'project has another build that is running' do
     create(:ci_build, pipeline: @pipeline, name: 'second build', status: 'running')
+    @pipeline.reload_status!
   end
 
   step 'I visit recent build details page' do

lib/api/access_requests.rb

+module API
+  class AccessRequests < Grape::API
+    before { authenticate! }
+
+    helpers ::API::Helpers::MembersHelpers
+
+    %w[group project].each do |source_type|
+      resource source_type.pluralize do
+        # Get a list of group/project access requests viewable by the authenticated user.
+        #
+        # Parameters:
+        #   id (required) - The group/project ID
+        #
+        # Example Request:
+        #  GET /groups/:id/access_requests
+        #  GET /projects/:id/access_requests
+        get ":id/access_requests" do
+          source = find_source(source_type, params[:id])
+          authorize_admin_source!(source_type, source)
+
+          access_requesters = paginate(source.requesters.includes(:user))
+
+          present access_requesters.map(&:user), with: Entities::AccessRequester, access_requesters: access_requesters
+        end
+
+        # Request access to the group/project
+        #
+        # Parameters:
+        #   id (required) - The group/project ID
+        #
+        # Example Request:
+        #  POST /groups/:id/access_requests
+        #  POST /projects/:id/access_requests
+        post ":id/access_requests" do
+          source = find_source(source_type, params[:id])
+          access_requester = source.request_access(current_user)
+
+          if access_requester.persisted?
+            present access_requester.user, with: Entities::AccessRequester, access_requester: access_requester
+          else
+            render_validation_error!(access_requester)
+          end
+        end
+
+        # Approve a group/project access request
+        #
+        # Parameters:
+        #   id (required) - The group/project ID
+        #   user_id (required) - The user ID of the access requester
+        #   access_level (optional) - Access level
+        #
+        # Example Request:
+        #   PUT /groups/:id/access_requests/:user_id/approve
+        #   PUT /projects/:id/access_requests/:user_id/approve
+        put ':id/access_requests/:user_id/approve' do
+          required_attributes! [:user_id]
+          source = find_source(source_type, params[:id])
+          authorize_admin_source!(source_type, source)
+
+          member = source.requesters.find_by!(user_id: params[:user_id])
+          if params[:access_level]
+            member.update(access_level: params[:access_level])
+          end
+          member.accept_request
+
+          status :created
+          present member.user, with: Entities::Member, member: member
+        end
+
+        # Deny a group/project access request
+        #
+        # Parameters:
+        #   id (required) - The group/project ID
+        #   user_id (required) - The user ID of the access requester
+        #
+        # Example Request:
+        #   DELETE /groups/:id/access_requests/:user_id
+        #   DELETE /projects/:id/access_requests/:user_id
+        delete ":id/access_requests/:user_id" do
+          required_attributes! [:user_id]
+          source = find_source(source_type, params[:id])
+
+          access_requester = source.requesters.find_by!(user_id: params[:user_id])
+
+          ::Members::DestroyService.new(access_requester, current_user).execute
+        end
+      end
+    end
+  end
+end

lib/api/api.rb

@@ -3,6 +3,10 @@ module API
     include APIGuard
     version 'v3', using: :path
 
+    rescue_from Gitlab::Access::AccessDeniedError do
+      rack_response({ 'message' => '403 Forbidden' }.to_json, 403)
+    end
+
     rescue_from ActiveRecord::RecordNotFound do
       rack_response({ 'message' => '404 Not found' }.to_json, 404)
     end
@@ -32,6 +36,7 @@ module API
     # Ensure the namespace is right, otherwise we might load Grape::API::Helpers
     helpers ::API::Helpers
 
+    mount ::API::AccessRequests
     mount ::API::AwardEmoji
     mount ::API::Branches
     mount ::API::Builds
@@ -40,19 +45,18 @@ module API
     mount ::API::DeployKeys
     mount ::API::Environments
     mount ::API::Files
-    mount ::API::GroupMembers
     mount ::API::Groups
     mount ::API::Internal
     mount ::API::Issues
     mount ::API::Keys
     mount ::API::Labels
     mount ::API::LicenseTemplates
+    mount ::API::Members
     mount ::API::MergeRequests
     mount ::API::Milestones
     mount ::API::Namespaces
     mount ::API::Notes
     mount ::API::ProjectHooks
-    mount ::API::ProjectMembers
     mount ::API::ProjectSnippets
     mount ::API::Projects
     mount ::API::Repositories

lib/api/entities.rb

@@ -92,9 +92,17 @@ module API
       end
     end
 
-    class ProjectMember < UserBasic
+    class Member < UserBasic
       expose :access_level do |user, options|
-        options[:project].project_members.find_by(user_id: user.id).access_level
+        member = options[:member] || options[:members].find { |m| m.user_id == user.id }
+        member.access_level
+      end
+    end
+
+    class AccessRequester < UserBasic
+      expose :requested_at do |user, options|
+        access_requester = options[:access_requester] || options[:access_requesters].find { |m| m.user_id == user.id }
+        access_requester.requested_at
       end
     end
 
@@ -109,12 +117,6 @@ module API
       expose :shared_projects, using: Entities::Project
     end
 
-    class GroupMember < UserBasic
-      expose :access_level do |user, options|
-        options[:group].group_members.find_by(user_id: user.id).access_level
-      end
-    end
-
     class RepoBranch < Grape::Entity
       expose :name
 
@@ -326,7 +328,7 @@ module API
       expose :id, :path, :kind
     end
 
-    class Member < Grape::Entity
+    class MemberAccess < Grape::Entity
       expose :access_level
       expose :notification_level do |member, options|
         if member.notification_setting
@@ -335,10 +337,10 @@ module API
       end
     end
 
-    class ProjectAccess < Member
+    class ProjectAccess < MemberAccess
     end
 
-    class GroupAccess < Member
+    class GroupAccess < MemberAccess
     end
 
     class ProjectService < Grape::Entity

lib/api/group_members.rb

-module API
-  class GroupMembers < Grape::API
-    before { authenticate! }
-
-    resource :groups do
-      # Get a list of group members viewable by the authenticated user.
-      #
-      # Example Request:
-      #  GET /groups/:id/members
-      get ":id/members" do
-        group = find_group(params[:id])
-        users = group.users
-        present users, with: Entities::GroupMember, group: group
-      end
-
-      # Add a user to the list of group members
-      #
-      # Parameters:
-      #   id (required) - group id
-      #   user_id (required) - the users id
-      #   access_level (required) - Project access level
-      # Example Request:
-      #  POST /groups/:id/members
-      post ":id/members" do
-        group = find_group(params[:id])
-        authorize! :admin_group, group
-        required_attributes! [:user_id, :access_level]
-
-        unless validate_access_level?(params[:access_level])
-          render_api_error!("Wrong access level", 422)
-        end
-
-        if group.group_members.find_by(user_id: params[:user_id])
-          render_api_error!("Already exists", 409)
-        end
-
-        group.add_users([params[:user_id]], params[:access_level], current_user)
-        member = group.group_members.find_by(user_id: params[:user_id])
-        present member.user, with: Entities::GroupMember, group: group
-      end
-
-      # Update group member
-      #
-      # Parameters:
-      #   id (required) - The ID of a group
-      #   user_id (required) - The ID of a group member
-      #   access_level (required) - Project access level
-      # Example Request:
-      #   PUT /groups/:id/members/:user_id
-      put ':id/members/:user_id' do
-        group = find_group(params[:id])
-        authorize! :admin_group, group
-        required_attributes! [:access_level]
-
-        group_member = group.group_members.find_by(user_id: params[:user_id])
-        not_found!('User can not be found') if group_member.nil?
-
-        if group_member.update_attributes(access_level: params[:access_level])
-          @member = group_member.user
-          present @member, with: Entities::GroupMember, group: group
-        else
-          handle_member_errors group_member.errors
-        end
-      end
-
-      # Remove member.
-      #
-      # Parameters:
-      #   id (required) - group id
-      #   user_id (required) - the users id
-      #
-      # Example Request:
-      #   DELETE /groups/:id/members/:user_id
-      delete ":id/members/:user_id" do
-        group = find_group(params[:id])
-        authorize! :admin_group, group
-        member = group.group_members.find_by(user_id: params[:user_id])
-
-        if member.nil?
-          render_api_error!("404 Not Found - user_id:#{params[:user_id]} not a member of group #{group.name}", 404)
-        else
-          member.destroy
-        end
-      end
-    end
-  end
-end

lib/api/helpers.rb

@@ -28,7 +28,7 @@ module API
 
       # If the sudo is the current user do nothing
       if identifier && !(@current_user.id == identifier || @current_user.username == identifier)
-        render_api_error!('403 Forbidden: Must be admin to use sudo', 403) unless @current_user.is_admin?
+        forbidden!('Must be admin to use sudo') unless @current_user.is_admin?
         @current_user = User.by_username_or_id(identifier)
         not_found!("No user id or username for: #{identifier}") if @current_user.nil?
       end
@@ -49,16 +49,15 @@ module API
 
     def user_project
       @project ||= find_project(params[:id])
-      @project || not_found!("Project")
     end
 
     def find_project(id)
       project = Project.find_with_namespace(id) || Project.find_by(id: id)
 
-      if project && can?(current_user, :read_project, project)
+      if can?(current_user, :read_project, project)
         project
       else
-        nil
+        not_found!('Project')
       end
     end
 
@@ -89,11 +88,7 @@ module API
     end
 
     def find_group(id)
-      begin
-        group = Group.find(id)
-      rescue ActiveRecord::RecordNotFound
-        group = Group.find_by!(path: id)
-      end
+      group = Group.find_by(path: id) || Group.find_by(id: id)
 
       if can?(current_user, :read_group, group)
         group
@@ -135,7 +130,7 @@ module API
     end
 
     def authorize!(action, subject)
-      forbidden! unless abilities.allowed?(current_user, action, subject)
+      forbidden! unless can?(current_user, action, subject)
     end
 
     def authorize_push_project
@@ -197,10 +192,6 @@ module API
       errors
     end
 
-    def validate_access_level?(level)
-      Gitlab::Access.options_with_owner.values.include? level.to_i
-    end
-
     # Checks the occurrences of datetime attributes, each attribute if present in the params hash must be in ISO 8601
     # format (YYYY-MM-DDTHH:MM:SSZ) or a Bad Request error is invoked.
     #
@@ -411,11 +402,6 @@ module API
       File.read(Gitlab.config.gitlab_shell.secret_file).chomp
     end
 
-    def handle_member_errors(errors)
-      error!(errors[:access_level], 422) if errors[:access_level].any?
-      not_found!(errors)
-    end
-
     def send_git_blob(repository, blob)
       env['api.format'] = :txt
       content_type 'text/plain'

lib/api/helpers/members_helpers.rb

+module API
+  module Helpers
+    module MembersHelpers
+      def find_source(source_type, id)
+        public_send("find_#{source_type}", id)
+      end
+
+      def authorize_admin_source!(source_type, source)
+        authorize! :"admin_#{source_type}", source
+      end
+    end
+  end
+end

lib/api/members.rb

+module API
+  class Members < Grape::API
+    before { authenticate! }
+
+    helpers ::API::Helpers::MembersHelpers
+
+    %w[group project].each do |source_type|
+      resource source_type.pluralize do
+        # Get a list of group/project members viewable by the authenticated user.
+        #
+        # Parameters:
+        #   id (required) - The group/project ID
+        #   query         - Query string
+        #
+        # Example Request:
+        #   GET /groups/:id/members
+        #   GET /projects/:id/members
+        get ":id/members" do
+          source = find_source(source_type, params[:id])
+
+          members = source.members.includes(:user)
+          members = members.joins(:user).merge(User.search(params[:query])) if params[:query]
+          members = paginate(members)
+
+          present members.map(&:user), with: Entities::Member, members: members
+        end
+
+        # Get a group/project member
+        #
+        # Parameters:
+        #   id (required) - The group/project ID
+        #   user_id (required) - The user ID of the member
+        #
+        # Example Request:
+        #   GET /groups/:id/members/:user_id
+        #   GET /projects/:id/members/:user_id
+        get ":id/members/:user_id" do
+          source = find_source(source_type, params[:id])
+
+          members = source.members
+          member = members.find_by!(user_id: params[:user_id])
+
+          present member.user, with: Entities::Member, member: member
+        end
+
+        # Add a new group/project member
+        #
+        # Parameters:
+        #   id (required) - The group/project ID
+        #   user_id (required) - The user ID of the new member
+        #   access_level (required) - A valid access level
+        #
+        # Example Request:
+        #   POST /groups/:id/members
+        #   POST /projects/:id/members
+        post ":id/members" do
+          source = find_source(source_type, params[:id])
+          authorize_admin_source!(source_type, source)
+          required_attributes! [:user_id, :access_level]
+
+          access_requester = source.requesters.find_by(user_id: params[:user_id])
+          if access_requester
+            # We pass current_user = access_requester so that the requester doesn't
+            # receive a "access denied" email
+            ::Members::DestroyService.new(access_requester, access_requester.user).execute
+          end
+
+          member = source.members.find_by(user_id: params[:user_id])
+
+          # This is to ensure back-compatibility but 409 behavior should be used
+          # for both project and group members in 9.0!
+          conflict!('Member already exists') if source_type == 'group' && member
+
+          unless member
+            source.add_user(params[:user_id], params[:access_level], current_user)
+            member = source.members.find_by(user_id: params[:user_id])
+          end
+
+          if member
+            present member.user, with: Entities::Member, member: member
+          else
+            # Since `source.add_user` doesn't return a member object, we have to
+            # build a new one and populate its errors in order to render them.
+            member = source.members.build(attributes_for_keys([:user_id, :access_level]))
+            member.valid? # populate the errors
+
+            # This is to ensure back-compatibility but 400 behavior should be used
+            # for all validation errors in 9.0!
+            render_api_error!('Access level is not known', 422) if member.errors.key?(:access_level)
+            render_validation_error!(member)
+          end
+        end
+
+        # Update a group/project member
+        #
+        # Parameters:
+        #   id (required) - The group/project ID
+        #   user_id (required) - The user ID of the member
+        #   access_level (required) - A valid access level
+        #
+        # Example Request:
+        #   PUT /groups/:id/members/:user_id
+        #   PUT /projects/:id/members/:user_id
+        put ":id/members/:user_id" do
+          source = find_source(source_type, params[:id])
+          authorize_admin_source!(source_type, source)
+          required_attributes! [:user_id, :access_level]
+
+          member = source.members.find_by!(user_id: params[:user_id])
+
+          if member.update_attributes(access_level: params[:access_level])
+            present member.user, with: Entities::Member, member: member
+          else
+            # This is to ensure back-compatibility but 400 behavior should be used
+            # for all validation errors in 9.0!
+            render_api_error!('Access level is not known', 422) if member.errors.key?(:access_level)
+            render_validation_error!(member)
+          end
+        end
+
+        # Remove a group/project member
+        #
+        # Parameters:
+        #   id (required) - The group/project ID
+        #   user_id (required) - The user ID of the member
+        #
+        # Example Request:
+        #   DELETE /groups/:id/members/:user_id
+        #   DELETE /projects/:id/members/:user_id
+        delete ":id/members/:user_id" do
+          source = find_source(source_type, params[:id])
+          required_attributes! [:user_id]
+
+          # This is to ensure back-compatibility but find_by! should be used
+          # in that casse in 9.0!
+          member = source.members.find_by(user_id: params[:user_id])
+
+          # This is to ensure back-compatibility but this should be removed in
+          # favor of find_by! in 9.0!
+          not_found!("Member: user_id:#{params[:user_id]}") if source_type == 'group' && member.nil?
+
+          # This is to ensure back-compatibility but 204 behavior should be used
+          # for all DELETE endpoints in 9.0!
+          if member.nil?
+            { message: "Access revoked", id: params[:user_id].to_i }
+          else
+            ::Members::DestroyService.new(member, current_user).execute
+
+            present member.user, with: Entities::Member, member: member
+          end
+        end
+      end
+    end
+  end
+end

lib/api/project_members.rb

-module API
-  # Projects members API
-  class ProjectMembers < Grape::API
-    before { authenticate! }
-
-    resource :projects do
-      # Get a project team members
-      #
-      # Parameters:
-      #   id (required) - The ID of a project
-      #   query         - Query string
-      # Example Request:
-      #   GET /projects/:id/members
-      get ":id/members" do
-        if params[:query].present?
-          @members = paginate user_project.users.where("username LIKE ?", "%#{params[:query]}%")
-        else
-          @members = paginate user_project.users
-        end
-        present @members, with: Entities::ProjectMember, project: user_project
-      end
-
-      # Get a project team members
-      #
-      # Parameters:
-      #   id (required) - The ID of a project
-      #   user_id (required) - The ID of a user
-      # Example Request:
-      #   GET /projects/:id/members/:user_id
-      get ":id/members/:user_id" do
-        @member = user_project.users.find params[:user_id]
-        present @member, with: Entities::ProjectMember, project: user_project
-      end
-
-      # Add a new project team member
-      #
-      # Parameters:
-      #   id (required) - The ID of a project
-      #   user_id (required) - The ID of a user
-      #   access_level (required) - Project access level
-      # Example Request:
-      #   POST /projects/:id/members
-      post ":id/members" do
-        authorize! :admin_project, user_project
-        required_attributes! [:user_id, :access_level]
-
-        # either the user is already a team member or a new one
-        project_member = user_project.project_member(params[:user_id])
-        if project_member.nil?
-          project_member = user_project.project_members.new(
-            user_id: params[:user_id],
-            access_level: params[:access_level]
-          )
-        end
-
-        if project_member.save
-          @member = project_member.user
-          present @member, with: Entities::ProjectMember, project: user_project
-        else
-          handle_member_errors project_member.errors
-        end
-      end
-
-      # Update project team member
-      #
-      # Parameters:
-      #   id (required) - The ID of a project
-      #   user_id (required) - The ID of a team member
-      #   access_level (required) - Project access level
-      # Example Request:
-      #   PUT /projects/:id/members/:user_id
-      put ":id/members/:user_id" do
-        authorize! :admin_project, user_project
-        required_attributes! [:access_level]
-
-        project_member = user_project.project_members.find_by(user_id: params[:user_id])
-        not_found!("User can not be found") if project_member.nil?
-
-        if project_member.update_attributes(access_level: params[:access_level])
-          @member = project_member.user
-          present @member, with: Entities::ProjectMember, project: user_project
-        else
-          handle_member_errors project_member.errors
-        end
-      end
-
-      # Remove a team member from project
-      #
-      # Parameters:
-      #   id (required) - The ID of a project
-      #   user_id (required) - The ID of a team member
-      # Example Request:
-      #   DELETE /projects/:id/members/:user_id
-      delete ":id/members/:user_id" do
-        project_member = user_project.project_members.find_by(user_id: params[:user_id])
-
-        unless current_user.can?(:admin_project, user_project) ||
-                current_user.can?(:destroy_project_member, project_member)
-          forbidden!
-        end
-
-        if project_member.nil?
-          { message: "Access revoked", id: params[:user_id].to_i }
-        else
-          project_member.destroy
-        end
-      end
-    end
-  end
-end

lib/backup/files.rb

@@ -27,7 +27,7 @@ module Backup
 
     def backup_existing_files_dir
       timestamped_files_path = File.join(files_parent_dir, "#{name}.#{Time.now.to_i}")
-      if File.exists?(app_files_dir)
+      if File.exist?(app_files_dir)
         FileUtils.mv(app_files_dir, File.expand_path(timestamped_files_path))
       end
     end

lib/backup/manager.rb

@@ -114,7 +114,7 @@ module Backup
 
       tar_file = ENV["BACKUP"].nil? ? File.join("#{file_list.first}_gitlab_backup.tar") : File.join(ENV["BACKUP"] + "_gitlab_backup.tar")
 
-      unless File.exists?(tar_file)
+      unless File.exist?(tar_file)
         puts "The specified backup doesn't exist!"
         exit 1
       end

lib/backup/repository.rb

@@ -28,7 +28,7 @@ module Backup
 
         wiki = ProjectWiki.new(project)
 
-        if File.exists?(path_to_repo(wiki))
+        if File.exist?(path_to_repo(wiki))
           $progress.print " * #{wiki.path_with_namespace} ... "
           if wiki.repository.empty?
             $progress.puts " [SKIPPED]".color(:cyan)
@@ -49,7 +49,7 @@ module Backup
 
     def restore
       Gitlab.config.repositories.storages.each do |name, path|
-        next unless File.exists?(path)
+        next unless File.exist?(path)
 
         # Move repos dir to 'repositories.old' dir
         bk_repos_path = File.join(path, '..', 'repositories.old.' + Time.now.to_i.to_s)
@@ -63,7 +63,7 @@ module Backup
 
         project.ensure_dir_exist
 
-        if File.exists?(path_to_bundle(project))
+        if File.exist?(path_to_bundle(project))
           FileUtils.mkdir_p(path_to_repo(project))
           cmd = %W(tar -xf #{path_to_bundle(project)} -C #{path_to_repo(project)})
         else
@@ -80,7 +80,7 @@ module Backup
 
         wiki = ProjectWiki.new(project)
 
-        if File.exists?(path_to_bundle(wiki))
+        if File.exist?(path_to_bundle(wiki))
           $progress.print " * #{wiki.path_with_namespace} ... "
 
           # If a wiki bundle exists, first remove the empty repo

lib/ci/gitlab_ci_yaml_processor.rb

@@ -62,7 +62,7 @@ module Ci
         #  - before script should be a concatenated command
         commands: [job[:before_script] || @before_script, job[:script]].flatten.compact.join("\n"),
         tag_list: job[:tags] || [],
-        name: job[:name],
+        name: job[:name].to_s,
         allow_failure: job[:allow_failure] || false,
         when: job[:when] || 'on_success',
         environment: job[:environment],

lib/extracts_path.rb

@@ -94,7 +94,7 @@ module ExtractsPath
     @options = params.select {|key, value| allowed_options.include?(key) && !value.blank? }
     @options = HashWithIndifferentAccess.new(@options)
 
-    @id = Addressable::URI.unescape(get_id)
+    @id = Addressable::URI.normalize_component(get_id)
     @ref, @path = extract_ref(@id)
     @repo = @project.repository
     if @options[:extended_sha1].blank?

lib/gitlab/backend/grack_auth.rb

-module Grack
-  class AuthSpawner
-    def self.call(env)
-      # Avoid issues with instance variables in Grack::Auth persisting across
-      # requests by creating a new instance for each request.
-      Auth.new({}).call(env)
-    end
-  end
-
-  class Auth < Rack::Auth::Basic
-    attr_accessor :user, :project, :env
-
-    def call(env)
-      @env = env
-      @request = Rack::Request.new(env)
-      @auth = Request.new(env)
-
-      @ci = false
-
-      # Need this patch due to the rails mount
-      # Need this if under RELATIVE_URL_ROOT
-      unless Gitlab.config.gitlab.relative_url_root.empty?
-        # If website is mounted using relative_url_root need to remove it first
-        @env['PATH_INFO'] = @request.path.sub(Gitlab.config.gitlab.relative_url_root, '')
-      else
-        @env['PATH_INFO'] = @request.path
-      end
-
-      @env['SCRIPT_NAME'] = ""
-
-      auth!
-
-      lfs_response = Gitlab::Lfs::Router.new(project, @user, @ci, @request).try_call
-      return lfs_response unless lfs_response.nil?
-
-      if @user.nil? && !@ci
-        unauthorized
-      else
-        render_not_found
-      end
-    end
-
-    private
-
-    def auth!
-      return unless @auth.provided?
-
-      return bad_request unless @auth.basic?
-
-      # Authentication with username and password
-      login, password = @auth.credentials
-
-      # Allow authentication for GitLab CI service
-      # if valid token passed
-      if ci_request?(login, password)
-        @ci = true
-        return
-      end
-
-      @user = authenticate_user(login, password)
-    end
-
-    def ci_request?(login, password)
-      matched_login = /(?<s>^[a-zA-Z]*-ci)-token$/.match(login)
-
-      if project && matched_login.present?
-        underscored_service = matched_login['s'].underscore
-
-        if underscored_service == 'gitlab_ci'
-          return project && project.valid_build_token?(password)
-        elsif Service.available_services_names.include?(underscored_service)
-          service_method = "#{underscored_service}_service"
-          service = project.send(service_method)
-
-          return service && service.activated? && service.valid_token?(password)
-        end
-      end
-
-      false
-    end
-
-    def oauth_access_token_check(login, password)
-      if login == "oauth2" && git_cmd == 'git-upload-pack' && password.present?
-        token = Doorkeeper::AccessToken.by_token(password)
-        token && token.accessible? && User.find_by(id: token.resource_owner_id)
-      end
-    end
-
-    def authenticate_user(login, password)
-      user = Gitlab::Auth.find_with_user_password(login, password)
-
-      unless user
-        user = oauth_access_token_check(login, password)
-      end
-
-      # If the user authenticated successfully, we reset the auth failure count
-      # from Rack::Attack for that IP. A client may attempt to authenticate
-      # with a username and blank password first, and only after it receives
-      # a 401 error does it present a password. Resetting the count prevents
-      # false positives from occurring.
-      #
-      # Otherwise, we let Rack::Attack know there was a failed authentication
-      # attempt from this IP. This information is stored in the Rails cache
-      # (Redis) and will be used by the Rack::Attack middleware to decide
-      # whether to block requests from this IP.
-      config = Gitlab.config.rack_attack.git_basic_auth
-
-      if config.enabled
-        if user
-          # A successful login will reset the auth failure count from this IP
-          Rack::Attack::Allow2Ban.reset(@request.ip, config)
-        else
-          banned = Rack::Attack::Allow2Ban.filter(@request.ip, config) do
-            # Unless the IP is whitelisted, return true so that Allow2Ban
-            # increments the counter (stored in Rails.cache) for the IP
-            if config.ip_whitelist.include?(@request.ip)
-              false
-            else
-              true
-            end
-          end
-
-          if banned
-            Rails.logger.info "IP #{@request.ip} failed to login " \
-              "as #{login} but has been temporarily banned from Git auth"
-          end
-        end
-      end
-
-      user
-    end
-
-    def git_cmd
-      if @request.get?
-        @request.params['service']
-      elsif @request.post?
-        File.basename(@request.path)
-      else
-        nil
-      end
-    end
-
-    def project
-      return @project if defined?(@project)
-
-      @project = project_by_path(@request.path_info)
-    end
-
-    def project_by_path(path)
-      if m = /^([\w\.\/-]+)\.git/.match(path).to_a
-        path_with_namespace = m.last
-        path_with_namespace.gsub!(/\.wiki$/, '')
-
-        path_with_namespace[0] = '' if path_with_namespace.start_with?('/')
-        Project.find_with_namespace(path_with_namespace)
-      end
-    end
-
-    def render_not_found
-      [404, { "Content-Type" => "text/plain" }, ["Not Found"]]
-    end
-  end
-end

lib/gitlab/git_access.rb

@@ -134,7 +134,7 @@ module Gitlab
     end
 
     def build_status_object(status, message = '')
-      GitAccessStatus.new(status, message)
+      Gitlab::GitAccessStatus.new(status, message)
     end
   end
 end

lib/gitlab/lfs/response.rb

-module Gitlab
-  module Lfs
-    class Response
-      def initialize(project, user, ci, request)
-        @origin_project = project
-        @project = storage_project(project)
-        @user = user
-        @ci = ci
-        @env = request.env
-        @request = request
-      end
-
-      def render_download_object_response(oid)
-        render_response_to_download do
-          if check_download_sendfile_header?
-            render_lfs_sendfile(oid)
-          else
-            render_not_found
-          end
-        end
-      end
-
-      def render_batch_operation_response
-        request_body = JSON.parse(@request.body.read)
-        case request_body["operation"]
-        when "download"
-          render_batch_download(request_body)
-        when "upload"
-          render_batch_upload(request_body)
-        else
-          render_not_found
-        end
-      end
-
-      def render_storage_upload_authorize_response(oid, size)
-        render_response_to_push do
-          [
-            200,
-            { "Content-Type" => "application/json; charset=utf-8" },
-            [JSON.dump({
-              'StoreLFSPath' => "#{Gitlab.config.lfs.storage_path}/tmp/upload",
-              'LfsOid' => oid,
-              'LfsSize' => size
-            })]
-          ]
-        end
-      end
-
-      def render_storage_upload_store_response(oid, size, tmp_file_name)
-        return render_forbidden unless tmp_file_name
-
-        render_response_to_push do
-          render_lfs_upload_ok(oid, size, tmp_file_name)
-        end
-      end
-
-      def render_unsupported_deprecated_api
-        [
-          501,
-          { "Content-Type" => "application/json; charset=utf-8" },
-          [JSON.dump({
-            'message' => 'Server supports batch API only, please update your Git LFS client to version 1.0.1 and up.',
-            'documentation_url' => "#{Gitlab.config.gitlab.url}/help",
-          })]
-        ]
-      end
-
-      private
-
-      def render_not_enabled
-        [
-          501,
-          {
-            "Content-Type" => "application/json; charset=utf-8",
-          },
-          [JSON.dump({
-            'message' => 'Git LFS is not enabled on this GitLab server, contact your admin.',
-            'documentation_url' => "#{Gitlab.config.gitlab.url}/help",
-          })]
-        ]
-      end
-
-      def render_unauthorized
-        [
-          401,
-          {
-            'Content-Type' => 'text/plain'
-          },
-          ['Unauthorized']
-        ]
-      end
-
-      def render_not_found
-        [
-          404,
-          {
-            "Content-Type" => "application/vnd.git-lfs+json"
-          },
-          [JSON.dump({
-            'message' => 'Not found.',
-            'documentation_url' => "#{Gitlab.config.gitlab.url}/help",
-          })]
-        ]
-      end
-
-      def render_forbidden
-        [
-          403,
-          {
-            "Content-Type" => "application/vnd.git-lfs+json"
-          },
-          [JSON.dump({
-            'message' => 'Access forbidden. Check your access level.',
-            'documentation_url' => "#{Gitlab.config.gitlab.url}/help",
-          })]
-        ]
-      end
-
-      def render_lfs_sendfile(oid)
-        return render_not_found unless oid.present?
-
-        lfs_object = object_for_download(oid)
-
-        if lfs_object && lfs_object.file.exists?
-          [
-            200,
-            {
-              # GitLab-workhorse will forward Content-Type header
-              "Content-Type" => "application/octet-stream",
-              "X-Sendfile" => lfs_object.file.path
-            },
-            []
-          ]
-        else
-          render_not_found
-        end
-      end
-
-      def render_batch_upload(body)
-        return render_not_found if body.empty? || body['objects'].nil?
-
-        render_response_to_push do
-          response = build_upload_batch_response(body['objects'])
-          [
-            200,
-            {
-              "Content-Type" => "application/json; charset=utf-8",
-              "Cache-Control" => "private",
-            },
-            [JSON.dump(response)]
-          ]
-        end
-      end
-
-      def render_batch_download(body)
-        return render_not_found if body.empty? || body['objects'].nil?
-
-        render_response_to_download do
-          response = build_download_batch_response(body['objects'])
-          [
-            200,
-            {
-              "Content-Type" => "application/json; charset=utf-8",
-              "Cache-Control" => "private",
-            },
-            [JSON.dump(response)]
-          ]
-        end
-      end
-
-      def render_lfs_upload_ok(oid, size, tmp_file)
-        if store_file(oid, size, tmp_file)
-          [
-            200,
-            {
-              'Content-Type' => 'text/plain',
-              'Content-Length' => 0
-            },
-            []
-          ]
-        else
-          [
-            422,
-            { 'Content-Type' => 'text/plain' },
-            ["Unprocessable entity"]
-          ]
-        end
-      end
-
-      def render_response_to_download
-        return render_not_enabled unless Gitlab.config.lfs.enabled
-
-        unless @project.public?
-          return render_unauthorized unless @user || @ci
-          return render_forbidden unless user_can_fetch?
-        end
-
-        yield
-      end
-
-      def render_response_to_push
-        return render_not_enabled unless Gitlab.config.lfs.enabled
-        return render_unauthorized unless @user
-        return render_forbidden unless user_can_push?
-
-        yield
-      end
-
-      def check_download_sendfile_header?
-        @env['HTTP_X_SENDFILE_TYPE'].to_s == "X-Sendfile"
-      end
-
-      def user_can_fetch?
-        # Check user access against the project they used to initiate the pull
-        @ci || @user.can?(:download_code, @origin_project)
-      end
-
-      def user_can_push?
-        # Check user access against the project they used to initiate the push
-        @user.can?(:push_code, @origin_project)
-      end
-
-      def storage_project(project)
-        if project.forked?
-          storage_project(project.forked_from_project)
-        else
-          project
-        end
-      end
-
-      def store_file(oid, size, tmp_file)
-        tmp_file_path = File.join("#{Gitlab.config.lfs.storage_path}/tmp/upload", tmp_file)
-
-        object = LfsObject.find_or_create_by(oid: oid, size: size)
-        if object.file.exists?
-          success = true
-        else
-          success = move_tmp_file_to_storage(object, tmp_file_path)
-        end
-
-        if success
-          success = link_to_project(object)
-        end
-
-        success
-      ensure
-        # Ensure that the tmp file is removed
-        FileUtils.rm_f(tmp_file_path)
-      end
-
-      def object_for_download(oid)
-        @project.lfs_objects.find_by(oid: oid)
-      end
-
-      def move_tmp_file_to_storage(object, path)
-        File.open(path) do |f|
-          object.file = f
-        end
-
-        object.file.store!
-        object.save
-      end
-
-      def link_to_project(object)
-        if object && !object.projects.exists?(@project.id)
-          object.projects << @project
-          object.save
-        end
-      end
-
-      def select_existing_objects(objects)
-        objects_oids = objects.map { |o| o['oid'] }
-        @project.lfs_objects.where(oid: objects_oids).pluck(:oid).to_set
-      end
-
-      def build_upload_batch_response(objects)
-        selected_objects = select_existing_objects(objects)
-
-        upload_hypermedia_links(objects, selected_objects)
-      end
-
-      def build_download_batch_response(objects)
-        selected_objects = select_existing_objects(objects)
-
-        download_hypermedia_links(objects, selected_objects)
-      end
-
-      def download_hypermedia_links(all_objects, existing_objects)
-        all_objects.each do |object|
-          if existing_objects.include?(object['oid'])
-            object['actions'] = {
-              'download' => {
-                'href' => "#{@origin_project.http_url_to_repo}/gitlab-lfs/objects/#{object['oid']}",
-                'header' => {
-                  'Authorization' => @env['HTTP_AUTHORIZATION']
-                }.compact
-              }
-            }
-          else
-            object['error'] = {
-              'code' => 404,
-              'message' => "Object does not exist on the server or you don't have permissions to access it",
-            }
-          end
-        end
-
-        { 'objects' => all_objects }
-      end
-
-      def upload_hypermedia_links(all_objects, existing_objects)
-        all_objects.each do |object|
-          # generate actions only for non-existing objects
-          next if existing_objects.include?(object['oid'])
-
-          object['actions'] = {
-            'upload' => {
-              'href' => "#{@origin_project.http_url_to_repo}/gitlab-lfs/objects/#{object['oid']}/#{object['size']}",
-              'header' => {
-                'Authorization' => @env['HTTP_AUTHORIZATION']
-              }.compact
-            }
-          }
-        end
-
-        { 'objects' => all_objects }
-      end
-    end
-  end
-end

lib/gitlab/lfs/router.rb

-module Gitlab
-  module Lfs
-    class Router
-      attr_reader :project, :user, :ci, :request
-
-      def initialize(project, user, ci, request)
-        @project = project
-        @user = user
-        @ci = ci
-        @env = request.env
-        @request = request
-      end
-
-      def try_call
-        return unless @request && @request.path.present?
-
-        case @request.request_method
-        when 'GET'
-          get_response
-        when 'POST'
-          post_response
-        when 'PUT'
-          put_response
-        else
-          nil
-        end
-      end
-
-      private
-
-      def get_response
-        path_match = @request.path.match(/\/(info\/lfs|gitlab-lfs)\/objects\/([0-9a-f]{64})$/)
-        return nil unless path_match
-
-        oid = path_match[2]
-        return nil unless oid
-
-        case path_match[1]
-        when "info/lfs"
-          lfs.render_unsupported_deprecated_api
-        when "gitlab-lfs"
-          lfs.render_download_object_response(oid)
-        else
-          nil
-        end
-      end
-
-      def post_response
-        post_path = @request.path.match(/\/info\/lfs\/objects(\/batch)?$/)
-        return nil unless post_path
-
-        # Check for Batch API
-        if post_path[0].ends_with?("/info/lfs/objects/batch")
-          lfs.render_batch_operation_response
-        elsif post_path[0].ends_with?("/info/lfs/objects")
-          lfs.render_unsupported_deprecated_api
-        else
-          nil
-        end
-      end
-
-      def put_response
-        object_match = @request.path.match(/\/gitlab-lfs\/objects\/([0-9a-f]{64})\/([0-9]+)(|\/authorize){1}$/)
-        return nil if object_match.nil?
-
-        oid = object_match[1]
-        size = object_match[2].try(:to_i)
-        return nil if oid.nil? || size.nil?
-
-        # GitLab-workhorse requests
-        # 1. Try to authorize the request
-        # 2. send a request with a header containing the name of the temporary file
-        if object_match[3] && object_match[3] == '/authorize'
-          lfs.render_storage_upload_authorize_response(oid, size)
-        else
-          tmp_file_name = sanitize_tmp_filename(@request.env['HTTP_X_GITLAB_LFS_TMP'])
-          lfs.render_storage_upload_store_response(oid, size, tmp_file_name)
-        end
-      end
-
-      def lfs
-        return unless @project
-
-        Gitlab::Lfs::Response.new(@project, @user, @ci, @request)
-      end
-
-      def sanitize_tmp_filename(name)
-        if name.present?
-          name.gsub!(/^.*(\\|\/)/, '')
-          name = name.match(/[0-9a-f]{73}/)
-          name[0] if name
-        else
-          nil
-        end
-      end
-    end
-  end
-end

lib/tasks/gitlab/check.rake

@@ -64,7 +64,7 @@ namespace :gitlab do
           for_more_information(
             see_installation_guide_section "GitLab"
           )
-       end
+        end
       end
     end
 
@@ -73,7 +73,7 @@ namespace :gitlab do
 
       database_config_file = Rails.root.join("config", "database.yml")
 
-      if File.exists?(database_config_file)
+      if File.exist?(database_config_file)
         puts "yes".color(:green)
       else
         puts "no".color(:red)
@@ -94,7 +94,7 @@ namespace :gitlab do
 
       gitlab_config_file = Rails.root.join("config", "gitlab.yml")
 
-      if File.exists?(gitlab_config_file)
+      if File.exist?(gitlab_config_file)
         puts "yes".color(:green)
       else
         puts "no".color(:red)
@@ -113,7 +113,7 @@ namespace :gitlab do
       print "GitLab config outdated? ... "
 
       gitlab_config_file = Rails.root.join("config", "gitlab.yml")
-      unless File.exists?(gitlab_config_file)
+      unless File.exist?(gitlab_config_file)
         puts "can't check because of previous errors".color(:magenta)
       end
 
@@ -144,7 +144,7 @@ namespace :gitlab do
 
       script_path = "/etc/init.d/gitlab"
 
-      if File.exists?(script_path)
+      if File.exist?(script_path)
         puts "yes".color(:green)
       else
         puts "no".color(:red)
@@ -169,7 +169,7 @@ namespace :gitlab do
       recipe_path = Rails.root.join("lib/support/init.d/", "gitlab")
       script_path = "/etc/init.d/gitlab"
 
-      unless File.exists?(script_path)
+      unless File.exist?(script_path)
         puts "can't check because of previous errors".color(:magenta)
         return
       end
@@ -361,7 +361,7 @@ namespace :gitlab do
       Gitlab.config.repositories.storages.each do |name, repo_base_path|
         print "#{name}... "
 
-        if File.exists?(repo_base_path)
+        if File.exist?(repo_base_path)
           puts "yes".color(:green)
         else
           puts "no".color(:red)
@@ -385,7 +385,7 @@ namespace :gitlab do
       Gitlab.config.repositories.storages.each do |name, repo_base_path|
         print "#{name}... "
 
-        unless File.exists?(repo_base_path)
+        unless File.exist?(repo_base_path)
           puts "can't check because of previous errors".color(:magenta)
           return
         end
@@ -408,7 +408,7 @@ namespace :gitlab do
       Gitlab.config.repositories.storages.each do |name, repo_base_path|
         print "#{name}... "
 
-        unless File.exists?(repo_base_path)
+        unless File.exist?(repo_base_path)
           puts "can't check because of previous errors".color(:magenta)
           return
         end
@@ -438,7 +438,7 @@ namespace :gitlab do
       Gitlab.config.repositories.storages.each do |name, repo_base_path|
         print "#{name}... "
 
-        unless File.exists?(repo_base_path)
+        unless File.exist?(repo_base_path)
           puts "can't check because of previous errors".color(:magenta)
           return
         end

lib/tasks/gitlab/shell.rake

@@ -90,7 +90,7 @@ namespace :gitlab do
     task build_missing_projects: :environment do
       Project.find_each(batch_size: 1000) do |project|
         path_to_repo = project.repository.path_to_repo
-        if File.exists?(path_to_repo)
+        if File.exist?(path_to_repo)
           print '-'
         else
           if Gitlab::Shell.new.add_repository(project.repository_storage_path,

lib/tasks/spinach.rake

@@ -46,7 +46,7 @@ def run_spinach_tests(tags)
   success = run_spinach_command(%W(--tags #{tags}))
   3.times do |_|
     break if success
-    break unless File.exists?('tmp/spinach-rerun.txt')
+    break unless File.exist?('tmp/spinach-rerun.txt')
 
     tests = File.foreach('tmp/spinach-rerun.txt').map(&:chomp)
     puts ''

spec/factories/ci/builds.rb

@@ -7,6 +7,7 @@ FactoryGirl.define do
     stage_idx 0
     ref 'master'
     tag false
+    status 'pending'
     created_at 'Di 29. Okt 09:50:00 CET 2013'
     started_at 'Di 29. Okt 09:51:28 CET 2013'
     finished_at 'Di 29. Okt 09:53:28 CET 2013'
@@ -45,6 +46,10 @@ FactoryGirl.define do
       status 'pending'
     end
 
+    trait :created do
+      status 'created'
+    end
+
     trait :manual do
       status 'skipped'
       self.when 'manual'

spec/factories/ci/pipelines.rb

@@ -18,7 +18,9 @@
 
 FactoryGirl.define do
   factory :ci_empty_pipeline, class: Ci::Pipeline do
+    ref 'master'
     sha '97de212e80737a608d939f648d959671fb0a0142'
+    status 'pending'
 
     project factory: :empty_project
 

spec/factories/commit_statuses.rb

@@ -7,6 +7,30 @@ FactoryGirl.define do
     started_at 'Tue, 26 Jan 2016 08:21:42 +0100'
     finished_at 'Tue, 26 Jan 2016 08:23:42 +0100'
 
+    trait :success do
+      status 'success'
+    end
+
+    trait :failed do
+      status 'failed'
+    end
+
+    trait :canceled do
+      status 'canceled'
+    end
+
+    trait :running do
+      status 'running'
+    end
+
+    trait :pending do
+      status 'pending'
+    end
+
+    trait :created do
+      status 'created'
+    end
+
     after(:build) do |build, evaluator|
       build.project = build.pipeline.project
     end

spec/features/merge_requests/created_from_fork_spec.rb

@@ -29,12 +29,16 @@ feature 'Merge request created from fork' do
     include WaitForAjax
 
     given(:pipeline) do
-      create(:ci_pipeline_with_two_job, project: fork_project,
-                                        sha: merge_request.diff_head_sha,
-                                        ref: merge_request.source_branch)
+      create(:ci_pipeline,
+             project: fork_project,
+             sha: merge_request.diff_head_sha,
+             ref: merge_request.source_branch)
     end
 
-    background { pipeline.create_builds(user) }
+    background do
+      create(:ci_build, pipeline: pipeline, name: 'rspec')
+      create(:ci_build, pipeline: pipeline, name: 'spinach')
+    end
 
     scenario 'user visits a pipelines page', js: true do
       visit_merge_request(merge_request)

spec/features/pipelines_spec.rb

@@ -33,7 +33,10 @@ describe "Pipelines" do
     context 'cancelable pipeline' do
       let!(:running) { create(:ci_build, :running, pipeline: pipeline, stage: 'test', commands: 'test') }
 
-      before { visit namespace_project_pipelines_path(project.namespace, project) }
+      before do
+        pipeline.reload_status!
+        visit namespace_project_pipelines_path(project.namespace, project)
+      end
 
       it { expect(page).to have_link('Cancel') }
       it { expect(page).to have_selector('.ci-running') }
@@ -49,7 +52,10 @@ describe "Pipelines" do
     context 'retryable pipelines' do
       let!(:failed) { create(:ci_build, :failed, pipeline: pipeline, stage: 'test', commands: 'test') }
 
-      before { visit namespace_project_pipelines_path(project.namespace, project) }
+      before do
+        pipeline.reload_status!
+        visit namespace_project_pipelines_path(project.namespace, project)
+      end
 
       it { expect(page).to have_link('Retry') }
       it { expect(page).to have_selector('.ci-failed') }
@@ -80,7 +86,10 @@ describe "Pipelines" do
       context 'when running' do
         let!(:running) { create(:generic_commit_status, status: 'running', pipeline: pipeline, stage: 'test') }
 
-        before { visit namespace_project_pipelines_path(project.namespace, project) }
+        before do
+          pipeline.reload_status!
+          visit namespace_project_pipelines_path(project.namespace, project)
+        end
 
         it 'is not cancelable' do
           expect(page).not_to have_link('Cancel')
@@ -92,9 +101,12 @@ describe "Pipelines" do
       end
 
       context 'when failed' do
-        let!(:running) { create(:generic_commit_status, status: 'failed', pipeline: pipeline, stage: 'test') }
+        let!(:failed) { create(:generic_commit_status, status: 'failed', pipeline: pipeline, stage: 'test') }
 
-        before { visit namespace_project_pipelines_path(project.namespace, project) }
+        before do
+          pipeline.reload_status!
+          visit namespace_project_pipelines_path(project.namespace, project)
+        end
 
         it 'is not retryable' do
           expect(page).not_to have_link('Retry')
@@ -211,7 +223,7 @@ describe "Pipelines" do
 
     context 'for invalid commit' do
       before do
-        fill_in('Create for', with: 'invalid reference')
+        fill_in('Create for', with: 'invalid-reference')
         click_on 'Create pipeline'
       end
 

spec/fixtures/project_services/campfire/rooms.json

+{
+  "rooms": [
+    {
+      "name": "test-room",
+      "locked": false,
+      "created_at": "2009/01/07 20:43:11 +0000",
+      "updated_at": "2009/03/18 14:31:39 +0000",
+      "topic": "The room topic\n",
+      "id": 123,
+      "membership_limit": 4
+    },
+    {
+      "name": "another room",
+      "locked": true,
+      "created_at": "2009/03/18 14:30:42 +0000",
+      "updated_at": "2013/01/27 14:14:27 +0000",
+      "topic": "Comment, ideas, GitHub notifications for eCommittee App",
+      "id": 456,
+      "membership_limit": 4
+    }
+  ]
+}

spec/fixtures/project_services/campfire/rooms2.json

+{
+  "rooms": [
+    {
+      "name": "test-room-not-found",
+      "locked": false,
+      "created_at": "2009/01/07 20:43:11 +0000",
+      "updated_at": "2009/03/18 14:31:39 +0000",
+      "topic": "The room topic\n",
+      "id": 123,
+      "membership_limit": 4
+    },
+    {
+      "name": "another room",
+      "locked": true,
+      "created_at": "2009/03/18 14:30:42 +0000",
+      "updated_at": "2013/01/27 14:14:27 +0000",
+      "topic": "Comment, ideas, GitHub notifications for eCommittee App",
+      "id": 456,
+      "membership_limit": 4
+    }
+  ]
+}

spec/lib/ci/charts_spec.rb

@@ -5,6 +5,7 @@ describe Ci::Charts, lib: true do
     before do
       @pipeline = FactoryGirl.create(:ci_pipeline)
       FactoryGirl.create(:ci_build, pipeline: @pipeline)
+      @pipeline.reload_status!
     end
 
     it 'returns build times in minutes' do

spec/lib/ci/gitlab_ci_yaml_processor_spec.rb

@@ -19,7 +19,7 @@ module Ci
         expect(config_processor.builds_for_stage_and_ref(type, "master").first).to eq({
           stage: "test",
           stage_idx: 1,
-          name: :rspec,
+          name: "rspec",
           commands: "pwd\nrspec",
           tag_list: [],
           options: {},
@@ -433,7 +433,7 @@ module Ci
         expect(config_processor.builds_for_stage_and_ref("test", "master").first).to eq({
           stage: "test",
           stage_idx: 1,
-          name: :rspec,
+          name: "rspec",
           commands: "pwd\nrspec",
           tag_list: [],
           options: {
@@ -461,7 +461,7 @@ module Ci
         expect(config_processor.builds_for_stage_and_ref("test", "master").first).to eq({
           stage: "test",
           stage_idx: 1,
-          name: :rspec,
+          name: "rspec",
           commands: "pwd\nrspec",
           tag_list: [],
           options: {
@@ -700,7 +700,7 @@ module Ci
         expect(config_processor.builds_for_stage_and_ref("test", "master").first).to eq({
           stage: "test",
           stage_idx: 1,
-          name: :rspec,
+          name: "rspec",
           commands: "pwd\nrspec",
           tag_list: [],
           options: {
@@ -837,7 +837,7 @@ module Ci
           expect(subject.first).to eq({
             stage: "test",
             stage_idx: 1,
-            name: :normal_job,
+            name: "normal_job",
             commands: "test",
             tag_list: [],
             options: {},
@@ -882,7 +882,7 @@ module Ci
           expect(subject.first).to eq({
             stage: "build",
             stage_idx: 0,
-            name: :job1,
+            name: "job1",
             commands: "execute-script-for-job",
             tag_list: [],
             options: {},
@@ -894,7 +894,7 @@ module Ci
           expect(subject.second).to eq({
             stage: "build",
             stage_idx: 0,
-            name: :job2,
+            name: "job2",
             commands: "execute-script-for-job",
             tag_list: [],
             options: {},

spec/lib/extracts_path_spec.rb

@@ -30,15 +30,28 @@ describe ExtractsPath, lib: true do
       expect(@logs_path).to eq("/#{@project.path_with_namespace}/refs/#{ref}/logs_tree/files/ruby/popen.rb")
     end
 
-    context 'escaped sequences in ref' do
-      let(:ref) { "improve%2Fawesome" }
+    context 'escaped slash character in ref' do
+      let(:ref) { 'improve%2Fawesome' }
 
-      it "id has no escape sequences" do
+      it 'has no escape sequences in @ref or @logs_path' do
         assign_ref_vars
+
         expect(@ref).to eq('improve/awesome')
         expect(@logs_path).to eq("/#{@project.path_with_namespace}/refs/#{ref}/logs_tree/files/ruby/popen.rb")
       end
     end
+
+    context 'ref contains %20' do
+      let(:ref) { 'foo%20bar' }
+
+      it 'is not converted to a space in @id' do
+        @project.repository.add_branch(@project.owner, 'foo%20bar', 'master')
+
+        assign_ref_vars
+
+        expect(@id).to start_with('foo%20bar/')
+      end
+    end
   end
 
   describe '#extract_ref' do

spec/lib/gitlab/badge/build_spec.rb

@@ -96,9 +96,10 @@ describe Gitlab::Badge::Build do
   end
 
   def create_build(project, sha, branch)
-    pipeline = create(:ci_pipeline, project: project,
-                                    sha: sha,
-                                    ref: branch)
+    pipeline = create(:ci_empty_pipeline,
+                      project: project,
+                      sha: sha,
+                      ref: branch)
 
     create(:ci_build, pipeline: pipeline, stage: 'notify')
   end

spec/models/build_spec.rb

@@ -765,6 +765,53 @@ describe Ci::Build, models: true do
     end
   end
 
+  describe '#when' do
+    subject { build.when }
+
+    context 'if is undefined' do
+      before do
+        build.when = nil
+      end
+
+      context 'use from gitlab-ci.yml' do
+        before do
+          stub_ci_pipeline_yaml_file(config)
+        end
+
+        context 'if config is not found' do
+          let(:config) { nil }
+
+          it { is_expected.to eq('on_success') }
+        end
+
+        context 'if config does not have a questioned job' do
+          let(:config) do
+            YAML.dump({
+                        test_other: {
+                          script: 'Hello World'
+                        }
+                      })
+          end
+
+          it { is_expected.to eq('on_success') }
+        end
+
+        context 'if config has when' do
+          let(:config) do
+            YAML.dump({
+                        test: {
+                          script: 'Hello World',
+                          when: 'always'
+                        }
+                      })
+          end
+
+          it { is_expected.to eq('always') }
+        end
+      end
+    end
+  end
+
   describe '#retryable?' do
     context 'when build is running' do
       before do

spec/models/ci/pipeline_spec.rb

@@ -38,9 +38,6 @@ describe Ci::Pipeline, models: true do
     it { expect(pipeline.sha).to start_with(subject) }
   end
 
-  describe '#create_next_builds' do
-  end
-
   describe '#retried' do
     subject { pipeline.retried }
 
@@ -54,304 +51,20 @@ describe Ci::Pipeline, models: true do
     end
   end
 
-  describe '#create_builds' do
-    let!(:pipeline) { FactoryGirl.create :ci_pipeline, project: project, ref: 'master', tag: false }
-
-    def create_builds(trigger_request = nil)
-      pipeline.create_builds(nil, trigger_request)
-    end
-
-    def create_next_builds
-      pipeline.create_next_builds(pipeline.builds.order(:id).last)
-    end
-
-    it 'creates builds' do
-      expect(create_builds).to be_truthy
-      pipeline.builds.update_all(status: "success")
-      expect(pipeline.builds.count(:all)).to eq(2)
-
-      expect(create_next_builds).to be_truthy
-      pipeline.builds.update_all(status: "success")
-      expect(pipeline.builds.count(:all)).to eq(4)
-
-      expect(create_next_builds).to be_truthy
-      pipeline.builds.update_all(status: "success")
-      expect(pipeline.builds.count(:all)).to eq(5)
-
-      expect(create_next_builds).to be_falsey
-    end
-
-    context 'custom stage with first job allowed to fail' do
-      let(:yaml) do
-        {
-          stages: ['clean', 'test'],
-          clean_job: {
-            stage: 'clean',
-            allow_failure: true,
-            script: 'BUILD',
-          },
-          test_job: {
-            stage: 'test',
-            script: 'TEST',
-          },
-        }
-      end
-
-      before do
-        stub_ci_pipeline_yaml_file(YAML.dump(yaml))
-        create_builds
-      end
-
-      it 'properly schedules builds' do
-        expect(pipeline.builds.pluck(:status)).to contain_exactly('pending')
-        pipeline.builds.running_or_pending.each(&:drop)
-        expect(pipeline.builds.pluck(:status)).to contain_exactly('pending', 'failed')
-      end
-    end
-
-    context 'properly creates builds when "when" is defined' do
-      let(:yaml) do
-        {
-          stages: ["build", "test", "test_failure", "deploy", "cleanup"],
-          build: {
-            stage: "build",
-            script: "BUILD",
-          },
-          test: {
-            stage: "test",
-            script: "TEST",
-          },
-          test_failure: {
-            stage: "test_failure",
-            script: "ON test failure",
-            when: "on_failure",
-          },
-          deploy: {
-            stage: "deploy",
-            script: "PUBLISH",
-          },
-          cleanup: {
-            stage: "cleanup",
-            script: "TIDY UP",
-            when: "always",
-          }
-        }
-      end
-
-      before do
-        stub_ci_pipeline_yaml_file(YAML.dump(yaml))
-      end
-
-      context 'when builds are successful' do
-        it 'properly creates builds' do
-          expect(create_builds).to be_truthy
-          expect(pipeline.builds.pluck(:name)).to contain_exactly('build')
-          expect(pipeline.builds.pluck(:status)).to contain_exactly('pending')
-          pipeline.builds.running_or_pending.each(&:success)
-
-          expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test')
-          expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'pending')
-          pipeline.builds.running_or_pending.each(&:success)
-
-          expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy')
-          expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'success', 'pending')
-          pipeline.builds.running_or_pending.each(&:success)
-
-          expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy', 'cleanup')
-          expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'success', 'success', 'pending')
-          pipeline.builds.running_or_pending.each(&:success)
-
-          expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'success', 'success', 'success')
-          pipeline.reload
-          expect(pipeline.status).to eq('success')
-        end
-      end
-
-      context 'when test job fails' do
-        it 'properly creates builds' do
-          expect(create_builds).to be_truthy
-          expect(pipeline.builds.pluck(:name)).to contain_exactly('build')
-          expect(pipeline.builds.pluck(:status)).to contain_exactly('pending')
-          pipeline.builds.running_or_pending.each(&:success)
-
-          expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test')
-          expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'pending')
-          pipeline.builds.running_or_pending.each(&:drop)
-
-          expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure')
-          expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'failed', 'pending')
-          pipeline.builds.running_or_pending.each(&:success)
-
-          expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure', 'cleanup')
-          expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'failed', 'success', 'pending')
-          pipeline.builds.running_or_pending.each(&:success)
-
-          expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'failed', 'success', 'success')
-          pipeline.reload
-          expect(pipeline.status).to eq('failed')
-        end
-      end
-
-      context 'when test and test_failure jobs fail' do
-        it 'properly creates builds' do
-          expect(create_builds).to be_truthy
-          expect(pipeline.builds.pluck(:name)).to contain_exactly('build')
-          expect(pipeline.builds.pluck(:status)).to contain_exactly('pending')
-          pipeline.builds.running_or_pending.each(&:success)
-
-          expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test')
-          expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'pending')
-          pipeline.builds.running_or_pending.each(&:drop)
-
-          expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure')
-          expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'failed', 'pending')
-          pipeline.builds.running_or_pending.each(&:drop)
-
-          expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure', 'cleanup')
-          expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'failed', 'failed', 'pending')
-          pipeline.builds.running_or_pending.each(&:success)
-
-          expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure', 'cleanup')
-          expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'failed', 'failed', 'success')
-          pipeline.reload
-          expect(pipeline.status).to eq('failed')
-        end
-      end
-
-      context 'when deploy job fails' do
-        it 'properly creates builds' do
-          expect(create_builds).to be_truthy
-          expect(pipeline.builds.pluck(:name)).to contain_exactly('build')
-          expect(pipeline.builds.pluck(:status)).to contain_exactly('pending')
-          pipeline.builds.running_or_pending.each(&:success)
-
-          expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test')
-          expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'pending')
-          pipeline.builds.running_or_pending.each(&:success)
-
-          expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy')
-          expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'success', 'pending')
-          pipeline.builds.running_or_pending.each(&:drop)
-
-          expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy', 'cleanup')
-          expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'success', 'failed', 'pending')
-          pipeline.builds.running_or_pending.each(&:success)
-
-          expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'success', 'failed', 'success')
-          pipeline.reload
-          expect(pipeline.status).to eq('failed')
-        end
-      end
-
-      context 'when build is canceled in the second stage' do
-        it 'does not schedule builds after build has been canceled' do
-          expect(create_builds).to be_truthy
-          expect(pipeline.builds.pluck(:name)).to contain_exactly('build')
-          expect(pipeline.builds.pluck(:status)).to contain_exactly('pending')
-          pipeline.builds.running_or_pending.each(&:success)
-
-          expect(pipeline.builds.running_or_pending).not_to be_empty
-
-          expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test')
-          expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'pending')
-          pipeline.builds.running_or_pending.each(&:cancel)
-
-          expect(pipeline.builds.running_or_pending).to be_empty
-          expect(pipeline.reload.status).to eq('canceled')
-        end
-      end
-
-      context 'when listing manual actions' do
-        let(:yaml) do
-          {
-            stages: ["build", "test", "staging", "production", "cleanup"],
-            build: {
-              stage: "build",
-              script: "BUILD",
-            },
-            test: {
-              stage: "test",
-              script: "TEST",
-            },
-            staging: {
-              stage: "staging",
-              script: "PUBLISH",
-            },
-            production: {
-              stage: "production",
-              script: "PUBLISH",
-              when: "manual",
-            },
-            cleanup: {
-              stage: "cleanup",
-              script: "TIDY UP",
-              when: "always",
-            },
-            clear_cache: {
-              stage: "cleanup",
-              script: "CLEAR CACHE",
-              when: "manual",
-            }
-          }
-        end
-
-        it 'returns only for skipped builds' do
-          # currently all builds are created
-          expect(create_builds).to be_truthy
-          expect(manual_actions).to be_empty
-
-          # succeed stage build
-          pipeline.builds.running_or_pending.each(&:success)
-          expect(manual_actions).to be_empty
-
-          # succeed stage test
-          pipeline.builds.running_or_pending.each(&:success)
-          expect(manual_actions).to be_empty
-
-          # succeed stage staging and skip stage production
-          pipeline.builds.running_or_pending.each(&:success)
-          expect(manual_actions).to be_many # production and clear cache
-
-          # succeed stage cleanup
-          pipeline.builds.running_or_pending.each(&:success)
-
-          # after processing a pipeline we should have 6 builds, 5 succeeded
-          expect(pipeline.builds.count).to eq(6)
-          expect(pipeline.builds.success.count).to eq(4)
-        end
-
-        def manual_actions
-          pipeline.manual_actions
-        end
-      end
-    end
-
-    context 'when no builds created' do
-      let(:pipeline) { build(:ci_pipeline) }
-
-      before do
-        stub_ci_pipeline_yaml_file(YAML.dump(before_script: ['ls']))
-      end
-
-      it 'returns false' do
-        expect(pipeline.create_builds(nil)).to be_falsey
-        expect(pipeline).not_to be_persisted
-      end
-    end
-  end
-
   describe "#finished_at" do
     let(:pipeline) { FactoryGirl.create :ci_pipeline }
 
     it "returns finished_at of latest build" do
       build = FactoryGirl.create :ci_build, pipeline: pipeline, finished_at: Time.now - 60
       FactoryGirl.create :ci_build, pipeline: pipeline, finished_at: Time.now - 120
+      pipeline.reload_status!
 
       expect(pipeline.finished_at.to_i).to eq(build.finished_at.to_i)
     end
 
     it "returns nil if there is no finished build" do
       FactoryGirl.create :ci_not_started_build, pipeline: pipeline
+      pipeline.reload_status!
 
       expect(pipeline.finished_at).to be_nil
     end
@@ -359,7 +72,7 @@ describe Ci::Pipeline, models: true do
 
   describe "coverage" do
     let(:project) { FactoryGirl.create :empty_project, build_coverage_regex: "/.*/" }
-    let(:pipeline) { FactoryGirl.create :ci_pipeline, project: project }
+    let(:pipeline) { FactoryGirl.create :ci_empty_pipeline, project: project }
 
     it "calculates average when there are two builds with coverage" do
       FactoryGirl.create :ci_build, name: "rspec", coverage: 30, pipeline: pipeline
@@ -426,31 +139,30 @@ describe Ci::Pipeline, models: true do
     end
   end
 
-  describe '#update_state' do
-    it 'executes update_state after touching object' do
-      expect(pipeline).to receive(:update_state).and_return(true)
-      pipeline.touch
-    end
+  describe '#reload_status!' do
+    let(:pipeline) { create :ci_empty_pipeline, project: project }
 
     context 'dependent objects' do
-      let(:commit_status) { build :commit_status, pipeline: pipeline }
+      let(:commit_status) { create :commit_status, :pending, pipeline: pipeline }
+
+      it 'executes reload_status! after succeeding dependent object' do
+        expect(pipeline).to receive(:reload_status!).and_return(true)
 
-      it 'executes update_state after saving dependent object' do
-        expect(pipeline).to receive(:update_state).and_return(true)
-        commit_status.save
+        commit_status.success
       end
     end
 
-    context 'update state' do
+    context 'updates' do
       let(:current) { Time.now.change(usec: 0) }
-      let(:build) { FactoryGirl.create :ci_build, :success, pipeline: pipeline, started_at: current - 120, finished_at: current - 60 }
+      let(:build) { FactoryGirl.create :ci_build, pipeline: pipeline, started_at: current - 120, finished_at: current - 60 }
 
       before do
         build
+        pipeline.reload_status!
       end
 
       [:status, :started_at, :finished_at, :duration].each do |param|
-        it "update #{param}" do
+        it "#{param}" do
           expect(pipeline.send(param)).to eq(build.send(param))
         end
       end

spec/models/member_spec.rb

@@ -10,7 +10,7 @@ describe Member, models: true do
 
     it { is_expected.to validate_presence_of(:user) }
     it { is_expected.to validate_presence_of(:source) }
-    it { is_expected.to validate_inclusion_of(:access_level).in_array(Gitlab::Access.values) }
+    it { is_expected.to validate_inclusion_of(:access_level).in_array(Gitlab::Access.all_values) }
 
     it_behaves_like 'an object with email-formated attributes', :invite_email do
       subject { build(:project_member) }

spec/models/members/project_member_spec.rb

@@ -27,6 +27,7 @@ describe ProjectMember, models: true do
   describe 'validations' do
     it { is_expected.to allow_value('Project').for(:source_type) }
     it { is_expected.not_to allow_value('project').for(:source_type) }
+    it { is_expected.to validate_inclusion_of(:access_level).in_array(Gitlab::Access.values) }
   end
 
   describe 'modules' do
@@ -40,7 +41,7 @@ describe ProjectMember, models: true do
   end
 
   describe "#destroy" do
-    let(:owner)   { create(:project_member, access_level: ProjectMember::OWNER) }
+    let(:owner)   { create(:project_member, access_level: ProjectMember::MASTER) }
     let(:project) { owner.project }
     let(:master)  { create(:project_member, project: project) }
 

spec/models/project_services/campfire_service_spec.rb

@@ -39,4 +39,62 @@ describe CampfireService, models: true do
       it { is_expected.not_to validate_presence_of(:token) }
     end
   end
+
+  describe "#execute" do
+    let(:user)    { create(:user) }
+    let(:project) { create(:project) }
+
+    before do
+      @campfire_service = CampfireService.new
+      allow(@campfire_service).to receive_messages(
+        project_id: project.id,
+        project: project,
+        service_hook: true,
+        token: 'verySecret',
+        subdomain: 'project-name',
+        room: 'test-room'
+      )
+      @sample_data = Gitlab::PushDataBuilder.build_sample(project, user)
+      @rooms_url = 'https://verySecret:X@project-name.campfirenow.com/rooms.json'
+      @headers = { 'Content-Type' => 'application/json; charset=utf-8' }
+    end
+
+    it "calls Campfire API to get a list of rooms and speak in a room" do
+      # make sure a valid list of rooms is returned
+      body = File.read(Rails.root + 'spec/fixtures/project_services/campfire/rooms.json')
+      WebMock.stub_request(:get, @rooms_url).to_return(
+        body: body,
+        status: 200,
+        headers: @headers
+      )
+      # stub the speak request with the room id found in the previous request's response
+      speak_url = 'https://verySecret:X@project-name.campfirenow.com/room/123/speak.json'
+      WebMock.stub_request(:post, speak_url)
+
+      @campfire_service.execute(@sample_data)
+
+      expect(WebMock).to have_requested(:get, @rooms_url).once
+      expect(WebMock).to have_requested(:post, speak_url).with(
+        body: /#{project.path}.*#{@sample_data[:before]}.*#{@sample_data[:after]}/
+      ).once
+    end
+
+    it "calls Campfire API to get a list of rooms but shouldn't speak in a room" do
+      # return a list of rooms that do not contain a room named 'test-room'
+      body = File.read(Rails.root + 'spec/fixtures/project_services/campfire/rooms2.json')
+      WebMock.stub_request(:get, @rooms_url).to_return(
+        body: body,
+        status: 200,
+        headers: @headers
+      )
+      # we want to make sure no request is sent to the /speak endpoint, here is a basic
+      # regexp that matches this endpoint
+      speak_url = 'https://verySecret:X@project-name.campfirenow.com/room/.*/speak.json'
+
+      @campfire_service.execute(@sample_data)
+
+      expect(WebMock).to have_requested(:get, @rooms_url).once
+      expect(WebMock).not_to have_requested(:post, /#{speak_url}/)
+    end
+  end
 end

spec/models/project_services/hipchat_service_spec.rb

@@ -301,7 +301,8 @@ describe HipchatService, models: true do
     end
 
     context 'build events' do
-      let(:build) { create(:ci_build) }
+      let(:pipeline) { create(:ci_empty_pipeline) }
+      let(:build) { create(:ci_build, pipeline: pipeline) }
       let(:data) { Gitlab::DataBuilder::BuildDataBuilder.build(build) }
 
       context 'for failed' do

spec/requests/api/access_requests_spec.rb

+require 'spec_helper'
+
+describe API::AccessRequests, api: true  do
+  include ApiHelpers
+
+  let(:master) { create(:user) }
+  let(:developer) { create(:user) }
+  let(:access_requester) { create(:user) }
+  let(:stranger) { create(:user) }
+
+  let(:project) do
+    project = create(:project, :public, creator_id: master.id, namespace: master.namespace)
+    project.team << [developer, :developer]
+    project.team << [master, :master]
+    project.request_access(access_requester)
+    project
+  end
+
+  let(:group) do
+    group = create(:group, :public)
+    group.add_developer(developer)
+    group.add_owner(master)
+    group.request_access(access_requester)
+    group
+  end
+
+  shared_examples 'GET /:sources/:id/access_requests' do |source_type|
+    context "with :sources == #{source_type.pluralize}" do
+      it_behaves_like 'a 404 response when source is private' do
+        let(:route) { get api("/#{source_type.pluralize}/#{source.id}/access_requests", stranger) }
+      end
+
+      context 'when authenticated as a non-master/owner' do
+        %i[developer access_requester stranger].each do |type|
+          context "as a #{type}" do
+            it 'returns 403' do
+              user = public_send(type)
+              get api("/#{source_type.pluralize}/#{source.id}/access_requests", user)
+
+              expect(response).to have_http_status(403)
+            end
+          end
+        end
+      end
+
+      context 'when authenticated as a master/owner' do
+        it 'returns access requesters' do
+          get api("/#{source_type.pluralize}/#{source.id}/access_requests", master)
+
+          expect(response).to have_http_status(200)
+          expect(json_response).to be_an Array
+          expect(json_response.size).to eq(1)
+        end
+      end
+    end
+  end
+
+  shared_examples 'POST /:sources/:id/access_requests' do |source_type|
+    context "with :sources == #{source_type.pluralize}" do
+      it_behaves_like 'a 404 response when source is private' do
+        let(:route) { post api("/#{source_type.pluralize}/#{source.id}/access_requests", stranger) }
+      end
+
+      context 'when authenticated as a member' do
+        %i[developer master].each do |type|
+          context "as a #{type}" do
+            it 'returns 400' do
+              expect do
+                user = public_send(type)
+                post api("/#{source_type.pluralize}/#{source.id}/access_requests", user)
+
+                expect(response).to have_http_status(400)
+              end.not_to change { source.requesters.count }
+            end
+          end
+        end
+      end
+
+      context 'when authenticated as an access requester' do
+        it 'returns 400' do
+          expect do
+            post api("/#{source_type.pluralize}/#{source.id}/access_requests", access_requester)
+
+            expect(response).to have_http_status(400)
+          end.not_to change { source.requesters.count }
+        end
+      end
+
+      context 'when authenticated as a stranger' do
+        it 'returns 201' do
+          expect do
+            post api("/#{source_type.pluralize}/#{source.id}/access_requests", stranger)
+
+            expect(response).to have_http_status(201)
+          end.to change { source.requesters.count }.by(1)
+
+          # User attributes
+          expect(json_response['id']).to eq(stranger.id)
+          expect(json_response['name']).to eq(stranger.name)
+          expect(json_response['username']).to eq(stranger.username)
+          expect(json_response['state']).to eq(stranger.state)
+          expect(json_response['avatar_url']).to eq(stranger.avatar_url)
+          expect(json_response['web_url']).to eq(Gitlab::Routing.url_helpers.user_url(stranger))
+
+          # Member attributes
+          expect(json_response['requested_at']).to be_present
+        end
+      end
+    end
+  end
+
+  shared_examples 'PUT /:sources/:id/access_requests/:user_id/approve' do |source_type|
+    context "with :sources == #{source_type.pluralize}" do
+      it_behaves_like 'a 404 response when source is private' do
+        let(:route) { put api("/#{source_type.pluralize}/#{source.id}/access_requests/#{access_requester.id}/approve", stranger) }
+      end
+
+      context 'when authenticated as a non-master/owner' do
+        %i[developer access_requester stranger].each do |type|
+          context "as a #{type}" do
+            it 'returns 403' do
+              user = public_send(type)
+              put api("/#{source_type.pluralize}/#{source.id}/access_requests/#{access_requester.id}/approve", user)
+
+              expect(response).to have_http_status(403)
+            end
+          end
+        end
+      end
+
+      context 'when authenticated as a master/owner' do
+        it 'returns 201' do
+          expect do
+            put api("/#{source_type.pluralize}/#{source.id}/access_requests/#{access_requester.id}/approve", master),
+                access_level: Member::MASTER
+
+            expect(response).to have_http_status(201)
+          end.to change { source.members.count }.by(1)
+          # User attributes
+          expect(json_response['id']).to eq(access_requester.id)
+          expect(json_response['name']).to eq(access_requester.name)
+          expect(json_response['username']).to eq(access_requester.username)
+          expect(json_response['state']).to eq(access_requester.state)
+          expect(json_response['avatar_url']).to eq(access_requester.avatar_url)
+          expect(json_response['web_url']).to eq(Gitlab::Routing.url_helpers.user_url(access_requester))
+
+          # Member attributes
+          expect(json_response['access_level']).to eq(Member::MASTER)
+        end
+
+        context 'user_id does not match an existing access requester' do
+          it 'returns 404' do
+            expect do
+              put api("/#{source_type.pluralize}/#{source.id}/access_requests/#{stranger.id}/approve", master)
+
+              expect(response).to have_http_status(404)
+            end.not_to change { source.members.count }
+          end
+        end
+      end
+    end
+  end
+
+  shared_examples 'DELETE /:sources/:id/access_requests/:user_id' do |source_type|
+    context "with :sources == #{source_type.pluralize}" do
+      it_behaves_like 'a 404 response when source is private' do
+        let(:route) { delete api("/#{source_type.pluralize}/#{source.id}/access_requests/#{access_requester.id}", stranger) }
+      end
+
+      context 'when authenticated as a non-master/owner' do
+        %i[developer stranger].each do |type|
+          context "as a #{type}" do
+            it 'returns 403' do
+              user = public_send(type)
+              delete api("/#{source_type.pluralize}/#{source.id}/access_requests/#{access_requester.id}", user)
+
+              expect(response).to have_http_status(403)
+            end
+          end
+        end
+      end
+
+      context 'when authenticated as the access requester' do
+        it 'returns 200' do
+          expect do
+            delete api("/#{source_type.pluralize}/#{source.id}/access_requests/#{access_requester.id}", access_requester)
+
+            expect(response).to have_http_status(200)
+          end.to change { source.requesters.count }.by(-1)
+        end
+      end
+
+      context 'when authenticated as a master/owner' do
+        it 'returns 200' do
+          expect do
+            delete api("/#{source_type.pluralize}/#{source.id}/access_requests/#{access_requester.id}", master)
+
+            expect(response).to have_http_status(200)
+          end.to change { source.requesters.count }.by(-1)
+        end
+
+        context 'user_id does not match an existing access requester' do
+          it 'returns 404' do
+            expect do
+              delete api("/#{source_type.pluralize}/#{source.id}/access_requests/#{stranger.id}", master)
+
+              expect(response).to have_http_status(404)
+            end.not_to change { source.requesters.count }
+          end
+        end
+      end
+    end
+  end
+
+  it_behaves_like 'GET /:sources/:id/access_requests', 'project' do
+    let(:source) { project }
+  end
+
+  it_behaves_like 'GET /:sources/:id/access_requests', 'group' do
+    let(:source) { group }
+  end
+
+  it_behaves_like 'POST /:sources/:id/access_requests', 'project' do
+    let(:source) { project }
+  end
+
+  it_behaves_like 'POST /:sources/:id/access_requests', 'group' do
+    let(:source) { group }
+  end
+
+  it_behaves_like 'PUT /:sources/:id/access_requests/:user_id/approve', 'project' do
+    let(:source) { project }
+  end
+
+  it_behaves_like 'PUT /:sources/:id/access_requests/:user_id/approve', 'group' do
+    let(:source) { group }
+  end
+
+  it_behaves_like 'DELETE /:sources/:id/access_requests/:user_id', 'project' do
+    let(:source) { project }
+  end
+
+  it_behaves_like 'DELETE /:sources/:id/access_requests/:user_id', 'group' do
+    let(:source) { group }
+  end
+end

spec/requests/api/builds_spec.rb

@@ -238,6 +238,10 @@ describe API::API, api: true do
         it { expect(response.headers).to include(download_headers) }
       end
 
+      before do
+        pipeline.reload_status!
+      end
+
       context 'with regular branch' do
         before do
           pipeline.update(ref: 'master',

spec/requests/api/commits_spec.rb

@@ -89,16 +89,29 @@ describe API::API, api: true  do
 
       it "returns nil for commit without CI" do
         get api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}", user)
+
         expect(response).to have_http_status(200)
         expect(json_response['status']).to be_nil
       end
 
       it "returns status for CI" do
         pipeline = project.ensure_pipeline(project.repository.commit.sha, 'master')
+        pipeline.update(status: 'success')
+
         get api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}", user)
+
         expect(response).to have_http_status(200)
         expect(json_response['status']).to eq(pipeline.status)
       end
+
+      it "returns status for CI when pipeline is created" do
+        project.ensure_pipeline(project.repository.commit.sha, 'master')
+
+        get api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}", user)
+
+        expect(response).to have_http_status(200)
+        expect(json_response['status']).to be_nil
+      end
     end
 
     context "unauthorized user" do

spec/requests/api/group_members_spec.rb

-require 'spec_helper'
-
-describe API::API, api: true  do
-  include ApiHelpers
-
-  let(:owner) { create(:user) }
-  let(:reporter) { create(:user) }
-  let(:developer) { create(:user) }
-  let(:master) { create(:user) }
-  let(:guest) { create(:user) }
-  let(:stranger) { create(:user) }
-
-  let!(:group_with_members) do
-    group = create(:group, :private)
-    group.add_users([reporter.id], GroupMember::REPORTER)
-    group.add_users([developer.id], GroupMember::DEVELOPER)
-    group.add_users([master.id], GroupMember::MASTER)
-    group.add_users([guest.id], GroupMember::GUEST)
-    group
-  end
-
-  let!(:group_no_members) { create(:group) }
-
-  before do
-    group_with_members.add_owner owner
-    group_no_members.add_owner owner
-  end
-
-  describe "GET /groups/:id/members" do
-    context "when authenticated as user that is part or the group" do
-      it "each user: returns an array of members groups of group3" do
-        [owner, master, developer, reporter, guest].each do |user|
-          get api("/groups/#{group_with_members.id}/members", user)
-          expect(response).to have_http_status(200)
-          expect(json_response).to be_an Array
-          expect(json_response.size).to eq(5)
-          expect(json_response.find { |e| e['id'] == owner.id }['access_level']).to eq(GroupMember::OWNER)
-          expect(json_response.find { |e| e['id'] == reporter.id }['access_level']).to eq(GroupMember::REPORTER)
-          expect(json_response.find { |e| e['id'] == developer.id }['access_level']).to eq(GroupMember::DEVELOPER)
-          expect(json_response.find { |e| e['id'] == master.id }['access_level']).to eq(GroupMember::MASTER)
-          expect(json_response.find { |e| e['id'] == guest.id }['access_level']).to eq(GroupMember::GUEST)
-        end
-      end
-
-      it 'users not part of the group should get access error' do
-        get api("/groups/#{group_with_members.id}/members", stranger)
-
-        expect(response).to have_http_status(404)
-      end
-    end
-  end
-
-  describe "POST /groups/:id/members" do
-    context "when not a member of the group" do
-      it "does not add guest as member of group_no_members when adding being done by person outside the group" do
-        post api("/groups/#{group_no_members.id}/members", reporter), user_id: guest.id, access_level: GroupMember::MASTER
-        expect(response).to have_http_status(403)
-      end
-    end
-
-    context "when a member of the group" do
-      it "returns ok and add new member" do
-        new_user = create(:user)
-
-        expect do
-          post api("/groups/#{group_no_members.id}/members", owner), user_id: new_user.id, access_level: GroupMember::MASTER
-        end.to change { group_no_members.members.count }.by(1)
-
-        expect(response).to have_http_status(201)
-        expect(json_response['name']).to eq(new_user.name)
-        expect(json_response['access_level']).to eq(GroupMember::MASTER)
-      end
-
-      it "does not allow guest to modify group members" do
-        new_user = create(:user)
-
-        expect do
-          post api("/groups/#{group_with_members.id}/members", guest), user_id: new_user.id, access_level: GroupMember::MASTER
-        end.not_to change { group_with_members.members.count }
-
-        expect(response).to have_http_status(403)
-      end
-
-      it "returns error if member already exists" do
-        post api("/groups/#{group_with_members.id}/members", owner), user_id: master.id, access_level: GroupMember::MASTER
-        expect(response).to have_http_status(409)
-      end
-
-      it "returns a 400 error when user id is not given" do
-        post api("/groups/#{group_no_members.id}/members", owner), access_level: GroupMember::MASTER
-        expect(response).to have_http_status(400)
-      end
-
-      it "returns a 400 error when access level is not given" do
-        post api("/groups/#{group_no_members.id}/members", owner), user_id: master.id
-        expect(response).to have_http_status(400)
-      end
-
-      it "returns a 422 error when access level is not known" do
-        post api("/groups/#{group_no_members.id}/members", owner), user_id: master.id, access_level: 1234
-        expect(response).to have_http_status(422)
-      end
-    end
-  end
-
-  describe 'PUT /groups/:id/members/:user_id' do
-    context 'when not a member of the group' do
-      it 'returns a 409 error if the user is not a group member' do
-        put(
-          api("/groups/#{group_no_members.id}/members/#{developer.id}",
-              owner), access_level: GroupMember::MASTER
-        )
-        expect(response).to have_http_status(404)
-      end
-    end
-
-    context 'when a member of the group' do
-      it 'returns ok and update member access level' do
-        put(
-          api("/groups/#{group_with_members.id}/members/#{reporter.id}",
-              owner),
-          access_level: GroupMember::MASTER
-        )
-
-        expect(response).to have_http_status(200)
-
-        get api("/groups/#{group_with_members.id}/members", owner)
-        json_reporter = json_response.find do |e|
-          e['id'] == reporter.id
-        end
-
-        expect(json_reporter['access_level']).to eq(GroupMember::MASTER)
-      end
-
-      it 'does not allow guest to modify group members' do
-        put(
-          api("/groups/#{group_with_members.id}/members/#{developer.id}",
-              guest),
-          access_level: GroupMember::MASTER
-        )
-
-        expect(response).to have_http_status(403)
-
-        get api("/groups/#{group_with_members.id}/members", owner)
-        json_developer = json_response.find do |e|
-          e['id'] == developer.id
-        end
-
-        expect(json_developer['access_level']).to eq(GroupMember::DEVELOPER)
-      end
-
-      it 'returns a 400 error when access level is not given' do
-        put(
-          api("/groups/#{group_with_members.id}/members/#{master.id}", owner)
-        )
-        expect(response).to have_http_status(400)
-      end
-
-      it 'returns a 422 error when access level is not known' do
-        put(
-          api("/groups/#{group_with_members.id}/members/#{master.id}", owner),
-          access_level: 1234
-        )
-        expect(response).to have_http_status(422)
-      end
-    end
-  end
-
-  describe 'DELETE /groups/:id/members/:user_id' do
-    context 'when not a member of the group' do
-      it "does not delete guest's membership of group_with_members" do
-        random_user = create(:user)
-        delete api("/groups/#{group_with_members.id}/members/#{owner.id}", random_user)
-
-        expect(response).to have_http_status(404)
-      end
-    end
-
-    context "when a member of the group" do
-      it "deletes guest's membership of group" do
-        expect do
-          delete api("/groups/#{group_with_members.id}/members/#{guest.id}", owner)
-        end.to change { group_with_members.members.count }.by(-1)
-
-        expect(response).to have_http_status(200)
-      end
-
-      it "returns a 404 error when user id is not known" do
-        delete api("/groups/#{group_with_members.id}/members/1328", owner)
-        expect(response).to have_http_status(404)
-      end
-
-      it "does not allow guest to modify group members" do
-        delete api("/groups/#{group_with_members.id}/members/#{master.id}", guest)
-        expect(response).to have_http_status(403)
-      end
-    end
-  end
-end

spec/requests/api/members_spec.rb

+require 'spec_helper'
+
+describe API::Members, api: true  do
+  include ApiHelpers
+
+  let(:master) { create(:user) }
+  let(:developer) { create(:user) }
+  let(:access_requester) { create(:user) }
+  let(:stranger) { create(:user) }
+
+  let(:project) do
+    project = create(:project, :public, creator_id: master.id, namespace: master.namespace)
+    project.team << [developer, :developer]
+    project.team << [master, :master]
+    project.request_access(access_requester)
+    project
+  end
+
+  let!(:group) do
+    group = create(:group, :public)
+    group.add_developer(developer)
+    group.add_owner(master)
+    group.request_access(access_requester)
+    group
+  end
+
+  shared_examples 'GET /:sources/:id/members' do |source_type|
+    context "with :sources == #{source_type.pluralize}" do
+      it_behaves_like 'a 404 response when source is private' do
+        let(:route) { get api("/#{source_type.pluralize}/#{source.id}/members", stranger) }
+      end
+
+      context 'when authenticated as a non-member' do
+        %i[access_requester stranger].each do |type|
+          context "as a #{type}" do
+            it 'returns 200' do
+              user = public_send(type)
+              get api("/#{source_type.pluralize}/#{source.id}/members", user)
+
+              expect(response).to have_http_status(200)
+              expect(json_response.size).to eq(2)
+            end
+          end
+        end
+      end
+
+      it 'finds members with query string' do
+        get api("/#{source_type.pluralize}/#{source.id}/members", developer), query: master.username
+
+        expect(response).to have_http_status(200)
+        expect(json_response.count).to eq(1)
+        expect(json_response.first['username']).to eq(master.username)
+      end
+    end
+  end
+
+  shared_examples 'GET /:sources/:id/members/:user_id' do |source_type|
+    context "with :sources == #{source_type.pluralize}" do
+      it_behaves_like 'a 404 response when source is private' do
+        let(:route) { get api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", stranger) }
+      end
+
+      context 'when authenticated as a non-member' do
+        %i[access_requester stranger].each do |type|
+          context "as a #{type}" do
+            it 'returns 200' do
+              user = public_send(type)
+              get api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", user)
+
+              expect(response).to have_http_status(200)
+              # User attributes
+              expect(json_response['id']).to eq(developer.id)
+              expect(json_response['name']).to eq(developer.name)
+              expect(json_response['username']).to eq(developer.username)
+              expect(json_response['state']).to eq(developer.state)
+              expect(json_response['avatar_url']).to eq(developer.avatar_url)
+              expect(json_response['web_url']).to eq(Gitlab::Routing.url_helpers.user_url(developer))
+
+              # Member attributes
+              expect(json_response['access_level']).to eq(Member::DEVELOPER)
+            end
+          end
+        end
+      end
+    end
+  end
+
+  shared_examples 'POST /:sources/:id/members' do |source_type|
+    context "with :sources == #{source_type.pluralize}" do
+      it_behaves_like 'a 404 response when source is private' do
+        let(:route) { post api("/#{source_type.pluralize}/#{source.id}/members", stranger) }
+      end
+
+      context 'when authenticated as a non-member or member with insufficient rights' do
+        %i[access_requester stranger developer].each do |type|
+          context "as a #{type}" do
+            it 'returns 403' do
+              user = public_send(type)
+              post api("/#{source_type.pluralize}/#{source.id}/members", user)
+
+              expect(response).to have_http_status(403)
+            end
+          end
+        end
+      end
+
+      context 'when authenticated as a master/owner' do
+        context 'and new member is already a requester' do
+          it 'transforms the requester into a proper member' do
+            expect do
+              post api("/#{source_type.pluralize}/#{source.id}/members", master),
+                   user_id: access_requester.id, access_level: Member::MASTER
+
+              expect(response).to have_http_status(201)
+            end.to change { source.members.count }.by(1)
+            expect(source.requesters.count).to eq(0)
+            expect(json_response['id']).to eq(access_requester.id)
+            expect(json_response['access_level']).to eq(Member::MASTER)
+          end
+        end
+
+        it 'creates a new member' do
+          expect do
+            post api("/#{source_type.pluralize}/#{source.id}/members", master),
+                 user_id: stranger.id, access_level: Member::DEVELOPER
+
+            expect(response).to have_http_status(201)
+          end.to change { source.members.count }.by(1)
+          expect(json_response['id']).to eq(stranger.id)
+          expect(json_response['access_level']).to eq(Member::DEVELOPER)
+        end
+      end
+
+      it "returns #{source_type == 'project' ? 201 : 409} if member already exists" do
+        post api("/#{source_type.pluralize}/#{source.id}/members", master),
+             user_id: master.id, access_level: Member::MASTER
+
+        expect(response).to have_http_status(source_type == 'project' ? 201 : 409)
+      end
+
+      it 'returns 400 when user_id is not given' do
+        post api("/#{source_type.pluralize}/#{source.id}/members", master),
+             access_level: Member::MASTER
+
+        expect(response).to have_http_status(400)
+      end
+
+      it 'returns 400 when access_level is not given' do
+        post api("/#{source_type.pluralize}/#{source.id}/members", master),
+             user_id: stranger.id
+
+        expect(response).to have_http_status(400)
+      end
+
+      it 'returns 422 when access_level is not valid' do
+        post api("/#{source_type.pluralize}/#{source.id}/members", master),
+             user_id: stranger.id, access_level: 1234
+
+        expect(response).to have_http_status(422)
+      end
+    end
+  end
+
+  shared_examples 'PUT /:sources/:id/members/:user_id' do |source_type|
+    context "with :sources == #{source_type.pluralize}" do
+      it_behaves_like 'a 404 response when source is private' do
+        let(:route) { put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", stranger) }
+      end
+
+      context 'when authenticated as a non-member or member with insufficient rights' do
+        %i[access_requester stranger developer].each do |type|
+          context "as a #{type}" do
+            it 'returns 403' do
+              user = public_send(type)
+              put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", user)
+
+              expect(response).to have_http_status(403)
+            end
+          end
+        end
+      end
+
+      context 'when authenticated as a master/owner' do
+        it 'updates the member' do
+          put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", master),
+              access_level: Member::MASTER
+
+          expect(response).to have_http_status(200)
+          expect(json_response['id']).to eq(developer.id)
+          expect(json_response['access_level']).to eq(Member::MASTER)
+        end
+      end
+
+      it 'returns 409 if member does not exist' do
+        put api("/#{source_type.pluralize}/#{source.id}/members/123", master),
+            access_level: Member::MASTER
+
+        expect(response).to have_http_status(404)
+      end
+
+      it 'returns 400 when access_level is not given' do
+        put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", master)
+
+        expect(response).to have_http_status(400)
+      end
+
+      it 'returns 422 when access level is not valid' do
+        put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", master),
+            access_level: 1234
+
+        expect(response).to have_http_status(422)
+      end
+    end
+  end
+
+  shared_examples 'DELETE /:sources/:id/members/:user_id' do |source_type|
+    context "with :sources == #{source_type.pluralize}" do
+      it_behaves_like 'a 404 response when source is private' do
+        let(:route) { delete api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", stranger) }
+      end
+
+      context 'when authenticated as a non-member or member with insufficient rights' do
+        %i[access_requester stranger].each do |type|
+          context "as a #{type}" do
+            it 'returns 403' do
+              user = public_send(type)
+              delete api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", user)
+
+              expect(response).to have_http_status(403)
+            end
+          end
+        end
+      end
+
+      context 'when authenticated as a member and deleting themself' do
+        it 'deletes the member' do
+          expect do
+            delete api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", developer)
+
+            expect(response).to have_http_status(200)
+          end.to change { source.members.count }.by(-1)
+        end
+      end
+
+      context 'when authenticated as a master/owner' do
+        context 'and member is a requester' do
+          it "returns #{source_type == 'project' ? 200 : 404}" do
+            expect do
+              delete api("/#{source_type.pluralize}/#{source.id}/members/#{access_requester.id}", master)
+
+              expect(response).to have_http_status(source_type == 'project' ? 200 : 404)
+            end.not_to change { source.requesters.count }
+          end
+        end
+
+        it 'deletes the member' do
+          expect do
+            delete api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", master)
+
+            expect(response).to have_http_status(200)
+          end.to change { source.members.count }.by(-1)
+        end
+      end
+
+      it "returns #{source_type == 'project' ? 200 : 404} if member does not exist" do
+        delete api("/#{source_type.pluralize}/#{source.id}/members/123", master)
+
+        expect(response).to have_http_status(source_type == 'project' ? 200 : 404)
+      end
+    end
+  end
+
+  it_behaves_like 'GET /:sources/:id/members', 'project' do
+    let(:source) { project }
+  end
+
+  it_behaves_like 'GET /:sources/:id/members', 'group' do
+    let(:source) { group }
+  end
+
+  it_behaves_like 'GET /:sources/:id/members/:user_id', 'project' do
+    let(:source) { project }
+  end
+
+  it_behaves_like 'GET /:sources/:id/members/:user_id', 'group' do
+    let(:source) { group }
+  end
+
+  it_behaves_like 'POST /:sources/:id/members', 'project' do
+    let(:source) { project }
+  end
+
+  it_behaves_like 'POST /:sources/:id/members', 'group' do
+    let(:source) { group }
+  end
+
+  it_behaves_like 'PUT /:sources/:id/members/:user_id', 'project' do
+    let(:source) { project }
+  end
+
+  it_behaves_like 'PUT /:sources/:id/members/:user_id', 'group' do
+    let(:source) { group }
+  end
+
+  it_behaves_like 'DELETE /:sources/:id/members/:user_id', 'project' do
+    let(:source) { project }
+  end
+
+  it_behaves_like 'DELETE /:sources/:id/members/:user_id', 'group' do
+    let(:source) { group }
+  end
+end

spec/requests/api/project_members_spec.rb

-require 'spec_helper'
-
-describe API::API, api: true  do
-  include ApiHelpers
-  let(:user) { create(:user) }
-  let(:user2) { create(:user) }
-  let(:user3) { create(:user) }
-  let(:project) { create(:project, creator_id: user.id, namespace: user.namespace) }
-  let(:project_member) { create(:project_member, :master, user: user, project: project) }
-  let(:project_member2) { create(:project_member, :developer, user: user3, project: project) }
-
-  describe "GET /projects/:id/members" do
-    before { project_member }
-    before { project_member2 }
-
-    it "returns project team members" do
-      get api("/projects/#{project.id}/members", user)
-      expect(response).to have_http_status(200)
-      expect(json_response).to be_an Array
-      expect(json_response.count).to eq(2)
-      expect(json_response.map { |u| u['username'] }).to include user.username
-    end
-
-    it "finds team members with query string" do
-      get api("/projects/#{project.id}/members", user), query: user.username
-      expect(response).to have_http_status(200)
-      expect(json_response).to be_an Array
-      expect(json_response.count).to eq(1)
-      expect(json_response.first['username']).to eq(user.username)
-    end
-
-    it "returns a 404 error if id not found" do
-      get api("/projects/9999/members", user)
-      expect(response).to have_http_status(404)
-    end
-  end
-
-  describe "GET /projects/:id/members/:user_id" do
-    before { project_member }
-
-    it "returns project team member" do
-      get api("/projects/#{project.id}/members/#{user.id}", user)
-      expect(response).to have_http_status(200)
-      expect(json_response['username']).to eq(user.username)
-      expect(json_response['access_level']).to eq(ProjectMember::MASTER)
-    end
-
-    it "returns a 404 error if user id not found" do
-      get api("/projects/#{project.id}/members/1234", user)
-      expect(response).to have_http_status(404)
-    end
-  end
-
-  describe "POST /projects/:id/members" do
-    it "adds user to project team" do
-      expect do
-        post api("/projects/#{project.id}/members", user), user_id: user2.id, access_level: ProjectMember::DEVELOPER
-      end.to change { ProjectMember.count }.by(1)
-
-      expect(response).to have_http_status(201)
-      expect(json_response['username']).to eq(user2.username)
-      expect(json_response['access_level']).to eq(ProjectMember::DEVELOPER)
-    end
-
-    it "returns a 201 status if user is already project member" do
-      post api("/projects/#{project.id}/members", user),
-           user_id: user2.id,
-           access_level: ProjectMember::DEVELOPER
-      expect do
-        post api("/projects/#{project.id}/members", user), user_id: user2.id, access_level: ProjectMember::DEVELOPER
-      end.not_to change { ProjectMember.count }
-
-      expect(response).to have_http_status(201)
-      expect(json_response['username']).to eq(user2.username)
-      expect(json_response['access_level']).to eq(ProjectMember::DEVELOPER)
-    end
-
-    it "returns a 400 error when user id is not given" do
-      post api("/projects/#{project.id}/members", user), access_level: ProjectMember::MASTER
-      expect(response).to have_http_status(400)
-    end
-
-    it "returns a 400 error when access level is not given" do
-      post api("/projects/#{project.id}/members", user), user_id: user2.id
-      expect(response).to have_http_status(400)
-    end
-
-    it "returns a 422 error when access level is not known" do
-      post api("/projects/#{project.id}/members", user), user_id: user2.id, access_level: 1234
-      expect(response).to have_http_status(422)
-    end
-  end
-
-  describe "PUT /projects/:id/members/:user_id" do
-    before { project_member2 }
-
-    it "updates project team member" do
-      put api("/projects/#{project.id}/members/#{user3.id}", user), access_level: ProjectMember::MASTER
-      expect(response).to have_http_status(200)
-      expect(json_response['username']).to eq(user3.username)
-      expect(json_response['access_level']).to eq(ProjectMember::MASTER)
-    end
-
-    it "returns a 404 error if user_id is not found" do
-      put api("/projects/#{project.id}/members/1234", user), access_level: ProjectMember::MASTER
-      expect(response).to have_http_status(404)
-    end
-
-    it "returns a 400 error when access level is not given" do
-      put api("/projects/#{project.id}/members/#{user3.id}", user)
-      expect(response).to have_http_status(400)
-    end
-
-    it "returns a 422 error when access level is not known" do
-      put api("/projects/#{project.id}/members/#{user3.id}", user), access_level: 123
-      expect(response).to have_http_status(422)
-    end
-  end
-
-  describe "DELETE /projects/:id/members/:user_id" do
-    before do
-      project_member
-      project_member2
-    end
-
-    it "removes user from project team" do
-      expect do
-        delete api("/projects/#{project.id}/members/#{user3.id}", user)
-      end.to change { ProjectMember.count }.by(-1)
-    end
-
-    it "returns 200 if team member is not part of a project" do
-      delete api("/projects/#{project.id}/members/#{user3.id}", user)
-      expect do
-        delete api("/projects/#{project.id}/members/#{user3.id}", user)
-      end.not_to change { ProjectMember.count }
-      expect(response).to have_http_status(200)
-    end
-
-    it "returns 200 if team member already removed" do
-      delete api("/projects/#{project.id}/members/#{user3.id}", user)
-      delete api("/projects/#{project.id}/members/#{user3.id}", user)
-      expect(response).to have_http_status(200)
-    end
-
-    it "returns 200 OK when the user was not member" do
-      expect do
-        delete api("/projects/#{project.id}/members/1000000", user)
-      end.to change { ProjectMember.count }.by(0)
-      expect(response).to have_http_status(200)
-      expect(json_response['id']).to eq(1000000)
-      expect(json_response['message']).to eq('Access revoked')
-    end
-
-    context 'when the user is not an admin or owner' do
-      it 'can leave the project' do
-        expect do
-          delete api("/projects/#{project.id}/members/#{user3.id}", user3)
-        end.to change { ProjectMember.count }.by(-1)
-
-        expect(response).to have_http_status(200)
-        expect(json_response['id']).to eq(project_member2.id)
-      end
-    end
-  end
-end

spec/requests/api/triggers_spec.rb

@@ -50,7 +50,8 @@ describe API::API do
         post api("/projects/#{project.id}/trigger/builds"), options.merge(ref: 'master')
         expect(response).to have_http_status(201)
         pipeline.builds.reload
-        expect(pipeline.builds.size).to eq(2)
+        expect(pipeline.builds.pending.size).to eq(2)
+        expect(pipeline.builds.size).to eq(5)
       end
 
       it 'returns bad request with no builds created if there\'s no commit for that ref' do

spec/requests/ci/api/builds_spec.rb

@@ -6,112 +6,102 @@ describe Ci::API::API do
   let(:runner) { FactoryGirl.create(:ci_runner, tag_list: ["mysql", "ruby"]) }
   let(:project) { FactoryGirl.create(:empty_project) }
 
-  before do
-    stub_ci_pipeline_to_return_yaml_file
-  end
-
   describe "Builds API for runners" do
-    let(:shared_runner) { FactoryGirl.create(:ci_runner, token: "SharedRunner") }
-    let(:shared_project) { FactoryGirl.create(:empty_project, name: "SharedProject") }
+    let(:pipeline) { create(:ci_pipeline_without_jobs, project: project, ref: 'master') }
 
     before do
-      FactoryGirl.create :ci_runner_project, project: project, runner: runner
+      project.runners << runner
     end
 
     describe "POST /builds/register" do
-      it "starts a build" do
-        pipeline = FactoryGirl.create(:ci_pipeline, project: project, ref: 'master')
-        pipeline.create_builds(nil)
-        build = pipeline.builds.first
+      let!(:build) { create(:ci_build, pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0) }
 
-        post ci_api("/builds/register"), token: runner.token, info: { platform: :darwin }
+      it "starts a build" do
+        register_builds info: { platform: :darwin }
 
         expect(response).to have_http_status(201)
         expect(json_response['sha']).to eq(build.sha)
         expect(runner.reload.platform).to eq("darwin")
+        expect(json_response["options"]).to eq({ "image" => "ruby:2.1", "services" => ["postgres"] })
+        expect(json_response["variables"]).to include(
+          { "key" => "CI_BUILD_NAME", "value" => "spinach", "public" => true },
+          { "key" => "CI_BUILD_STAGE", "value" => "test", "public" => true },
+          { "key" => "DB_NAME", "value" => "postgres", "public" => true }
+        )
       end
 
-      it "returns 404 error if no pending build found" do
-        post ci_api("/builds/register"), token: runner.token
-
-        expect(response).to have_http_status(404)
-      end
-
-      it "returns 404 error if no builds for specific runner" do
-        pipeline = FactoryGirl.create(:ci_pipeline, project: shared_project)
-        FactoryGirl.create(:ci_build, pipeline: pipeline, status: 'pending')
+      context 'when builds are finished' do
+        before do
+          build.success
+        end
 
-        post ci_api("/builds/register"), token: runner.token
+        it "returns 404 error if no builds for specific runner" do
+          register_builds
 
-        expect(response).to have_http_status(404)
+          expect(response).to have_http_status(404)
+        end
       end
 
-      it "returns 404 error if no builds for shared runner" do
-        pipeline = FactoryGirl.create(:ci_pipeline, project: project)
-        FactoryGirl.create(:ci_build, pipeline: pipeline, status: 'pending')
+      context 'for other project with builds' do
+        before do
+          build.success
+          create(:ci_build, :pending)
+        end
 
-        post ci_api("/builds/register"), token: shared_runner.token
+        it "returns 404 error if no builds for shared runner" do
+          register_builds
 
-        expect(response).to have_http_status(404)
+          expect(response).to have_http_status(404)
+        end
       end
 
-      it "returns options" do
-        pipeline = FactoryGirl.create(:ci_pipeline, project: project, ref: 'master')
-        pipeline.create_builds(nil)
+      context 'for shared runner' do
+        let(:shared_runner) { create(:ci_runner, token: "SharedRunner") }
 
-        post ci_api("/builds/register"), token: runner.token, info: { platform: :darwin }
+        it "should return 404 error if no builds for shared runner" do
+          register_builds shared_runner.token
 
-        expect(response).to have_http_status(201)
-        expect(json_response["options"]).to eq({ "image" => "ruby:2.1", "services" => ["postgres"] })
+          expect(response).to have_http_status(404)
+        end
       end
 
-      it "returns variables" do
-        pipeline = FactoryGirl.create(:ci_pipeline, project: project, ref: 'master')
-        pipeline.create_builds(nil)
-        project.variables << Ci::Variable.new(key: "SECRET_KEY", value: "secret_value")
-
-        post ci_api("/builds/register"), token: runner.token, info: { platform: :darwin }
+      context 'for triggered build' do
+        before do
+          trigger = create(:ci_trigger, project: project)
+          create(:ci_trigger_request_with_variables, pipeline: pipeline, builds: [build], trigger: trigger)
+          project.variables << Ci::Variable.new(key: "SECRET_KEY", value: "secret_value")
+        end
 
-        expect(response).to have_http_status(201)
-        expect(json_response["variables"]).to include(
-          { "key" => "CI_BUILD_NAME", "value" => "spinach", "public" => true },
-          { "key" => "CI_BUILD_STAGE", "value" => "test", "public" => true },
-          { "key" => "DB_NAME", "value" => "postgres", "public" => true },
-          { "key" => "SECRET_KEY", "value" => "secret_value", "public" => false }
-        )
+        it "returns variables for triggers" do
+          register_builds info: { platform: :darwin }
+
+          expect(response).to have_http_status(201)
+          expect(json_response["variables"]).to include(
+            { "key" => "CI_BUILD_NAME", "value" => "spinach", "public" => true },
+            { "key" => "CI_BUILD_STAGE", "value" => "test", "public" => true },
+            { "key" => "CI_BUILD_TRIGGERED", "value" => "true", "public" => true },
+            { "key" => "DB_NAME", "value" => "postgres", "public" => true },
+            { "key" => "SECRET_KEY", "value" => "secret_value", "public" => false },
+            { "key" => "TRIGGER_KEY_1", "value" => "TRIGGER_VALUE_1", "public" => false },
+          )
+        end
       end
 
-      it "returns variables for triggers" do
-        trigger = FactoryGirl.create(:ci_trigger, project: project)
-        pipeline = FactoryGirl.create(:ci_pipeline, project: project, ref: 'master')
-
-        trigger_request = FactoryGirl.create(:ci_trigger_request_with_variables, pipeline: pipeline, trigger: trigger)
-        pipeline.create_builds(nil, trigger_request)
-        project.variables << Ci::Variable.new(key: "SECRET_KEY", value: "secret_value")
-
-        post ci_api("/builds/register"), token: runner.token, info: { platform: :darwin }
-
-        expect(response).to have_http_status(201)
-        expect(json_response["variables"]).to include(
-          { "key" => "CI_BUILD_NAME", "value" => "spinach", "public" => true },
-          { "key" => "CI_BUILD_STAGE", "value" => "test", "public" => true },
-          { "key" => "CI_BUILD_TRIGGERED", "value" => "true", "public" => true },
-          { "key" => "DB_NAME", "value" => "postgres", "public" => true },
-          { "key" => "SECRET_KEY", "value" => "secret_value", "public" => false },
-          { "key" => "TRIGGER_KEY_1", "value" => "TRIGGER_VALUE_1", "public" => false }
-        )
-      end
+      context 'with multiple builds' do
+        before do
+          build.success
+        end
 
-      it "returns dependent builds" do
-        pipeline = FactoryGirl.create(:ci_pipeline, project: project, ref: 'master')
-        pipeline.create_builds(nil, nil)
-        pipeline.builds.where(stage: 'test').each(&:success)
+        let!(:test_build) { create(:ci_build, pipeline: pipeline, name: 'deploy', stage: 'deploy', stage_idx: 1) }
 
-        post ci_api("/builds/register"), token: runner.token, info: { platform: :darwin }
+        it "returns dependent builds" do
+          register_builds info: { platform: :darwin }
 
-        expect(response).to have_http_status(201)
-        expect(json_response["depends_on_builds"].count).to eq(2)
-        expect(json_response["depends_on_builds"][0]["name"]).to eq("rspec")
+          expect(response).to have_http_status(201)
+          expect(json_response["id"]).to eq(test_build.id)
+          expect(json_response["depends_on_builds"].count).to eq(1)
+          expect(json_response["depends_on_builds"][0]).to include('id' => build.id, 'name' => 'spinach')
+        end
       end
 
       %w(name version revision platform architecture).each do |param|
@@ -121,8 +111,9 @@ describe Ci::API::API do
           subject { runner.read_attribute(param.to_sym) }
 
           it do
-            post ci_api("/builds/register"), token: runner.token, info: { param => value }
-            expect(response).to have_http_status(404)
+            register_builds info: { param => value }
+
+            expect(response).to have_http_status(201)
             runner.reload
             is_expected.to eq(value)
           end
@@ -131,8 +122,7 @@ describe Ci::API::API do
 
       context 'when build has no tags' do
         before do
-          pipeline = create(:ci_pipeline, project: project)
-          create(:ci_build, pipeline: pipeline, tags: [])
+          build.update(tags: [])
         end
 
         context 'when runner is allowed to pick untagged builds' do
@@ -154,17 +144,15 @@ describe Ci::API::API do
             expect(response).to have_http_status 404
           end
         end
+      end
 
-        def register_builds
-          post ci_api("/builds/register"), token: runner.token,
-                                           info: { platform: :darwin }
-        end
+      def register_builds(token = runner.token, **params)
+        post ci_api("/builds/register"), params.merge(token: token)
       end
     end
 
     describe "PUT /builds/:id" do
-      let(:pipeline) {create(:ci_pipeline, project: project)}
-      let(:build) { create(:ci_build, :trace, pipeline: pipeline, runner_id: runner.id) }
+      let(:build) { create(:ci_build, :pending, :trace, pipeline: pipeline, runner_id: runner.id) }
 
       before do
         build.run!
@@ -189,7 +177,7 @@ describe Ci::API::API do
     end
 
     describe 'PATCH /builds/:id/trace.txt' do
-      let(:build) { create(:ci_build, :trace, runner_id: runner.id) }
+      let(:build) { create(:ci_build, :pending, :trace, runner_id: runner.id) }
       let(:headers) { { Ci::API::Helpers::BUILD_TOKEN_HEADER => build.token, 'Content-Type' => 'text/plain' } }
       let(:headers_with_range) { headers.merge({ 'Content-Range' => '11-20' }) }
 
@@ -237,8 +225,7 @@ describe Ci::API::API do
     context "Artifacts" do
       let(:file_upload) { fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif') }
       let(:file_upload2) { fixture_file_upload(Rails.root + 'spec/fixtures/dk.png', 'image/gif') }
-      let(:pipeline) { create(:ci_pipeline, project: project) }
-      let(:build) { create(:ci_build, pipeline: pipeline, runner_id: runner.id) }
+      let(:build) { create(:ci_build, :pending, pipeline: pipeline, runner_id: runner.id) }
       let(:authorize_url) { ci_api("/builds/#{build.id}/artifacts/authorize") }
       let(:post_url) { ci_api("/builds/#{build.id}/artifacts") }
       let(:delete_url) { ci_api("/builds/#{build.id}/artifacts") }

spec/requests/ci/api/triggers_spec.rb

@@ -42,7 +42,8 @@ describe Ci::API::API do
         post ci_api("/projects/#{project.ci_id}/refs/master/trigger"), options
         expect(response).to have_http_status(201)
         pipeline.builds.reload
-        expect(pipeline.builds.size).to eq(2)
+        expect(pipeline.builds.pending.size).to eq(2)
+        expect(pipeline.builds.size).to eq(5)
       end
 
       it 'returns bad request with no builds created if there\'s no commit for that ref' do

spec/requests/lfs_http_spec.rb

 require 'spec_helper'
 
-describe Gitlab::Lfs::Router do
+describe 'Git LFS API and storage' do
   let(:user) { create(:user) }
   let!(:lfs_object) { create(:lfs_object, :with_file) }
 
@@ -31,10 +31,11 @@ describe Gitlab::Lfs::Router do
         'operation' => 'upload'
       }
     end
+    let(:authorization) { authorize_user }
 
     before do
       allow(Gitlab.config.lfs).to receive(:enabled).and_return(false)
-      post_json "#{project.http_url_to_repo}/info/lfs/objects/batch", body, headers
+      post_lfs_json "#{project.http_url_to_repo}/info/lfs/objects/batch", body, headers
     end
 
     it 'responds with 501' do
@@ -71,8 +72,9 @@ describe Gitlab::Lfs::Router do
     end
 
     context 'when handling lfs request using deprecated API' do
+      let(:authorization) { authorize_user }
       before do
-        post_json "#{project.http_url_to_repo}/info/lfs/objects", nil, headers
+        post_lfs_json "#{project.http_url_to_repo}/info/lfs/objects", nil, headers
       end
 
       it_behaves_like 'a deprecated'
@@ -118,8 +120,8 @@ describe Gitlab::Lfs::Router do
               project.lfs_objects << lfs_object
             end
 
-            it 'responds with status 403' do
-              expect(response).to have_http_status(403)
+            it 'responds with status 404' do
+              expect(response).to have_http_status(404)
             end
           end
 
@@ -147,8 +149,8 @@ describe Gitlab::Lfs::Router do
       context 'without required headers' do
         let(:authorization) { authorize_user }
 
-        it 'responds with status 403' do
-          expect(response).to have_http_status(403)
+        it 'responds with status 404' do
+          expect(response).to have_http_status(404)
         end
       end
     end
@@ -162,7 +164,7 @@ describe Gitlab::Lfs::Router do
       enable_lfs
       update_lfs_permissions
       update_user_permissions
-      post_json "#{project.http_url_to_repo}/info/lfs/objects/batch", body, headers
+      post_lfs_json "#{project.http_url_to_repo}/info/lfs/objects/batch", body, headers
     end
 
     describe 'download' do
@@ -304,10 +306,10 @@ describe Gitlab::Lfs::Router do
         end
 
         context 'when user does is not member of the project' do
-          let(:role) { :guest }
+          let(:update_user_permissions) { nil }
 
-          it 'responds with 403' do
-            expect(response).to have_http_status(403)
+          it 'responds with 404' do
+            expect(response).to have_http_status(404)
           end
         end
 
@@ -510,6 +512,7 @@ describe Gitlab::Lfs::Router do
 
     describe 'unsupported' do
       let(:project) { create(:empty_project) }
+      let(:authorization) { authorize_user }
       let(:body) do
         { 'operation' => 'other',
           'objects' => [
@@ -553,11 +556,11 @@ describe Gitlab::Lfs::Router do
 
       context 'and request is sent with a malformed headers' do
         before do
-          put_finalize('cat /etc/passwd')
+          put_finalize('/etc/passwd')
         end
 
         it 'does not recognize it as a valid lfs command' do
-          expect(response).to have_http_status(403)
+          expect(response).to have_http_status(401)
         end
       end
     end
@@ -582,6 +585,16 @@ describe Gitlab::Lfs::Router do
           expect(response).to have_http_status(403)
         end
       end
+
+      context 'and request is sent with a malformed headers' do
+        before do
+          put_finalize('/etc/passwd')
+        end
+
+        it 'does not recognize it as a valid lfs command' do
+          expect(response).to have_http_status(403)
+        end
+      end
     end
 
     describe 'to one project' do
@@ -624,9 +637,25 @@ describe Gitlab::Lfs::Router do
               expect(lfs_object.projects.pluck(:id)).to include(project.id)
             end
           end
+
+          context 'invalid tempfiles' do
+            it 'rejects slashes in the tempfile name (path traversal' do
+              put_finalize('foo/bar')
+              expect(response).to have_http_status(403)
+            end
+
+            it 'rejects tempfile names that do not start with the oid' do
+              put_finalize("foo#{sample_oid}")
+              expect(response).to have_http_status(403)
+            end
+          end
         end
 
         describe 'and user does not have push access' do
+          before do
+            project.team << [user, :reporter]
+          end
+
           it_behaves_like 'forbidden'
         end
       end
@@ -758,8 +787,8 @@ describe Gitlab::Lfs::Router do
     Projects::ForkService.new(project, user, {}).execute
   end
 
-  def post_json(url, body = nil, headers = nil)
-    post(url, body.try(:to_json), (headers || {}).merge('Content-Type' => 'application/json'))
+  def post_lfs_json(url, body = nil, headers = nil)
+    post(url, body.try(:to_json), (headers || {}).merge('Content-Type' => 'application/vnd.git-lfs+json'))
   end
 
   def json_response

spec/services/ci/create_builds_service_spec.rb

-require 'spec_helper'
-
-describe Ci::CreateBuildsService, services: true do
-  let(:pipeline) { create(:ci_pipeline, ref: 'master') }
-  let(:user) { create(:user) }
-
-  describe '#execute' do
-    # Using stubbed .gitlab-ci.yml created in commit factory
-    #
-
-    subject do
-      described_class.new(pipeline).execute('test', user, status, nil)
-    end
-
-    context 'next builds available' do
-      let(:status) { 'success' }
-
-      it { is_expected.to be_an_instance_of Array }
-      it { is_expected.to all(be_an_instance_of Ci::Build) }
-
-      it 'does not persist created builds' do
-        expect(subject.first).not_to be_persisted
-      end
-    end
-
-    context 'builds skipped' do
-      let(:status) { 'skipped' }
-
-      it { is_expected.to be_empty }
-    end
-  end
-end

spec/services/create_commit_builds_service_spec.rb → spec/services/ci/create_pipeline_service_spec.rb

 require 'spec_helper'
 
-describe CreateCommitBuildsService, services: true do
-  let(:service) { CreateCommitBuildsService.new }
-  let(:project) { FactoryGirl.create(:empty_project) }
-  let(:user) { create(:user) }
+describe Ci::CreatePipelineService, services: true do
+  let(:project) { FactoryGirl.create(:project) }
+  let(:user) { create(:admin) }
 
   before do
     stub_ci_pipeline_to_return_yaml_file
   end
 
   describe '#execute' do
+    def execute(params)
+      described_class.new(project, user, params).execute
+    end
+
     context 'valid params' do
       let(:pipeline) do
-        service.execute(project, user,
-                        ref: 'refs/heads/master',
-                        before: '00000000',
-                        after: '31das312',
-                        commits: [{ message: "Message" }]
-                       )
+        execute(ref: 'refs/heads/master',
+                before: '00000000',
+                after: project.commit.id,
+                commits: [{ message: "Message" }])
       end
 
       it { expect(pipeline).to be_kind_of(Ci::Pipeline) }
@@ -30,38 +31,33 @@ describe CreateCommitBuildsService, services: true do
 
     context "skip tag if there is no build for it" do
       it "creates commit if there is appropriate job" do
-        result = service.execute(project, user,
-                                 ref: 'refs/tags/0_1',
-                                 before: '00000000',
-                                 after: '31das312',
-                                 commits: [{ message: "Message" }]
-                                )
+        result = execute(ref: 'refs/heads/master',
+                         before: '00000000',
+                         after: project.commit.id,
+                         commits: [{ message: "Message" }])
         expect(result).to be_persisted
       end
 
       it "creates commit if there is no appropriate job but deploy job has right ref setting" do
-        config = YAML.dump({ deploy: { script: "ls", only: ["0_1"] } })
+        config = YAML.dump({ deploy: { script: "ls", only: ["master"] } })
         stub_ci_pipeline_yaml_file(config)
+        result = execute(ref: 'refs/heads/master',
+                         before: '00000000',
+                         after: project.commit.id,
+                         commits: [{ message: "Message" }])
 
-        result = service.execute(project, user,
-                                 ref: 'refs/heads/0_1',
-                                 before: '00000000',
-                                 after: '31das312',
-                                 commits: [{ message: "Message" }]
-                                )
         expect(result).to be_persisted
       end
     end
 
     it 'skips creating pipeline for refs without .gitlab-ci.yml' do
       stub_ci_pipeline_yaml_file(nil)
-      result = service.execute(project, user,
-                               ref: 'refs/heads/0_1',
-                               before: '00000000',
-                               after: '31das312',
-                               commits: [{ message: 'Message' }]
-                              )
-      expect(result).to be_falsey
+      result = execute(ref: 'refs/heads/master',
+                       before: '00000000',
+                       after: project.commit.id,
+                       commits: [{ message: 'Message' }])
+
+      expect(result).not_to be_persisted
       expect(Ci::Pipeline.count).to eq(0)
     end
 
@@ -70,12 +66,11 @@ describe CreateCommitBuildsService, services: true do
       allow_any_instance_of(Ci::Pipeline).to receive(:git_commit_message) { message }
       stub_ci_pipeline_yaml_file('invalid: file: file')
       commits = [{ message: message }]
-      pipeline = service.execute(project, user,
-                                 ref: 'refs/tags/0_1',
-                                 before: '00000000',
-                                 after: '31das312',
-                                 commits: commits
-                                )
+      pipeline = execute(ref: 'refs/heads/master',
+                         before: '00000000',
+                         after: project.commit.id,
+                         commits: commits)
+
       expect(pipeline).to be_persisted
       expect(pipeline.builds.any?).to be false
       expect(pipeline.status).to eq('failed')
@@ -94,12 +89,10 @@ describe CreateCommitBuildsService, services: true do
 
       it "skips builds creation if there is [ci skip] tag in commit message" do
         commits = [{ message: message }]
-        pipeline = service.execute(project, user,
-                                   ref: 'refs/tags/0_1',
-                                   before: '00000000',
-                                   after: '31das312',
-                                   commits: commits
-                                  )
+        pipeline = execute(ref: 'refs/heads/master',
+                           before: '00000000',
+                           after: project.commit.id,
+                           commits: commits)
 
         expect(pipeline).to be_persisted
         expect(pipeline.builds.any?).to be false
@@ -108,12 +101,10 @@ describe CreateCommitBuildsService, services: true do
 
       it "skips builds creation if there is [skip ci] tag in commit message" do
         commits = [{ message: messageFlip }]
-        pipeline = service.execute(project, user,
-                                   ref: 'refs/tags/0_1',
-                                   before: '00000000',
-                                   after: '31das312',
-                                   commits: commits
-                                  )
+        pipeline = execute(ref: 'refs/heads/master',
+                           before: '00000000',
+                           after: project.commit.id,
+                           commits: commits)
 
         expect(pipeline).to be_persisted
         expect(pipeline.builds.any?).to be false
@@ -122,12 +113,10 @@ describe CreateCommitBuildsService, services: true do
 
       it "skips builds creation if there is [CI SKIP] tag in commit message" do
         commits = [{ message: capMessage }]
-        pipeline = service.execute(project, user,
-                                   ref: 'refs/tags/0_1',
-                                   before: '00000000',
-                                   after: '31das312',
-                                   commits: commits
-                                  )
+        pipeline = execute(ref: 'refs/heads/master',
+                           before: '00000000',
+                           after: project.commit.id,
+                           commits: commits)
 
         expect(pipeline).to be_persisted
         expect(pipeline.builds.any?).to be false
@@ -136,12 +125,10 @@ describe CreateCommitBuildsService, services: true do
 
       it "skips builds creation if there is [SKIP CI] tag in commit message" do
         commits = [{ message: capMessageFlip }]
-        pipeline = service.execute(project, user,
-                                   ref: 'refs/tags/0_1',
-                                   before: '00000000',
-                                   after: '31das312',
-                                   commits: commits
-                                  )
+        pipeline = execute(ref: 'refs/heads/master',
+                           before: '00000000',
+                           after: project.commit.id,
+                           commits: commits)
 
         expect(pipeline).to be_persisted
         expect(pipeline.builds.any?).to be false
@@ -152,67 +139,37 @@ describe CreateCommitBuildsService, services: true do
         allow_any_instance_of(Ci::Pipeline).to receive(:git_commit_message) { "some message" }
 
         commits = [{ message: "some message" }]
-        pipeline = service.execute(project, user,
-                                   ref: 'refs/tags/0_1',
-                                   before: '00000000',
-                                   after: '31das312',
-                                   commits: commits
-                                  )
+        pipeline = execute(ref: 'refs/heads/master',
+                           before: '00000000',
+                           after: project.commit.id,
+                           commits: commits)
 
         expect(pipeline).to be_persisted
-        expect(pipeline.builds.first.name).to eq("staging")
+        expect(pipeline.builds.first.name).to eq("rspec")
       end
 
-      it "skips builds creation if there is [ci skip] tag in commit message and yaml is invalid" do
+      it "fails builds creation if there is [ci skip] tag in commit message and yaml is invalid" do
         stub_ci_pipeline_yaml_file('invalid: file: fiile')
         commits = [{ message: message }]
-        pipeline = service.execute(project, user,
-                                   ref: 'refs/tags/0_1',
-                                   before: '00000000',
-                                   after: '31das312',
-                                   commits: commits
-                                  )
+        pipeline = execute(ref: 'refs/heads/master',
+                           before: '00000000',
+                           after: project.commit.id,
+                           commits: commits)
+
         expect(pipeline).to be_persisted
         expect(pipeline.builds.any?).to be false
-        expect(pipeline.status).to eq("skipped")
-        expect(pipeline.yaml_errors).to be_nil
+        expect(pipeline.status).to eq("failed")
+        expect(pipeline.yaml_errors).not_to be_nil
       end
     end
 
-    it "skips build creation if there are already builds" do
-      allow_any_instance_of(Ci::Pipeline).to receive(:ci_yaml_file) { gitlab_ci_yaml }
-
-      commits = [{ message: "message" }]
-      pipeline = service.execute(project, user,
-                                 ref: 'refs/heads/master',
-                                 before: '00000000',
-                                 after: '31das312',
-                                 commits: commits
-                                )
-      expect(pipeline).to be_persisted
-      expect(pipeline.builds.count(:all)).to eq(2)
-
-      pipeline = service.execute(project, user,
-                                 ref: 'refs/heads/master',
-                                 before: '00000000',
-                                 after: '31das312',
-                                 commits: commits
-                                )
-      expect(pipeline).to be_persisted
-      expect(pipeline.builds.count(:all)).to eq(2)
-    end
-
     it "creates commit with failed status if yaml is invalid" do
       stub_ci_pipeline_yaml_file('invalid: file')
-
       commits = [{ message: "some message" }]
-
-      pipeline = service.execute(project, user,
-                                 ref: 'refs/tags/0_1',
-                                 before: '00000000',
-                                 after: '31das312',
-                                 commits: commits
-                                )
+      pipeline = execute(ref: 'refs/heads/master',
+                         before: '00000000',
+                         after: project.commit.id,
+                         commits: commits)
 
       expect(pipeline).to be_persisted
       expect(pipeline.status).to eq("failed")
@@ -226,16 +183,32 @@ describe CreateCommitBuildsService, services: true do
       end
 
       it 'does not create a new pipeline' do
-        result = service.execute(project, user,
-                                 ref: 'refs/heads/master',
-                                 before: '00000000',
-                                 after: '31das312',
-                                 commits: [{ message: 'some msg' }])
+        result = execute(ref: 'refs/heads/master',
+                         before: '00000000',
+                         after: project.commit.id,
+                         commits: [{ message: 'some msg' }])
 
-        expect(result).to be_falsey
+        expect(result).not_to be_persisted
         expect(Ci::Build.all).to be_empty
         expect(Ci::Pipeline.count).to eq(0)
       end
     end
+
+    context 'with manual actions' do
+      before do
+        config = YAML.dump({ deploy: { script: 'ls', when: 'manual' } })
+        stub_ci_pipeline_yaml_file(config)
+      end
+
+      it 'does not create a new pipeline' do
+        result = execute(ref: 'refs/heads/master',
+                         before: '00000000',
+                         after: project.commit.id,
+                         commits: [{ message: 'some msg' }])
+
+        expect(result).to be_persisted
+        expect(result.manual_actions).not_to be_empty
+      end
+    end
   end
 end

spec/services/ci/create_trigger_request_service_spec.rb

 require 'spec_helper'
 
 describe Ci::CreateTriggerRequestService, services: true do
-  let(:service) { Ci::CreateTriggerRequestService.new }
+  let(:service) { described_class.new }
   let(:project) { create(:project) }
   let(:trigger) { create(:ci_trigger, project: project) }
 
@@ -27,8 +27,7 @@ describe Ci::CreateTriggerRequestService, services: true do
       subject { service.execute(project, trigger, 'master') }
 
       before do
-        stub_ci_pipeline_yaml_file('{}')
-        FactoryGirl.create :ci_pipeline, project: project
+        stub_ci_pipeline_yaml_file('script: { only: [develop], script: hello World }')
       end
 
       it { expect(subject).to be_nil }

spec/services/ci/image_for_build_service_spec.rb

@@ -5,8 +5,8 @@ module Ci
     let(:service) { ImageForBuildService.new }
     let(:project) { FactoryGirl.create(:empty_project) }
     let(:commit_sha) { '01234567890123456789' }
-    let(:commit) { project.ensure_pipeline(commit_sha, 'master') }
-    let(:build) { FactoryGirl.create(:ci_build, pipeline: commit) }
+    let(:pipeline) { project.ensure_pipeline(commit_sha, 'master') }
+    let(:build) { FactoryGirl.create(:ci_build, pipeline: pipeline) }
 
     describe '#execute' do
       before { build }
@@ -14,6 +14,7 @@ module Ci
       context 'branch name' do
         before { allow(project).to receive(:commit).and_return(OpenStruct.new(sha: commit_sha)) }
         before { build.run! }
+        before { pipeline.reload_status! }
         let(:image) { service.execute(project, ref: 'master') }
 
         it { expect(image).to be_kind_of(OpenStruct) }
@@ -31,6 +32,7 @@ module Ci
 
       context 'commit sha' do
         before { build.run! }
+        before { pipeline.reload_status! }
         let(:image) { service.execute(project, sha: build.sha) }
 
         it { expect(image).to be_kind_of(OpenStruct) }

spec/services/ci/process_pipeline_service_spec.rb

+require 'spec_helper'
+
+describe Ci::ProcessPipelineService, services: true do
+  let(:pipeline) { create(:ci_pipeline, ref: 'master') }
+  let(:user) { create(:user) }
+  let(:all_builds) { pipeline.builds }
+  let(:builds) { all_builds.where.not(status: [:created, :skipped]) }
+  let(:config) { nil }
+
+  before do
+    allow(pipeline).to receive(:ci_yaml_file).and_return(config)
+  end
+
+  describe '#execute' do
+    def create_builds
+      described_class.new(pipeline.project, user).execute(pipeline)
+    end
+
+    def succeed_pending
+      builds.pending.update_all(status: 'success')
+    end
+
+    context 'start queuing next builds' do
+      before do
+        create(:ci_build, :created, pipeline: pipeline, name: 'linux', stage_idx: 0)
+        create(:ci_build, :created, pipeline: pipeline, name: 'mac', stage_idx: 0)
+        create(:ci_build, :created, pipeline: pipeline, name: 'rspec', stage_idx: 1)
+        create(:ci_build, :created, pipeline: pipeline, name: 'rubocop', stage_idx: 1)
+        create(:ci_build, :created, pipeline: pipeline, name: 'deploy', stage_idx: 2)
+      end
+
+      it 'processes a pipeline' do
+        expect(create_builds).to be_truthy
+        succeed_pending
+        expect(builds.success.count).to eq(2)
+
+        expect(create_builds).to be_truthy
+        succeed_pending
+        expect(builds.success.count).to eq(4)
+
+        expect(create_builds).to be_truthy
+        succeed_pending
+        expect(builds.success.count).to eq(5)
+
+        expect(create_builds).to be_falsey
+      end
+
+      it 'does not process pipeline if existing stage is running' do
+        expect(create_builds).to be_truthy
+        expect(builds.pending.count).to eq(2)
+        
+        expect(create_builds).to be_falsey
+        expect(builds.pending.count).to eq(2)
+      end
+    end
+
+    context 'custom stage with first job allowed to fail' do
+      before do
+        create(:ci_build, :created, pipeline: pipeline, name: 'clean_job', stage_idx: 0, allow_failure: true)
+        create(:ci_build, :created, pipeline: pipeline, name: 'test_job', stage_idx: 1, allow_failure: true)
+      end
+
+      it 'automatically triggers a next stage when build finishes' do
+        expect(create_builds).to be_truthy
+        expect(builds.pluck(:status)).to contain_exactly('pending')
+
+        pipeline.builds.running_or_pending.each(&:drop)
+        expect(builds.pluck(:status)).to contain_exactly('failed', 'pending')
+      end
+    end
+
+    context 'properly creates builds when "when" is defined' do
+      before do
+        create(:ci_build, :created, pipeline: pipeline, name: 'build', stage_idx: 0)
+        create(:ci_build, :created, pipeline: pipeline, name: 'test', stage_idx: 1)
+        create(:ci_build, :created, pipeline: pipeline, name: 'test_failure', stage_idx: 2, when: 'on_failure')
+        create(:ci_build, :created, pipeline: pipeline, name: 'deploy', stage_idx: 3)
+        create(:ci_build, :created, pipeline: pipeline, name: 'production', stage_idx: 3, when: 'manual')
+        create(:ci_build, :created, pipeline: pipeline, name: 'cleanup', stage_idx: 4, when: 'always')
+        create(:ci_build, :created, pipeline: pipeline, name: 'clear cache', stage_idx: 4, when: 'manual')
+      end
+
+      context 'when builds are successful' do
+        it 'properly creates builds' do
+          expect(create_builds).to be_truthy
+          expect(builds.pluck(:name)).to contain_exactly('build')
+          expect(builds.pluck(:status)).to contain_exactly('pending')
+          pipeline.builds.running_or_pending.each(&:success)
+
+          expect(builds.pluck(:name)).to contain_exactly('build', 'test')
+          expect(builds.pluck(:status)).to contain_exactly('success', 'pending')
+          pipeline.builds.running_or_pending.each(&:success)
+
+          expect(builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy')
+          expect(builds.pluck(:status)).to contain_exactly('success', 'success', 'pending')
+          pipeline.builds.running_or_pending.each(&:success)
+
+          expect(builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy', 'cleanup')
+          expect(builds.pluck(:status)).to contain_exactly('success', 'success', 'success', 'pending')
+          pipeline.builds.running_or_pending.each(&:success)
+
+          expect(builds.pluck(:status)).to contain_exactly('success', 'success', 'success', 'success')
+          pipeline.reload
+          expect(pipeline.status).to eq('success')
+        end
+      end
+
+      context 'when test job fails' do
+        it 'properly creates builds' do
+          expect(create_builds).to be_truthy
+          expect(builds.pluck(:name)).to contain_exactly('build')
+          expect(builds.pluck(:status)).to contain_exactly('pending')
+          pipeline.builds.running_or_pending.each(&:success)
+
+          expect(builds.pluck(:name)).to contain_exactly('build', 'test')
+          expect(builds.pluck(:status)).to contain_exactly('success', 'pending')
+          pipeline.builds.running_or_pending.each(&:drop)
+
+          expect(builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure')
+          expect(builds.pluck(:status)).to contain_exactly('success', 'failed', 'pending')
+          pipeline.builds.running_or_pending.each(&:success)
+
+          expect(builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure', 'cleanup')
+          expect(builds.pluck(:status)).to contain_exactly('success', 'failed', 'success', 'pending')
+          pipeline.builds.running_or_pending.each(&:success)
+
+          expect(builds.pluck(:status)).to contain_exactly('success', 'failed', 'success', 'success')
+          pipeline.reload
+          expect(pipeline.status).to eq('failed')
+        end
+      end
+
+      context 'when test and test_failure jobs fail' do
+        it 'properly creates builds' do
+          expect(create_builds).to be_truthy
+          expect(builds.pluck(:name)).to contain_exactly('build')
+          expect(builds.pluck(:status)).to contain_exactly('pending')
+          pipeline.builds.running_or_pending.each(&:success)
+
+          expect(builds.pluck(:name)).to contain_exactly('build', 'test')
+          expect(builds.pluck(:status)).to contain_exactly('success', 'pending')
+          pipeline.builds.running_or_pending.each(&:drop)
+
+          expect(builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure')
+          expect(builds.pluck(:status)).to contain_exactly('success', 'failed', 'pending')
+          pipeline.builds.running_or_pending.each(&:drop)
+
+          expect(builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure', 'cleanup')
+          expect(builds.pluck(:status)).to contain_exactly('success', 'failed', 'failed', 'pending')
+          pipeline.builds.running_or_pending.each(&:success)
+
+          expect(builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure', 'cleanup')
+          expect(builds.pluck(:status)).to contain_exactly('success', 'failed', 'failed', 'success')
+          pipeline.reload
+          expect(pipeline.status).to eq('failed')
+        end
+      end
+
+      context 'when deploy job fails' do
+        it 'properly creates builds' do
+          expect(create_builds).to be_truthy
+          expect(builds.pluck(:name)).to contain_exactly('build')
+          expect(builds.pluck(:status)).to contain_exactly('pending')
+          pipeline.builds.running_or_pending.each(&:success)
+
+          expect(builds.pluck(:name)).to contain_exactly('build', 'test')
+          expect(builds.pluck(:status)).to contain_exactly('success', 'pending')
+          pipeline.builds.running_or_pending.each(&:success)
+
+          expect(builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy')
+          expect(builds.pluck(:status)).to contain_exactly('success', 'success', 'pending')
+          pipeline.builds.running_or_pending.each(&:drop)
+
+          expect(builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy', 'cleanup')
+          expect(builds.pluck(:status)).to contain_exactly('success', 'success', 'failed', 'pending')
+          pipeline.builds.running_or_pending.each(&:success)
+
+          expect(builds.pluck(:status)).to contain_exactly('success', 'success', 'failed', 'success')
+          pipeline.reload
+          expect(pipeline.status).to eq('failed')
+        end
+      end
+
+      context 'when build is canceled in the second stage' do
+        it 'does not schedule builds after build has been canceled' do
+          expect(create_builds).to be_truthy
+          expect(builds.pluck(:name)).to contain_exactly('build')
+          expect(builds.pluck(:status)).to contain_exactly('pending')
+          pipeline.builds.running_or_pending.each(&:success)
+
+          expect(builds.running_or_pending).not_to be_empty
+
+          expect(builds.pluck(:name)).to contain_exactly('build', 'test')
+          expect(builds.pluck(:status)).to contain_exactly('success', 'pending')
+          pipeline.builds.running_or_pending.each(&:cancel)
+
+          expect(builds.running_or_pending).to be_empty
+          expect(pipeline.reload.status).to eq('canceled')
+        end
+      end
+
+      context 'when listing manual actions' do
+        it 'returns only for skipped builds' do
+          # currently all builds are created
+          expect(create_builds).to be_truthy
+          expect(manual_actions).to be_empty
+
+          # succeed stage build
+          pipeline.builds.running_or_pending.each(&:success)
+          expect(manual_actions).to be_empty
+
+          # succeed stage test
+          pipeline.builds.running_or_pending.each(&:success)
+          expect(manual_actions).to be_one # production
+
+          # succeed stage deploy
+          pipeline.builds.running_or_pending.each(&:success)
+          expect(manual_actions).to be_many # production and clear cache
+        end
+
+        def manual_actions
+          pipeline.manual_actions
+        end
+      end
+    end
+
+    context 'creates a builds from .gitlab-ci.yml' do
+      let(:config) do
+        YAML.dump({
+          rspec: {
+            stage: 'test',
+            script: 'rspec'
+          },
+          rubocop: {
+            stage: 'test',
+            script: 'rubocop'
+          },
+          deploy: {
+            stage: 'deploy',
+            script: 'deploy'
+          }
+        })
+      end
+
+      # Using stubbed .gitlab-ci.yml created in commit factory
+      #
+
+      before do
+        stub_ci_pipeline_yaml_file(config)
+        create(:ci_build, :created, pipeline: pipeline, name: 'linux', stage: 'build', stage_idx: 0)
+        create(:ci_build, :created, pipeline: pipeline, name: 'mac', stage: 'build', stage_idx: 0)
+      end
+
+      it 'when processing a pipeline' do
+        # Currently we have two builds with state created
+        expect(builds.count).to eq(0)
+        expect(all_builds.count).to eq(2)
+
+        # Create builds will mark the created as pending
+        expect(create_builds).to be_truthy
+        expect(builds.count).to eq(2)
+        expect(all_builds.count).to eq(2)
+
+        # When we builds succeed we will create a rest of pipeline from .gitlab-ci.yml
+        # We will have 2 succeeded, 2 pending (from stage test), total 5 (one more build from deploy)
+        succeed_pending
+        expect(create_builds).to be_truthy
+        expect(builds.success.count).to eq(2)
+        expect(builds.pending.count).to eq(2)
+        expect(all_builds.count).to eq(5)
+
+        # When we succeed the 2 pending from stage test,
+        # We will queue a deploy stage, no new builds will be created
+        succeed_pending
+        expect(create_builds).to be_truthy
+        expect(builds.pending.count).to eq(1)
+        expect(builds.success.count).to eq(4)
+        expect(all_builds.count).to eq(5)
+
+        # When we succeed last pending build, we will have a total of 5 succeeded builds, no new builds will be created
+        succeed_pending
+        expect(create_builds).to be_falsey
+        expect(builds.success.count).to eq(5)
+        expect(all_builds.count).to eq(5)
+      end
+    end
+  end
+end

spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb

@@ -110,19 +110,15 @@ describe MergeRequests::MergeWhenBuildSucceedsService do
 
     context 'properly handles multiple stages' do
       let(:ref) { mr_merge_if_green_enabled.source_branch }
-      let(:build) { create(:ci_build, pipeline: pipeline, ref: ref, name: 'build', stage: 'build') }
-      let(:test) { create(:ci_build, pipeline: pipeline, ref: ref, name: 'test', stage: 'test') }
+      let!(:build) { create(:ci_build, :created, pipeline: pipeline, ref: ref, name: 'build', stage: 'build') }
+      let!(:test) { create(:ci_build, :created, pipeline: pipeline, ref: ref, name: 'test', stage: 'test') }
+      let(:pipeline) { create(:ci_empty_pipeline, ref: mr_merge_if_green_enabled.source_branch, project: project) }
 
       before do
         # This behavior of MergeRequest: we instantiate a new object
         allow_any_instance_of(MergeRequest).to receive(:pipeline).and_wrap_original do
           Ci::Pipeline.find(pipeline.id)
         end
-
-        # We create test after the build
-        allow(pipeline).to receive(:create_next_builds).and_wrap_original do
-          test
-        end
       end
 
       it "doesn't merge if some stages failed" do

spec/support/api/members_shared_examples.rb

+shared_examples 'a 404 response when source is private' do
+  before do
+    source.update_column(:visibility_level, Gitlab::VisibilityLevel::PRIVATE)
+  end
+
+  it 'returns 404' do
+    route
+
+    expect(response).to have_http_status(404)
+  end
+end

spec/tasks/gitlab/backup_rake_spec.rb

@@ -42,7 +42,7 @@ describe 'gitlab:app namespace rake task' do
       before do
         allow(Dir).to receive(:glob).and_return([])
         allow(Dir).to receive(:chdir)
-        allow(File).to receive(:exists?).and_return(true)
+        allow(File).to receive(:exist?).and_return(true)
         allow(Kernel).to receive(:system).and_return(true)
         allow(FileUtils).to receive(:cp_r).and_return(true)
         allow(FileUtils).to receive(:mv).and_return(true)

spec/workers/post_receive_spec.rb

@@ -53,7 +53,13 @@ describe PostReceive do
       subject { PostReceive.new.perform(pwd(project), key_id, base64_changes) }
 
       context "creates a Ci::Pipeline for every change" do
-        before { stub_ci_pipeline_to_return_yaml_file }
+        before do
+          allow_any_instance_of(Ci::CreatePipelineService).to receive(:commit) do
+            OpenStruct.new(id: '123456')
+          end
+          allow_any_instance_of(Ci::CreatePipelineService).to receive(:branch?).and_return(true)
+          stub_ci_pipeline_to_return_yaml_file
+        end
 
         it { expect{ subject }.to change{ Ci::Pipeline.count }.by(2) }
       end