Merge branch 'ce_upstream' into 'master'

CE upstream

CE changes https://gitlab.com/gitlab-org/gitlab-ce/compare/d636efbe26ee51030bb108d889c2ef867570358a...0c36b810abede73b3cf59f951e7467dbac54ec98?expand_all_diffs=1

See merge request !850

features/steps/profile/profile.rb

@@ -104,18 +104,6 @@ class Spinach::Features::Profile < Spinach::FeatureSteps
     end
   end
-  step 'I reset my token' do
-    page.within '.private-token' do
-      @old_token = @user.private_token
-      click_button "Reset private token"
-    end
-  end
-
-  step 'I should see new token' do
-    expect(find("#token").value).not_to eq @old_token
-    expect(find("#token").value).to eq @user.reload.private_token
-  end
-
   step 'I have activity' do
     create(:closed_issue_event, author: current_user)
   end

features/steps/project/network_graph.rb

@@ -109,4 +109,8 @@ class Spinach::Features::ProjectNetworkGraph < Spinach::FeatureSteps
       find('button').click
     end
   end
+
+  step 'I should see non-existent git revision error message' do
+    expect(page).to have_selector '.flash-alert', text: "Git revision ';' does not exist."
+  end
 end

lib/api/entities.rb

@@ -468,11 +468,14 @@ module API
     end
     class LabelBasic < Grape::Entity
-      expose :name, :color, :description
+      expose :id, :name, :color, :description
     end
     class Label < LabelBasic
       expose :open_issues_count, :closed_issues_count, :open_merge_requests_count
+      expose :priority do |label, options|
+        label.priority(options[:project])
+      end
       expose :subscribed do |label, options|
         label.subscribed?(options[:current_user])

lib/api/labels.rb

@@ -11,7 +11,7 @@ module API
         success Entities::Label
       end
       get ':id/labels' do
-        present available_labels, with: Entities::Label, current_user: current_user
+        present available_labels, with: Entities::Label, current_user: current_user, project: user_project
       end
       desc 'Create a new label' do
@@ -21,6 +21,7 @@ module API
         requires :name, type: String, desc: 'The name of the label to be created'
         requires :color, type: String, desc: "The color of the label given in 6-digit hex notation with leading '#' sign (e.g. #FFAABB)"
         optional :description, type: String, desc: 'The description of label to be created'
+        optional :priority, type: Integer, desc: 'The priority of the label', allow_blank: true
       end
       post ':id/labels' do
         authorize! :admin_label, user_project
@@ -28,10 +29,15 @@ module API
         label = available_labels.find_by(title: params[:name])
         conflict!('Label already exists') if label
-        label = user_project.labels.create(declared(params, include_parent_namespaces: false).to_h)
+        priority = params.delete(:priority)
+        label_params = declared(params,
+                                include_parent_namespaces: false,
+                                include_missing: false).to_h
+        label = user_project.labels.create(label_params)
         if label.valid?
-          present label, with: Entities::Label, current_user: current_user
+          label.prioritize!(user_project, priority) if priority
+          present label, with: Entities::Label, current_user: current_user, project: user_project
         else
           render_validation_error!(label)
         end
@@ -49,7 +55,7 @@ module API
         label = user_project.labels.find_by(title: params[:name])
         not_found!('Label') unless label
-        present label.destroy, with: Entities::Label, current_user: current_user
+        present label.destroy, with: Entities::Label, current_user: current_user, project: user_project
       end
       desc 'Update an existing label. At least one optional parameter is required.' do
@@ -60,7 +66,8 @@ module API
         optional :new_name, type: String, desc: 'The new name of the label'
         optional :color, type: String, desc: "The new color of the label given in 6-digit hex notation with leading '#' sign (e.g. #FFAABB)"
         optional :description, type: String, desc: 'The new description of label'
-        at_least_one_of :new_name, :color, :description
+        optional :priority, type: Integer, desc: 'The priority of the label', allow_blank: true
+        at_least_one_of :new_name, :color, :description, :priority
       end
       put ':id/labels' do
         authorize! :admin_label, user_project
