Merge branch 'master' of gitlab.com:gitlab-org/gitlab-ee into ce_upstream

CHANGELOG-EE

 Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.11.0 (unreleased)
+  - Allow projects to be moved between repository storages
+  - Add rake task to remove old repository copies from repositories moved to another storage
   - Performance improvement of push rules
   - Change LdapGroupSyncWorker to use new LDAP group sync classes
   - [Elastic][Fix] Commit search breaks for some URLs on gitlab-ce project

GITLAB_SHELL_VERSION

-3.2.1
+3.3.0

app/models/ability.rb

@@ -184,6 +184,8 @@ class Ability
         # Push abilities on the users team role
         rules.push(*project_team_rules(project.team, user))
 
+        rules << :change_repository_storage if user.admin?
+
         owner = user.admin? ||
                 project.owner == user ||
                 (project.group && project.group.has_owner?(user))

app/models/project.rb

@@ -1491,6 +1491,16 @@ class Project < ActiveRecord::Base
     end
   end
 
+  def change_repository_storage(new_repository_storage_key)
+    return if repository_read_only?
+    return if repository_storage == new_repository_storage_key
+
+    raise ArgumentError unless Gitlab.config.repositories.storages.keys.include?(new_repository_storage_key)
+
+    run_after_commit { ProjectUpdateRepositoryStorageWorker.perform_async(id, new_repository_storage_key) }
+    self.repository_read_only = true
+  end
+
   private
 
   def default_branch_protected?

app/services/projects/update_repository_storage_service.rb

+module Projects
+  class UpdateRepositoryStorageService < BaseService
+    include Gitlab::ShellAdapter
+
+    def initialize(project)
+      @project = project
+    end
+
+    def execute(new_repository_storage_key)
+      new_storage_path = Gitlab.config.repositories.storages[new_repository_storage_key]
+      result = move_storage(project.path_with_namespace, new_storage_path)
+
+      if project.wiki.repository_exists?
+        result &&= move_storage("#{project.path_with_namespace}.wiki", new_storage_path)
+      end
+
+      if result
+        mark_old_paths_for_archive
+
+        project.update(repository_storage: new_repository_storage_key, repository_read_only: false)
+      else
+        project.update(repository_read_only: false)
+      end
+    end
+
+    private
+
+    def move_storage(project_path, new_storage_path)
+      gitlab_shell.mv_storage(project.repository_storage_path, project_path, new_storage_path)
+    end
+
+    def mark_old_paths_for_archive
+      old_repository_storage_path = project.repository_storage_path
+      new_project_path = moved_path(project.path_with_namespace)
+
+      # Notice that the block passed to `run_after_commit` will run with `project`
+      # as its context
+      project.run_after_commit do
+        GitlabShellWorker.perform_async(:mv_repository,
+                                        old_repository_storage_path,
+                                        path_with_namespace,
+                                        new_project_path)
+
+        if wiki.repository_exists?
+          GitlabShellWorker.perform_async(:mv_repository,
+                                          old_repository_storage_path,
+                                          "#{path_with_namespace}.wiki",
+                                          "#{new_project_path}.wiki")
+        end
+      end
+    end
+
+    def moved_path(path)
+      "#{path}+#{project.id}+moved+#{Time.now.to_i}"
+    end
+  end
+end

app/services/projects/update_service.rb

@@ -13,13 +13,20 @@ module Projects
         end
       end
 
-      new_branch = params[:default_branch]
+      new_branch = params.delete(:default_branch)
+      new_repository_storage = params.delete(:repository_storage)
 
-      if project.repository.exists? && new_branch && new_branch != project.default_branch
+      if project.repository.exists?
+        if new_branch && new_branch != project.default_branch
           project.change_head(new_branch)
         end
 
-      if project.update_attributes(params.except(:default_branch))
+        if new_repository_storage && can?(current_user, :change_repository_storage, project)
+          project.change_repository_storage(new_repository_storage)
+        end
+      end
+
+      if project.update_attributes(params)
         if project.previous_changes.include?('path')
           project.rename_repo
         end

app/workers/project_update_repository_storage_worker.rb

+class ProjectUpdateRepositoryStorageWorker
+  include Sidekiq::Worker
+
+  sidekiq_options queue: :default
+
+  def perform(project_id, new_repository_storage_key)
+    project = Project.find(project_id)
+
+    ::Projects::UpdateRepositoryStorageService.new(project).execute(new_repository_storage_key)
+  end
+end

db/migrate/20160713200638_add_repository_read_only_to_projects.rb

+class AddRepositoryReadOnlyToProjects < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    add_column :projects, :repository_read_only, :boolean
+  end
+end

doc/api/projects.md

@@ -280,7 +280,8 @@ Parameters:
       "group_name": "Gitlab Org",
       "group_access_level": 10
     }
-  ]
+  ],
+  "repository_storage": "default"
 }
 ```
 
@@ -448,6 +449,7 @@ Parameters:
 - `visibility_level` (optional)
 - `import_url` (optional)
 - `public_builds` (optional)
+- `repository_storage` (optional, available only for admins)
 
 ### Create project for user
 
@@ -473,6 +475,7 @@ Parameters:
 - `visibility_level` (optional)
 - `import_url` (optional)
 - `public_builds` (optional)
+- `repository_storage` (optional, available only for admins)
 
 ### Edit project
 
@@ -499,6 +502,7 @@ Parameters:
 - `public` (optional) - if `true` same as setting visibility_level = 20
 - `visibility_level` (optional)
 - `public_builds` (optional)
+- `repository_storage` (optional, available only for admins)
 
 On success, method returns 200 with the updated project. If parameters are
 invalid, 400 is returned.

doc/raketasks/cleanup.md

@@ -22,3 +22,13 @@ sudo gitlab-rake gitlab:cleanup:repos
 # installation from source
 bundle exec rake gitlab:cleanup:repos RAILS_ENV=production
 ```
+
+Remove old repository copies from repositories moved to another storage.
+
+```
+# omnibus-gitlab
+sudo gitlab-rake gitlab:cleanup:moved
+
+# installation from source
+bundle exec rake gitlab:cleanup:moved RAILS_ENV=production
+```

lib/api/entities.rb

@@ -94,6 +94,7 @@ module API
       expose :shared_with_groups do |project, options|
         SharedGroup.represent(project.project_group_links.all, options)
       end
+      expose :repository_storage, if: lambda { |_project, options| options[:user].try(:admin?) }
     end
 
     class Member < UserBasic

lib/api/projects.rb

@@ -105,6 +105,7 @@ module API
       #   visibility_level (optional) - 0 by default
       #   import_url (optional)
       #   public_builds (optional)
+      #   repository_storage (optional)
       # Example Request
       #   POST /projects
       post do
@@ -123,7 +124,8 @@ module API
                                      :public,
                                      :visibility_level,
                                      :import_url,
-                                     :public_builds]
+                                     :public_builds,
+                                     :repository_storage]
         attrs = map_public_to_visibility_level(attrs)
         @project = ::Projects::CreateService.new(current_user, attrs).execute
         if @project.saved?
@@ -155,6 +157,7 @@ module API
       #   visibility_level (optional)
       #   import_url (optional)
       #   public_builds (optional)
+      #   repository_storage (optional)
       # Example Request
       #   POST /projects/user/:user_id
       post "user/:user_id" do
@@ -172,7 +175,8 @@ module API
                                      :public,
                                      :visibility_level,
                                      :import_url,
-                                     :public_builds]
+                                     :public_builds,
+                                     :repository_storage]
         attrs = map_public_to_visibility_level(attrs)
         @project = ::Projects::CreateService.new(user, attrs).execute
         if @project.saved?
@@ -218,6 +222,7 @@ module API
       #   public (optional) - if true same as setting visibility_level = 20
       #   visibility_level (optional) - visibility level of a project
       #   public_builds (optional)
+      #   repository_storage (optional)
       # Example Request
       #   PUT /projects/:id
       put ':id' do
@@ -234,7 +239,8 @@ module API
                                      :shared_runners_enabled,
                                      :public,
                                      :visibility_level,
-                                     :public_builds]
+                                     :public_builds,
+                                     :repository_storage]
         attrs = map_public_to_visibility_level(attrs)
         authorize_admin_project
         authorize! :rename_project, user_project if attrs[:name].present?

lib/gitlab/backend/shell.rb

@@ -106,6 +106,20 @@ module Gitlab
                                    storage, "#{path}.git", "#{new_path}.git"])
     end
 
+    # Move repository storage
+    #
+    # current_storage - project's current storage path
+    # path - project path with namespace
+    # new_storage - new storage path
+    #
+    # Ex.
+    #   mv_storage("/path/to/storage", "randx/gitlab-ci", "/new/storage/path")
+    #
+    def mv_storage(current_storage, path, new_storage)
+      Gitlab::Utils.system_silent([gitlab_shell_projects_path, 'mv-storage',
+                                   current_storage, "#{path}.git", new_storage])
+    end
+
     # Fork repository to new namespace
     # forked_from_storage - forked-from project's storage path
     # path - project path with namespace

lib/gitlab/git_access.rb

@@ -59,6 +59,10 @@ module Gitlab
     end
 
     def push_access_check(changes)
