diff --git a/app/models/project.rb b/app/models/project.rb index d5324ceac31..b2c70777e97 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -222,6 +222,7 @@ class Project < ActiveRecord::Base validates :import_url, importable_url: true, if: [:external_import?, :import_url_changed?] validates :star_count, numericality: { greater_than_or_equal_to: 0 } validate :check_limit, on: :create + validate :can_create_repository?, on: [:create, :update], if: ->(project) { !project.persisted? || project.renamed? } validate :avatar_type, if: ->(project) { project.avatar.present? && project.avatar_changed? } validates :avatar, file_size: { maximum: 200.kilobytes.to_i } @@ -468,7 +469,7 @@ class Project < ActiveRecord::Base end def repository_storage_path - Gitlab.config.repositories.storages[repository_storage]['path'] + Gitlab.config.repositories.storages[repository_storage].try(:[], 'path') end def team @@ -583,7 +584,7 @@ class Project < ActiveRecord::Base end def valid_import_url? - valid? || errors.messages[:import_url].nil? + valid?(:import_url) || errors.messages[:import_url].nil? end def create_or_update_import_data(data: nil, credentials: nil) @@ -1000,6 +1001,20 @@ class Project < ActiveRecord::Base end end + # Check if repository already exists on disk + def can_create_repository? + return false unless repository_storage_path + + expires_full_path_cache # we need to clear cache to validate renames correctly + + if gitlab_shell.exists?(repository_storage_path, "#{disk_path}.git") + errors.add(:base, 'There is already a repository with that name on disk') + return false + end + + true + end + def create_repository(force: false) # Forked import is handled asynchronously return if forked? && !force @@ -1486,6 +1501,10 @@ class Project < ActiveRecord::Base self.storage_version.nil? end + def renamed? + persisted? && path_changed? + end + private def storage diff --git a/features/steps/shared/group.rb b/features/steps/shared/group.rb index de119f2c6c0..03bc7e798e0 100644 --- a/features/steps/shared/group.rb +++ b/features/steps/shared/group.rb @@ -36,14 +36,12 @@ module SharedGroup protected def is_member_of(username, groupname, role) - @project_count ||= 0 user = User.find_by(name: username) || create(:user, name: username) group = Group.find_by(name: groupname) || create(:group, name: groupname) group.add_user(user, role) - project ||= create(:project, :repository, namespace: group, path: "project#{@project_count}") + project ||= create(:project, :repository, namespace: group) create(:closed_issue_event, project: project) project.team << [user, :master] - @project_count += 1 end def owned_group diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 9ebda0ba03b..7493b0a8b35 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -101,8 +101,6 @@ FactoryGirl.define do # Test repository - https://gitlab.com/gitlab-org/gitlab-test trait :repository do - path { 'gitlabhq' } - test_repo transient do diff --git a/spec/helpers/blob_helper_spec.rb b/spec/helpers/blob_helper_spec.rb index c654151564e..04620f6d88c 100644 --- a/spec/helpers/blob_helper_spec.rb +++ b/spec/helpers/blob_helper_spec.rb @@ -95,13 +95,13 @@ describe BlobHelper do it 'returns a link with the proper route' do link = edit_blob_link(project, 'master', 'README.md') - expect(Capybara.string(link).find_link('Edit')[:href]).to eq('/gitlab/gitlabhq/edit/master/README.md') + expect(Capybara.string(link).find_link('Edit')[:href]).to eq("/#{project.full_path}/edit/master/README.md") end it 'returns a link with the passed link_opts on the expected route' do link = edit_blob_link(project, 'master', 'README.md', link_opts: { mr_id: 10 }) - expect(Capybara.string(link).find_link('Edit')[:href]).to eq('/gitlab/gitlabhq/edit/master/README.md?mr_id=10') + expect(Capybara.string(link).find_link('Edit')[:href]).to eq("/#{project.full_path}/edit/master/README.md?mr_id=10") end end diff --git a/spec/javascripts/fixtures/blob.rb b/spec/javascripts/fixtures/blob.rb index 2dffc42b0ef..81e8a51a902 100644 --- a/spec/javascripts/fixtures/blob.rb +++ b/spec/javascripts/fixtures/blob.rb @@ -17,6 +17,10 @@ describe Projects::BlobController, '(JavaScript fixtures)', type: :controller do sign_in(admin) end + after do + remove_repository(project) + end + it 'blob/show.html.raw' do |example| get(:show, namespace_id: project.namespace, diff --git a/spec/javascripts/fixtures/branches.rb b/spec/javascripts/fixtures/branches.rb index bb3bdf7c215..4fc072d2585 100644 --- a/spec/javascripts/fixtures/branches.rb +++ b/spec/javascripts/fixtures/branches.rb @@ -17,6 +17,10 @@ describe Projects::BranchesController, '(JavaScript fixtures)', type: :controlle sign_in(admin) end + after do + remove_repository(project) + end + it 'branches/new_branch.html.raw' do |example| get :new, namespace_id: project.namespace.to_param, diff --git a/spec/javascripts/fixtures/dashboard.rb b/spec/javascripts/fixtures/dashboard.rb index 793ffa7c220..7fa351680c9 100644 --- a/spec/javascripts/fixtures/dashboard.rb +++ b/spec/javascripts/fixtures/dashboard.rb @@ -17,6 +17,10 @@ describe Dashboard::ProjectsController, '(JavaScript fixtures)', type: :controll sign_in(admin) end + after do + remove_repository(project) + end + it 'dashboard/user-callout.html.raw' do |example| rendered = render_template('shared/_user_callout') store_frontend_fixture(rendered, example.description) diff --git a/spec/javascripts/fixtures/deploy_keys.rb b/spec/javascripts/fixtures/deploy_keys.rb index bea161c514f..580894ceaf9 100644 --- a/spec/javascripts/fixtures/deploy_keys.rb +++ b/spec/javascripts/fixtures/deploy_keys.rb @@ -16,6 +16,10 @@ describe Projects::DeployKeysController, '(JavaScript fixtures)', type: :control sign_in(admin) end + after do + remove_repository(project) + end + render_views it 'deploy_keys/keys.json' do |example| diff --git a/spec/javascripts/fixtures/issues.rb b/spec/javascripts/fixtures/issues.rb index d3ad50af1b9..0ee2f82dfd6 100644 --- a/spec/javascripts/fixtures/issues.rb +++ b/spec/javascripts/fixtures/issues.rb @@ -17,6 +17,10 @@ describe Projects::IssuesController, '(JavaScript fixtures)', type: :controller sign_in(admin) end + after do + remove_repository(project) + end + it 'issues/open-issue.html.raw' do |example| render_issue(example.description, create(:issue, project: project)) end diff --git a/spec/javascripts/fixtures/jobs.rb b/spec/javascripts/fixtures/jobs.rb index 83a96797506..87d131dfe28 100644 --- a/spec/javascripts/fixtures/jobs.rb +++ b/spec/javascripts/fixtures/jobs.rb @@ -21,6 +21,10 @@ describe Projects::JobsController, '(JavaScript fixtures)', type: :controller do sign_in(admin) end + after do + remove_repository(project) + end + it 'builds/build-with-artifacts.html.raw' do |example| get :show, namespace_id: project.namespace.to_param, diff --git a/spec/javascripts/fixtures/labels.rb b/spec/javascripts/fixtures/labels.rb index 814f065f3a4..b730d557e21 100644 --- a/spec/javascripts/fixtures/labels.rb +++ b/spec/javascripts/fixtures/labels.rb @@ -19,6 +19,10 @@ describe 'Labels (JavaScript fixtures)' do clean_frontend_fixtures('labels/') end + after do + remove_repository(project) + end + describe Groups::LabelsController, '(JavaScript fixtures)', type: :controller do render_views diff --git a/spec/javascripts/fixtures/merge_requests.rb b/spec/javascripts/fixtures/merge_requests.rb index f97a5d2b5de..767db7e1868 100644 --- a/spec/javascripts/fixtures/merge_requests.rb +++ b/spec/javascripts/fixtures/merge_requests.rb @@ -37,6 +37,10 @@ describe Projects::MergeRequestsController, '(JavaScript fixtures)', type: :cont sign_in(admin) end + after do + remove_repository(project) + end + it 'merge_requests/merge_request_with_task_list.html.raw' do |example| create(:ci_build, :pending, pipeline: pipeline) diff --git a/spec/javascripts/fixtures/merge_requests_diffs.rb b/spec/javascripts/fixtures/merge_requests_diffs.rb index 6e0a97d2e3f..ddce00bc0fe 100644 --- a/spec/javascripts/fixtures/merge_requests_diffs.rb +++ b/spec/javascripts/fixtures/merge_requests_diffs.rb @@ -29,6 +29,10 @@ describe Projects::MergeRequests::DiffsController, '(JavaScript fixtures)', type sign_in(admin) end + after do + remove_repository(project) + end + it 'merge_request_diffs/inline_changes_tab_with_comments.json' do |example| create(:diff_note_on_merge_request, project: project, author: admin, position: position, noteable: merge_request) create(:note_on_merge_request, author: admin, project: project, noteable: merge_request) diff --git a/spec/javascripts/fixtures/projects.rb b/spec/javascripts/fixtures/projects.rb index f09d44a49d1..2a100e7fab5 100644 --- a/spec/javascripts/fixtures/projects.rb +++ b/spec/javascripts/fixtures/projects.rb @@ -17,6 +17,10 @@ describe ProjectsController, '(JavaScript fixtures)', type: :controller do sign_in(admin) end + after do + remove_repository(project) + end + it 'projects/dashboard.html.raw' do |example| get :show, namespace_id: project.namespace.to_param, diff --git a/spec/javascripts/fixtures/prometheus_service.rb b/spec/javascripts/fixtures/prometheus_service.rb index 7968c9425f2..f95f8038ffb 100644 --- a/spec/javascripts/fixtures/prometheus_service.rb +++ b/spec/javascripts/fixtures/prometheus_service.rb @@ -18,6 +18,10 @@ describe Projects::ServicesController, '(JavaScript fixtures)', type: :controlle sign_in(admin) end + after do + remove_repository(project) + end + it 'services/prometheus/prometheus_service.html.raw' do |example| get :edit, namespace_id: namespace, diff --git a/spec/javascripts/fixtures/raw.rb b/spec/javascripts/fixtures/raw.rb index 25f5a3b0bb3..82770beb39b 100644 --- a/spec/javascripts/fixtures/raw.rb +++ b/spec/javascripts/fixtures/raw.rb @@ -10,6 +10,10 @@ describe 'Raw files', '(JavaScript fixtures)', type: :controller do clean_frontend_fixtures('blob/notebook/') end + after do + remove_repository(project) + end + it 'blob/notebook/basic.json' do |example| blob = project.repository.blob_at('6d85bb69', 'files/ipython/basic.ipynb') diff --git a/spec/javascripts/fixtures/services.rb b/spec/javascripts/fixtures/services.rb index 80915c32a74..9280ed5a7f1 100644 --- a/spec/javascripts/fixtures/services.rb +++ b/spec/javascripts/fixtures/services.rb @@ -18,6 +18,10 @@ describe Projects::ServicesController, '(JavaScript fixtures)', type: :controlle sign_in(admin) end + after do + remove_repository(project) + end + it 'services/edit_service.html.raw' do |example| get :edit, namespace_id: namespace, diff --git a/spec/javascripts/fixtures/snippet.rb b/spec/javascripts/fixtures/snippet.rb index 01bfb87b0c1..fa97f352e31 100644 --- a/spec/javascripts/fixtures/snippet.rb +++ b/spec/javascripts/fixtures/snippet.rb @@ -18,6 +18,10 @@ describe SnippetsController, '(JavaScript fixtures)', type: :controller do sign_in(admin) end + after do + remove_repository(project) + end + it 'snippets/show.html.raw' do |example| get(:show, id: snippet.to_param) diff --git a/spec/javascripts/fixtures/todos.rb b/spec/javascripts/fixtures/todos.rb index ba630365c18..426b854fe8b 100644 --- a/spec/javascripts/fixtures/todos.rb +++ b/spec/javascripts/fixtures/todos.rb @@ -15,6 +15,10 @@ describe 'Todos (JavaScript fixtures)' do clean_frontend_fixtures('todos/') end + after do + remove_repository(project) + end + describe Dashboard::TodosController, '(JavaScript fixtures)', type: :controller do render_views diff --git a/spec/lib/container_registry/tag_spec.rb b/spec/lib/container_registry/tag_spec.rb index e76463b5e7c..cb4ae3be525 100644 --- a/spec/lib/container_registry/tag_spec.rb +++ b/spec/lib/container_registry/tag_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe ContainerRegistry::Tag do let(:group) { create(:group, name: 'group') } - let(:project) { create(:project, :repository, path: 'test', group: group) } + let(:project) { create(:project, path: 'test', group: group) } let(:repository) do create(:container_repository, name: '', project: project) diff --git a/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb b/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb index bd36d1d309d..6568a0b1bb0 100644 --- a/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb +++ b/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb @@ -13,7 +13,7 @@ describe Gitlab::Email::Handler::CreateIssueHandler do let(:email_raw) { fixture_file('emails/valid_new_issue.eml') } let(:namespace) { create(:namespace, path: 'gitlabhq') } - let!(:project) { create(:project, :public, :repository, namespace: namespace) } + let!(:project) { create(:project, :public, namespace: namespace, path: 'gitlabhq') } let!(:user) do create( :user, diff --git a/spec/lib/gitlab/email/message/repository_push_spec.rb b/spec/lib/gitlab/email/message/repository_push_spec.rb index 83c4d177cae..0ec1f931037 100644 --- a/spec/lib/gitlab/email/message/repository_push_spec.rb +++ b/spec/lib/gitlab/email/message/repository_push_spec.rb @@ -4,7 +4,7 @@ describe Gitlab::Email::Message::RepositoryPush do include RepoHelpers let!(:group) { create(:group, name: 'my_group') } - let!(:project) { create(:project, :repository, name: 'my_project', namespace: group) } + let!(:project) { create(:project, :repository, namespace: group) } let!(:author) { create(:author, name: 'Author') } let(:message) do @@ -38,7 +38,7 @@ describe Gitlab::Email::Message::RepositoryPush do describe '#project_name_with_namespace' do subject { message.project_name_with_namespace } - it { is_expected.to eq 'my_group / my_project' } + it { is_expected.to eq "#{group.name} / #{project.path}" } end describe '#author' do diff --git a/spec/models/container_repository_spec.rb b/spec/models/container_repository_spec.rb index bae88cb1d24..e46945e301e 100644 --- a/spec/models/container_repository_spec.rb +++ b/spec/models/container_repository_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe ContainerRepository do let(:group) { create(:group, name: 'group') } - let(:project) { create(:project, :repository, path: 'test', group: group) } + let(:project) { create(:project, path: 'test', group: group) } let(:repository) do create(:container_repository, name: 'my_image', project: project) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 2e613c44357..eb590f7dae5 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -181,7 +181,7 @@ describe Project do end end - context 'repository storages inclussion' do + context 'repository storages inclusion' do let(:project2) { build(:project, repository_storage: 'missing') } before do diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index 088b7b4fc04..5da634e2fb1 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -1,11 +1,12 @@ require 'spec_helper' describe Projects::CreateService, '#execute' do + let(:gitlab_shell) { Gitlab::Shell.new } let(:user) { create :user } let(:opts) do { - name: "GitLab", - namespace: user.namespace + name: 'GitLab', + namespace_id: user.namespace.id } end @@ -146,6 +147,41 @@ describe Projects::CreateService, '#execute' do expect(project.owner).to eq(user) expect(project.namespace).to eq(user.namespace) end + + context 'when another repository already exists on disk' do + let(:opts) do + { + name: 'Existing', + namespace_id: user.namespace.id + } + end + + let(:repository_storage_path) { Gitlab.config.repositories.storages['default']['path'] } + + before do + gitlab_shell.add_repository(repository_storage_path, "#{user.namespace.full_path}/existing") + end + + after do + gitlab_shell.remove_repository(repository_storage_path, "#{user.namespace.full_path}/existing") + end + + it 'does not allow to create project with same path' do + project = create_project(user, opts) + + expect(project).to respond_to(:errors) + expect(project.errors.messages).to have_key(:base) + expect(project.errors.messages[:base].first).to match('There is already a repository with that name on disk') + end + + it 'does not allow to import a project with the same path' do + project = create_project(user, opts.merge({ import_url: 'https://gitlab.com/gitlab-org/gitlab-test.git' })) + + expect(project).to respond_to(:errors) + expect(project.errors.messages).to have_key(:base) + expect(project.errors.messages[:base].first).to match('There is already a repository with that name on disk') + end + end end context 'when there is an active service template' do diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb index 21c4b30734c..a6e0364d44c 100644 --- a/spec/services/projects/fork_service_spec.rb +++ b/spec/services/projects/fork_service_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Projects::ForkService do + let(:gitlab_shell) { Gitlab::Shell.new } + describe 'fork by user' do before do @from_user = create(:user) @@ -73,6 +75,26 @@ describe Projects::ForkService do end end + context 'repository already exists' do + let(:repository_storage_path) { Gitlab.config.repositories.storages['default']['path'] } + + before do + gitlab_shell.add_repository(repository_storage_path, "#{@to_user.namespace.full_path}/#{@from_project.path}") + end + + after do + gitlab_shell.remove_repository(repository_storage_path, "#{@to_user.namespace.full_path}/#{@from_project.path}") + end + + it 'does not allow creation' do + to_project = fork_project(@from_project, @to_user) + + expect(to_project).not_to be_persisted + expect(to_project.errors.messages).to have_key(:base) + expect(to_project.errors.messages[:base].first).to match('There is already a repository with that name on disk') + end + end + context 'GitLab CI is enabled' do it "forks and enables CI for fork" do @from_project.enable_ci diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index 2cb60cbcfc4..a14ed526f68 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -1,6 +1,7 @@ require 'spec_helper' 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) } @@ -119,6 +120,25 @@ describe Projects::TransferService do it { expect(project.namespace).to eq(user.namespace) } end + context 'namespace which contains orphan repository with same projects path name' do + let(:repository_storage_path) { Gitlab.config.repositories.storages['default']['path'] } + + before do + group.add_owner(user) + gitlab_shell.add_repository(repository_storage_path, "#{group.full_path}/#{project.path}") + + @result = transfer_project(project, user, group) + end + + after do + gitlab_shell.remove_repository(repository_storage_path, "#{group.full_path}/#{project.path}") + end + + it { expect(@result).to eq false } + it { expect(project.namespace).to eq(user.namespace) } + it { expect(project.errors[:new_namespace]).to include('Cannot move project') } + end + def transfer_project(project, user, new_namespace) service = Projects::TransferService.new(project, user) diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index 1b282e82187..92cc9a37795 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -1,6 +1,7 @@ require 'spec_helper' describe Projects::UpdateService, '#execute' do + let(:gitlab_shell) { Gitlab::Shell.new } let(:user) { create(:user) } let(:admin) { create(:admin) } @@ -132,6 +133,28 @@ describe Projects::UpdateService, '#execute' do end end + context 'when renaming a project' do + let(:repository_storage_path) { Gitlab.config.repositories.storages['default']['path'] } + + before do + gitlab_shell.add_repository(repository_storage_path, "#{user.namespace.full_path}/existing") + end + + after do + gitlab_shell.remove_repository(repository_storage_path, "#{user.namespace.full_path}/existing") + end + + it 'does not allow renaming when new path matches existing repository on disk' do + result = update_project(project, admin, path: 'existing') + + expect(result).to include(status: :error) + expect(result[:message]).to match('Project could not be updated!') + expect(project).not_to be_valid + expect(project.errors.messages).to have_key(:base) + expect(project.errors.messages[:base]).to include('There is already a repository with that name on disk') + end + end + context 'when passing invalid parameters' do it 'returns an error result when record cannot be updated' do result = update_project(project, admin, { name: 'foo&bar' }) diff --git a/spec/support/javascript_fixtures_helpers.rb b/spec/support/javascript_fixtures_helpers.rb index aace4b3adee..923c8080e6c 100644 --- a/spec/support/javascript_fixtures_helpers.rb +++ b/spec/support/javascript_fixtures_helpers.rb @@ -31,6 +31,10 @@ module JavaScriptFixturesHelpers File.write(fixture_file_name, fixture) end + def remove_repository(project) + Gitlab::Shell.new.remove_repository(project.repository_storage_path, project.disk_path) + end + private # Private: Prepare a response object for use as a frontend fixture