Merge branch '40744-hashed-storage-specs' into 'master'

Use hashed storage in the specs

Closes #40744

See merge request gitlab-org/gitlab-ce!15681

lib/api/internal.rb

@@ -13,7 +13,7 @@ module API
       #   key_id - ssh key id for Git over SSH
       #   user_id - user id for Git over HTTP
       #   protocol - Git access protocol being used, e.g. HTTP or SSH
-      #   project - project path with namespace
+      #   project - project full_path (not path on disk)
       #   action - git action (git-upload-pack or git-receive-pack)
       #   changes - changes as "oldrev newrev ref", see Gitlab::ChangesList
       post "/allowed" do

spec/controllers/profiles_controller_spec.rb

@@ -69,9 +69,8 @@ describe ProfilesController, :request_store do
 
   describe 'PUT update_username' do
     let(:namespace) { user.namespace }
-    let(:project) { create(:project_empty_repo, namespace: namespace) }
     let(:gitlab_shell) { Gitlab::Shell.new }
-    let(:new_username) { 'renamedtosomethingelse' }
+    let(:new_username) { generate(:username) }
 
     it 'allows username change' do
       sign_in(user)
@@ -85,16 +84,39 @@ describe ProfilesController, :request_store do
       expect(user.username).to eq(new_username)
     end
 
-    it 'moves dependent projects to new namespace' do
-      sign_in(user)
+    context 'with legacy storage' do
+      it 'moves dependent projects to new namespace' do
+        project = create(:project_empty_repo, :legacy_storage, namespace: namespace)
 
-      put :update_username,
-        user: { username: new_username }
+        sign_in(user)
 
-      user.reload
+        put :update_username,
+          user: { username: new_username }
 
-      expect(response.status).to eq(302)
-      expect(gitlab_shell.exists?(project.repository_storage_path, "#{new_username}/#{project.path}.git")).to be_truthy
+        user.reload
+
+        expect(response.status).to eq(302)
+        expect(gitlab_shell.exists?(project.repository_storage_path, "#{new_username}/#{project.path}.git")).to be_truthy
+      end
+    end
+
+    context 'with hashed storage' do
+      it 'keeps repository location unchanged on disk' do
+        project = create(:project_empty_repo, namespace: namespace)
+
+        before_disk_path = project.disk_path
+
+        sign_in(user)
+
+        put :update_username,
+          user: { username: new_username }
+
+        user.reload
+
+        expect(response.status).to eq(302)
+        expect(gitlab_shell.exists?(project.repository_storage_path, "#{project.disk_path}.git")).to be_truthy
+        expect(before_disk_path).to eq(project.disk_path)
+      end
     end
   end
 end

spec/controllers/projects_controller_spec.rb

@@ -288,62 +288,82 @@ describe ProjectsController do
     render_views
 
     let(:admin) { create(:admin) }
-    let(:project) { create(:project, :repository) }
 
     before do
       sign_in(admin)
     end
 
-    context 'when only renaming a project path' do
-      it "sets the repository to the right path after a rename" do
-        expect { update_project path: 'renamed_path' }
-          .to change { project.reload.path }
+    shared_examples_for 'updating a project' do
+      context 'when only renaming a project path' do
+        it "sets the repository to the right path after a rename" do
+          original_repository_path = project.repository.path
 
-        expect(project.path).to include 'renamed_path'
-        expect(assigns(:repository).path).to include project.path
-        expect(response).to have_gitlab_http_status(302)
-      end
-    end
+          expect { update_project path: 'renamed_path' }
+            .to change { project.reload.path }
+          expect(project.path).to include 'renamed_path'
 
-    context 'when project has container repositories with tags' do
-      before do
-        stub_container_registry_config(enabled: true)
-        stub_container_registry_tags(repository: /image/, tags: %w[rc1])
-        create(:container_repository, project: project, name: :image)
+          if project.hashed_storage?(:repository)
+            expect(assigns(:repository).path).to eq(original_repository_path)
+          else
+            expect(assigns(:repository).path).to include(project.path)
+          end
+
+          expect(response).to have_gitlab_http_status(302)
+        end
       end
 
-      it 'does not allow to rename the project' do
-        expect { update_project path: 'renamed_path' }
-          .not_to change { project.reload.path }
+      context 'when project has container repositories with tags' do
+        before do
+          stub_container_registry_config(enabled: true)
+          stub_container_registry_tags(repository: /image/, tags: %w[rc1])
+          create(:container_repository, project: project, name: :image)
+        end
 
-        expect(controller).to set_flash[:alert].to(/container registry tags/)
-        expect(response).to have_gitlab_http_status(200)
+        it 'does not allow to rename the project' do
+          expect { update_project path: 'renamed_path' }
+            .not_to change { project.reload.path }
+
+          expect(controller).to set_flash[:alert].to(/container registry tags/)
+          expect(response).to have_gitlab_http_status(200)
+        end
       end
-    end
 
-    it 'updates Fast Forward Merge attributes' do
-      controller.instance_variable_set(:@project, project)
+      it 'updates Fast Forward Merge attributes' do
+        controller.instance_variable_set(:@project, project)
 