+      if project.repository_read_only?
+        return build_status_object(false, 'The repository is temporarily read-only. Please try again later.')
+      end
+
       if Gitlab::Geo.secondary?
         return build_status_object(false, "You can't push code on a secondary GitLab Geo node.")
       end

lib/tasks/gitlab/cleanup.rake

@@ -27,7 +27,7 @@ namespace :gitlab do
         all_dirs.each do |dir_path|
 
           if remove_flag
-            if FileUtils.rm_rf dir_path
+            if FileUtils.rm_rf(dir_path)
               puts "Removed...#{dir_path}".color(:red)
             else
               puts "Cannot remove #{dir_path}".color(:red)
@@ -43,6 +43,34 @@ namespace :gitlab do
       end
     end
 
+    desc "GitLab | Cleanup | Delete moved repositories"
+    task moved: :environment  do
+      warn_user_is_not_gitlab
+      remove_flag = ENV['REMOVE']
+
+      Gitlab.config.repositories.storages.each do |name, repo_root|
+        # Look for global repos (legacy, depth 1) and normal repos (depth 2)
+        IO.popen(%W(find #{repo_root.chomp('/')} -mindepth 1 -maxdepth 2 -name *+moved*.git)) do |find|
+          find.each_line do |path|
+            path.chomp!
+            if remove_flag
+              if FileUtils.rm_rf(path)
+                puts "Removed...#{path}".color(:green)
+              else
+                puts "Cannot remove #{path}".color(:red)
+              end
+            else
+              puts "Can be removed: #{path}".color(:green)
+            end
+          end
+        end
+      end
+
+      unless remove_flag
+        puts "To cleanup these repositories run this command with REMOVE=true".color(:yellow)
+      end
+    end
+
     desc "GitLab | Cleanup | Clean repositories"
     task repos: :environment  do
       warn_user_is_not_gitlab

spec/factories/projects.rb

@@ -44,6 +44,10 @@ FactoryGirl.define do
         project.create_repository
       end
     end
+
+    trait :read_only_repository do
+      repository_read_only true
+    end
   end
 
   # Project with empty repository

spec/features/issues/issue_sidebar_spec.rb

@@ -79,7 +79,7 @@ feature 'Issue Sidebar', feature: true do
       visit_issue(project, issue)
     end
 
-    it 'should update weight in sidebar to 1' do
+    it 'updates weight in sidebar to 1' do
       page.within '.weight' do
         click_link 'Edit'
         click_link '1'
@@ -90,7 +90,7 @@ feature 'Issue Sidebar', feature: true do
       end
     end
 
-    it 'should update weight in sidebar to no weight' do
+    it 'updates weight in sidebar to no weight' do
       page.within '.weight' do
         click_link 'Edit'
         click_link 'No Weight'

spec/features/issues_spec.rb

@@ -443,7 +443,7 @@ describe 'Issues', feature: true do
       visit namespace_project_issue_path(project.namespace, project, issue)
     end
 
-    it 'should allow user to update to a weight' do
+    it 'allows user to update to a weight' do
       page.within('.weight') do
         expect(page).to have_content "None"
         click_link 'Edit'

spec/features/notes_on_merge_requests_spec.rb

@@ -91,7 +91,7 @@ describe 'Comments', feature: true do
         end
 
         # TODO: fix after 7.7 release
-        # it "should reset the edit note form textarea with the original content of the note if cancelled" do
+        # it "resets the edit note form textarea with the original content of the note if cancelled" do
         #   within(".current-note-edit-form") do
         #     fill_in "note[note]", with: "Some new content"
         #     find(".btn-cancel").click
@@ -184,7 +184,7 @@ describe 'Comments', feature: true do
       end
 
       describe 'the note form' do
-        it "shouldn't add a second form for same row" do
+        it "does not add a second form for same row" do
           click_diff_line
 
           is_expected.
@@ -192,7 +192,7 @@ describe 'Comments', feature: true do
                         count: 1)
         end
 
-        it 'should be removed when canceled' do
+        it 'is removed when canceled' do
           is_expected.to have_css('.js-temp-notes-holder')
 
           page.within("form[data-line-code='#{line_code}']") do

spec/finders/merge_requests_finder_spec.rb

@@ -30,7 +30,7 @@ describe MergeRequestsFinder do
       expect(merge_requests.size).to eq(1)
     end
 
-    it 'should ignore sorting by weight' do
+    it 'ignores sorting by weight' do
       params = { project_id: project1.id, scope: 'authored', state: 'opened', weight: Issue::WEIGHT_ANY }
       merge_requests = MergeRequestsFinder.new(user, params).execute
       expect(merge_requests.size).to eq(1)

spec/helpers/diff_helper_spec.rb

@@ -91,7 +91,7 @@ describe DiffHelper do
     let(:new_pos) { 50 }
     let(:text) { 'some_text' }
 
-    it "should generate foldable top match line for inline view with empty text by default" do
+    it "generates foldable top match line for inline view with empty text by default" do
       output = diff_match_line old_pos, new_pos
 
       expect(output).to be_html_safe
@@ -100,7 +100,7 @@ describe DiffHelper do
       expect(output).to have_css 'td:nth-child(3):not(.parallel).line_content.match', text: ''
     end
 
-    it "should allow to define text and bottom option" do
+    it "allows to define text and bottom option" do
       output = diff_match_line old_pos, new_pos, text: text, bottom: true
 
       expect(output).to be_html_safe
@@ -109,7 +109,7 @@ describe DiffHelper do
       expect(output).to have_css 'td:nth-child(3):not(.parallel).line_content.match', text: text
     end
 
-    it "should generate match line for parallel view" do
+    it "generates match line for parallel view" do
       output = diff_match_line old_pos, new_pos, text: text, view: :parallel
 
       expect(output).to be_html_safe
@@ -119,7 +119,7 @@ describe DiffHelper do
       expect(output).to have_css 'td:nth-child(4).line_content.match.parallel', text: text
     end
 
-    it "should allow to generate only left match line for parallel view" do
+    it "allows to generate only left match line for parallel view" do
       output = diff_match_line old_pos, nil, text: text, view: :parallel
 
       expect(output).to be_html_safe
@@ -128,7 +128,7 @@ describe DiffHelper do
       expect(output).not_to have_css 'td:nth-child(3)'
     end
 
-    it "should allow to generate only right match line for parallel view" do
+    it "allows to generate only right match line for parallel view" do
       output = diff_match_line nil, new_pos, text: text, view: :parallel
 
       expect(output).to be_html_safe

spec/javascripts/awards_handler_spec.js

@@ -111,7 +111,7 @@
       });
     });
     describe('::getAwardUrl', function() {
-      return it('should return the url for request', function() {
+      return it('returns the url for request', function() {
         return expect(awardsHandler.getAwardUrl()).toBe('/gitlab-org/gitlab-test/issues/8/toggle_award_emoji');
       });
     });

spec/javascripts/datetime_utility_spec.js.coffee

@@ -2,30 +2,30 @@
 
 describe 'Date time utils', ->
   describe 'get day name', ->
-    it 'should return Sunday', ->
+    it 'returns Sunday', ->
       day = gl.utils.getDayName(new Date('07/17/2016'))
       expect(day).toBe('Sunday')
 
-    it 'should return Monday', ->
+    it 'returns Monday', ->
       day = gl.utils.getDayName(new Date('07/18/2016'))
       expect(day).toBe('Monday')
 
-    it 'should return Tuesday', ->
+    it 'returns Tuesday', ->
       day = gl.utils.getDayName(new Date('07/19/2016'))
       expect(day).toBe('Tuesday')
 
-    it 'should return Wednesday', ->
+    it 'returns Wednesday', ->
       day = gl.utils.getDayName(new Date('07/20/2016'))
       expect(day).toBe('Wednesday')
 
-    it 'should return Thursday', ->
+    it 'returns Thursday', ->
       day = gl.utils.getDayName(new Date('07/21/2016'))
       expect(day).toBe('Thursday')
 
-    it 'should return Friday', ->
+    it 'returns Friday', ->
       day = gl.utils.getDayName(new Date('07/22/2016'))
       expect(day).toBe('Friday')
 
-    it 'should return Saturday', ->
+    it 'returns Saturday', ->
       day = gl.utils.getDayName(new Date('07/23/2016'))
       expect(day).toBe('Saturday')

spec/lib/ee/gitlab/ldap/sync/groups_spec.rb

@@ -37,7 +37,7 @@ describe EE::Gitlab::LDAP::Sync::Groups, lib: true do
       context 'and admin_group and external_groups are not present' do
         before { stub_ldap_config(group_base: 'dc=example,dc=com') }
 
-        it 'should call EE::Gitlab::LDAP::Sync::Group#execute' do
+        it 'calls EE::Gitlab::LDAP::Sync::Group#execute' do
           expect(EE::Gitlab::LDAP::Sync::Group).to receive(:execute).twice
         end
 
@@ -58,7 +58,7 @@ describe EE::Gitlab::LDAP::Sync::Groups, lib: true do
           )
         end
 
-        it 'should call EE::Gitlab::LDAP::Sync::Group#execute' do
+        it 'calls EE::Gitlab::LDAP::Sync::Group#execute' do
           expect(EE::Gitlab::LDAP::Sync::Group).to receive(:execute).twice
         end
 
@@ -79,7 +79,7 @@ describe EE::Gitlab::LDAP::Sync::Groups, lib: true do
           )
         end
 