@@ -68,17 +75,25 @@ module API
         label = user_project.labels.find_by(title: params[:name])
         not_found!('Label not found') unless label
-        update_params = declared(params,
-                                 include_parent_namespaces: false,
-                                 include_missing: false).to_h
+        update_priority = params.key?(:priority)
+        priority = params.delete(:priority)
+        label_params = declared(params,
+                                include_parent_namespaces: false,
+                                include_missing: false).to_h
         # Rename new name to the actual label attribute name
-        update_params['name'] = update_params.delete('new_name') if update_params.key?('new_name')
-        if label.update(update_params)
-          present label, with: Entities::Label, current_user: current_user
-        else
-          render_validation_error!(label)
+        label_params[:name] = label_params.delete('new_name') if label_params.key?('new_name')
+        render_validation_error!(label) unless label.update(label_params)
+
+        if update_priority
+          if priority.nil?
+            label.unprioritize!(user_project)
+          else
+            label.prioritize!(user_project, priority)
+          end
         end
+
+        present label, with: Entities::Label, current_user: current_user, project: user_project
       end
     end
   end

lib/api/system_hooks.rb

@@ -32,7 +32,7 @@ module API
         if hook.save
           present hook, with: Entities::Hook
         else
-          not_found!
+          render_validation_error!(hook)
         end
       end

lib/constraints/namespace_url_constrainer.rb → lib/constraints/constrainer_helper.rb

-class NamespaceUrlConstrainer
-  def matches?(request)
-    id = request.path
-    id = id.sub(/\A#{relative_url_root}/, '') if relative_url_root
-    id = id.sub(/\A\/+/, '').split('/').first
-    id = id.sub(/.atom\z/, '') if id
-
-    if id =~ Gitlab::Regex.namespace_regex
-      find_resource(id)
-    end
-  end
-
-  def find_resource(id)
-    Namespace.find_by_path(id)
+module ConstrainerHelper
+  def extract_resource_path(path)
+    id = path.dup
+    id.sub!(/\A#{relative_url_root}/, '') if relative_url_root
+    id.sub(/\A\/+/, '').sub(/\/+\z/, '').sub(/.atom\z/, '')
   end
   private

lib/constraints/group_url_constrainer.rb

-require 'constraints/namespace_url_constrainer'
-class GroupUrlConstrainer < NamespaceUrlConstrainer
-  def find_resource(id)
-    Group.find_by_path(id)
+require_relative 'constrainer_helper'
+class GroupUrlConstrainer
+  include ConstrainerHelper
+
+  def matches?(request)
+    id = extract_resource_path(request.path)
+
+    if id =~ Gitlab::Regex.namespace_regex
+      Group.find_by(path: id).present?
+    else
+      false
+    end
   end
 end

lib/constraints/user_url_constrainer.rb

-require 'constraints/namespace_url_constrainer'
-class UserUrlConstrainer < NamespaceUrlConstrainer
-  def find_resource(id)
-    User.find_by('lower(username) = ?', id.downcase)
+require_relative 'constrainer_helper'
+class UserUrlConstrainer
+  include ConstrainerHelper
+
+  def matches?(request)
+    id = extract_resource_path(request.path)
+
+    if id =~ Gitlab::Regex.namespace_regex
+      User.find_by('lower(username) = ?', id.downcase).present?
+    else
+      false
+    end
   end
 end

lib/extracts_path.rb

@@ -106,7 +106,7 @@ module ExtractsPath
   # resolved (e.g., when a user inserts an invalid path or ref).
   def assign_ref_vars
     # assign allowed options
-    allowed_options = ["filter_ref", "extended_sha1"]
+    allowed_options = ["filter_ref"]
     @options = params.select {|key, value| allowed_options.include?(key) && !value.blank? }
     @options = HashWithIndifferentAccess.new(@options)
@@ -114,17 +114,13 @@ module ExtractsPath
     @ref, @path = extract_ref(@id)
     @repo = @project.repository
-    if @options[:extended_sha1].present?
-      @commit = @repo.commit(@options[:extended_sha1])
-    else
-      @commit = @repo.commit(@ref)
-      if @path.empty? && !@commit && @id.ends_with?('.atom')
-        @id = @ref = extract_ref_without_atom(@id)
-        @commit = @repo.commit(@ref)
-        request.format = :atom if @commit
-      end
+    @commit = @repo.commit(@ref)
+    if @path.empty? && !@commit && @id.ends_with?('.atom')
+      @id = @ref = extract_ref_without_atom(@id)
+      @commit = @repo.commit(@ref)
+      request.format = :atom if @commit
     end
     raise InvalidPathError unless @commit

lib/gitlab/backend/shell.rb

@@ -188,19 +188,6 @@ module Gitlab
                                    'rm-project', storage, "#{name}.git"])
     end