-      params = {
-        merge_method: :ff
-      }
+        params = {
+          merge_method: :ff
+        }
 
-      put :update,
-          namespace_id: project.namespace,
-          id: project.id,
-          project: params
+        put :update,
+            namespace_id: project.namespace,
+            id: project.id,
+            project: params
 
-      expect(response).to have_gitlab_http_status(302)
-      params.each do |param, value|
-        expect(project.public_send(param)).to eq(value)
+        expect(response).to have_gitlab_http_status(302)
+        params.each do |param, value|
+          expect(project.public_send(param)).to eq(value)
+        end
+      end
+
+      def update_project(**parameters)
+        put :update,
+            namespace_id: project.namespace.path,
+            id: project.path,
+            project: parameters
       end
     end
 
-    def update_project(**parameters)
-      put :update,
-          namespace_id: project.namespace.path,
-          id: project.path,
-          project: parameters
+    context 'hashed storage' do
+      let(:project) { create(:project, :repository) }
+
+      it_behaves_like 'updating a project'
+    end
+
+    context 'legacy storage' do
+      let(:project) { create(:project, :repository, :legacy_storage) }
+
+      it_behaves_like 'updating a project'
     end
   end
 

spec/factories/projects.rb

@@ -81,8 +81,10 @@ FactoryBot.define do
       archived true
     end
 
-    trait :hashed do
-      storage_version Project::LATEST_STORAGE_VERSION
+    storage_version Project::LATEST_STORAGE_VERSION
+
+    trait :legacy_storage do
+      storage_version nil
     end
 
     trait :access_requestable do

spec/features/projects/import_export/namespace_export_file_spec.rb

 require 'spec_helper'
 
-feature 'Import/Export - Namespace export file cleanup', :js do
+describe 'Import/Export - Namespace export file cleanup', :js do
   let(:export_path) { Dir.mktmpdir('namespace_export_file_spec') }
 
   before do
@@ -42,13 +42,13 @@ feature 'Import/Export - Namespace export file cleanup', :js do
   end
 
   describe 'legacy storage' do
-    let(:project) { create(:project) }
+    let(:project) { create(:project, :legacy_storage) }
 
     it_behaves_like 'handling project exports on namespace change'
   end
 
   describe 'hashed storage' do
-    let(:project) { create(:project, :hashed) }
+    let(:project) { create(:project) }
 
     it_behaves_like 'handling project exports on namespace change'
   end

spec/lib/backup/repository_spec.rb

@@ -33,10 +33,22 @@ describe Backup::Repository do
         allow(Gitlab::Popen).to receive(:popen).and_return(['error', 1])
       end
 
-      it 'shows the appropriate error' do
-        described_class.new.restore
+      context 'hashed storage' do
+        it 'shows the appropriate error' do
+          described_class.new.restore
 
-        expect(progress).to have_received(:puts).with("Ignoring error on #{project.full_path} - error")
+          expect(progress).to have_received(:puts).with("Ignoring error on #{project.full_path} (#{project.disk_path}) - error")
+        end
+      end
+
+      context 'legacy storage' do
+        let!(:project) { create(:project, :legacy_storage) }
+
+        it 'shows the appropriate error' do
+          described_class.new.restore
+
+          expect(progress).to have_received(:puts).with("Ignoring error on #{project.full_path} - error")
+        end
       end
     end
   end

spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb

@@ -23,8 +23,8 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :sidekiq do
     let!(:appearance) { create_or_update_appearance(logo: uploaded_file, header_logo: uploaded_file) }
     let!(:user1) { create(:user, :with_avatar) }
     let!(:user2) { create(:user, :with_avatar) }
-    let!(:project1) { create(:project, :with_avatar) }
-    let!(:project2) { create(:project, :with_avatar) }
+    let!(:project1) { create(:project, :legacy_storage, :with_avatar) }
+    let!(:project2) { create(:project, :legacy_storage, :with_avatar) }
 
     before do
       UploadService.new(project1, uploaded_file, FileUploader).execute # Markdown upload
@@ -48,7 +48,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :sidekiq do
 
     it 'adds untracked files to the uploads table' do
       expect do
-        subject.perform(1, untracked_files_for_uploads.last.id)
+        subject.perform(1, untracked_files_for_uploads.reorder(:id).last.id)
       end.to change { uploads.count }.from(4).to(8)
 
       expect(user2.uploads.count).to eq(1)
@@ -213,13 +213,13 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :sidekiq do
     end
 
     context 'for a project avatar file path' do
-      let(:model) { create(:project, :with_avatar) }
+      let(:model) { create(:project, :legacy_storage, :with_avatar) }
 
       it_behaves_like 'non_markdown_file'
     end
 
     context 'for a project Markdown attachment (notes, issues, MR descriptions) file path' do
-      let(:model) { create(:project) }
+      let(:model) { create(:project, :legacy_storage) }
 
       before do
         # Upload the file
@@ -304,7 +304,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do
 
     context 'for a project Markdown attachment (notes, issues, MR descriptions) file path' do
       it 'returns the file path relative to the project directory in uploads' do
-        project = create(:project)
+        project = create(:project, :legacy_storage)
         random_hex = SecureRandom.hex
 
         assert_upload_path("/#{project.full_path}/#{random_hex}/Some file.jpg", "#{random_hex}/Some file.jpg")