-        it 'should call EE::Gitlab::LDAP::Sync::Group#execute' do
+        it 'calls EE::Gitlab::LDAP::Sync::Group#execute' do
           expect(EE::Gitlab::LDAP::Sync::Group).to receive(:execute).twice
         end
 

spec/lib/gitlab/backend/shell_spec.rb

@@ -44,6 +44,30 @@ describe Gitlab::Shell, lib: true do
     end
   end
 
+  describe 'projects commands' do
+    let(:projects_path) { 'tmp/tests/shell-projects-test/bin/gitlab-projects' }
+
+    before do
+      allow(Gitlab.config.gitlab_shell).to receive(:path).and_return('tmp/tests/shell-projects-test')
+    end
+
+    describe 'mv_repository' do
+      it 'executes the command' do
+        expect(Gitlab::Utils).to receive(:system_silent).
+          with([projects_path, 'mv-project', 'storage/path', 'project/path.git', 'new/path.git'])
+        gitlab_shell.mv_repository('storage/path', 'project/path', 'new/path')
+      end
+    end
+
+    describe 'mv_storage' do
+      it 'executes the command' do
+        expect(Gitlab::Utils).to receive(:system_silent).
+          with([projects_path, 'mv-storage', 'current/storage', 'project/path.git', 'new/storage'])
+        gitlab_shell.mv_storage('current/storage', 'project/path', 'new/storage')
+      end
+    end
+  end
+
   describe Gitlab::Shell::KeyAdder, lib: true do
     describe '#add_key' do
       it 'normalizes space characters in the key' do

spec/lib/gitlab/diff/highlight_spec.rb

@@ -12,11 +12,11 @@ describe Gitlab::Diff::Highlight, lib: true do
     context "with a diff file" do
       let(:subject) { Gitlab::Diff::Highlight.new(diff_file, repository: project.repository).highlight }
 
-      it 'should return Gitlab::Diff::Line elements' do
+      it 'returns Gitlab::Diff::Line elements' do
         expect(subject.first).to be_an_instance_of(Gitlab::Diff::Line)
       end
 
-      it 'should not modify "match" lines' do
+      it 'does not modify "match" lines' do
         expect(subject[0].text).to eq('@@ -6,12 +6,18 @@ module Popen')
         expect(subject[22].text).to eq('@@ -19,6 +25,7 @@ module Popen')
       end
@@ -43,11 +43,11 @@ describe Gitlab::Diff::Highlight, lib: true do
     context "with diff lines" do
       let(:subject) { Gitlab::Diff::Highlight.new(diff_file.diff_lines, repository: project.repository).highlight }
 
-      it 'should return Gitlab::Diff::Line elements' do
+      it 'returns Gitlab::Diff::Line elements' do
         expect(subject.first).to be_an_instance_of(Gitlab::Diff::Line)
       end
 
-      it 'should not modify "match" lines' do
+      it 'does not modify "match" lines' do
         expect(subject[0].text).to eq('@@ -6,12 +6,18 @@ module Popen')
         expect(subject[22].text).to eq('@@ -19,6 +25,7 @@ module Popen')
       end

spec/lib/gitlab/diff/parallel_diff_spec.rb

@@ -12,7 +12,7 @@ describe Gitlab::Diff::ParallelDiff, lib: true do
   subject { described_class.new(diff_file) }
 
   describe '#parallelize' do
