diff options
author | Douglas Barbosa Alexandre <dbalexandre@gmail.com> | 2017-04-03 21:48:09 +0300 |
---|---|---|
committer | Douglas Barbosa Alexandre <dbalexandre@gmail.com> | 2017-04-03 21:50:22 +0300 |
commit | 6143642a5ff2bbc63736b4b05d9583e72527fd5e (patch) | |
tree | 5d7818ad8d53e9bcc3cbcc0894872a2a0647f522 | |
parent | 35fe9c660cc5309e244ce1bde5f65b4e7ec73e4e (diff) |
Refactoring Projects::ImportService
-rw-r--r-- | app/models/repository.rb | 4 | ||||
-rw-r--r-- | app/services/projects/import_service.rb | 50 | ||||
-rw-r--r-- | spec/models/repository_spec.rb | 8 | ||||
-rw-r--r-- | spec/services/projects/import_service_spec.rb | 83 |
4 files changed, 75 insertions, 70 deletions
diff --git a/app/models/repository.rb b/app/models/repository.rb index adc2fd563ff..6b2383b73eb 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -402,10 +402,6 @@ class Repository expire_tags_cache end - def before_import - expire_content_cache - end - # Runs code after the HEAD of a repository is changed. def after_change_head expire_method_caches(METHOD_CACHES_FOR_FILE_TYPES.keys) diff --git a/app/services/projects/import_service.rb b/app/services/projects/import_service.rb index 794cc1556a4..45ad47c8f44 100644 --- a/app/services/projects/import_service.rb +++ b/app/services/projects/import_service.rb @@ -11,7 +11,7 @@ module Projects success rescue => e - error(e.message) + error("Error importing repository #{project.import_url} into #{project.path_with_namespace} - #{e.message}") end private @@ -21,11 +21,7 @@ module Projects # In this case, we only want to import issues, not a repository. create_repository elsif !project.repository_exists? - if project.github_import? || project.gitea_import? - fetch_repository - else - import_repository - end + import_repository end end @@ -36,42 +32,40 @@ module Projects end def import_repository + raise Error, 'Blocked import URL.' if Gitlab::UrlBlocker.blocked_url?(project.import_url) + begin - raise Error, "Blocked import URL." if Gitlab::UrlBlocker.blocked_url?(project.import_url) - gitlab_shell.import_repository(project.repository_storage_path, project.path_with_namespace, project.import_url) - rescue => e + if project.github_import? || project.gitea_import? + fetch_repository + else + clone_repository + end + rescue Gitlab::Shell::Error => e # Expire cache to prevent scenarios such as: # 1. First import failed, but the repo was imported successfully, so +exists?+ returns true # 2. Retried import, repo is broken or not imported but +exists?+ still returns true - project.repository.before_import if project.repository_exists? + project.repository.expire_content_cache if project.repository_exists? - raise Error, "Error importing repository #{project.import_url} into #{project.path_with_namespace} - #{e.message}" + raise Error, e.message end end - def fetch_repository - begin - raise Error, 'Blocked import URL.' if Gitlab::UrlBlocker.blocked_url?(project.import_url) - - project.create_repository - project.repository.add_remote(project.import_type, project.import_url) - project.repository.set_remote_as_mirror (project.import_type) - project.repository.fetch_remote(project.import_type, forced: true) - project.repository.remove_remote(project.import_type) - rescue => e - # Expire cache to prevent scenarios such as: - # 1. First import failed, but the repo was imported successfully, so +exists?+ returns true - # 2. Retried import, repo is broken or not imported but +exists?+ still returns true - project.repository.before_import if project.repository_exists? + def clone_repository + gitlab_shell.import_repository(project.repository_storage_path, project.path_with_namespace, project.import_url) + end - raise Error, "Error importing repository #{project.import_url} into #{project.path_with_namespace} - #{e.message}" - end + def fetch_repository + project.create_repository + project.repository.add_remote(project.import_type, project.import_url) + project.repository.set_remote_as_mirror (project.import_type) + project.repository.fetch_remote(project.import_type, forced: true) + project.repository.remove_remote(project.import_type) end def import_data return unless has_importer? - project.repository.before_import unless project.gitlab_project_import? + project.repository.expire_content_cache unless project.gitlab_project_import? unless importer.execute raise Error, 'The remote data could not be imported.' diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index df742ee8084..b16848f6f4f 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1293,14 +1293,6 @@ describe Repository, models: true do end end - describe '#before_import' do - it 'flushes the repository caches' do - expect(repository).to receive(:expire_content_cache) - - repository.before_import - end - end - describe '#after_import' do it 'flushes and builds the cache' do expect(repository).to receive(:expire_content_cache) diff --git a/spec/services/projects/import_service_spec.rb b/spec/services/projects/import_service_spec.rb index e5917bb0b7a..09cfa36b3b9 100644 --- a/spec/services/projects/import_service_spec.rb +++ b/spec/services/projects/import_service_spec.rb @@ -26,30 +26,59 @@ describe Projects::ImportService, services: true do result = subject.execute expect(result[:status]).to eq :error - expect(result[:message]).to eq 'The repository could not be created.' + expect(result[:message]).to eq "Error importing repository #{project.import_url} into #{project.path_with_namespace} - The repository could not be created." end end context 'with known url' do before do project.import_url = 'https://github.com/vim/vim.git' + project.import_type = 'github' end - it 'succeeds if repository import is successfully' do - expect_any_instance_of(Gitlab::Shell).to receive(:import_repository).with(project.repository_storage_path, project.path_with_namespace, project.import_url).and_return(true) + context 'with a Github repository' do + it 'succeeds if repository import is successfully' do + expect_any_instance_of(Repository).to receive(:fetch_remote).and_return(true) + expect_any_instance_of(Gitlab::GithubImport::Importer).to receive(:execute).and_return(true) - result = subject.execute + result = subject.execute - expect(result[:status]).to eq :success + expect(result[:status]).to eq :success + end + + it 'fails if repository import fails' do + expect_any_instance_of(Repository).to receive(:fetch_remote).and_raise(Gitlab::Shell::Error.new('Failed to import the repository')) + + result = subject.execute + + expect(result[:status]).to eq :error + expect(result[:message]).to eq "Error importing repository #{project.import_url} into #{project.path_with_namespace} - Failed to import the repository" + end end - it 'fails if repository import fails' do - expect_any_instance_of(Gitlab::Shell).to receive(:import_repository).with(project.repository_storage_path, project.path_with_namespace, project.import_url).and_raise(Gitlab::Shell::Error.new('Failed to import the repository')) + context 'with a non Github repository' do + before do + project.import_url = 'https://bitbucket.org/vim/vim.git' + project.import_type = 'bitbucket' + end - result = subject.execute + it 'succeeds if repository import is successfully' do + expect_any_instance_of(Gitlab::Shell).to receive(:import_repository).and_return(true) + expect_any_instance_of(Gitlab::BitbucketImport::Importer).to receive(:execute).and_return(true) - expect(result[:status]).to eq :error - expect(result[:message]).to eq "Error importing repository #{project.import_url} into #{project.path_with_namespace} - Failed to import the repository" + result = subject.execute + + expect(result[:status]).to eq :success + end + + it 'fails if repository import fails' do + expect_any_instance_of(Gitlab::Shell).to receive(:import_repository).and_raise(Gitlab::Shell::Error.new('Failed to import the repository')) + + result = subject.execute + + expect(result[:status]).to eq :error + expect(result[:message]).to eq "Error importing repository #{project.import_url} into #{project.path_with_namespace} - Failed to import the repository" + end end end @@ -64,8 +93,8 @@ describe Projects::ImportService, services: true do end it 'succeeds if importer succeeds' do - expect_any_instance_of(Gitlab::Shell).to receive(:import_repository).with(project.repository_storage_path, project.path_with_namespace, project.import_url).and_return(true) - expect_any_instance_of(Gitlab::GithubImport::Importer).to receive(:execute).and_return(true) + allow_any_instance_of(Repository).to receive(:fetch_remote).and_return(true) + allow_any_instance_of(Gitlab::GithubImport::Importer).to receive(:execute).and_return(true) result = subject.execute @@ -73,48 +102,42 @@ describe Projects::ImportService, services: true do end it 'flushes various caches' do - expect_any_instance_of(Gitlab::Shell).to receive(:import_repository). - with(project.repository_storage_path, project.path_with_namespace, project.import_url). + allow_any_instance_of(Repository).to receive(:fetch_remote). and_return(true) - expect_any_instance_of(Gitlab::GithubImport::Importer).to receive(:execute). + allow_any_instance_of(Gitlab::GithubImport::Importer).to receive(:execute). and_return(true) - expect_any_instance_of(Repository).to receive(:expire_emptiness_caches). - and_call_original - - expect_any_instance_of(Repository).to receive(:expire_exists_cache). - and_call_original + expect_any_instance_of(Repository).to receive(:expire_content_cache) subject.execute end it 'fails if importer fails' do - expect_any_instance_of(Gitlab::Shell).to receive(:import_repository).with(project.repository_storage_path, project.path_with_namespace, project.import_url).and_return(true) - expect_any_instance_of(Gitlab::GithubImport::Importer).to receive(:execute).and_return(false) + allow_any_instance_of(Repository).to receive(:fetch_remote).and_return(true) + allow_any_instance_of(Gitlab::GithubImport::Importer).to receive(:execute).and_return(false) result = subject.execute expect(result[:status]).to eq :error - expect(result[:message]).to eq 'The remote data could not be imported.' + expect(result[:message]).to eq "Error importing repository #{project.import_url} into #{project.path_with_namespace} - The remote data could not be imported." end it 'fails if importer raise an error' do - expect_any_instance_of(Gitlab::Shell).to receive(:import_repository).with(project.repository_storage_path, project.path_with_namespace, project.import_url).and_return(true) - expect_any_instance_of(Gitlab::GithubImport::Importer).to receive(:execute).and_raise(Projects::ImportService::Error.new('Github: failed to connect API')) + allow_any_instance_of(Gitlab::Shell).to receive(:fetch_remote).and_return(true) + allow_any_instance_of(Gitlab::GithubImport::Importer).to receive(:execute).and_raise(Projects::ImportService::Error.new('Github: failed to connect API')) result = subject.execute expect(result[:status]).to eq :error - expect(result[:message]).to eq 'Github: failed to connect API' + expect(result[:message]).to eq "Error importing repository #{project.import_url} into #{project.path_with_namespace} - Github: failed to connect API" end - it 'expires existence cache after error' do + it 'expires content cache after error' do allow_any_instance_of(Project).to receive(:repository_exists?).and_return(false, true) - expect_any_instance_of(Gitlab::Shell).to receive(:import_repository).with(project.repository_storage_path, project.path_with_namespace, project.import_url).and_raise(Gitlab::Shell::Error.new('Failed to import the repository')) - expect_any_instance_of(Repository).to receive(:expire_emptiness_caches).and_call_original - expect_any_instance_of(Repository).to receive(:expire_exists_cache).and_call_original + expect_any_instance_of(Gitlab::Shell).to receive(:fetch_remote).and_raise(Gitlab::Shell::Error.new('Failed to import the repository')) + expect_any_instance_of(Repository).to receive(:expire_content_cache) subject.execute end |