@@ -357,7 +357,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do
 
     context 'for a project Markdown attachment (notes, issues, MR descriptions) file path' do
       it 'returns FileUploader as a string' do
-        project = create(:project)
+        project = create(:project, :legacy_storage)
 
         assert_uploader("/#{project.full_path}/#{SecureRandom.hex}/Some file.jpg", 'FileUploader')
       end
@@ -409,7 +409,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do
 
     context 'for a project Markdown attachment (notes, issues, MR descriptions) file path' do
       it 'returns Project as a string' do
-        project = create(:project)
+        project = create(:project, :legacy_storage)
 
         assert_model_type("/#{project.full_path}/#{SecureRandom.hex}/Some file.jpg", 'Project')
       end
@@ -461,7 +461,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do
 
     context 'for a project Markdown attachment (notes, issues, MR descriptions) file path' do
       it 'returns the ID as a string' do
-        project = create(:project)
+        project = create(:project, :legacy_storage)
 
         assert_model_id("/#{project.full_path}/#{SecureRandom.hex}/Some file.jpg", project.id)
       end
@@ -483,7 +483,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do
     end
 
     context 'for a project avatar file path' do
-      let(:project) { create(:project, avatar: uploaded_file) }
+      let(:project) { create(:project, :legacy_storage, avatar: uploaded_file) }
       let(:untracked_file) { described_class.create!(path: project.uploads.first.path) }
 
       it 'returns the file size' do
@@ -496,7 +496,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do
     end
 
     context 'for a project Markdown attachment (notes, issues, MR descriptions) file path' do
-      let(:project) { create(:project) }
+      let(:project) { create(:project, :legacy_storage) }
       let(:untracked_file) { create_untracked_file("/#{project.full_path}/#{project.uploads.first.path}") }
 
       before do

spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb

@@ -77,7 +77,7 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do
     context 'when files were uploaded before and after hashed storage was enabled' do
       let!(:appearance) { create_or_update_appearance(logo: uploaded_file, header_logo: uploaded_file) }
       let!(:user) { create(:user, :with_avatar) }
-      let!(:project1) { create(:project, :with_avatar) }
+      let!(:project1) { create(:project, :with_avatar, :legacy_storage) }
       let(:project2) { create(:project) } # instantiate after enabling hashed_storage
 
       before do
@@ -149,7 +149,7 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do
     context 'when files were uploaded before and after hashed storage was enabled' do
       let!(:appearance) { create_or_update_appearance(logo: uploaded_file, header_logo: uploaded_file) }
       let!(:user) { create(:user, :with_avatar) }
-      let!(:project1) { create(:project, :with_avatar) }
+      let!(:project1) { create(:project, :with_avatar, :legacy_storage) }
       let(:project2) { create(:project) } # instantiate after enabling hashed_storage
 
       before do

spec/lib/gitlab/bare_repository_import/importer_spec.rb

@@ -8,11 +8,15 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do
   subject(:importer) { described_class.new(admin, bare_repository) }
 
   before do
+    @rainbow = Rainbow.enabled
+    Rainbow.enabled = false
+
     allow(described_class).to receive(:log)
   end
 
   after do
     FileUtils.rm_rf(base_dir)
+    Rainbow.enabled = @rainbow
   end
 
   shared_examples 'importing a repository' do
@@ -148,7 +152,7 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do
       # This is a quick way to get a valid repository instead of copying an
       # existing one. Since it's not persisted, the importer will try to
       # create the project.
-      project = build(:project, :repository)
+      project = build(:project, :legacy_storage, :repository)
       original_commit_count = project.repository.commit_count
 
       bare_repo = Gitlab::BareRepositoryImport::Repository.new(project.repository_storage_path, project.repository.path)

spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces_spec.rb

@@ -94,7 +94,7 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameNamespaces, :
   describe '#move_repositories' do
     let(:namespace) { create(:group, name: 'hello-group') }
     it 'moves a project for a namespace' do
-      create(:project, :repository, namespace: namespace, path: 'hello-project')
+      create(:project, :repository, :legacy_storage, namespace: namespace, path: 'hello-project')
       expected_path = File.join(TestEnv.repos_path, 'bye-group', 'hello-project.git')
 
       subject.move_repositories(namespace, 'hello-group', 'bye-group')
@@ -104,7 +104,7 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameNamespaces, :
 
     it 'moves a namespace in a subdirectory correctly' do
       child_namespace = create(:group, name: 'sub-group', parent: namespace)
-      create(:project, :repository, namespace: child_namespace, path: 'hello-project')
+      create(:project, :repository, :legacy_storage, namespace: child_namespace, path: 'hello-project')
 
       expected_path = File.join(TestEnv.repos_path, 'hello-group', 'renamed-sub-group', 'hello-project.git')
 
@@ -115,7 +115,7 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameNamespaces, :
 
     it 'moves a parent namespace with subdirectories' do
       child_namespace = create(:group, name: 'sub-group', parent: namespace)