-    it 'should return an array of arrays containing the parsed diff' do
+    it 'returns an array of arrays containing the parsed diff' do
       diff_lines = diff_file.highlighted_diff_lines
       expected = [
         # Unchanged lines

spec/lib/gitlab/elastic/project_search_results_spec.rb

@@ -78,7 +78,7 @@ describe Gitlab::Elastic::ProjectSearchResults, lib: true do
       Gitlab::Elastic::Helper.refresh_index
     end
 
-    it 'should not list project confidential issues for non project members' do
+    it 'does not list project confidential issues for non project members' do
       results = described_class.new(non_member, query, project.id)
       issues = results.objects('issues')
 
@@ -88,7 +88,7 @@ describe Gitlab::Elastic::ProjectSearchResults, lib: true do
       expect(results.issues_count).to eq 1
     end
 
-    it 'should list project confidential issues for author' do
+    it 'lists project confidential issues for author' do
       results = described_class.new(author, query, project.id)
       issues = results.objects('issues')
 
@@ -98,7 +98,7 @@ describe Gitlab::Elastic::ProjectSearchResults, lib: true do
       expect(results.issues_count).to eq 2
     end
 
-    it 'should list project confidential issues for assignee' do
+    it 'lists project confidential issues for assignee' do
       results = described_class.new(assignee, query, project.id)
       issues = results.objects('issues')
 
@@ -108,7 +108,7 @@ describe Gitlab::Elastic::ProjectSearchResults, lib: true do
       expect(results.issues_count).to eq 2
     end
 
-    it 'should list project confidential issues for project members' do
+    it 'lists project confidential issues for project members' do
       project.team << [member, :developer]
 
       results = described_class.new(member, query, project.id)
@@ -120,7 +120,7 @@ describe Gitlab::Elastic::ProjectSearchResults, lib: true do
       expect(results.issues_count).to eq 3
     end
 
-    it 'should not list project confidential issues for project members with guest role' do
+    it 'does not list project confidential issues for project members with guest role' do
       project.team << [member, :guest]
 
       results = described_class.new(member, query, project.id)
@@ -132,7 +132,7 @@ describe Gitlab::Elastic::ProjectSearchResults, lib: true do
       expect(results.issues_count).to eq 1
     end
 
-    it 'should list all project issues for admin' do
+    it 'lists all project issues for admin' do
       results = described_class.new(admin, query, project.id)
       issues = results.objects('issues')
 

spec/lib/gitlab/elastic/search_results_spec.rb

@@ -41,7 +41,7 @@ describe Gitlab::Elastic::SearchResults, lib: true do
       Gitlab::Elastic::Helper.refresh_index
     end
 
-    it 'should list issues that title or description contain the query' do
+    it 'lists issues that title or description contain the query' do
       results = described_class.new(user, 'hello world', limit_project_ids)
       issues = results.objects('issues')
 
@@ -51,14 +51,14 @@ describe Gitlab::Elastic::SearchResults, lib: true do
       expect(results.issues_count).to eq 2
     end
 
-    it 'should return empty list when issues title or description does not contain the query' do
+    it 'returns empty list when issues title or description does not contain the query' do
       results = described_class.new(user, 'security', limit_project_ids)
 
       expect(results.objects('issues')).to be_empty
       expect(results.issues_count).to eq 0
     end
 
-    it 'should list issue when search by a valid iid' do
+    it 'lists issue when search by a valid iid' do
       results = described_class.new(user, '#2', limit_project_ids)
       issues = results.objects('issues')
 
@@ -68,7 +68,7 @@ describe Gitlab::Elastic::SearchResults, lib: true do
       expect(results.issues_count).to eq 1
     end
 
-    it 'should return empty list when search by invalid iid' do
+    it 'returns empty list when search by invalid iid' do
       results = described_class.new(user, '#222', limit_project_ids)
 
       expect(results.objects('issues')).to be_empty
@@ -100,7 +100,7 @@ describe Gitlab::Elastic::SearchResults, lib: true do
     context 'search by term' do
       let(:query) { 'issue' }
 
-      it 'should not list confidential issues for guests' do
+      it 'does not list confidential issues for guests' do
         results = described_class.new(nil, query, limit_project_ids)
         issues = results.objects('issues')
 
@@ -113,7 +113,7 @@ describe Gitlab::Elastic::SearchResults, lib: true do
         expect(results.issues_count).to eq 1
       end
 
-      it 'should not list confidential issues for non project members' do
+      it 'does not list confidential issues for non project members' do
         results = described_class.new(non_member, query, limit_project_ids)
         issues = results.objects('issues')
 
@@ -126,7 +126,7 @@ describe Gitlab::Elastic::SearchResults, lib: true do
         expect(results.issues_count).to eq 1
       end
 
-      it 'should list confidential issues for author' do
+      it 'lists confidential issues for author' do
         results = described_class.new(author, query, limit_project_ids)
         issues = results.objects('issues')
 
@@ -139,7 +139,7 @@ describe Gitlab::Elastic::SearchResults, lib: true do
         expect(results.issues_count).to eq 3
       end
 
-      it 'should list confidential issues for assignee' do
+      it 'lists confidential issues for assignee' do
         results = described_class.new(assignee, query, limit_project_ids)
         issues = results.objects('issues')
 
@@ -152,7 +152,7 @@ describe Gitlab::Elastic::SearchResults, lib: true do
         expect(results.issues_count).to eq 3
       end
 
-      it 'should list confidential issues for project members' do
+      it 'lists confidential issues for project members' do
         project_1.team << [member, :developer]
         project_2.team << [member, :developer]
 
@@ -168,7 +168,7 @@ describe Gitlab::Elastic::SearchResults, lib: true do
         expect(results.issues_count).to eq 4
       end
 
-      it 'should list all issues for admin' do
+      it 'lists all issues for admin' do
         results = described_class.new(admin, query, limit_project_ids)
         issues = results.objects('issues')
 
@@ -185,7 +185,7 @@ describe Gitlab::Elastic::SearchResults, lib: true do
     context 'search by iid' do
       let(:query) { '#1' }
 
-      it 'should not list confidential issues for guests' do
+      it 'does not list confidential issues for guests' do
         results = described_class.new(nil, query, limit_project_ids)
         issues = results.objects('issues')
 
@@ -198,7 +198,7 @@ describe Gitlab::Elastic::SearchResults, lib: true do
         expect(results.issues_count).to eq 1
       end
 
-      it 'should not list confidential issues for non project members' do
+      it 'does not list confidential issues for non project members' do
         results = described_class.new(non_member, query, limit_project_ids)
         issues = results.objects('issues')
 
@@ -211,7 +211,7 @@ describe Gitlab::Elastic::SearchResults, lib: true do
         expect(results.issues_count).to eq 1
       end
 
-      it 'should list confidential issues for author' do
+      it 'lists confidential issues for author' do
         results = described_class.new(author, query, limit_project_ids)
         issues = results.objects('issues')
 
@@ -224,7 +224,7 @@ describe Gitlab::Elastic::SearchResults, lib: true do
         expect(results.issues_count).to eq 2
       end
 
-      it 'should list confidential issues for assignee' do
+      it 'lists confidential issues for assignee' do
         results = described_class.new(assignee, query, limit_project_ids)
         issues = results.objects('issues')
 
@@ -237,7 +237,7 @@ describe Gitlab::Elastic::SearchResults, lib: true do
         expect(results.issues_count).to eq 2
       end
 
-      it 'should list confidential issues for project members' do
+      it 'lists confidential issues for project members' do
         project_2.team << [member, :developer]
         project_3.team << [member, :developer]
 
@@ -253,7 +253,7 @@ describe Gitlab::Elastic::SearchResults, lib: true do
         expect(results.issues_count).to eq 3
       end
 
-      it 'should list all issues for admin' do
+      it 'lists all issues for admin' do
         results = described_class.new(admin, query, limit_project_ids)
         issues = results.objects('issues')
 
@@ -297,7 +297,7 @@ describe Gitlab::Elastic::SearchResults, lib: true do
       Gitlab::Elastic::Helper.refresh_index
     end
 
-    it 'should list merge requests that title or description contain the query' do
+    it 'lists merge requests that title or description contain the query' do
       results = described_class.new(user, 'hello world', limit_project_ids)
       merge_requests = results.objects('merge_requests')
 
@@ -307,14 +307,14 @@ describe Gitlab::Elastic::SearchResults, lib: true do
       expect(results.merge_requests_count).to eq 2
     end
 
-    it 'should return empty list when merge requests title or description does not contain the query' do
+    it 'returns empty list when merge requests title or description does not contain the query' do
       results = described_class.new(user, 'security', limit_project_ids)
 
       expect(results.objects('merge_requests')).to be_empty
       expect(results.merge_requests_count).to eq 0
     end
 
-    it 'should list merge request when search by a valid iid' do
+    it 'lists merge request when search by a valid iid' do
       results = described_class.new(user, '#2', limit_project_ids)
       merge_requests = results.objects('merge_requests')
 
@@ -324,7 +324,7 @@ describe Gitlab::Elastic::SearchResults, lib: true do
       expect(results.merge_requests_count).to eq 1
     end
 
-    it 'should return empty list when search by invalid iid' do
+    it 'returns empty list when search by invalid iid' do
       results = described_class.new(user, '#222', limit_project_ids)
 
       expect(results.objects('merge_requests')).to be_empty

spec/lib/gitlab/git_access_spec.rb

@@ -555,4 +555,15 @@ describe Gitlab::GitAccess, lib: true do
       end
     end
   end
+
+  context 'when the repository is read only' do
+    it 'denies push access' do
+      project = create(:project, :read_only_repository)
+      project.team << [user, :master]
+
+      check = access.check('git-receive-pack')
+
+      expect(check).not_to be_allowed
+    end
+  end
 end

spec/lib/gitlab/highlight_spec.rb

@@ -12,7 +12,7 @@ describe Gitlab::Highlight, lib: true do
       Gitlab::Highlight.highlight_lines(project.repository, commit.id, 'files/ruby/popen.rb')
     end
 
-    it 'should properly highlight all the lines' do
+    it 'highlights all the lines properly' do
       expect(lines[4]).to eq(%Q{<span id="LC5" class="line">  <span class="kp">extend</span> <span class="nb">self</span></span>\n})
       expect(lines[21]).to eq(%Q{<span id="LC22" class="line">    <span class="k">unless</span> <span class="no">File</span><span class="p">.</span><span class="nf">directory?</span><span class="p">(</span><span class="n">path</span><span class="p">)</span></span>\n})
       expect(lines[26]).to eq(%Q{<span id="LC27" class="line">    <span class="vi">@cmd_status</span> <span class="o">=</span> <span class="mi">0</span></span>\n})

spec/lib/gitlab/ldap/access_spec.rb

@@ -14,7 +14,7 @@ describe Gitlab::LDAP::Access, lib: true do
 
       it { is_expected.to be_falsey }
 
-      it 'should block user in GitLab' do
+      it 'blocks user in GitLab' do
         access.allowed?
         expect(user).to be_blocked
         expect(user).to be_ldap_blocked
@@ -150,14 +150,14 @@ describe Gitlab::LDAP::Access, lib: true do
       allow(access).to receive_messages(ldap_user: Gitlab::LDAP::Person.new(entry, user.ldap_identity.provider))
     end
 
-    it "should add a Kerberos identity if it is in Active Directory but not in GitLab" do
+    it "adds a Kerberos identity if it is in Active Directory but not in GitLab" do
       allow_any_instance_of(Gitlab::LDAP::Person).to receive_messages(kerberos_principal: "mylogin@FOO.COM")
 
       expect{ access.update_kerberos_identity }.to change(user.identities.where(provider: :kerberos), :count).from(0).to(1)
       expect(user.identities.where(provider: "kerberos").last.extern_uid).to eq("mylogin@FOO.COM")
     end
 
-    it "should update existing Kerberos identity in GitLab if Active Directory has a different one" do
+    it "updates existing Kerberos identity in GitLab if Active Directory has a different one" do
       allow_any_instance_of(Gitlab::LDAP::Person).to receive_messages(kerberos_principal: "otherlogin@BAR.COM")
       user.identities.build(provider: "kerberos", extern_uid: "mylogin@FOO.COM").save
 
@@ -165,7 +165,7 @@ describe Gitlab::LDAP::Access, lib: true do
       expect(user.identities.where(provider: "kerberos").last.extern_uid).to eq("otherlogin@BAR.COM")
     end
 
-    it "should not remove Kerberos identities from GitLab if they are none in the LDAP provider" do
+    it "does not remove Kerberos identities from GitLab if they are none in the LDAP provider" do
       allow_any_instance_of(Gitlab::LDAP::Person).to receive_messages(kerberos_principal: nil)
       user.identities.build(provider: "kerberos", extern_uid: "otherlogin@BAR.COM").save
 
@@ -173,7 +173,7 @@ describe Gitlab::LDAP::Access, lib: true do
       expect(user.identities.where(provider: "kerberos").last.extern_uid).to eq("otherlogin@BAR.COM")
     end
 
-    it "should not modify identities in GitLab if they are no kerberos principal in the LDAP provider" do
+    it "does not modify identities in GitLab if they are no kerberos principal in the LDAP provider" do
       allow_any_instance_of(Gitlab::LDAP::Person).to receive_messages(kerberos_principal: nil)
 
       expect{ access.update_kerberos_identity }.not_to change(user.identities, :count)
@@ -192,13 +192,13 @@ describe Gitlab::LDAP::Access, lib: true do
       allow(access).to receive_messages(sync_ssh_keys?: true)
     end
 
-    it "should add a SSH key if it is in LDAP but not in gitlab" do
+    it "adds a SSH key if it is in LDAP but not in gitlab" do
       allow_any_instance_of(Gitlab::LDAP::Adapter).to receive(:user) { Gitlab::LDAP::Person.new(entry, 'ldapmain') }
 
       expect{ access.update_ssh_keys }.to change(user.keys, :count).from(0).to(1)
     end
 
-    it "should add a SSH key and give it a proper name" do
+    it "adds a SSH key and give it a proper name" do
       allow_any_instance_of(Gitlab::LDAP::Adapter).to receive(:user) { Gitlab::LDAP::Person.new(entry, 'ldapmain') }
 
       access.update_ssh_keys
@@ -206,7 +206,7 @@ describe Gitlab::LDAP::Access, lib: true do
       expect(user.keys.last.title).to match(/#{access.ldap_config.sync_ssh_keys}/)
     end
 
-    it "should not add a SSH key if it is invalid" do
+    it "does not add a SSH key if it is invalid" do
       entry = Net::LDAP::Entry.from_single_ldif_string("dn: cn=foo, dc=bar, dc=com\n#{ssh_key_attribute_name}: I am not a valid key")
       allow_any_instance_of(Gitlab::LDAP::Adapter).to receive(:user) { Gitlab::LDAP::Person.new(entry, 'ldapmain') }
 
@@ -216,14 +216,14 @@ describe Gitlab::LDAP::Access, lib: true do
     context 'user has at least one LDAPKey' do
       before { user.keys.ldap.create key: ssh_key, title: 'to be removed' }
 
-      it "should remove a SSH key if it is no longer in LDAP" do
+      it "removes a SSH key if it is no longer in LDAP" do
         entry = Net::LDAP::Entry.from_single_ldif_string("dn: cn=foo, dc=bar, dc=com\n#{ssh_key_attribute_name}:\n")
         allow_any_instance_of(Gitlab::LDAP::Adapter).to receive(:user) { Gitlab::LDAP::Person.new(entry, 'ldapmain') }
 
         expect{ access.update_ssh_keys }.to change(user.keys, :count).from(1).to(0)
       end
 
-      it "should remove a SSH key if the ldap attribute was removed" do
+      it "removes a SSH key if the ldap attribute was removed" do
         entry = Net::LDAP::Entry.from_single_ldif_string("dn: cn=foo, dc=bar, dc=com")
         allow_any_instance_of(Gitlab::LDAP::Adapter).to receive(:user) { Gitlab::LDAP::Person.new(entry, 'ldapmain') }
 
@@ -239,21 +239,21 @@ describe Gitlab::LDAP::Access, lib: true do
       allow(access).to receive_messages(ldap_user: Gitlab::LDAP::Person.new(entry, user.ldap_identity.provider))
     end
 
-    it "should not update email if email attribute is not set" do
+    it "does not update email if email attribute is not set" do
       expect{ access.update_email }.not_to change(user, :email)
     end
 
-    it "should not update the email if the user has the same email in GitLab and in LDAP" do
+    it "does not update the email if the user has the same email in GitLab and in LDAP" do
       entry['mail'] = [user.email]
       expect{ access.update_email }.not_to change(user, :email)
     end
 
-    it "should not update the email if the user has the same email GitLab and in LDAP, but with upper case in LDAP" do
+    it "does not update the email if the user has the same email GitLab and in LDAP, but with upper case in LDAP" do
       entry['mail'] = [user.email.upcase]
       expect{ access.update_email }.not_to change(user, :email)
     end
 
-    it "should update the email if the user email is different" do
+    it "updates the email if the user email is different" do
       entry['mail'] = ["new_email@example.com"]
       expect{ access.update_email }.to change(user, :email)
     end

spec/lib/gitlab/saml/user_spec.rb

@@ -100,7 +100,7 @@ describe Gitlab::Saml::User, lib: true do
         end
 
         context 'user was admin, now should not be' do
-          it 'should make user non admin' do
+          it 'makes user non admin' do
             existing_user.update_attribute('admin', true)
             saml_user.save
             expect(gl_user).to be_valid

spec/models/project_services/jenkins_deprecated_service_spec.rb

@@ -49,7 +49,7 @@ eos
 
       statuses = { 'blue.png' => 'success', 'yellow.png' => 'failed', 'red.png' => 'failed', 'aborted.png' => 'failed', 'blue-anime.gif' => 'running', 'grey.png' => 'pending' }
       statuses.each do |icon, state|
-        it "should have a status of #{state} when the icon #{icon} exists." do
+        it "has a status of #{state} when the icon #{icon} exists." do
           stub_request(:get, "http://jenkins.gitlab.org/job/2/scm/bySHA1/2ab7834c").to_return(status: 200, body: status_body_for_icon(icon), headers: {})
           expect(@service.commit_status("2ab7834c", 'master')).to eq(state)
         end
@@ -68,7 +68,7 @@ eos
         )
       end
 
-      it "should have a status of success when the icon yellow exists." do
+      it "has a status of success when the icon yellow exists." do
         stub_request(:get, "http://jenkins.gitlab.org/job/2/scm/bySHA1/2ab7834c").to_return(status: 200, body: status_body_for_icon('yellow.png'), headers: {})
         expect(@service.commit_status("2ab7834c", 'master')).to eq('success')
       end

spec/models/project_services/jenkins_service_spec.rb

@@ -150,7 +150,7 @@ describe JenkinsService do
         expect(@jenkins_service.jenkins_url).to eq('http://jenkins_edited.example.com/')
       end
 
-      it 'should reset password if url changed, even if setter called multiple times' do
+      it 'resets password if url changed, even if setter called multiple times' do
         @jenkins_service.jenkins_url = 'http://jenkins1.example.com/'
         @jenkins_service.jenkins_url = 'http://jenkins1.example.com/'
         @jenkins_service.save

spec/models/project_spec.rb

@@ -251,7 +251,7 @@ describe Project, models: true do
   describe "#kerberos_url_to_repo" do
     let(:project) { create(:empty_project, path: "somewhere") }
 
-    it 'should return valid kerberos url for this repo' do
+    it 'returns valid kerberos url for this repo' do
       expect(project.kerberos_url_to_repo).to eq("#{Gitlab.config.build_gitlab_kerberos_url}/#{project.namespace.path}/somewhere.git")
     end
   end
@@ -1594,4 +1594,51 @@ describe Project, models: true do
       expect(shared_project.authorized_for_user?(master, Gitlab::Access::MASTER)).to be(true)
     end
   end
+
+  describe '#change_repository_storage' do
+    let(:project) { create(:project, repository_storage: 'a') }
+    let(:read_only_project) { create(:project, repository_storage: 'a', repository_read_only: true) }
+
+    before do
+      FileUtils.mkdir('tmp/tests/storage_a')
+      FileUtils.mkdir('tmp/tests/storage_b')
+
+      storages = { 'a' => 'tmp/tests/storage_a', 'b' => 'tmp/tests/storage_b' }
+      allow(Gitlab.config.repositories).to receive(:storages).and_return(storages)
+    end
+
+    after do
+      FileUtils.rm_rf('tmp/tests/storage_a')
+      FileUtils.rm_rf('tmp/tests/storage_b')
+    end
+
+    it 'schedule the transfer of the repository to the new storage and locks the project' do
+      expect(ProjectUpdateRepositoryStorageWorker).to receive(:perform_async).with(project.id, 'b')
+
+      project.change_repository_storage('b')
+      project.save
+
+      expect(project).to be_repository_read_only
+    end
+
+    it "doesn't schedule the transfer if the repository is already read-only" do
+      expect(ProjectUpdateRepositoryStorageWorker).not_to receive(:perform_async)
+
+      read_only_project.change_repository_storage('b')
+      read_only_project.save
+    end
+
+    it "doesn't lock or schedule the transfer if the storage hasn't changed" do
+      expect(ProjectUpdateRepositoryStorageWorker).not_to receive(:perform_async)
+
+      project.change_repository_storage('a')
+      project.save
+
+      expect(project).not_to be_repository_read_only
+    end
+
+    it 'throws an error if an invalid repository storage is provided' do
+      expect { project.change_repository_storage('c') }.to raise_error
+    end
+  end
 end

spec/models/remote_mirror_spec.rb

@@ -22,19 +22,19 @@ require 'rails_helper'
 describe RemoteMirror do
   describe 'encrypting credentials' do
     context 'when setting URL for a first time' do
-      it 'should store the URL without credentials' do
+      it 'stores the URL without credentials' do
         mirror = create_mirror(url: 'http://foo:bar@test.com')
 
         expect(mirror.read_attribute(:url)).to eq('http://test.com')
       end
 
-      it 'should store the credentials on a separate field' do
+      it 'stores the credentials on a separate field' do
         mirror = create_mirror(url: 'http://foo:bar@test.com')
 
         expect(mirror.credentials).to eq({ user: 'foo', password: 'bar' })
       end
 
-      it 'should handle credentials with large content' do
+      it 'handles credentials with large content' do
         mirror = create_mirror(url: 'http://bxnhm8dote33ct932r3xavslj81wxmr7o8yux8do10oozckkif:9ne7fuvjn40qjt35dgt8v86q9m9g9essryxj76sumg2ccl2fg26c0krtz2gzfpyq4hf22h328uhq6npuiq6h53tpagtsj7vsrz75@test.com')
 
         expect(mirror.credentials).to eq({
@@ -45,7 +45,7 @@ describe RemoteMirror do
     end
 
     context 'when updating the URL' do
-      it 'should allow a new URL without credentials' do
+      it 'allows a new URL without credentials' do
         mirror = create_mirror(url: 'http://foo:bar@test.com')
 
         mirror.update_attribute(:url, 'http://test.com')
@@ -54,7 +54,7 @@ describe RemoteMirror do
         expect(mirror.credentials).to eq({ user: nil, password: nil })
       end
 
-      it 'should allow a new URL with credentials' do
+      it 'allows a new URL with credentials' do
         mirror = create_mirror(url: 'http://test.com')
 
         mirror.update_attribute(:url, 'http://foo:bar@test.com')
@@ -63,7 +63,7 @@ describe RemoteMirror do
         expect(mirror.credentials).to eq({ user: 'foo', password: 'bar' })
       end
 
-      it 'should update the remote config if credentials changed' do
+      it 'updates the remote config if credentials changed' do
         mirror = create_mirror(url: 'http://foo:bar@test.com')
         repo = mirror.project.repository
 
@@ -76,7 +76,7 @@ describe RemoteMirror do
 
   describe '#safe_url' do
     context 'when URL contains credentials' do
-      it 'should mask the credentials' do
+      it 'masks the credentials' do
         mirror = create_mirror(url: 'http://foo:bar@test.com')
 
         expect(mirror.safe_url).to eq('http://*****:*****@test.com')
@@ -84,7 +84,7 @@ describe RemoteMirror do
     end
 
     context 'when URL does not contain credentials' do
-      it 'should show the full URL' do
+      it 'shows the full URL' do
         mirror = create_mirror(url: 'http://test.com')
 
         expect(mirror.safe_url).to eq('http://test.com')

spec/requests/api/builds_spec.rb

@@ -56,7 +56,7 @@ describe API::API, api: true do
     context 'unauthorized user' do
       let(:api_user) { nil }
 
-      it 'should not return project builds' do
+      it 'does not return project builds' do
         expect(response).to have_http_status(401)
       end
     end

spec/requests/api/groups_spec.rb

@@ -191,7 +191,7 @@ describe API::API, api: true  do
         expect(response).to have_http_status(404)
       end
 
-      it "should only return projects to which user has access" do
+      it "only returns projects to which user has access" do
         project3.team << [user3, :developer]
 
         get api("/groups/#{group1.id}/projects", user3)
@@ -203,21 +203,21 @@ describe API::API, api: true  do
     end
 
     context "when authenticated as admin" do
-      it "should return any existing group" do
+      it "returns any existing group" do
         get api("/groups/#{group2.id}/projects", admin)
         expect(response).to have_http_status(200)
         expect(json_response.length).to eq(1)
         expect(json_response.first['name']).to eq(project2.name)
       end
 
-      it "should not return a non existing group" do
+      it "does not return a non existing group" do
         get api("/groups/1328/projects", admin)
         expect(response).to have_http_status(404)
       end
     end
 
     context 'when using group path in URL' do
-      it 'should return any existing group' do
+      it 'returns any existing group' do
         get api("/groups/#{group1.path}/projects", admin)
 
         expect(response).to have_http_status(200)
@@ -277,14 +277,14 @@ describe API::API, api: true  do
 
   describe "PUT /groups" do
     context "when authenticated as user without group permissions" do
-      it "should not create group" do
+      it "does not create group" do
         put api("/groups/#{group2.id}", user1), attributes_for(:group)
         expect(response.status).to eq(404)
       end
     end
 
     context "when authenticated as user with group permissions" do
-      it "should update group" do
+      it "updates group" do
         group2.update(owner: user2)
         put api("/groups/#{group2.id}", user2), { name: 'Renamed' }
         expect(response.status).to eq(200)

spec/requests/api/ldap_group_links_spec.rb

@@ -21,14 +21,14 @@ describe API::API, api: true  do
 
   describe "POST /groups/:id/ldap_group_links" do
     context "when unauthenticated" do
-      it "should return authentication error" do
+      it "returns authentication error" do
         post api("/groups/#{group_with_ldap_links.id}/ldap_group_links")
         expect(response.status).to eq 401
       end
     end
 
     context "when a less priviledged user" do
-      it "should not allow less priviledged user to add LDAP group link" do
+      it "does not allow less priviledged user to add LDAP group link" do
         expect do
           post api("/groups/#{group_with_ldap_links.id}/ldap_group_links", user),
           cn: 'ldap-group4', group_access: GroupMember::GUEST
@@ -39,7 +39,7 @@ describe API::API, api: true  do
     end
 
     context "when owner of the group" do
-      it "should return ok and add ldap group link" do
+      it "returns ok and add ldap group link" do
         expect do
           post api("/groups/#{group_with_ldap_links.id}/ldap_group_links", owner),
           cn: 'ldap-group3', group_access: GroupMember::GUEST, provider: 'ldap3'
@@ -52,7 +52,7 @@ describe API::API, api: true  do
       end
 
       # TODO: Correct and activate this test once issue #329 is fixed
-      xit "should return ok and add ldap group link even if no provider specified" do
+      xit "returns ok and add ldap group link even if no provider specified" do
         expect do
           post api("/groups/#{group_with_ldap_links.id}/ldap_group_links", owner),
           cn: 'ldap-group3', group_access: GroupMember::GUEST
@@ -64,22 +64,22 @@ describe API::API, api: true  do
         expect(json_response['provider']).to eq('ldapmain')
       end
 
-      it "should return error if LDAP group link already exists" do
+      it "returns error if LDAP group link already exists" do
         post api("//groups/#{group_with_ldap_links.id}/ldap_group_links", owner), provider: 'ldap1', cn: 'ldap-group1', group_access: GroupMember::GUEST
         expect(response.status).to eq(409)
       end
 
-      it "should return a 400 error when cn is not given" do
+      it "returns a 400 error when cn is not given" do
         post api("//groups/#{group_with_ldap_links.id}/ldap_group_links", owner), group_access: GroupMember::GUEST
         expect(response.status).to eq(400)
       end
 
-      it "should return a 400 error when group access is not given" do
+      it "returns a 400 error when group access is not given" do
         post api("//groups/#{group_with_ldap_links.id}/ldap_group_links", owner), cn: 'ldap-group3'
         expect(response.status).to eq(400)
       end
 
-      it "should return a 422 error when group access is not known" do
+      it "returns a 422 error when group access is not known" do
         post api("//groups/#{group_with_ldap_links.id}/ldap_group_links", owner), cn: 'ldap-group3', group_access: 11, provider: 'ldap1'
         expect(response.status).to eq(422)
       end
@@ -88,14 +88,14 @@ describe API::API, api: true  do
 
   describe 'DELETE /groups/:id/ldap_group_links/:cn' do
     context "when unauthenticated" do
-      it "should return authentication error" do
+      it "returns authentication error" do
         delete api("/groups/#{group_with_ldap_links.id}/ldap_group_links/ldap-group1")
         expect(response.status).to eq 401
       end
     end
 
     context "when a less priviledged user" do
-      it "should not remove the LDAP group link" do
+      it "does not remove the LDAP group link" do
         expect do
           delete api("/groups/#{group_with_ldap_links.id}/ldap_group_links/ldap-group1", user)
         end.not_to change { group_with_ldap_links.ldap_group_links.count }
@@ -105,7 +105,7 @@ describe API::API, api: true  do
     end
 
     context "when owner of the group" do
-      it "should remove ldap group link" do
+      it "removes ldap group link" do
         expect do
           delete api("/groups/#{group_with_ldap_links.id}/ldap_group_links/ldap-group1", owner)
         end.to change { group_with_ldap_links.ldap_group_links.count }.by(-1)
@@ -113,7 +113,7 @@ describe API::API, api: true  do
         expect(response.status).to eq(200)
       end
 
-      it "should return 404 if LDAP group cn not used for a LDAP group link" do
+      it "returns 404 if LDAP group cn not used for a LDAP group link" do
         expect do
           delete api("/groups/#{group_with_ldap_links.id}/ldap_group_links/ldap-group1356", owner)
         end.not_to change { group_with_ldap_links.ldap_group_links.count }
@@ -125,14 +125,14 @@ describe API::API, api: true  do
 
   describe 'DELETE /groups/:id/ldap_group_links/:provider/:cn' do
     context "when unauthenticated" do
-      it "should return authentication error" do
+      it "returns authentication error" do
         delete api("/groups/#{group_with_ldap_links.id}/ldap_group_links/ldap2/ldap-group2")
         expect(response.status).to eq 401
       end
     end
 
     context "when a less priviledged user" do
-      it "should not remove the LDAP group link" do
+      it "does not remove the LDAP group link" do
         expect do
           delete api("/groups/#{group_with_ldap_links.id}/ldap_group_links/ldap2/ldap-group2", user)
         end.not_to change { group_with_ldap_links.ldap_group_links.count }
@@ -142,7 +142,7 @@ describe API::API, api: true  do
     end
 
     context "when owner of the group" do
-      it "should return 404 if LDAP group cn not used for a LDAP group link for the specified provider" do
+      it "returns 404 if LDAP group cn not used for a LDAP group link for the specified provider" do
         expect do
           delete api("/groups/#{group_with_ldap_links.id}/ldap_group_links/ldap1/ldap-group2", owner)
         end.not_to change { group_with_ldap_links.ldap_group_links.count }
@@ -150,7 +150,7 @@ describe API::API, api: true  do
         expect(response.status).to eq(404)
       end
 
-      it "should remove ldap group link" do
+      it "removes ldap group link" do
         expect do
           delete api("/groups/#{group_with_ldap_links.id}/ldap_group_links/ldap2/ldap-group2", owner)
         end.to change { group_with_ldap_links.ldap_group_links.count }.by(-1)

spec/requests/api/ldap_spec.rb

@@ -15,14 +15,14 @@ describe API::API do
 
   describe "GET /ldap/groups" do
     context "when unauthenticated" do
-      it "should return authentication error" do
+      it "returns authentication error" do
         get api("/ldap/groups")
         expect(response.status).to eq 401
       end
     end
 
     context "when authenticated as user" do
-      it "should return an array of ldap groups" do
+      it "returns an array of ldap groups" do
         get api("/ldap/groups", user)
         expect(response.status).to eq 200
         expect(json_response).to be_an Array
@@ -34,14 +34,14 @@ describe API::API do
 
   describe "GET /ldap/ldapmain/groups" do
     context "when unauthenticated" do
-      it "should return authentication error" do
+      it "returns authentication error" do
         get api("/ldap/ldapmain/groups")
         expect(response.status).to eq 401
       end
     end
 
     context "when authenticated as user" do
-      it "should return an array of ldap groups" do
+      it "returns an array of ldap groups" do
         get api("/ldap/ldapmain/groups", user)
         expect(response.status).to eq 200
         expect(json_response).to be_an Array

spec/requests/api/license_spec.rb

@@ -9,7 +9,7 @@ describe API::API, api: true  do
   let(:user) { create(:user) }
 
   describe 'GET /license' do
-    it 'should retrieve the license information if admin is logged in' do
+    it 'retrieves the license information if admin is logged in' do
       get api('/license', admin)
       expect(response.status).to eq 200
       expect(json_response['user_limit']).to eq 0
@@ -19,7 +19,7 @@ describe API::API, api: true  do
       expect(json_response['licensee']).not_to be_empty
     end
 
-    it 'should deny access if not admin' do
+    it 'denies access if not admin' do
       get api('/license', user)
       expect(response.status).to eq 403
     end

spec/requests/api/project_push_rule_spec.rb

@@ -17,7 +17,7 @@ describe API::API, 'ProjectPushRule', api: true  do
     end
 
     context "authorized user" do
-      it "should return project push rule" do
+      it "returns project push rule" do
         get api("/projects/#{project.id}/push_rule", user)
         expect(response.status).to eq(200)
 
@@ -27,7 +27,7 @@ describe API::API, 'ProjectPushRule', api: true  do
     end
 
     context "unauthorized user" do
-      it "should not access project push rule" do
+      it "does not have access to project push rule" do
         get api("/projects/#{project.id}/push_rule", user3)
         expect(response.status).to eq(403)
       end
@@ -36,7 +36,7 @@ describe API::API, 'ProjectPushRule', api: true  do
 
   describe "POST /projects/:id/push_rule" do
     context "authorized user" do
-      it "should add push rule to project" do
+      it "adds push rule to project" do
         post api("/projects/#{project.id}/push_rule", user),
           deny_delete_tag: true
         expect(response.status).to eq(201)
@@ -48,7 +48,7 @@ describe API::API, 'ProjectPushRule', api: true  do
     end
 
     context "unauthorized user" do
-      it "should not add push rule to project" do
+      it "does not add push rule to project" do
         post api("/projects/#{project.id}/push_rule", user3),
           deny_delete_tag: true
         expect(response.status).to eq(403)
@@ -62,7 +62,7 @@ describe API::API, 'ProjectPushRule', api: true  do
     end
 
     context "with existing push rule" do
-      it "should not add push rule to project" do
+      it "does not add push rule to project" do
         post api("/projects/#{project.id}/push_rule", user),
           deny_delete_tag: true
         expect(response.status).to eq(422)
@@ -75,7 +75,7 @@ describe API::API, 'ProjectPushRule', api: true  do
       create(:push_rule, project: project)
     end
 
-    it "should update an existing project push rule" do
+    it "updates an existing project push rule" do
       put api("/projects/#{project.id}/push_rule", user),
         deny_delete_tag: false, commit_message_regex: 'Fixes \d+\..*'
       expect(response.status).to eq(200)
@@ -86,13 +86,13 @@ describe API::API, 'ProjectPushRule', api: true  do
   end
 
   describe "PUT /projects/:id/push_rule" do
-    it "should error on non existing project push rule" do
+    it "gets error on non existing project push rule" do
       put api("/projects/#{project.id}/push_rule", user),
         deny_delete_tag: false, commit_message_regex: 'Fixes \d+\..*'
       expect(response.status).to eq(404)
     end
 
-    it "should not update push rule for unauthorized user" do
+    it "does not update push rule for unauthorized user" do
       post api("/projects/#{project.id}/push_rule", user3),
         deny_delete_tag: true
       expect(response.status).to eq(403)
@@ -105,7 +105,7 @@ describe API::API, 'ProjectPushRule', api: true  do
     end
 
     context "authorized user" do
-      it "should delete push rule from project" do
+      it "deletes push rule from project" do
         delete api("/projects/#{project.id}/push_rule", user)
         expect(response.status).to eq(200)
 
@@ -114,7 +114,7 @@ describe API::API, 'ProjectPushRule', api: true  do
     end
 
     context "unauthorized user" do
-      it "should return a 403 error" do
+      it "returns a 403 error" do
         delete api("/projects/#{project.id}/push_rule", user3)
         expect(response.status).to eq(403)
       end
@@ -123,7 +123,7 @@ describe API::API, 'ProjectPushRule', api: true  do
 
   describe "DELETE /projects/:id/push_rule" do
     context "for non existing push rule" do
-      it "should delete push rule from project" do
+      it "deletes push rule from project" do
         delete api("/projects/#{project.id}/push_rule", user)
         expect(response.status).to eq(404)
 
@@ -131,7 +131,7 @@ describe API::API, 'ProjectPushRule', api: true  do
         expect(json_response['message']).to eq("404 Not Found")
       end
 
-      it "should return a 403 error if not authorized" do
+      it "returns a 403 error if not authorized" do
         delete api("/projects/#{project.id}/push_rule", user3)
         expect(response.status).to eq(403)
       end

spec/requests/api/projects_spec.rb

@@ -305,7 +305,7 @@ describe API::API, api: true  do
     before { project }
     before { admin }
 
-    it 'should create new project without path and return 201' do
+    it 'creates new project without path and return 201' do
       expect { post api("/projects/user/#{user.id}", admin), name: 'foo' }.to change {Project.count}.by(1)
       expect(response).to have_http_status(201)
     end

spec/requests/api/users_spec.rb

@@ -71,7 +71,7 @@ describe API::API, api: true  do
     end
 
     context "when authenticated and ldap is enabled" do
-      it "should return non-ldap user" do
+      it "returns non-ldap user" do
         User.delete_all
         create :omniauth_user, provider: "ldapserver1"
         get api("/users", user), skip_ldap: "true"

spec/requests/ci/api/builds_spec.rb

@@ -236,7 +236,7 @@ describe Ci::API::API do
       before { build.run! }
 
       describe "POST /builds/:id/artifacts/authorize" do
-        context "should authorize posting artifact to running build" do
+        context "authorizes posting artifact to running build" do
           it "using token as parameter" do
             post authorize_url, { token: build.token }, headers
             expect(response).to have_http_status(200)
@@ -250,7 +250,7 @@ describe Ci::API::API do
           end
         end
 
-        context "should fail to post too large artifact" do
+        context "fails to post too large artifact" do
           it "using token as parameter" do
             stub_application_setting(max_artifacts_size: 0)
             post authorize_url, { token: build.token, filesize: 100 }, headers

spec/services/event_create_service_spec.rb

@@ -9,7 +9,7 @@ describe EventCreateService, services: true do
 
       it { expect(service.open_issue(issue, issue.author)).to be_truthy }
 
-      it "should create new event" do
+      it "creates new event" do
         expect { service.open_issue(issue, issue.author) }.to change { Event.count }
       end
     end
@@ -19,7 +19,7 @@ describe EventCreateService, services: true do
 
       it { expect(service.close_issue(issue, issue.author)).to be_truthy }
 
-      it "should create new event" do
+      it "creates new event" do
         expect { service.close_issue(issue, issue.author) }.to change { Event.count }
       end
     end
@@ -29,7 +29,7 @@ describe EventCreateService, services: true do
 
       it { expect(service.reopen_issue(issue, issue.author)).to be_truthy }
 
-      it "should create new event" do
+      it "creates new event" do
         expect { service.reopen_issue(issue, issue.author) }.to change { Event.count }
       end
     end

spec/services/merge_requests/close_service_spec.rb

@@ -27,7 +27,7 @@ describe MergeRequests::CloseService, services: true do
       it { expect(@merge_request).to be_valid }
       it { expect(@merge_request).to be_closed }
 
-      it 'should execute hooks with close action' do
+      it 'executes hooks with close action' do
         expect(service).to have_received(:execute_hooks).
                                with(@merge_request, 'close')
       end

spec/services/merge_requests/ff_merge_service_spec.rb

@@ -28,7 +28,7 @@ describe MergeRequests::FfMergeService do
         end
       end
 
-      it "should not create merge commit" do
+      it "does not create merge commit" do
         source_branch_sha = merge_request.source_project.repository.commit(merge_request.source_branch).sha
         target_branch_sha = merge_request.target_project.repository.commit(merge_request.target_branch).sha
         expect(source_branch_sha).to eq(target_branch_sha)
@@ -37,13 +37,13 @@ describe MergeRequests::FfMergeService do
       it { expect(merge_request).to be_valid }
       it { expect(merge_request).to be_merged }
 
-      it 'should send email to user2 about merge of new merge_request' do
+      it 'sends email to user2 about merge of new merge_request' do
         email = ActionMailer::Base.deliveries.last
         expect(email.to.first).to eq(user2.email)
         expect(email.subject).to include(merge_request.title)
       end
 
-      it 'should create system note about merge_request merge' do
+      it 'creates system note about merge_request merge' do
         note = merge_request.notes.last
         expect(note.note).to include 'Status changed to merged'
       end

spec/services/merge_requests/rebase_service_spec.rb

@@ -21,7 +21,7 @@ describe MergeRequests::RebaseService do
         service.execute(merge_request)
       end
 
-      it "should rebase source branch" do
+      it 'rebases source branch' do
         parent_sha = merge_request.source_project.repository.commit(merge_request.source_branch).parents.first.sha
         target_branch_sha = merge_request.target_project.repository.commit(merge_request.target_branch).sha
         expect(parent_sha).to eq(target_branch_sha)

spec/services/pages_service_spec.rb

@@ -15,7 +15,7 @@ describe PagesService, services: true do
     context 'on success' do
       before { build.success }
 
-      it 'should execute worker' do
+      it 'executes worker' do
         expect(PagesWorker).to receive(:perform_async)
         service.execute
       end
@@ -25,7 +25,7 @@ describe PagesService, services: true do
       context "on #{status}" do
         before { build.status = status }
 
-        it 'should not execute worker' do
+        it 'does not execute worker' do
           expect(PagesWorker).not_to receive(:perform_async)
           service.execute
         end
@@ -39,7 +39,7 @@ describe PagesService, services: true do
       build.success
     end
 
-    it 'should not execute worker' do
+    it 'does not execute worker' do
       expect(PagesWorker).not_to receive(:perform_async)
       service.execute
     end

spec/services/projects/update_remote_mirror_service_spec.rb

@@ -91,7 +91,7 @@ describe Projects::UpdateRemoteMirrorService do
       end
 
       context 'when there are not tags to push' do
-        it 'should not try to push tags' do
+        it 'does not try to push tags' do
           allow(repository).to receive(:remote_tags) { {} }
           allow(repository).to receive(:tags) { [] }
 
@@ -102,7 +102,7 @@ describe Projects::UpdateRemoteMirrorService do
       end
 
       context 'when there are some tags to push' do
-        it 'should push tags to remote' do
+        it 'pushes tags to remote' do
           allow(repository).to receive(:remote_tags) { {} }
 
           expect(repository).to receive(:push_remote_branches).with(remote_mirror.ref_name, ['v1.0.0', 'v1.1.0'])
@@ -112,7 +112,7 @@ describe Projects::UpdateRemoteMirrorService do
       end
 
       context 'when there are some tags to delete' do
-        it 'should delete tags from remote' do
+        it 'deletes tags from remote' do
           allow(repository).to receive(:remote_tags) { generate_tags(repository, 'v1.0.0', 'v1.1.0') }
           repository.rm_tag('v1.0.0')
 

spec/services/projects/update_repository_storage_service_spec.rb

+require 'spec_helper'
+
+describe Projects::UpdateRepositoryStorageService, services: true do
+
+  subject { described_class.new(project) }
+
+  describe "#execute" do
+    let(:gitlab_shell) { Gitlab::Shell.new }
+    let(:time) { Time.now }
+
+    before do
+      FileUtils.mkdir('tmp/tests/storage_a')
+      FileUtils.mkdir('tmp/tests/storage_b')
+
+      storages = { 'a' => 'tmp/tests/storage_a', 'b' => 'tmp/tests/storage_b' }
+      allow(Gitlab.config.repositories).to receive(:storages).and_return(storages)
+      allow(subject).to receive(:gitlab_shell).and_return(gitlab_shell)
+
+      allow(Time).to receive(:now).and_return(time)
+    end
+
+    after do
+      FileUtils.rm_rf('tmp/tests/storage_a')
+      FileUtils.rm_rf('tmp/tests/storage_b')
+    end
+
+    context 'without wiki' do
+      let(:project) { create(:project, repository_storage: 'a', repository_read_only: true, wiki_enabled: false) }
+
+      context 'when the move succeeds' do
+        it 'moves the repository to the new storage and unmarks the repository as read only' do
+          expect(gitlab_shell).to receive(:mv_storage).
+            with('tmp/tests/storage_a', project.path_with_namespace, 'tmp/tests/storage_b').
+            and_return(true)
+          expect(GitlabShellWorker).to receive(:perform_async).
+            with(:mv_repository,
+              'tmp/tests/storage_a',
+              project.path_with_namespace,
+              "#{project.path_with_namespace}+#{project.id}+moved+#{time.to_i}")
+
+          subject.execute('b')
+
+          expect(project).not_to be_repository_read_only
+          expect(project.repository_storage).to eq('b')
+        end
+      end
+
+      context 'when the move fails' do
+        it 'unmarks the repository as read-only without updating the repository storage' do
+          expect(gitlab_shell).to receive(:mv_storage).
+            with('tmp/tests/storage_a', project.path_with_namespace, 'tmp/tests/storage_b').
+            and_return(false)
+          expect(GitlabShellWorker).not_to receive(:perform_async)
+
+          subject.execute('b')
+
+          expect(project).not_to be_repository_read_only
+          expect(project.repository_storage).to eq('a')
+        end
+      end
+    end
+
+    context 'with wiki' do
+      let(:project) { create(:project, repository_storage: 'a', repository_read_only: true, wiki_enabled: true) }
+
+      before { project.create_wiki }
+
+      context 'when the move succeeds' do
+        it 'moves the repository and its wiki to the new storage and unmarks the repository as read only' do
+          expect(gitlab_shell).to receive(:mv_storage).
+            with('tmp/tests/storage_a', project.path_with_namespace, 'tmp/tests/storage_b').
+            and_return(true)
+          expect(GitlabShellWorker).to receive(:perform_async).
+            with(:mv_repository,
+              'tmp/tests/storage_a',
+              project.path_with_namespace,
+              "#{project.path_with_namespace}+#{project.id}+moved+#{time.to_i}")
+
+          expect(gitlab_shell).to receive(:mv_storage).
+            with('tmp/tests/storage_a', "#{project.path_with_namespace}.wiki", 'tmp/tests/storage_b').
+            and_return(true)
+          expect(GitlabShellWorker).to receive(:perform_async).
+            with(:mv_repository,
+              'tmp/tests/storage_a',
+              "#{project.path_with_namespace}.wiki",
+              "#{project.path_with_namespace}+#{project.id}+moved+#{time.to_i}.wiki")
+
+          subject.execute('b')
+
+          expect(project).not_to be_repository_read_only
+          expect(project.repository_storage).to eq('b')
+        end
+      end
+
+      context 'when the move of the wiki fails' do
+        it 'unmarks the repository as read-only without updating the repository storage' do
+          expect(gitlab_shell).to receive(:mv_storage).
+            with('tmp/tests/storage_a', project.path_with_namespace, 'tmp/tests/storage_b').
+            and_return(true)
+          expect(gitlab_shell).to receive(:mv_storage).
+            with('tmp/tests/storage_a', "#{project.path_with_namespace}.wiki", 'tmp/tests/storage_b').
+            and_return(false)
+          expect(GitlabShellWorker).not_to receive(:perform_async)
+
+          subject.execute('b')
+
+          expect(project).not_to be_repository_read_only
+          expect(project.repository_storage).to eq('a')
+        end
+      end
+    end
+  end
+end

spec/services/projects/update_service_spec.rb

@@ -139,6 +139,38 @@ describe Projects::UpdateService, services: true do
     end
   end
 
+  describe 'repository_storage' do
+    let(:admin_user) { create(:user, admin: true) }
+    let(:user) { create(:user) }
+    let(:project) { create(:project, repository_storage: 'a') }
+    let(:opts) { { repository_storage: 'b' } }
+
+    before do
+      FileUtils.mkdir('tmp/tests/storage_a')
+      FileUtils.mkdir('tmp/tests/storage_b')
+
+      storages = { 'a' => 'tmp/tests/storage_a', 'b' => 'tmp/tests/storage_b' }
+      allow(Gitlab.config.repositories).to receive(:storages).and_return(storages)
+    end
+
+    after do
+      FileUtils.rm_rf('tmp/tests/storage_a')
+      FileUtils.rm_rf('tmp/tests/storage_b')
+    end
+
+    it 'calls the change repository storage method if the storage changed' do
+      expect(project).to receive(:change_repository_storage).with('b')
+
+      update_project(project, admin_user, opts).inspect
+    end
+
+    it "doesn't call the change repository storage for non-admin users" do
+      expect(project).not_to receive(:change_repository_storage)
+
+      update_project(project, user, opts).inspect
+    end
+  end
+
   def update_project(project, user, opts)
     Projects::UpdateService.new(project, user, opts).execute
   end

spec/tasks/gitlab/mail_google_schema_whitelisting.rb

@@ -20,7 +20,7 @@ describe 'gitlab:mail_google_schema_whitelisting rake task' do
       Rake.application.invoke_task "gitlab:mail_google_schema_whitelisting"
     end
 
-    it 'should run the task without errors' do
+    it 'runs the task without errors' do
       expect { run_rake_task }.not_to raise_error
     end
   end

spec/workers/project_update_repository_storage_worker_spec.rb

+require 'spec_helper'
+
+describe ProjectUpdateRepositoryStorageWorker do
+  let(:project) { create(:project) }
+
+  subject { ProjectUpdateRepositoryStorageWorker.new }
+
+  describe "#perform" do
+    it "should call the update repository storage service" do
+      expect_any_instance_of(Projects::UpdateRepositoryStorageService).
+        to receive(:execute).with('new_storage')
+
+      subject.perform(project.id, 'new_storage')
+    end
+  end
+end