-    # Gc repository
-    #
-    # storage - project storage path
-    # path - project path with namespace
-    #
-    # Ex.
-    #   gc("/path/to/storage", "gitlab/gitlab-ci")
-    #
-    def gc(storage, path)
-      Gitlab::Utils.system_silent([gitlab_shell_projects_path, 'gc',
-                                   storage, "#{path}.git"])
-    end
-
     # Add new key to gitlab-shell
     #
     # Ex.

lib/gitlab/email/handler.rb

@@ -4,8 +4,7 @@ require 'gitlab/email/handler/create_issue_handler'
 module Gitlab
   module Email
     module Handler
-      # The `CreateIssueHandler` feature is disabled for the time being.
-      HANDLERS = [CreateNoteHandler]
+      HANDLERS = [CreateNoteHandler, CreateIssueHandler]
       def self.for(mail, mail_key)
         HANDLERS.find do |klass|

lib/gitlab/email/handler/create_issue_handler.rb

@@ -5,16 +5,16 @@ module Gitlab
   module Email
     module Handler
       class CreateIssueHandler < BaseHandler
-        attr_reader :project_path, :authentication_token
+        attr_reader :project_path, :incoming_email_token
         def initialize(mail, mail_key)
           super(mail, mail_key)
-          @project_path, @authentication_token =
+          @project_path, @incoming_email_token =
             mail_key && mail_key.split('+', 2)
         end
         def can_handle?
-          !authentication_token.nil?
+          !incoming_email_token.nil?
         end
         def execute
@@ -29,7 +29,7 @@ module Gitlab
         end
         def author
-          @author ||= User.find_by(authentication_token: authentication_token)
+          @author ||= User.find_by(incoming_email_token: incoming_email_token)
         end
         def project

lib/gitlab/exclusive_lease.rb

+require 'securerandom'
+
 module Gitlab
   # This class implements an 'exclusive lease'. We call it a 'lease'
   # because it has a set expiry time. We call it 'exclusive' because only
   # one caller may obtain a lease for a given key at a time. The
   # implementation is intended to work across GitLab processes and across
-  # servers. It is a 'cheap' alternative to using SQL queries and updates:
+  # servers. It is a cheap alternative to using SQL queries and updates:
   # you do not need to change the SQL schema to start using
   # ExclusiveLease.
   #