-      create(:project, :repository, namespace: child_namespace, path: 'hello-project')
+      create(:project, :repository, :legacy_storage, namespace: child_namespace, path: 'hello-project')
       expected_path = File.join(TestEnv.repos_path, 'renamed-group', 'sub-group', 'hello-project.git')
 
       subject.move_repositories(child_namespace, 'hello-group', 'renamed-group')
@@ -166,7 +166,7 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameNamespaces, :
 
   describe '#rename_namespace_dependencies' do
     it "moves the the repository for a project in the namespace" do
-      create(:project, :repository, namespace: namespace, path: "the-path-project")
+      create(:project, :repository, :legacy_storage, namespace: namespace, path: "the-path-project")
       expected_repo = File.join(TestEnv.repos_path, "the-path0", "the-path-project.git")
 
       subject.rename_namespace_dependencies(namespace, 'the-path', 'the-path0')
@@ -187,7 +187,7 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameNamespaces, :
     end
 
     it 'invalidates the markdown cache of related projects' do
-      project = create(:project, namespace: namespace, path: "the-path-project")
+      project = create(:project, :legacy_storage, namespace: namespace, path: "the-path-project")
 
       expect(subject).to receive(:remove_cached_html_for_projects).with([project.id])
 
@@ -243,7 +243,7 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameNamespaces, :
 
   describe '#revert_renames', :redis do
     it 'renames the routes back to the previous values' do
-      project = create(:project, :repository, path: 'a-project', namespace: namespace)
+      project = create(:project, :legacy_storage, :repository, path: 'a-project', namespace: namespace)
       subject.rename_namespace(namespace)
 
       expect(subject).to receive(:perform_rename)
@@ -261,7 +261,7 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameNamespaces, :
     end
 
     it 'moves the repositories back to their original place' do
-      project = create(:project, :repository, path: 'a-project', namespace: namespace)
+      project = create(:project, :repository, :legacy_storage, path: 'a-project', namespace: namespace)
       project.create_repository
       subject.rename_namespace(namespace)
 

spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects_spec.rb

@@ -5,6 +5,7 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameProjects, :de
   let(:subject) { described_class.new(['the-path'], migration) }
   let(:project) do
     create(:project,
+           :legacy_storage,
            path: 'the-path',
            namespace: create(:namespace, path: 'known-parent' ))
   end
@@ -17,7 +18,7 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameProjects, :de
   describe '#projects_for_paths' do
     it 'searches using nested paths' do
       namespace = create(:namespace, path: 'hello')
-      project = create(:project, path: 'THE-path', namespace: namespace)
+      project = create(:project, :legacy_storage, path: 'THE-path', namespace: namespace)
 
       result_ids = described_class.new(['Hello/the-path'], migration)
                      .projects_for_paths.map(&:id)
@@ -26,8 +27,8 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameProjects, :de
     end
 
     it 'includes the correct projects' do
-      project = create(:project, path: 'THE-path')
-      _other_project = create(:project)
+      project = create(:project, :legacy_storage, path: 'THE-path')
+      _other_project = create(:project, :legacy_storage)
 
       result_ids = subject.projects_for_paths.map(&:id)
 
@@ -36,7 +37,7 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameProjects, :de
   end
 
   describe '#rename_projects' do
-    let!(:projects) { create_list(:project, 2, path: 'the-path') }
+    let!(:projects) { create_list(:project, 2, :legacy_storage, path: 'the-path') }
 
     it 'renames each project' do
       expect(subject).to receive(:rename_project).twice
@@ -120,7 +121,7 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameProjects, :de
 
   describe '#move_repository' do
     let(:known_parent) { create(:namespace, path: 'known-parent') }
-    let(:project) { create(:project, :repository, path: 'the-path', namespace: known_parent) }
+    let(:project) { create(:project, :repository, :legacy_storage, path: 'the-path', namespace: known_parent) }
 
     it 'moves the repository for a project' do
       expected_path = File.join(TestEnv.repos_path, 'known-parent', 'new-repo.git')

spec/lib/gitlab/email/attachment_uploader_spec.rb

@@ -2,7 +2,7 @@ require "spec_helper"
 
 describe Gitlab::Email::AttachmentUploader do
   describe "#execute" do
-    let(:project) { build(:project) }
+    let(:project) { create(:project) }
     let(:message_raw) { fixture_file("emails/attachment.eml") }
     let(:message) { Mail::Message.new(message_raw) }
 

spec/lib/gitlab/gfm/uploads_rewriter_spec.rb

@@ -39,8 +39,8 @@ describe Gitlab::Gfm::UploadsRewriter do
       it 'copies files' do
         expect(new_files).to all(exist)
         expect(old_paths).not_to match_array new_paths
-        expect(old_paths).to all(include(old_project.full_path))
-        expect(new_paths).to all(include(new_project.full_path))
+        expect(old_paths).to all(include(old_project.disk_path))
+        expect(new_paths).to all(include(new_project.disk_path))
       end
 
       it 'does not remove old files' do

spec/lib/gitlab/import_export/uploads_restorer_spec.rb

@@ -16,7 +16,7 @@ describe Gitlab::ImportExport::UploadsRestorer do
     end
 
     describe 'legacy storage' do
