Merge branch 'master' into dev-master

* master: (98 commits)
  Enable Style/EmptyLines cop, remove redundant ones
  Update CHANGELOG
  Cache results from jQuery selectors to retrieve namespace name
  Fix import button when import fail due the namespace already been taken
  Fix snippets comments not displayed
  Fix emoji paths in relative root configurations
  Exclude requesters from Project#members, Group#members and User#members
  Upgrade Thin from 1.6.1 to 1.7.0.
  Many squashed commits
  Cache autocomplete results
  Upgrade Sidekiq from 4.1.2 to 4.1.4.
  Upgrade seed-fu from 2.3.5 to 2.3.6
  use has_many relationship with events
  Support creating a todo on issuables via API
  Expose target, filter by state as string
  Add todos API documentation and changelog
  Improve the request / withdraw access button
  Metrics for Rouge::Plugins::Redcarpet and Rouge::Formatters::HTMLGitlab
  Groundwork for Kerberos SPNEGO (EE feature)
  Update CHANGELOG 8.9.5 for runners related fixes
  ...

17 files changed, 159 insertions, 49 deletions

diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb
index d84f3e998f5..2ea1320267c 100644
--- a/spec/models/application_setting_spec.rb
+++ b/spec/models/application_setting_spec.rb
@@ -40,6 +40,16 @@ describe ApplicationSetting, models: true do
     it_behaves_like 'an object with email-formated attributes', :admin_notification_email do
       subject { setting }
     end
+
+    context 'repository storages inclussion' do
+      before do
+        storages = { 'custom' => 'tmp/tests/custom_repositories' }
+        allow(Gitlab.config.repositories).to receive(:storages).and_return(storages)
+      end
+
+      it { is_expected.to allow_value('custom').for(:repository_storage) }
+      it { is_expected.not_to allow_value('alternative').for(:repository_storage) }
+    end
   end
 
   context 'restricted signup domains' do
diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb
index 8154001cf46..e8171788872 100644
--- a/spec/models/build_spec.rb
+++ b/spec/models/build_spec.rb
@@ -323,7 +323,6 @@ describe Ci::Build, models: true do
         expect_any_instance_of(Ci::Runner).to receive(:can_pick?).and_return(false)
         is_expected.to be_falsey
       end
-
     end
   end
 
@@ -669,4 +668,22 @@ describe Ci::Build, models: true do
       expect(build.commit).to eq project.commit
     end
   end
+
+  describe '#retryable?' do
+    context 'when build is running' do
+      before { build.run! }
+
+      it 'should return false' do
+        expect(build.retryable?).to be false
+      end
+    end
+
+    context 'when build is finished' do
+      before { build.success! }
+
+      it 'should return true' do
+        expect(build.retryable?).to be true
+      end
+    end
+  end
 end
diff --git a/spec/models/concerns/access_requestable_spec.rb b/spec/models/concerns/access_requestable_spec.rb
index 98307876962..96eee0e8bdd 100644
--- a/spec/models/concerns/access_requestable_spec.rb
+++ b/spec/models/concerns/access_requestable_spec.rb
@@ -16,7 +16,7 @@ describe AccessRequestable do
 
       before { group.request_access(user) }
 
-      it { expect(group.members.request.exists?(user_id: user)).to be_truthy }
+      it { expect(group.requesters.exists?(user_id: user)).to be_truthy }
     end
   end
 
@@ -34,7 +34,7 @@ describe AccessRequestable do
 
       before { project.request_access(user) }
 
-      it { expect(project.members.request.exists?(user_id: user)).to be_truthy }
+      it { expect(project.requesters.exists?(user_id: user)).to be_truthy }
     end
   end
 end
diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb
index 89730ab8eb8..60e4bbc8564 100644
--- a/spec/models/concerns/issuable_spec.rb
+++ b/spec/models/concerns/issuable_spec.rb
@@ -170,7 +170,6 @@ describe Issue, "Issuable" do
     end
   end
 
-
   describe '#subscribed?' do
     context 'user is not a participant in the issue' do
       before { allow(issue).to receive(:participants).with(user).and_return([]) }