-  # It is important to choose the timeout wisely. If the timeout is very
-  # high (1 hour) then the throughput of your operation gets very low (at
-  # most once an hour). If the timeout is lower than how long your
-  # operation may take then you cannot count on exclusivity. For example,
-  # if the timeout is 10 seconds and you do an operation which may take 20
-  # seconds then two overlapping operations may hold a lease for the same
-  # key at the same time.
-  #
-  # This class has no 'cancel' method. I originally decided against adding
-  # it because it would add complexity and a false sense of security. The
-  # complexity: instead of setting '1' we would have to set a UUID, and to
-  # delete it we would have to execute Lua on the Redis server to only
-  # delete the key if the value was our own UUID. Otherwise there is a
-  # chance that when you intend to cancel your lease you actually delete
-  # someone else's. The false sense of security: you cannot design your
-  # system to rely too much on the lease being cancelled after use because
-  # the calling (Ruby) process may crash or be killed. You _cannot_ count
-  # on begin/ensure blocks to cancel a lease, because the 'ensure' does
-  # not always run. Think of 'kill -9' from the Unicorn master for
-  # instance.
-  #
-  # If you find that leases are getting in your way, ask yourself: would
-  # it be enough to lower the lease timeout? Another thing that might be
-  # appropriate is to only use a lease for bulk/automated operations, and
-  # to ignore the lease when you get a single 'manual' user request (a
-  # button click).
-  #
   class ExclusiveLease
+    LUA_CANCEL_SCRIPT = <<-EOS
+      local key, uuid = KEYS[1], ARGV[1]
+      if redis.call("get", key) == uuid then
+        redis.call("del", key)
+      end
+    EOS
+
+    def self.cancel(key, uuid)
+      Gitlab::Redis.with do |redis|
+        redis.eval(LUA_CANCEL_SCRIPT, keys: [redis_key(key)], argv: [uuid])
+      end
+    end
+
+    def self.redis_key(key)
+      "gitlab:exclusive_lease:#{key}"
+    end
+
     def initialize(key, timeout:)
-      @key, @timeout = key, timeout
+      @redis_key = self.class.redis_key(key)
+      @timeout = timeout
+      @uuid = SecureRandom.uuid
     end
-    # Try to obtain the lease. Return true on success,
+    # Try to obtain the lease. Return lease UUID on success,
     # false if the lease is already taken.
     def try_obtain
       # Performing a single SET is atomic
       Gitlab::Redis.with do |redis|
-        !!redis.set(redis_key, '1', nx: true, ex: @timeout)
+        redis.set(@redis_key, @uuid, nx: true, ex: @timeout) && @uuid
       end
     end
     # Returns true if the key for this lease is set.
     def exists?
       Gitlab::Redis.with do |redis|
-        redis.exists(redis_key)
+        redis.exists(@redis_key)
       end
     end
-
-    # No #cancel method. See comments above!
-
-    private
-
-    def redis_key
-      "gitlab:exclusive_lease:#{@key}"
-    end
   end
 end

lib/gitlab/incoming_email.rb

 module Gitlab
   module IncomingEmail
+    WILDCARD_PLACEHOLDER = '%{key}'.freeze
+
     class << self
       FALLBACK_MESSAGE_ID_REGEX = /\Areply\-(.+)@#{Gitlab.config.gitlab.host}\Z/.freeze
@@ -7,8 +9,16 @@ module Gitlab
         config.enabled && config.address
       end
+      def supports_wildcard?
+        config.address && config.address.include?(WILDCARD_PLACEHOLDER)
+      end
+
+      def supports_issue_creation?
+        enabled? && supports_wildcard?
+      end
+
       def reply_address(key)
-        config.address.gsub('%{key}', key)
+        config.address.gsub(WILDCARD_PLACEHOLDER, key)
       end
       def key_from_address(address)

lib/gitlab/project_search_results.rb

@@ -5,11 +5,7 @@ module Gitlab
     def initialize(current_user, project, query, repository_ref = nil)
       @current_user = current_user
       @project = project
-      @repository_ref = if repository_ref.present?
-                          repository_ref
-                        else
-                          nil
-                        end
+      @repository_ref = repository_ref.presence
       @query = query
     end
@@ -47,33 +43,31 @@ module Gitlab
     private
     def blobs
-      if project.empty_repo? || query.blank?
-        []
-      else
-        project.repository.search_files(query, repository_ref)
-      end
+      @blobs ||= project.repository.search_files(query, repository_ref)
     end
     def wiki_blobs