-      let(:project) { create(:project) }
+      let(:project) { create(:project, :legacy_storage) }
 
       subject(:restorer) { described_class.new(project: project, shared: shared) }
 
@@ -34,7 +34,7 @@ describe Gitlab::ImportExport::UploadsRestorer do
     end
 
     describe 'hashed storage' do
-      let(:project) { create(:project, :hashed) }
+      let(:project) { create(:project) }
 
       subject(:restorer) { described_class.new(project: project, shared: shared) }
 

spec/lib/gitlab/import_export/uploads_saver_spec.rb

@@ -15,7 +15,7 @@ describe Gitlab::ImportExport::UploadsSaver do
     end
 
     describe 'legacy storage' do
-      let(:project) { create(:project) }
+      let(:project) { create(:project, :legacy_storage) }
 
       subject(:saver) { described_class.new(shared: shared, project: project) }
 
@@ -37,7 +37,7 @@ describe Gitlab::ImportExport::UploadsSaver do
     end
 
     describe 'hashed storage' do
-      let(:project) { create(:project, :hashed) }
+      let(:project) { create(:project) }
 
       subject(:saver) { described_class.new(shared: shared, project: project) }
 

spec/lib/gitlab/repo_path_spec.rb

@@ -6,11 +6,11 @@ describe ::Gitlab::RepoPath do
 
     context 'a repository storage path' do
       it 'parses a full repository path' do
-        expect(described_class.parse(project.repository.path)).to eq([project, false, nil])
+        expect(described_class.parse(project.repository.full_path)).to eq([project, false, nil])
       end
 
       it 'parses a full wiki path' do
-        expect(described_class.parse(project.wiki.repository.path)).to eq([project, true, nil])
+        expect(described_class.parse(project.wiki.repository.full_path)).to eq([project, true, nil])
       end
     end
 

spec/lib/gitlab/shell_spec.rb

@@ -443,7 +443,7 @@ describe Gitlab::Shell do
     end
 
     describe '#remove_repository' do
-      let!(:project) { create(:project, :repository) }
+      let!(:project) { create(:project, :repository, :legacy_storage) }
       let(:disk_path) { "#{project.disk_path}.git" }
 
       it 'returns true when the command succeeds' do

spec/lib/gitlab/workhorse_spec.rb

@@ -324,7 +324,7 @@ describe Gitlab::Workhorse do
       it 'includes a Repository param' do
         repo_param = {
           storage_name: 'default',
-          relative_path: project.full_path + '.git',
+          relative_path: project.disk_path + '.git',
           gl_repository: "project-#{project.id}"
         }
 

spec/migrations/migrate_process_commit_worker_jobs_spec.rb

@@ -4,7 +4,7 @@ require 'spec_helper'
 require Rails.root.join('db', 'migrate', '20161124141322_migrate_process_commit_worker_jobs.rb')
 
 describe MigrateProcessCommitWorkerJobs do
-  let(:project) { create(:project, :repository) }
+  let(:project) { create(:project, :legacy_storage, :repository) }
   let(:user) { create(:user) }
   let(:commit) { project.commit.raw.rugged_commit }
 

spec/migrations/turn_nested_groups_into_regular_groups_for_mysql_spec.rb