diff --git a/spec/models/concerns/strip_attribute_spec.rb b/spec/models/concerns/strip_attribute_spec.rb
index 6445e29c3ef..c3af7a0960f 100644
--- a/spec/models/concerns/strip_attribute_spec.rb
+++ b/spec/models/concerns/strip_attribute_spec.rb
@@ -16,5 +16,4 @@ describe Milestone, "StripAttribute" do
 
     it { expect(milestone.title).to eq('8.3') }
   end
-
 end
diff --git a/spec/models/email_spec.rb b/spec/models/email_spec.rb
index 5d0bd31db5a..d9df9e0f907 100644
--- a/spec/models/email_spec.rb
+++ b/spec/models/email_spec.rb
@@ -1,11 +1,9 @@
 require 'spec_helper'
 
 describe Email, models: true do
-
   describe 'validations' do
     it_behaves_like 'an object with email-formated attributes', :email do
       subject { build(:email) }
     end
   end
-
 end
diff --git a/spec/models/forked_project_link_spec.rb b/spec/models/forked_project_link_spec.rb
index 3b817608ce0..fa1a0d4e0c7 100644
--- a/spec/models/forked_project_link_spec.rb
+++ b/spec/models/forked_project_link_spec.rb
@@ -23,14 +23,12 @@ describe :forked_from_project do
   let(:project_from) { create(:project) }
   let(:project_to) { create(:project, forked_project_link: forked_project_link) }
 
-
   before :each do
     forked_project_link.forked_from_project = project_from
     forked_project_link.forked_to_project = project_to
     forked_project_link.save!
   end
 
-
   it "project_to should know it is forked" do
     expect(project_to.forked?).to be_truthy
   end
@@ -43,7 +41,6 @@ describe :forked_from_project do
     expect(forked_project_link).to receive(:destroy)
     project_to.destroy
   end
-
 end
 
 def fork_project(from_project, user)
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index 2c19aa3f67f..a878ff1b227 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -7,9 +7,38 @@ describe Group, models: true do
     it { is_expected.to have_many :projects }
     it { is_expected.to have_many(:group_members).dependent(:destroy) }
     it { is_expected.to have_many(:users).through(:group_members) }
+    it { is_expected.to have_many(:owners).through(:group_members) }
+    it { is_expected.to have_many(:requesters).dependent(:destroy) }
     it { is_expected.to have_many(:project_group_links).dependent(:destroy) }
     it { is_expected.to have_many(:shared_projects).through(:project_group_links) }
     it { is_expected.to have_many(:notification_settings).dependent(:destroy) }
+
+    describe '#members & #requesters' do
+      let(:requester) { create(:user) }
+      let(:developer) { create(:user) }
+      before do
+        group.request_access(requester)
+        group.add_developer(developer)
+      end
+
+      describe '#members' do
+        it 'includes members and exclude requesters' do
+          member_user_ids = group.members.pluck(:user_id)
+
+          expect(member_user_ids).to include(developer.id)
+          expect(member_user_ids).not_to include(requester.id)
+        end
+      end
+
+      describe '#requesters' do
+        it 'does not include requesters' do
+          requester_user_ids = group.requesters.pluck(:user_id)
+
+          expect(requester_user_ids).to include(requester.id)
+          expect(requester_user_ids).not_to include(developer.id)
+        end
+      end
+    end
   end
 
   describe 'modules' do
diff --git a/spec/models/identity_spec.rb b/spec/models/identity_spec.rb
index 1b987588f59..b3aed66a5b6 100644
--- a/spec/models/identity_spec.rb
+++ b/spec/models/identity_spec.rb
@@ -1,7 +1,6 @@
 require 'spec_helper'
 
 RSpec.describe Identity, models: true do
-
   describe 'relations' do
     it { is_expected.to belong_to(:user) }
   end
diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb
index e9134a3d283..40181a8b906 100644
--- a/spec/models/member_spec.rb
+++ b/spec/models/member_spec.rb
@@ -73,10 +73,10 @@ describe Member, models: true do
       @accepted_invite_member = project.members.invite.find_by_invite_email('toto2@example.com').tap { |u| u.accept_invite!(accepted_invite_user) }
 
       requested_user = create(:user).tap { |u| project.request_access(u) }