-      if project.wiki_enabled? && query.present?
-        project_wiki = ProjectWiki.new(project)
-        unless project_wiki.empty?
-          project_wiki.search_files(query)
+      @wiki_blobs ||= begin
+        if project.wiki_enabled? && query.present?
+          project_wiki = ProjectWiki.new(project)
+          unless project_wiki.empty?
+            project_wiki.search_files(query)
+          else
+            []
+          end
         else
           []
         end
-      else
-        []
       end
     end
     def notes
-      project.notes.user.search(query, as_user: @current_user).order('updated_at DESC')
+      @notes ||= project.notes.user.search(query, as_user: @current_user).order('updated_at DESC')
     end
     def commits
-      project.repository.find_commits_by_message(query)
+      @commits ||= project.repository.find_commits_by_message(query)
     end
     def project_ids_relation

lib/gitlab/regex.rb

@@ -8,6 +8,10 @@ module Gitlab
       @namespace_regex ||= /\A#{NAMESPACE_REGEX_STR}\z/.freeze
     end
+    def namespace_route_regex
+      @namespace_route_regex ||= /#{NAMESPACE_REGEX_STR}/.freeze
+    end
+
     def namespace_regex_message
       "can contain only letters, digits, '_', '-' and '.'. " \
       "Cannot start with '-' or end in '.', '.git' or '.atom'." \

spec/controllers/projects_controller_spec.rb

@@ -7,6 +7,26 @@ describe ProjectsController do
   let(:jpg)     { fixture_file_upload(Rails.root + 'spec/fixtures/rails_sample.jpg', 'image/jpg') }
   let(:txt)     { fixture_file_upload(Rails.root + 'spec/fixtures/doc_sample.txt', 'text/plain') }
+  describe 'GET index' do
+    context 'as a user' do
+      it 'redirects to root page' do
+        sign_in(user)
+
+        get :index
+
+        expect(response).to redirect_to(root_path)
+      end
+    end
+
+    context 'as a guest' do
+      it 'redirects to Explore page' do
+        get :index
+
+        expect(response).to redirect_to(explore_root_path)
+      end
+    end
+  end
+
   describe "GET show" do
     context "user not project member" do
       before { sign_in(user) }
@@ -285,6 +305,33 @@ describe ProjectsController do
     end
   end
+  describe 'PUT #new_issue_address' do
+    subject do
+      put :new_issue_address,
+        namespace_id: project.namespace.to_param,
+        id: project.to_param
+      user.reload
+    end
+
+    before do
+      sign_in(user)
+      project.team << [user, :developer]
+      allow(Gitlab.config.incoming_email).to receive(:enabled).and_return(true)
+    end
+
+    it 'has http status 200' do
+      expect(response).to have_http_status(200)
+    end
+
+    it 'changes the user incoming email token' do
+      expect { subject }.to change { user.incoming_email_token }
+    end
+
+    it 'changes projects new issue address' do
+      expect { subject }.to change { project.new_issue_address(user) }
+    end
+  end
+
   describe "POST #toggle_star" do
     it "toggles star if user is signed in" do
       sign_in(user)

spec/features/es_global_search_spec.rb