@@ -4,7 +4,7 @@ require Rails.root.join('db', 'migrate', '20170503140202_turn_nested_groups_into
 describe TurnNestedGroupsIntoRegularGroupsForMysql do
   let!(:parent_group) { create(:group) }
   let!(:child_group) { create(:group, parent: parent_group) }
-  let!(:project) { create(:project, :empty_repo, namespace: child_group) }
+  let!(:project) { create(:project, :legacy_storage, :empty_repo, namespace: child_group) }
   let!(:member) { create(:user) }
   let(:migration) { described_class.new }
 

spec/models/project_spec.rb

@@ -2504,6 +2504,7 @@ describe Project do
   end
 
   describe '#remove_exports' do
+    let(:legacy_project) { create(:project, :legacy_storage, :with_export) }
     let(:project) { create(:project, :with_export) }
 
     it 'removes the exports directory for the project' do
@@ -2516,15 +2517,29 @@ describe Project do
       expect(File.exist?(project.export_path)).to be_falsy
     end
 
-    it 'is a no-op when there is no namespace' do
+    it 'is a no-op on legacy projects when there is no namespace' do
+      export_path = legacy_project.export_path
+
+      legacy_project.update_column(:namespace_id, nil)
+
+      expect(FileUtils).not_to receive(:rm_rf).with(export_path)
+
+      legacy_project.remove_exports
+
+      expect(File.exist?(export_path)).to be_truthy
+    end
+
+    it 'runs on hashed storage projects when there is no namespace' do
       export_path = project.export_path
+
       project.update_column(:namespace_id, nil)
 
-      expect(FileUtils).not_to receive(:rm_rf).with(export_path)
+      allow(FileUtils).to receive(:rm_rf).and_call_original
+      expect(FileUtils).to receive(:rm_rf).with(export_path).and_call_original
 
       project.remove_exports
 
-      expect(File.exist?(export_path)).to be_truthy
+      expect(File.exist?(export_path)).to be_falsy
     end
 
     it 'is run when the project is destroyed' do
@@ -2545,7 +2560,7 @@ describe Project do
   end
 
   context 'legacy storage' do
-    let(:project) { create(:project, :repository) }
+    let(:project) { create(:project, :repository, :legacy_storage) }
     let(:gitlab_shell) { Gitlab::Shell.new }
     let(:project_storage) { project.send(:storage) }
 
@@ -2719,6 +2734,8 @@ describe Project do
     let(:project) { create(:project, :repository, skip_disk_validation: true) }
     let(:gitlab_shell) { Gitlab::Shell.new }
     let(:hash) { Digest::SHA2.hexdigest(project.id.to_s) }
+    let(:hashed_prefix) { File.join('@hashed', hash[0..1], hash[2..3]) }
+    let(:hashed_path) { File.join(hashed_prefix, hash) }
 
     before do
       stub_application_setting(hashed_storage_enabled: true)
@@ -2744,14 +2761,12 @@ describe Project do
 
     describe '#base_dir' do
       it 'returns base_dir based on hash of project id' do
-        expect(project.base_dir).to eq("@hashed/#{hash[0..1]}/#{hash[2..3]}")
+        expect(project.base_dir).to eq(hashed_prefix)
       end
     end
 
     describe '#disk_path' do
       it 'returns disk_path based on hash of project id' do
-        hashed_path = "@hashed/#{hash[0..1]}/#{hash[2..3]}/#{hash}"
-
         expect(project.disk_path).to eq(hashed_path)
       end
     end
@@ -2760,7 +2775,7 @@ describe Project do
       it 'delegates to gitlab_shell to ensure namespace is created' do
         allow(project).to receive(:gitlab_shell).and_return(gitlab_shell)
 
-        expect(gitlab_shell).to receive(:add_namespace).with(project.repository_storage_path, "@hashed/#{hash[0..1]}/#{hash[2..3]}")
+        expect(gitlab_shell).to receive(:add_namespace).with(project.repository_storage_path, hashed_prefix)
 
         project.ensure_storage_path_exists
       end

spec/requests/api/internal_spec.rb

@@ -366,20 +366,9 @@ describe API::Internal do
           end
         end
 
-        context 'project as /namespace/project' do
-          it do
-            push(key, project_with_repo_path('/' + project.full_path))
-
-            expect(response).to have_gitlab_http_status(200)
-            expect(json_response["status"]).to be_truthy
-            expect(json_response["repository_path"]).to eq(project.repository.path_to_repo)
-            expect(json_response["gl_repository"]).to eq("project-#{project.id}")
-          end
-        end
-
         context 'project as namespace/project' do
           it do
-            push(key, project_with_repo_path(project.full_path))
+            push(key, project)
 
             expect(response).to have_gitlab_http_status(200)
             expect(json_response["status"]).to be_truthy
@@ -496,8 +485,10 @@ describe API::Internal do
     end
 
     context 'project does not exist' do
-      it do
-        pull(key, project_with_repo_path('gitlab/notexist'))
+      it 'returns a 200 response with status: false' do
+        project.destroy
+
+        pull(key, project)
 
         expect(response).to have_gitlab_http_status(200)
         expect(json_response["status"]).to be_falsey
@@ -569,6 +560,7 @@ describe API::Internal do
     end
 
     context 'the project path was changed' do
+      let(:project) { create(:project, :repository, :legacy_storage) }
       let!(:old_path_to_repo) { project.repository.path_to_repo }
       let!(:repository) { project.repository }
 
@@ -858,9 +850,14 @@ describe API::Internal do
     end
   end
 
-  def project_with_repo_path(path)
-    double().tap do |fake_project|
-      allow(fake_project).to receive_message_chain('repository.path_to_repo' => path)
+  def gl_repository_for(project_or_wiki)
+    case project_or_wiki
+    when ProjectWiki
+      project_or_wiki.project.gl_repository(is_wiki: true)
+    when Project
+      project_or_wiki.gl_repository(is_wiki: false)
+    else
+      nil
     end
   end
 
@@ -868,18 +865,8 @@ describe API::Internal do
     post(
       api("/internal/allowed"),
       key_id: key.id,
-      project: project.repository.path_to_repo,
-      action: 'git-upload-pack',
-      secret_token: secret_token,
-      protocol: protocol
-    )
-  end
-
-  def pull_with_path(key, path_to_repo, protocol = 'ssh')
-    post(
-      api("/internal/allowed"),
-      key_id: key.id,
-      project: path_to_repo,
+      project: project.full_path,
+      gl_repository: gl_repository_for(project),
       action: 'git-upload-pack',
       secret_token: secret_token,
       protocol: protocol
@@ -891,20 +878,8 @@ describe API::Internal do
       api("/internal/allowed"),
       changes: 'd14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master',
       key_id: key.id,
-      project: project.repository.path_to_repo,
-      action: 'git-receive-pack',
-      secret_token: secret_token,
-      protocol: protocol,
-      env: env
-    )
-  end
-
-  def push_with_path(key, path_to_repo, protocol = 'ssh', env: nil)
-    post(
-      api("/internal/allowed"),
-      changes: 'd14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master',
-      key_id: key.id,
-      project: path_to_repo,
+      project: project.full_path,
+      gl_repository: gl_repository_for(project),
       action: 'git-receive-pack',
       secret_token: secret_token,
       protocol: protocol,
@@ -917,7 +892,8 @@ describe API::Internal do
       api("/internal/allowed"),
       ref: 'master',
       key_id: key.id,
-      project: project.repository.path_to_repo,
+      project: project.full_path,
+      gl_repository: gl_repository_for(project),
       action: 'git-upload-archive',
       secret_token: secret_token,
       protocol: 'ssh'
@@ -929,7 +905,7 @@ describe API::Internal do
       api("/internal/lfs_authenticate"),
       key_id: key_id,
       secret_token: secret_token,
-      project: project.repository.path_to_repo
+      project: project.full_path
     )
   end
 end

spec/requests/api/projects_spec.rb

@@ -460,7 +460,7 @@ describe API::Projects do
       expect(response).to have_gitlab_http_status(201)
 
       project.each_pair do |k, v|
-        next if %i[has_external_issue_tracker issues_enabled merge_requests_enabled wiki_enabled].include?(k)
+        next if %i[has_external_issue_tracker issues_enabled merge_requests_enabled wiki_enabled storage_version].include?(k)
 
         expect(json_response[k.to_s]).to eq(v)
       end
@@ -622,12 +622,8 @@ describe API::Projects do
   end
 
   describe 'POST /projects/user/:id' do
-    before do
-      expect(project).to be_persisted
-    end
-
     it 'creates new project without path but with name and return 201' do
-      expect { post api("/projects/user/#{user.id}", admin), name: 'Foo Project' }.to change {Project.count}.by(1)
+      expect { post api("/projects/user/#{user.id}", admin), name: 'Foo Project' }.to change { Project.count }.by(1)
       expect(response).to have_gitlab_http_status(201)
 
       project = Project.last
@@ -666,8 +662,9 @@ describe API::Projects do
       post api("/projects/user/#{user.id}", admin), project
 
       expect(response).to have_gitlab_http_status(201)
+
       project.each_pair do |k, v|
-        next if %i[has_external_issue_tracker path].include?(k)
+        next if %i[has_external_issue_tracker path storage_version].include?(k)
 
         expect(json_response[k.to_s]).to eq(v)
       end

spec/requests/api/v3/projects_spec.rb

@@ -401,7 +401,7 @@ describe API::V3::Projects do
       post v3_api('/projects', user), project
 
       project.each_pair do |k, v|
-        next if %i[has_external_issue_tracker issues_enabled merge_requests_enabled wiki_enabled].include?(k)
+        next if %i[storage_version has_external_issue_tracker issues_enabled merge_requests_enabled wiki_enabled].include?(k)
 
         expect(json_response[k.to_s]).to eq(v)
       end
@@ -545,7 +545,7 @@ describe API::V3::Projects do
 
       expect(response).to have_gitlab_http_status(201)
       project.each_pair do |k, v|
-        next if %i[has_external_issue_tracker path].include?(k)
+        next if %i[storage_version has_external_issue_tracker path].include?(k)
 
         expect(json_response[k.to_s]).to eq(v)
       end

spec/requests/git_http_spec.rb

@@ -163,7 +163,7 @@ describe 'Git HTTP requests' do
             download(path) do |response|
               json_body = ActiveSupport::JSON.decode(response.body)
 
-              expect(json_body['RepoPath']).to include(wiki.repository.full_path)
+              expect(json_body['RepoPath']).to include(wiki.repository.disk_path)
             end
           end
         end

spec/services/groups/destroy_service_spec.rb

@@ -6,7 +6,7 @@ describe Groups::DestroyService do
   let!(:user)         { create(:user) }
   let!(:group)        { create(:group) }
   let!(:nested_group) { create(:group, parent: group) }
-  let!(:project)      { create(:project, namespace: group) }
+  let!(:project)      { create(:project, :legacy_storage, namespace: group) }
   let!(:notification_setting) { create(:notification_setting, source: group)}
   let(:gitlab_shell) { Gitlab::Shell.new }
   let(:remove_path)  { group.path + "+#{group.id}+deleted" }
@@ -141,7 +141,7 @@ describe Groups::DestroyService do
     end
 
     context 'legacy storage' do
-      let!(:project) { create(:project, :empty_repo, namespace: group) }
+      let!(:project) { create(:project, :legacy_storage, :empty_repo, namespace: group) }
 
       it 'removes repository' do
         expect(gitlab_shell.exists?(project.repository_storage_path, "#{project.disk_path}.git")).to be_falsey
@@ -149,7 +149,7 @@ describe Groups::DestroyService do
     end
 
     context 'hashed storage' do
-      let!(:project) { create(:project, :hashed, :empty_repo, namespace: group) }
+      let!(:project) { create(:project, :empty_repo, namespace: group) }
 
       it 'removes repository' do
         expect(gitlab_shell.exists?(project.repository_storage_path, "#{project.disk_path}.git")).to be_falsey

spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb

@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe Projects::HashedStorage::MigrateAttachmentsService do
   subject(:service) { described_class.new(project) }
-  let(:project) { create(:project) }
+  let(:project) { create(:project, :legacy_storage) }
   let(:legacy_storage) { Storage::LegacyProject.new(project) }
   let(:hashed_storage) { Storage::HashedProject.new(project) }
 

spec/services/projects/hashed_storage/migrate_repository_service_spec.rb

@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe Projects::HashedStorage::MigrateRepositoryService do
   let(:gitlab_shell) { Gitlab::Shell.new }
-  let(:project) { create(:project, :repository, :wiki_repo) }
+  let(:project) { create(:project, :legacy_storage, :repository, :wiki_repo) }
   let(:service) { described_class.new(project) }
   let(:legacy_storage) { Storage::LegacyProject.new(project) }
   let(:hashed_storage) { Storage::HashedProject.new(project) }

spec/services/projects/hashed_storage_migration_service_spec.rb

 require 'spec_helper'
 
 describe Projects::HashedStorageMigrationService do
-  let(:project) { create(:project, :empty_repo, :wiki_repo) }
+  let(:project) { create(:project, :empty_repo, :wiki_repo, :legacy_storage) }
   subject(:service) { described_class.new(project) }
 
   describe '#execute' do

spec/services/projects/transfer_service_spec.rb

@@ -4,7 +4,7 @@ describe Projects::TransferService do
   let(:gitlab_shell) { Gitlab::Shell.new }
   let(:user) { create(:user) }
   let(:group) { create(:group) }
-  let(:project) { create(:project, :repository, namespace: user.namespace) }
+  let(:project) { create(:project, :repository, :legacy_storage, namespace: user.namespace) }
 
   context 'namespace -> namespace' do
     before do
@@ -214,7 +214,7 @@ describe Projects::TransferService do
   end
 
   context 'when hashed storage in use' do
-    let(:hashed_project) { create(:project, :repository, :hashed, namespace: user.namespace) }
+    let(:hashed_project) { create(:project, :repository, namespace: user.namespace) }
 
     before do
       group.add_owner(user)

spec/services/projects/update_service_spec.rb

@@ -150,6 +150,8 @@ describe Projects::UpdateService do
       let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage]['path'] }
 
       context 'with legacy storage' do