-      @requested_member = project.members.request.find_by(user_id: requested_user.id)
+      @requested_member = project.requesters.find_by(user_id: requested_user.id)
 
       accepted_request_user = create(:user).tap { |u| project.request_access(u) }
-      @accepted_request_member = project.members.request.find_by(user_id: accepted_request_user.id).tap { |m| m.accept_request }
+      @accepted_request_member = project.requesters.find_by(user_id: accepted_request_user.id).tap { |m| m.accept_request }
     end
 
     describe '.invite' do
@@ -103,22 +103,6 @@ describe Member, models: true do
       it { expect(described_class.request).not_to include @accepted_request_member }
     end
 
-    describe '.non_request' do
-      it { expect(described_class.non_request).to include @master }
-      it { expect(described_class.non_request).to include @invited_member }
-      it { expect(described_class.non_request).to include @accepted_invite_member }
-      it { expect(described_class.non_request).not_to include @requested_member }
-      it { expect(described_class.non_request).to include @accepted_request_member }
-    end
-
-    describe '.non_pending' do
-      it { expect(described_class.non_pending).to include @master }
-      it { expect(described_class.non_pending).not_to include @invited_member }
-      it { expect(described_class.non_pending).to include @accepted_invite_member }
-      it { expect(described_class.non_pending).not_to include @requested_member }
-      it { expect(described_class.non_pending).to include @accepted_request_member }
-    end
-
     describe '.owners_and_masters' do
       it { expect(described_class.owners_and_masters).to include @owner }
       it { expect(described_class.owners_and_masters).to include @master }
diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb
index bbf65edb27c..4c103462433 100644
--- a/spec/models/members/project_member_spec.rb
+++ b/spec/models/members/project_member_spec.rb
@@ -119,7 +119,6 @@ describe ProjectMember, models: true do
     it { expect(@project_1.users).to include(@user_1) }
     it { expect(@project_1.users).to include(@user_2) }
 
-
     it { expect(@project_2.users).to include(@user_1) }
     it { expect(@project_2.users).to include(@user_2) }
   end
diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb
index 4e68ac5e63a..5f68cd2b066 100644
--- a/spec/models/namespace_spec.rb
+++ b/spec/models/namespace_spec.rb
@@ -57,6 +57,7 @@ describe Namespace, models: true do
   describe :move_dir do
     before do
       @namespace = create :namespace
+      @project = create :project, namespace: @namespace
       allow(@namespace).to receive(:path_changed?).and_return(true)
     end
 
@@ -87,8 +88,13 @@ describe Namespace, models: true do
   end
 
   describe :rm_dir do
-    it "should remove dir" do
-      expect(namespace.rm_dir).to be_truthy
+    let!(:project) { create(:project, namespace: namespace) }
+    let!(:path) { File.join(Gitlab.config.repositories.storages.default, namespace.path) }
+
+    before { namespace.destroy }
+
+    it "should remove its dirs when deleted" do
+      expect(File.exist?(path)).to be(false)
     end
   end
 
@@ -103,7 +109,6 @@ describe Namespace, models: true do
   end
 
   describe ".clean_path" do
-
     let!(:user)       { create(:user, username: "johngitlab-etc") }
     let!(:namespace)  { create(:namespace, path: "JohnGitLab-etc1") }
 
diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb
index c9517324541..5a97cf370da 100644
--- a/spec/models/project_services/jira_service_spec.rb
+++ b/spec/models/project_services/jira_service_spec.rb
@@ -154,11 +154,9 @@ describe JiraService, models: true do
         expect(@jira_service.password).to eq("password")
         expect(@jira_service.api_url).to eq("http://jira_edited.example.com/rest/api/2")
       end
-
     end
   end
 
-
   describe "Validations" do
     context "active" do
       before do
diff --git a/spec/models/project_services/slack_service/wiki_page_message_spec.rb b/spec/models/project_services/slack_service/wiki_page_message_spec.rb
index 6ecab645b49..46dedb66c7c 100644
--- a/spec/models/project_services/slack_service/wiki_page_message_spec.rb
+++ b/spec/models/project_services/slack_service/wiki_page_message_spec.rb
@@ -47,7 +47,6 @@ describe SlackService::WikiPageMessage, models: true do
     context 'when :action == "create"' do
       before { args[:object_attributes][:action] = 'create' }
 