+require 'spec_helper'
+
+feature 'Global elastic search', feature: true do
+  let(:user) { create(:user) }
+  let(:project) { create(:project, namespace: user.namespace) }
+
+  before do
+    stub_application_setting(elasticsearch_search: true, elasticsearch_indexing: true)
+    Gitlab::Elastic::Helper.create_empty_index
+
+    project.team << [user, :master]
+    login_with(user)
+  end
+
+  after do
+    Gitlab::Elastic::Helper.delete_index
+    stub_application_setting(elasticsearch_search: false, elasticsearch_indexing: false)
+  end
+
+  describe 'I search through the issues and I see pagination' do
+    before do
+      create_list(:issue, 21, project: project, title: 'initial')
+
+      Gitlab::Elastic::Helper.refresh_index
+    end
+
+    it "has a pagination" do
+      visit dashboard_projects_path
+
+      fill_in "search", with: "initial"
+      click_button "Go"
+
+      select_filter("Issues")
+      expect(page).to have_selector('.gl-pagination .page', count: 2)
+    end
+  end
+
+  describe 'I search through the blobs' do
+    before do
+      project.repository.index_blobs
+
+      Gitlab::Elastic::Helper.refresh_index
+    end
+
+    it "finds files" do
+      visit dashboard_projects_path
+
+      fill_in "search", with: "def"
+      click_button "Go"
+
+      select_filter("Code")
+
+      expect(page).to have_selector('.file-content .code')
+    end
+  end
+
+  describe 'I search through the commits' do
+    before do
+      project.repository.index_commits
+      Gitlab::Elastic::Helper.refresh_index
+    end
+
+    it "finds commits" do
+      visit dashboard_projects_path
+
+      fill_in "search", with: "add"
+      click_button "Go"
+
+      select_filter("Commits")
+
+      expect(page).to have_selector('.commit-row-description')
+    end
+  end
+end

spec/features/global_search_spec.rb

 require 'spec_helper'
-feature 'Global elastic search', feature: true do
+feature 'Global search', feature: true do
   let(:user) { create(:user) }
   let(:project) { create(:project, namespace: user.namespace) }
   before do
-    stub_application_setting(elasticsearch_search: true, elasticsearch_indexing: true)
-    Gitlab::Elastic::Helper.create_empty_index
-
     project.team << [user, :master]
     login_with(user)
   end
-  after do
-    Gitlab::Elastic::Helper.delete_index
-    stub_application_setting(elasticsearch_search: false, elasticsearch_indexing: false)
-  end
-
   describe 'I search through the issues and I see pagination' do
     before do
-      create_list(:issue, 21, project: project, title: 'initial')
-
-      Gitlab::Elastic::Helper.refresh_index
+      allow_any_instance_of(Gitlab::SearchResults).to receive(:per_page).and_return(1)
+      create_list(:issue, 2, project: project, title: 'initial')
     end
     it "has a pagination" do
@@ -34,41 +25,4 @@ feature 'Global elastic search', feature: true do
       expect(page).to have_selector('.gl-pagination .page', count: 2)
     end
   end
-
-  describe 'I search through the blobs' do
-    before do
-      project.repository.index_blobs
-
-      Gitlab::Elastic::Helper.refresh_index
-    end
-
-    it "finds files" do
-      visit dashboard_projects_path
-
-      fill_in "search", with: "def"
-      click_button "Go"
-
-      select_filter("Code")
-
-      expect(page).to have_selector('.file-content .code')
-    end
-  end
-
-  describe 'I search through the commits' do
-    before do
-      project.repository.index_commits
-      Gitlab::Elastic::Helper.refresh_index
-    end
-
-    it "finds commits" do
-      visit dashboard_projects_path
-
-      fill_in "search", with: "add"
-      click_button "Go"
-
-      select_filter("Commits")
-
-      expect(page).to have_selector('.commit-row-description')
-    end
-  end
 end

spec/features/issues/new_branch_button_spec.rb

@@ -18,8 +18,8 @@ feature 'Start new branch from an issue', feature: true do
     end
     context "when there is a referenced merge request" do
-      let(:note) do
-        create(:note, :on_issue, :system, project: project,
+      let!(:note) do
+        create(:note, :on_issue, :system, project: project, noteable: issue,
                                           note: "Mentioned in !#{referenced_mr.iid}")
       end
       let(:referenced_mr) do
@@ -28,12 +28,13 @@ feature 'Start new branch from an issue', feature: true do
       end
       before do
-        issue.notes << note
+        referenced_mr.cache_merge_request_closes_issues!(user)
         visit namespace_project_issue_path(project.namespace, project, issue)
       end
       it "hides the new branch button", js: true do
+        expect(page).to have_css('#new-branch .unavailable')
         expect(page).not_to have_css('#new-branch .available')
         expect(page).to have_content /1 Related Merge Request/
       end