+        let(:project) { create(:project, :legacy_storage, :repository, creator: user, namespace: user.namespace) }
+
         before do
           gitlab_shell.add_repository(repository_storage, "#{user.namespace.full_path}/existing")
         end

spec/services/users/destroy_service_spec.rb

@@ -173,7 +173,7 @@ describe Users::DestroyService do
       end
 
       context 'legacy storage' do
-        let!(:project) { create(:project, :empty_repo, namespace: user.namespace) }
+        let!(:project) { create(:project, :empty_repo, :legacy_storage, namespace: user.namespace) }
 
         it 'removes repository' do
           expect(gitlab_shell.exists?(project.repository_storage_path, "#{project.disk_path}.git")).to be_falsey
@@ -181,7 +181,7 @@ describe Users::DestroyService do
       end
 
       context 'hashed storage' do
-        let!(:project) { create(:project, :empty_repo, :hashed, namespace: user.namespace) }
+        let!(:project) { create(:project, :empty_repo, namespace: user.namespace) }
 
         it 'removes repository' do
           expect(gitlab_shell.exists?(project.repository_storage_path, "#{project.disk_path}.git")).to be_falsey

spec/uploaders/file_uploader_spec.rb

@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe FileUploader do
   let(:group) { create(:group, name: 'awesome') }