-
       it 'it returns the attachment for a new wiki page' do
         expect(subject.attachments).to eq([
           {
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index d305cd9ff1e..a8c777d1e3e 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -11,6 +11,8 @@ describe Project, models: true do
     it { is_expected.to have_many(:issues).dependent(:destroy) }
     it { is_expected.to have_many(:milestones).dependent(:destroy) }
     it { is_expected.to have_many(:project_members).dependent(:destroy) }
+    it { is_expected.to have_many(:users).through(:project_members) }
+    it { is_expected.to have_many(:requesters).dependent(:destroy) }
     it { is_expected.to have_many(:notes).dependent(:destroy) }
     it { is_expected.to have_many(:snippets).class_name('ProjectSnippet').dependent(:destroy) }
     it { is_expected.to have_many(:deploy_keys_projects).dependent(:destroy) }
@@ -31,6 +33,34 @@ describe Project, models: true do
     it { is_expected.to have_many(:environments).dependent(:destroy) }
     it { is_expected.to have_many(:deployments).dependent(:destroy) }
     it { is_expected.to have_many(:todos).dependent(:destroy) }
+
+    describe '#members & #requesters' do
+      let(:project) { create(:project) }
+      let(:requester) { create(:user) }
+      let(:developer) { create(:user) }
+      before do
+        project.request_access(requester)
+        project.team << [developer, :developer]
+      end
+
+      describe '#members' do
+        it 'includes members and exclude requesters' do
+          member_user_ids = project.members.pluck(:user_id)
+
+          expect(member_user_ids).to include(developer.id)
+          expect(member_user_ids).not_to include(requester.id)
+        end
+      end
+
+      describe '#requesters' do
+        it 'does not include requesters' do
+          requester_user_ids = project.requesters.pluck(:user_id)
+
+          expect(requester_user_ids).to include(requester.id)
+          expect(requester_user_ids).not_to include(developer.id)
+        end
+      end
+    end
   end
 
   describe 'modules' do
@@ -56,6 +86,7 @@ describe Project, models: true do
     it { is_expected.to validate_length_of(:description).is_within(0..2000) }
     it { is_expected.to validate_presence_of(:creator) }
     it { is_expected.to validate_presence_of(:namespace) }
+    it { is_expected.to validate_presence_of(:repository_storage) }
 
     it 'should not allow new projects beyond user limits' do
       project2 = build(:project)
@@ -84,6 +115,20 @@ describe Project, models: true do
         end
       end
     end
+
+    context 'repository storages inclussion' do
+      let(:project2) { build(:project, repository_storage: 'missing') }
+
+      before do
+        storages = { 'custom' => 'tmp/tests/custom_repositories' }
+        allow(Gitlab.config.repositories).to receive(:storages).and_return(storages)
+      end
+
+      it "should not allow repository storages that don't match a label in the configuration" do
+        expect(project2).not_to be_valid
+        expect(project2.errors[:repository_storage].first).to match(/is not included in the list/)
+      end
+    end
   end
 
   describe 'default_scope' do
@@ -131,6 +176,24 @@ describe Project, models: true do
     end
   end
 
+  describe '#repository_storage_path' do
+    let(:project) { create(:project, repository_storage: 'custom') }
+
+    before do
+      FileUtils.mkdir('tmp/tests/custom_repositories')
+      storages = { 'custom' => 'tmp/tests/custom_repositories' }
+      allow(Gitlab.config.repositories).to receive(:storages).and_return(storages)
+    end
+
+    after do
+      FileUtils.rm_rf('tmp/tests/custom_repositories')
+    end
+
+    it 'returns the repository storage path' do
+      expect(project.repository_storage_path).to eq('tmp/tests/custom_repositories')
+    end
+  end
+
   it 'should return valid url to repo' do
     project = Project.new(path: 'somewhere')
     expect(project.url_to_repo).to eq(Gitlab.config.gitlab_shell.ssh_path_prefix + 'somewhere.git')
@@ -574,6 +637,21 @@ describe Project, models: true do
     end
   end
 
+  context 'repository storage by default' do
+    let(:project) { create(:empty_project) }
+
+    subject { project.repository_storage }
+
+    before do
+      storages = { 'alternative_storage' => '/some/path' }
+      allow(Gitlab.config.repositories).to receive(:storages).and_return(storages)
+      stub_application_setting(repository_storage: 'alternative_storage')
+      allow_any_instance_of(Project).to receive(:ensure_dir_exist).and_return(true)
+    end
+
+    it { is_expected.to eq('alternative_storage') }
+  end
+
   context 'shared runners by default' do
     let(:project) { create(:empty_project) }
 
@@ -729,12 +807,12 @@ describe Project, models: true do
 
       expect(gitlab_shell).to receive(:mv_repository).
         ordered.
-        with("#{ns}/foo", "#{ns}/#{project.path}").
+        with(project.repository_storage_path, "#{ns}/foo", "#{ns}/#{project.path}").
         and_return(true)
 
       expect(gitlab_shell).to receive(:mv_repository).
         ordered.
-        with("#{ns}/foo.wiki", "#{ns}/#{project.path}.wiki").
+        with(project.repository_storage_path, "#{ns}/foo.wiki", "#{ns}/#{project.path}.wiki").
         and_return(true)
 
       expect_any_instance_of(SystemHooksService).
@@ -826,7 +904,7 @@ describe Project, models: true do
     context 'using a regular repository' do
       it 'creates the repository' do
         expect(shell).to receive(:add_repository).
-          with(project.path_with_namespace).
+          with(project.repository_storage_path, project.path_with_namespace).
           and_return(true)
 
         expect(project.repository).to receive(:after_create)
@@ -836,7 +914,7 @@ describe Project, models: true do
 
       it 'adds an error if the repository could not be created' do
         expect(shell).to receive(:add_repository).
-          with(project.path_with_namespace).
+          with(project.repository_storage_path, project.path_with_namespace).
           and_return(false)
 
         expect(project.repository).not_to receive(:after_create)
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index d8350000bf6..851b8b241d7 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -531,8 +531,6 @@ describe Repository, models: true do
   describe '#expire_cache' do
     it 'expires all caches' do
       expect(repository).to receive(:expire_branch_cache)
-      expect(repository).to receive(:expire_branch_count_cache)
-      expect(repository).to receive(:expire_tag_count_cache)
 
       repository.expire_cache
     end
@@ -857,7 +855,6 @@ describe Repository, models: true do
 
       repository.after_create
     end
-
   end
 
   describe "#copy_gitattributes" do
@@ -1055,12 +1052,14 @@ describe Repository, models: true do
     let(:cache) { repository.send(:cache) }
 
     it 'builds the caches if they do not already exist' do
+      cache_keys = repository.cache_keys + repository.cache_keys_for_branches_and_tags
+
       expect(cache).to receive(:exist?).
-        exactly(repository.cache_keys.length).
+        exactly(cache_keys.length).
         times.
         and_return(false)
 
-      repository.cache_keys.each do |key|
+      cache_keys.each do |key|
         expect(repository).to receive(key)
       end
 
@@ -1068,12 +1067,14 @@ describe Repository, models: true do
     end
 
     it 'does not build any caches that already exist' do
+      cache_keys = repository.cache_keys + repository.cache_keys_for_branches_and_tags
+
       expect(cache).to receive(:exist?).
-        exactly(repository.cache_keys.length).
+        exactly(cache_keys.length).
         times.
         and_return(true)
 
-      repository.cache_keys.each do |key|
+      cache_keys.each do |key|
         expect(repository).not_to receive(key)
       end
 
diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb
index 2f000dbc01a..96bbbec9ea1 100644
--- a/spec/models/service_spec.rb
+++ b/spec/models/service_spec.rb
@@ -1,7 +1,6 @@
 require 'spec_helper'
 
 describe Service, models: true do
-
   describe "Associations" do
     it { is_expected.to belong_to :project }
     it { is_expected.to have_one :service_hook }
@@ -176,7 +175,6 @@ describe Service, models: true do
       )
     end
 
-
     it "returns nil when the property has not been assigned a new value" do
       service.username = "key_changed"
       expect(service.bamboo_url_was).to be_nil