-  let(:project) { create(:project, namespace: group, name: 'project') }
+  let(:project) { create(:project, :legacy_storage, namespace: group, name: 'project') }
   let(:uploader) { described_class.new(project) }
   let(:upload)  { double(model: project, path: 'secret/foo.jpg') }
 
@@ -16,12 +16,12 @@ describe FileUploader do
 
   shared_examples 'uses hashed storage' do
     context 'when rolled out attachments' do
+      let(:project) { build_stubbed(:project, namespace: group, name: 'project') }
+
       before do
         allow(project).to receive(:disk_path).and_return('ca/fe/fe/ed')
       end
 
-      let(:project) { build_stubbed(:project, :hashed, namespace: group, name: 'project') }
-
       it_behaves_like 'builds correct paths',
                       store_dir: %r{ca/fe/fe/ed/\h+},
                       absolute_path: %r{#{described_class.root}/ca/fe/fe/ed/secret/foo.jpg}

spec/workers/repository_fork_worker_spec.rb

@@ -68,7 +68,7 @@ describe RepositoryForkWorker do
     end
 
     it "handles bad fork" do
-      error_message = "Unable to fork project #{fork_project.id} for repository #{project.full_path} -> #{fork_project.full_path}"
+      error_message = "Unable to fork project #{fork_project.id} for repository #{project.disk_path} -> #{fork_project.disk_path}"
 
       expect_fork_repository.and_return(false)
 

spec/workers/storage_migrator_worker_spec.rb

@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe StorageMigratorWorker do
   subject(:worker) { described_class.new }
-  let(:projects) { create_list(:project, 2) }
+  let(:projects) { create_list(:project, 2, :legacy_storage) }
 
   describe '#perform' do
     let(:ids) { projects.map(&:id) }