From 812a2c193b8c92e9bd5e640e806df9cbe929e9d6 Mon Sep 17 00:00:00 2001 From: Travis Miller Date: Tue, 5 Dec 2017 22:27:23 -0600 Subject: [PATCH 01/16] Add project export API documentation --- doc/api/project_import_export.md | 78 ++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/doc/api/project_import_export.md b/doc/api/project_import_export.md index e442442c750..7c2c619b97b 100644 --- a/doc/api/project_import_export.md +++ b/doc/api/project_import_export.md @@ -4,6 +4,84 @@ [See also the project import/export documentation](../user/project/settings/import_export.md) +## Export start + +Start a new export. + +```http +POST /projects/:id/export +``` + +| Attribute | Type | Required | Description | +| --------- | -------------- | -------- | ---------------------------------------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | + +```console +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/1/export +``` + +```json +{ + "message": "202 Accepted" +} +``` + +## Export status + +Get the status of export. + +```http +GET /projects/:id/export +``` + +| Attribute | Type | Required | Description | +| --------- | -------------- | -------- | ---------------------------------------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | + +```console +curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/1/export +``` + +Status can be one of `none`, `started`, or `finished`. + +```json +{ + "id": 1, + "description": "Itaque perspiciatis minima aspernatur corporis consequatur.", + "name": "Gitlab Test", + "name_with_namespace": "Gitlab Org / Gitlab Test", + "path": "gitlab-test", + "path_with_namespace": "gitlab-org/gitlab-test", + "created_at": "2017-08-29T04:36:44.383Z", + "export_status": "finished", + "_links": { + "api_url": "https://gitlab.example.com/api/v4/projects/1/export/download", + "web_url": "https://gitlab.example.com/gitlab-org/gitlab-test/download_export", + } +} +``` + +## Export download + +Download the finished export. + +```http +GET /projects/:id/export +``` + +| Attribute | Type | Required | Description | +| --------- | -------------- | -------- | ---------------------------------------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | + +```console +curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" --remote-header-name --remote-name https://gitlab.example.com/api/v4/projects/5/export/download +``` + +```console +ls *export.tar.gz +2017-12-05_22-11-148_namespace_project_export.tar.gz +``` + ## Import a file ```http From 0bf6ed2e367a5f110405336880f33443c47848f2 Mon Sep 17 00:00:00 2001 From: Travis Miller Date: Tue, 5 Dec 2017 22:27:45 -0600 Subject: [PATCH 02/16] Add project export API schema --- .../public_api/v4/project/export_status.json | 17 +++++++++++++++ .../public_api/v4/project/identity.json | 21 +++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 spec/fixtures/api/schemas/public_api/v4/project/export_status.json create mode 100644 spec/fixtures/api/schemas/public_api/v4/project/identity.json diff --git a/spec/fixtures/api/schemas/public_api/v4/project/export_status.json b/spec/fixtures/api/schemas/public_api/v4/project/export_status.json new file mode 100644 index 00000000000..d24a6f93f4b --- /dev/null +++ b/spec/fixtures/api/schemas/public_api/v4/project/export_status.json @@ -0,0 +1,17 @@ +{ + "type": "object", + "allOf": [ + { "$ref": "identity.json" }, + { + "required": [ + "export_status" + ], + "properties": { + "export_status": { + "type": "string", + "enum": ["none", "started", "finished"] + } + } + } + ] +} diff --git a/spec/fixtures/api/schemas/public_api/v4/project/identity.json b/spec/fixtures/api/schemas/public_api/v4/project/identity.json new file mode 100644 index 00000000000..e35ab023d44 --- /dev/null +++ b/spec/fixtures/api/schemas/public_api/v4/project/identity.json @@ -0,0 +1,21 @@ +{ + "type": "object", + "required": [ + "id", + "description", + "name", + "name_with_namespace", + "path", + "path_with_namespace", + "created_at" + ], + "properties": { + "id": { "type": "integer" }, + "description": { "type": ["string", "null"] }, + "name": { "type": "string" }, + "name_with_namespace": { "type": "string" }, + "path": { "type": "string" }, + "path_with_namespace": { "type": "string" }, + "created_at": { "type": "date" } + } +} From b20408372b6811520edc3e35702bfe9b053b013b Mon Sep 17 00:00:00 2001 From: Travis Miller Date: Tue, 5 Dec 2017 22:28:12 -0600 Subject: [PATCH 03/16] Add project export API tests --- spec/requests/api/project_export_spec.rb | 290 +++++++++++++++++++++++ 1 file changed, 290 insertions(+) create mode 100644 spec/requests/api/project_export_spec.rb diff --git a/spec/requests/api/project_export_spec.rb b/spec/requests/api/project_export_spec.rb new file mode 100644 index 00000000000..146bf3fd648 --- /dev/null +++ b/spec/requests/api/project_export_spec.rb @@ -0,0 +1,290 @@ +require 'spec_helper' + +describe API::ProjectExport do + set(:project) { create(:project) } + set(:project_none) { create(:project, path: 'export-none') } + set(:project_started) { create(:project, path: 'export-started') } + set(:project_finished) { create(:project, path: 'export-finished') } + set(:user) { create(:user) } + set(:admin) { create(:admin) } + + let(:path) { "/projects/#{project.id}/export" } + let(:path_none) { "/projects/#{project_none.id}/export" } + let(:path_started) { "/projects/#{project_started.id}/export" } + let(:path_finished) { "/projects/#{project_finished.id}/export" } + + let(:download_path) { "/projects/#{project.id}/export/download" } + let(:download_path_none) { "/projects/#{project_none.id}/export/download" } + let(:download_path_started) { "/projects/#{project_started.id}/export/download" } + let(:download_path_finished) { "/projects/#{project_finished.id}/export/download" } + + let(:export_path) { "#{Dir.tmpdir}/project_export_spec" } + + before do + allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) + + # simulate exporting work directory + FileUtils.mkdir_p File.join(project_started.export_path, 'securerandom-hex') + + # simulate exported + FileUtils.mkdir_p project_finished.export_path + FileUtils.touch File.join(project_finished.export_path, '_export.tar.gz') + end + + after do + FileUtils.rm_rf(export_path, secure: true) + end + + shared_examples_for 'when project export is disabled' do + before do + stub_application_setting(project_export_enabled?: false) + end + + it_behaves_like '404 response' + end + + describe 'GET /projects/:project_id/export' do + shared_examples_for 'get project export status not found' do + it_behaves_like '404 response' do + let(:request) { get api(path, user) } + end + end + + shared_examples_for 'get project export status denied' do + it_behaves_like '403 response' do + let(:request) { get api(path, user) } + end + end + + shared_examples_for 'get project export status ok' do + it 'is none' do + get api(path_none, user) + + expect(response).to have_gitlab_http_status(200) + expect(response).to match_response_schema('public_api/v4/project/export_status') + expect(json_response['export_status']).to eq('none') + end + + it 'is started' do + get api(path_started, user) + + expect(response).to have_gitlab_http_status(200) + expect(response).to match_response_schema('public_api/v4/project/export_status') + expect(json_response['export_status']).to eq('started') + end + + it 'is finished' do + get api(path_finished, user) + + expect(response).to have_gitlab_http_status(200) + expect(response).to match_response_schema('public_api/v4/project/export_status') + expect(json_response['export_status']).to eq('finished') + end + end + + it_behaves_like 'when project export is disabled' do + let(:request) { get api(path, admin) } + end + + context 'when project export is enabled' do + context 'when user is an admin' do + let(:user) { admin } + + it_behaves_like 'get project export status ok' + end + + context 'when user is a master' do + before do + project.add_master(user) + project_none.add_master(user) + project_started.add_master(user) + project_finished.add_master(user) + end + + it_behaves_like 'get project export status ok' + end + + context 'when user is a developer' do + before do + project.add_developer(user) + end + + it_behaves_like 'get project export status denied' + end + + context 'when user is a reporter' do + before do + project.add_reporter(user) + end + + it_behaves_like 'get project export status denied' + end + + context 'when user is a guest' do + before do + project.add_guest(user) + end + + it_behaves_like 'get project export status denied' + end + + context 'when user is not a member' do + it_behaves_like 'get project export status not found' + end + end + end + + describe 'GET /projects/:project_id/export/download' do + shared_examples_for 'get project export download not found' do + it_behaves_like '404 response' do + let(:request) { get api(download_path, user) } + end + end + + shared_examples_for 'get project export download denied' do + it_behaves_like '403 response' do + let(:request) { get api(download_path, user) } + end + end + + shared_examples_for 'get project export download' do + it_behaves_like '404 response' do + let(:request) { get api(download_path_none, user) } + end + + it_behaves_like '404 response' do + let(:request) { get api(download_path_started, user) } + end + + it 'downloads' do + get api(download_path_finished, user) + + expect(response).to have_gitlab_http_status(200) + end + end + + it_behaves_like 'when project export is disabled' do + let(:request) { get api(download_path, admin) } + end + + context 'when project export is enabled' do + context 'when user is an admin' do + let(:user) { admin } + + it_behaves_like 'get project export download' + end + + context 'when user is a master' do + before do + project.add_master(user) + project_none.add_master(user) + project_started.add_master(user) + project_finished.add_master(user) + end + + it_behaves_like 'get project export download' + end + + context 'when user is a developer' do + before do + project.add_developer(user) + end + + it_behaves_like 'get project export download denied' + end + + context 'when user is a reporter' do + before do + project.add_reporter(user) + end + + it_behaves_like 'get project export download denied' + end + + context 'when user is a guest' do + before do + project.add_guest(user) + end + + it_behaves_like 'get project export download denied' + end + + context 'when user is not a member' do + it_behaves_like 'get project export download not found' + end + end + end + + describe 'POST /projects/:project_id/export' do + shared_examples_for 'post project export start not found' do + it_behaves_like '404 response' do + let(:request) { post api(path, user) } + end + end + + shared_examples_for 'post project export start denied' do + it_behaves_like '403 response' do + let(:request) { post api(path, user) } + end + end + + shared_examples_for 'post project export start' do + it 'starts' do + post api(path, user) + + expect(response).to have_gitlab_http_status(202) + end + end + + it_behaves_like 'when project export is disabled' do + let(:request) { post api(path, admin) } + end + + context 'when project export is enabled' do + context 'when user is an admin' do + let(:user) { admin } + + it_behaves_like 'post project export start' + end + + context 'when user is a master' do + before do + project.add_master(user) + project_none.add_master(user) + project_started.add_master(user) + project_finished.add_master(user) + end + + it_behaves_like 'post project export start' + end + + context 'when user is a developer' do + before do + project.add_developer(user) + end + + it_behaves_like 'post project export start denied' + end + + context 'when user is a reporter' do + before do + project.add_reporter(user) + end + + it_behaves_like 'post project export start denied' + end + + context 'when user is a guest' do + before do + project.add_guest(user) + end + + it_behaves_like 'post project export start denied' + end + + context 'when user is not a member' do + it_behaves_like 'post project export start not found' + end + end + end +end From a4308c53e5db594a6e351b143e11ceba6033cdc7 Mon Sep 17 00:00:00 2001 From: Travis Miller Date: Sat, 24 Feb 2018 23:03:29 -0600 Subject: [PATCH 04/16] Add project export API entities --- app/models/project.rb | 15 +++++++++++++++ lib/api/entities.rb | 15 +++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/app/models/project.rb b/app/models/project.rb index ba278a49688..791b69bd9ee 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1567,6 +1567,21 @@ class Project < ActiveRecord::Base Dir.glob("#{export_path}/*export.tar.gz").max_by { |f| File.ctime(f) } end + def export_status + if export_in_progress? + :started + elsif export_project_path + :finished + else + :none + end + end + + def export_in_progress? + shared = Gitlab::ImportExport::Shared.new(relative_path: File.join(disk_path, 'work')) + File.directory?(shared.export_path) + end + def remove_exports return nil unless export_path.present? diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 167878ba600..f140563882f 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -91,6 +91,21 @@ module API expose :created_at end + class ProjectExportStatus < ProjectIdentity + include ::API::Helpers::RelatedResourcesHelpers + + expose :export_status + expose :_links, if: lambda { |project, options| project.export_status == :finished } do + expose :api_url do |project| + expose_url(api_v4_projects_export_download_path(id: project.id)) + end + + expose :web_url do |project| + Gitlab::Routing.url_helpers.download_export_project_url(project) + end + end + end + class ProjectImportStatus < ProjectIdentity expose :import_status From c75187df8ce7d376ddc2c7201ce74d45b49fe5d9 Mon Sep 17 00:00:00 2001 From: Travis Miller Date: Sat, 2 Dec 2017 13:49:01 -0600 Subject: [PATCH 05/16] Add project export API implementation --- app/models/project.rb | 5 ++- .../projects/import_export/export_service.rb | 2 +- lib/api/api.rb | 1 + lib/api/project_export.rb | 36 +++++++++++++++++++ lib/gitlab/import_export/importer.rb | 2 +- lib/gitlab/import_export/shared.rb | 18 +++++++--- 6 files changed, 54 insertions(+), 10 deletions(-) create mode 100644 lib/api/project_export.rb diff --git a/app/models/project.rb b/app/models/project.rb index 791b69bd9ee..62a3b655ad1 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1560,7 +1560,7 @@ class Project < ActiveRecord::Base def export_path return nil unless namespace.present? || hashed_storage?(:repository) - File.join(Gitlab::ImportExport.storage_path, disk_path) + Gitlab::ImportExport::Shared.new(self).archive_path end def export_project_path @@ -1578,8 +1578,7 @@ class Project < ActiveRecord::Base end def export_in_progress? - shared = Gitlab::ImportExport::Shared.new(relative_path: File.join(disk_path, 'work')) - File.directory?(shared.export_path) + Gitlab::ImportExport::Shared.new(self).active_export_count > 0 end def remove_exports diff --git a/app/services/projects/import_export/export_service.rb b/app/services/projects/import_export/export_service.rb index fe4e8ea10bf..e26ce3089bb 100644 --- a/app/services/projects/import_export/export_service.rb +++ b/app/services/projects/import_export/export_service.rb @@ -2,7 +2,7 @@ module Projects module ImportExport class ExportService < BaseService def execute(_options = {}) - @shared = Gitlab::ImportExport::Shared.new(relative_path: File.join(project.disk_path, 'work')) + @shared = Gitlab::ImportExport::Shared.new(project) save_all end diff --git a/lib/api/api.rb b/lib/api/api.rb index 754549f72f0..6c96dccc08a 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -138,6 +138,7 @@ module API mount ::API::PagesDomains mount ::API::Pipelines mount ::API::PipelineSchedules + mount ::API::ProjectExport mount ::API::ProjectImport mount ::API::ProjectHooks mount ::API::Projects diff --git a/lib/api/project_export.rb b/lib/api/project_export.rb new file mode 100644 index 00000000000..8b2c5bedc60 --- /dev/null +++ b/lib/api/project_export.rb @@ -0,0 +1,36 @@ +module API + class ProjectExport < Grape::API + before do + not_found! unless Gitlab::CurrentSettings.current_application_settings.project_export_enabled? + authorize_admin_project + end + + params do + requires :id, type: String, desc: 'The ID of a project' + end + resource :projects, requirements: { id: %r{[^/]+} } do + desc 'Get export status' do + success Entities::ProjectExportStatus + end + get ':id/export' do + present user_project, with: Entities::ProjectExportStatus + end + + desc 'Download export' + get ':id/export/download' do + path = user_project.export_project_path + + render_api_error!('404 Not found or has expired', 404) unless path + + present_file!(path, File.basename(path), 'application/gzip') + end + + desc 'Start export' + post ':id/export' do + user_project.add_export_job(current_user: current_user) + + accepted! + end + end + end +end diff --git a/lib/gitlab/import_export/importer.rb b/lib/gitlab/import_export/importer.rb index a00795f553e..070dd630c10 100644 --- a/lib/gitlab/import_export/importer.rb +++ b/lib/gitlab/import_export/importer.rb @@ -9,7 +9,7 @@ module Gitlab @archive_file = project.import_source @current_user = project.creator @project = project - @shared = Gitlab::ImportExport::Shared.new(relative_path: path_with_namespace) + @shared = Gitlab::ImportExport::Shared.new(project) end def execute diff --git a/lib/gitlab/import_export/shared.rb b/lib/gitlab/import_export/shared.rb index b34cafc6876..fef6cfc08f0 100644 --- a/lib/gitlab/import_export/shared.rb +++ b/lib/gitlab/import_export/shared.rb @@ -1,13 +1,17 @@ module Gitlab module ImportExport class Shared - attr_reader :errors, :opts + attr_reader :errors, :project - def initialize(opts) - @opts = opts + def initialize(project) + @project = project @errors = [] end + def active_export_count + Dir[File.join(archive_path, '*')].count { |name| File.directory?(name) } + end + def export_path @export_path ||= Gitlab::ImportExport.export_path(relative_path: relative_path) end @@ -31,11 +35,15 @@ module Gitlab private def relative_path - File.join(opts[:relative_path], SecureRandom.hex) + File.join(relative_archive_path, SecureRandom.hex) end def relative_archive_path - File.join(opts[:relative_path], '..') + if @project.is_a?(Project) + @project.disk_path + else + @project[:relative_path] + end end def error_out(message, caller) From 3b474052b82e5e82df8f34a2bdb7e305118aef0f Mon Sep 17 00:00:00 2001 From: Travis Miller Date: Mon, 11 Dec 2017 11:50:07 -0600 Subject: [PATCH 06/16] Add project export API changelog --- changelogs/unreleased/29130-api-project-export.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/29130-api-project-export.yml diff --git a/changelogs/unreleased/29130-api-project-export.yml b/changelogs/unreleased/29130-api-project-export.yml new file mode 100644 index 00000000000..7dee349232a --- /dev/null +++ b/changelogs/unreleased/29130-api-project-export.yml @@ -0,0 +1,5 @@ +--- +title: Add project export API +merge_request: 15860 +author: Travis Miller +type: added From b4fc556f10b9bed9af9a7d57de5b8fcdfa584ad9 Mon Sep 17 00:00:00 2001 From: Travis Miller Date: Sat, 24 Feb 2018 21:12:11 -0600 Subject: [PATCH 07/16] review: fix docs --- doc/api/project_import_export.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/api/project_import_export.md b/doc/api/project_import_export.md index 7c2c619b97b..c6923f2a439 100644 --- a/doc/api/project_import_export.md +++ b/doc/api/project_import_export.md @@ -1,10 +1,10 @@ -# Project import API +# Project import/export API [Introduced][ce-41899] in GitLab 10.6 [See also the project import/export documentation](../user/project/settings/import_export.md) -## Export start +## Schedule an export Start a new export. @@ -66,7 +66,7 @@ Status can be one of `none`, `started`, or `finished`. Download the finished export. ```http -GET /projects/:id/export +GET /projects/:id/export/download ``` | Attribute | Type | Required | Description | From b1e3237677f9cdc4e0e792f26dbc3fc642c7ef28 Mon Sep 17 00:00:00 2001 From: Travis Miller Date: Sat, 24 Feb 2018 21:18:46 -0600 Subject: [PATCH 08/16] review: add feature introduction detail --- lib/api/project_export.rb | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/api/project_export.rb b/lib/api/project_export.rb index 8b2c5bedc60..4041faae971 100644 --- a/lib/api/project_export.rb +++ b/lib/api/project_export.rb @@ -10,13 +10,16 @@ module API end resource :projects, requirements: { id: %r{[^/]+} } do desc 'Get export status' do + detail 'This feature was introduced in GitLab 10.6.' success Entities::ProjectExportStatus end get ':id/export' do present user_project, with: Entities::ProjectExportStatus end - desc 'Download export' + desc 'Download export' do + detail 'This feature was introduced in GitLab 10.6.' + end get ':id/export/download' do path = user_project.export_project_path @@ -25,7 +28,9 @@ module API present_file!(path, File.basename(path), 'application/gzip') end - desc 'Start export' + desc 'Start export' do + detail 'This feature was introduced in GitLab 10.6.' + end post ':id/export' do user_project.add_export_job(current_user: current_user) From 737b7e3e7d8098493804f2c7261eec58268af2d1 Mon Sep 17 00:00:00 2001 From: Travis Miller Date: Sat, 24 Feb 2018 22:43:16 -0600 Subject: [PATCH 09/16] review: normalize shared initizlier arguments --- lib/gitlab/import_export/shared.rb | 11 ++++++----- spec/lib/gitlab/import_export/avatar_restorer_spec.rb | 2 +- spec/lib/gitlab/import_export/avatar_saver_spec.rb | 2 +- spec/lib/gitlab/import_export/file_importer_spec.rb | 2 +- spec/lib/gitlab/import_export/fork_spec.rb | 2 +- .../import_export/project_tree_restorer_spec.rb | 2 +- .../gitlab/import_export/project_tree_saver_spec.rb | 2 +- spec/lib/gitlab/import_export/reader_spec.rb | 2 +- spec/lib/gitlab/import_export/repo_restorer_spec.rb | 2 +- spec/lib/gitlab/import_export/repo_saver_spec.rb | 2 +- .../lib/gitlab/import_export/uploads_restorer_spec.rb | 2 +- spec/lib/gitlab/import_export/uploads_saver_spec.rb | 2 +- spec/lib/gitlab/import_export/version_checker_spec.rb | 2 +- spec/lib/gitlab/import_export/wiki_repo_saver_spec.rb | 2 +- spec/lib/gitlab/import_export/wiki_restorer_spec.rb | 2 +- 15 files changed, 20 insertions(+), 19 deletions(-) diff --git a/lib/gitlab/import_export/shared.rb b/lib/gitlab/import_export/shared.rb index fef6cfc08f0..012228ca273 100644 --- a/lib/gitlab/import_export/shared.rb +++ b/lib/gitlab/import_export/shared.rb @@ -1,10 +1,11 @@ module Gitlab module ImportExport class Shared - attr_reader :errors, :project + attr_reader :errors, :project, :opts - def initialize(project) + def initialize(project, opts = {}) @project = project + @opts = opts @errors = [] end @@ -39,10 +40,10 @@ module Gitlab end def relative_archive_path - if @project.is_a?(Project) - @project.disk_path + if @opts[:relative_path] + @opts[:relative_path] else - @project[:relative_path] + @project.disk_path end end diff --git a/spec/lib/gitlab/import_export/avatar_restorer_spec.rb b/spec/lib/gitlab/import_export/avatar_restorer_spec.rb index a93a921e459..5b1d9cd1e6c 100644 --- a/spec/lib/gitlab/import_export/avatar_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/avatar_restorer_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Gitlab::ImportExport::AvatarRestorer do include UploadHelpers - let(:shared) { Gitlab::ImportExport::Shared.new(relative_path: 'test') } + let(:shared) { Gitlab::ImportExport::Shared.new(project) } let(:project) { create(:project) } before do diff --git a/spec/lib/gitlab/import_export/avatar_saver_spec.rb b/spec/lib/gitlab/import_export/avatar_saver_spec.rb index 3fb5ddde8b5..e688929b216 100644 --- a/spec/lib/gitlab/import_export/avatar_saver_spec.rb +++ b/spec/lib/gitlab/import_export/avatar_saver_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::ImportExport::AvatarSaver do - let(:shared) { Gitlab::ImportExport::Shared.new(relative_path: 'test') } + let(:shared) { Gitlab::ImportExport::Shared.new(project) } let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" } let(:project_with_avatar) { create(:project, avatar: fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "image/png")) } let(:project) { create(:project) } diff --git a/spec/lib/gitlab/import_export/file_importer_spec.rb b/spec/lib/gitlab/import_export/file_importer_spec.rb index 5cdc5138fda..a8447679ffd 100644 --- a/spec/lib/gitlab/import_export/file_importer_spec.rb +++ b/spec/lib/gitlab/import_export/file_importer_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::ImportExport::FileImporter do - let(:shared) { Gitlab::ImportExport::Shared.new(relative_path: 'test') } + let(:shared) { Gitlab::ImportExport::Shared.new(nil, relative_path: 'test') } let(:export_path) { "#{Dir.tmpdir}/file_importer_spec" } let(:valid_file) { "#{shared.export_path}/valid.json" } let(:symlink_file) { "#{shared.export_path}/invalid.json" } diff --git a/spec/lib/gitlab/import_export/fork_spec.rb b/spec/lib/gitlab/import_export/fork_spec.rb index cfb15ee7e8b..49690c1f8b5 100644 --- a/spec/lib/gitlab/import_export/fork_spec.rb +++ b/spec/lib/gitlab/import_export/fork_spec.rb @@ -7,7 +7,7 @@ describe 'forked project import' do let!(:project_with_repo) { create(:project, :repository, name: 'test-repo-restorer', path: 'test-repo-restorer') } let!(:project) { create(:project, name: 'test-repo-restorer-no-repo', path: 'test-repo-restorer-no-repo') } let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" } - let(:shared) { Gitlab::ImportExport::Shared.new(relative_path: project.full_path) } + let(:shared) { Gitlab::ImportExport::Shared.new(project) } let(:forked_from_project) { create(:project, :repository) } let(:forked_project) { fork_project(project_with_repo, nil, repository: true) } let(:repo_saver) { Gitlab::ImportExport::RepoSaver.new(project: project_with_repo, shared: shared) } diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb index d076007e4bc..c0b70535433 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -7,7 +7,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do @user = create(:user) RSpec::Mocks.with_temporary_scope do - @shared = Gitlab::ImportExport::Shared.new(relative_path: "", project_path: 'path') + @shared = Gitlab::ImportExport::Shared.new(@project) allow(@shared).to receive(:export_path).and_return('spec/lib/gitlab/import_export/') @project = create(:project, :builds_disabled, :issues_disabled, name: 'project', path: 'project') diff --git a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb index 5804c45871e..a1b7a996ac5 100644 --- a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Gitlab::ImportExport::ProjectTreeSaver do describe 'saves the project tree into a json object' do - let(:shared) { Gitlab::ImportExport::Shared.new(relative_path: project.full_path) } + let(:shared) { Gitlab::ImportExport::Shared.new(project) } let(:project_tree_saver) { described_class.new(project: project, current_user: user, shared: shared) } let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" } let(:user) { create(:user) } diff --git a/spec/lib/gitlab/import_export/reader_spec.rb b/spec/lib/gitlab/import_export/reader_spec.rb index e9f5273725d..1ef024d3078 100644 --- a/spec/lib/gitlab/import_export/reader_spec.rb +++ b/spec/lib/gitlab/import_export/reader_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::ImportExport::Reader do - let(:shared) { Gitlab::ImportExport::Shared.new(relative_path: '') } + let(:shared) { Gitlab::ImportExport::Shared.new(nil) } let(:test_config) { 'spec/support/import_export/import_export.yml' } let(:project_tree_hash) do { diff --git a/spec/lib/gitlab/import_export/repo_restorer_spec.rb b/spec/lib/gitlab/import_export/repo_restorer_spec.rb index c49af602a01..16aa1413fd8 100644 --- a/spec/lib/gitlab/import_export/repo_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/repo_restorer_spec.rb @@ -6,7 +6,7 @@ describe Gitlab::ImportExport::RepoRestorer do let!(:project_with_repo) { create(:project, :repository, name: 'test-repo-restorer', path: 'test-repo-restorer') } let!(:project) { create(:project) } let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" } - let(:shared) { Gitlab::ImportExport::Shared.new(relative_path: project.full_path) } + let(:shared) { Gitlab::ImportExport::Shared.new(project) } let(:bundler) { Gitlab::ImportExport::RepoSaver.new(project: project_with_repo, shared: shared) } let(:bundle_path) { File.join(shared.export_path, Gitlab::ImportExport.project_bundle_filename) } let(:restorer) do diff --git a/spec/lib/gitlab/import_export/repo_saver_spec.rb b/spec/lib/gitlab/import_export/repo_saver_spec.rb index 44f972fe530..29f94906ec7 100644 --- a/spec/lib/gitlab/import_export/repo_saver_spec.rb +++ b/spec/lib/gitlab/import_export/repo_saver_spec.rb @@ -5,7 +5,7 @@ describe Gitlab::ImportExport::RepoSaver do let(:user) { create(:user) } let!(:project) { create(:project, :public, name: 'searchable_project') } let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" } - let(:shared) { Gitlab::ImportExport::Shared.new(relative_path: project.full_path) } + let(:shared) { Gitlab::ImportExport::Shared.new(project) } let(:bundler) { described_class.new(project: project, shared: shared) } before do diff --git a/spec/lib/gitlab/import_export/uploads_restorer_spec.rb b/spec/lib/gitlab/import_export/uploads_restorer_spec.rb index 8a3a244be21..d466bfc7667 100644 --- a/spec/lib/gitlab/import_export/uploads_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/uploads_restorer_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Gitlab::ImportExport::UploadsRestorer do describe 'bundle a project Git repo' do let(:export_path) { "#{Dir.tmpdir}/uploads_saver_spec" } - let(:shared) { Gitlab::ImportExport::Shared.new(relative_path: project.full_path) } + let(:shared) { Gitlab::ImportExport::Shared.new(project) } before do allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) diff --git a/spec/lib/gitlab/import_export/uploads_saver_spec.rb b/spec/lib/gitlab/import_export/uploads_saver_spec.rb index 177036c109b..1714badfc3d 100644 --- a/spec/lib/gitlab/import_export/uploads_saver_spec.rb +++ b/spec/lib/gitlab/import_export/uploads_saver_spec.rb @@ -4,7 +4,7 @@ describe Gitlab::ImportExport::UploadsSaver do describe 'bundle a project Git repo' do let(:export_path) { "#{Dir.tmpdir}/uploads_saver_spec" } let(:file) { fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif') } - let(:shared) { Gitlab::ImportExport::Shared.new(relative_path: project.full_path) } + let(:shared) { Gitlab::ImportExport::Shared.new(project) } before do allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) diff --git a/spec/lib/gitlab/import_export/version_checker_spec.rb b/spec/lib/gitlab/import_export/version_checker_spec.rb index e7d50f75682..1388fca465e 100644 --- a/spec/lib/gitlab/import_export/version_checker_spec.rb +++ b/spec/lib/gitlab/import_export/version_checker_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' include ImportExport::CommonUtil describe Gitlab::ImportExport::VersionChecker do - let(:shared) { Gitlab::ImportExport::Shared.new(relative_path: '') } + let(:shared) { Gitlab::ImportExport::Shared.new(nil, relative_path: '') } describe 'bundle a project Git repo' do let(:version) { Gitlab::ImportExport.version } diff --git a/spec/lib/gitlab/import_export/wiki_repo_saver_spec.rb b/spec/lib/gitlab/import_export/wiki_repo_saver_spec.rb index 1d1e7e7f89a..d445c69a897 100644 --- a/spec/lib/gitlab/import_export/wiki_repo_saver_spec.rb +++ b/spec/lib/gitlab/import_export/wiki_repo_saver_spec.rb @@ -5,7 +5,7 @@ describe Gitlab::ImportExport::WikiRepoSaver do let(:user) { create(:user) } let!(:project) { create(:project, :public, name: 'searchable_project') } let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" } - let(:shared) { Gitlab::ImportExport::Shared.new(relative_path: project.full_path) } + let(:shared) { Gitlab::ImportExport::Shared.new(project) } let(:wiki_bundler) { described_class.new(project: project, shared: shared) } let!(:project_wiki) { ProjectWiki.new(project, user) } diff --git a/spec/lib/gitlab/import_export/wiki_restorer_spec.rb b/spec/lib/gitlab/import_export/wiki_restorer_spec.rb index 81b654e9c5f..4fcb1a0e13f 100644 --- a/spec/lib/gitlab/import_export/wiki_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/wiki_restorer_spec.rb @@ -6,7 +6,7 @@ describe Gitlab::ImportExport::WikiRestorer do let!(:project_without_wiki) { create(:project) } let!(:project) { create(:project) } let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" } - let(:shared) { Gitlab::ImportExport::Shared.new(relative_path: project.full_path) } + let(:shared) { Gitlab::ImportExport::Shared.new(project) } let(:bundler) { Gitlab::ImportExport::WikiRepoSaver.new(project: project_with_wiki, shared: shared) } let(:bundle_path) { File.join(shared.export_path, Gitlab::ImportExport.project_bundle_filename) } let(:restorer) do From 0d8aadeb9df3bd5e4f18e8c22510c2220b74420d Mon Sep 17 00:00:00 2001 From: Travis Miller Date: Tue, 27 Feb 2018 08:11:13 -0600 Subject: [PATCH 10/16] eliminate need or opts argument in shared object --- lib/gitlab/import_export/shared.rb | 11 +++-------- spec/lib/gitlab/import_export/file_importer_spec.rb | 3 ++- .../import_export/project_tree_restorer_spec.rb | 2 +- spec/lib/gitlab/import_export/version_checker_spec.rb | 3 ++- 4 files changed, 8 insertions(+), 11 deletions(-) diff --git a/lib/gitlab/import_export/shared.rb b/lib/gitlab/import_export/shared.rb index 012228ca273..3d3d998a6a3 100644 --- a/lib/gitlab/import_export/shared.rb +++ b/lib/gitlab/import_export/shared.rb @@ -1,11 +1,10 @@ module Gitlab module ImportExport class Shared - attr_reader :errors, :project, :opts + attr_reader :errors, :project - def initialize(project, opts = {}) + def initialize(project) @project = project - @opts = opts @errors = [] end @@ -40,11 +39,7 @@ module Gitlab end def relative_archive_path - if @opts[:relative_path] - @opts[:relative_path] - else - @project.disk_path - end + @project.disk_path end def error_out(message, caller) diff --git a/spec/lib/gitlab/import_export/file_importer_spec.rb b/spec/lib/gitlab/import_export/file_importer_spec.rb index a8447679ffd..58b9fb06cc5 100644 --- a/spec/lib/gitlab/import_export/file_importer_spec.rb +++ b/spec/lib/gitlab/import_export/file_importer_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::ImportExport::FileImporter do - let(:shared) { Gitlab::ImportExport::Shared.new(nil, relative_path: 'test') } + let(:shared) { Gitlab::ImportExport::Shared.new(nil) } let(:export_path) { "#{Dir.tmpdir}/file_importer_spec" } let(:valid_file) { "#{shared.export_path}/valid.json" } let(:symlink_file) { "#{shared.export_path}/invalid.json" } @@ -12,6 +12,7 @@ describe Gitlab::ImportExport::FileImporter do stub_const('Gitlab::ImportExport::FileImporter::MAX_RETRIES', 0) allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) allow_any_instance_of(Gitlab::ImportExport::CommandLineUtil).to receive(:untar_zxf).and_return(true) + allow_any_instance_of(Gitlab::ImportExport::Shared).to receive(:relative_archive_path).and_return('test') allow(SecureRandom).to receive(:hex).and_return('abcd') setup_files end diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb index c0b70535433..974297fb078 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -259,7 +259,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do context 'Light JSON' do let(:user) { create(:user) } - let(:shared) { Gitlab::ImportExport::Shared.new(relative_path: "", project_path: 'path') } + let(:shared) { Gitlab::ImportExport::Shared.new(project) } let!(:project) { create(:project, :builds_disabled, :issues_disabled, name: 'project', path: 'project') } let(:project_tree_restorer) { described_class.new(user: user, shared: shared, project: project) } let(:restored_project_json) { project_tree_restorer.restore } diff --git a/spec/lib/gitlab/import_export/version_checker_spec.rb b/spec/lib/gitlab/import_export/version_checker_spec.rb index 1388fca465e..49d857d9483 100644 --- a/spec/lib/gitlab/import_export/version_checker_spec.rb +++ b/spec/lib/gitlab/import_export/version_checker_spec.rb @@ -2,12 +2,13 @@ require 'spec_helper' include ImportExport::CommonUtil describe Gitlab::ImportExport::VersionChecker do - let(:shared) { Gitlab::ImportExport::Shared.new(nil, relative_path: '') } + let(:shared) { Gitlab::ImportExport::Shared.new(nil) } describe 'bundle a project Git repo' do let(:version) { Gitlab::ImportExport.version } before do + allow_any_instance_of(Gitlab::ImportExport::Shared).to receive(:relative_archive_path).and_return('') allow(File).to receive(:open).and_return(version) end From 2b176137c9b5617d38ff89b6f8ed4636018916c9 Mon Sep 17 00:00:00 2001 From: Travis Miller Date: Wed, 28 Feb 2018 21:25:54 -0600 Subject: [PATCH 11/16] review: instantiate shared import/export object in project method --- app/models/project.rb | 8 ++++++-- app/services/projects/import_export/export_service.rb | 2 +- lib/gitlab/import_export/importer.rb | 2 +- spec/lib/gitlab/import_export/avatar_restorer_spec.rb | 2 +- spec/lib/gitlab/import_export/avatar_saver_spec.rb | 2 +- spec/lib/gitlab/import_export/fork_spec.rb | 2 +- .../gitlab/import_export/project_tree_restorer_spec.rb | 6 +++--- spec/lib/gitlab/import_export/project_tree_saver_spec.rb | 2 +- spec/lib/gitlab/import_export/repo_restorer_spec.rb | 2 +- spec/lib/gitlab/import_export/repo_saver_spec.rb | 2 +- spec/lib/gitlab/import_export/uploads_restorer_spec.rb | 2 +- spec/lib/gitlab/import_export/uploads_saver_spec.rb | 2 +- spec/lib/gitlab/import_export/wiki_repo_saver_spec.rb | 2 +- spec/lib/gitlab/import_export/wiki_restorer_spec.rb | 2 +- 14 files changed, 21 insertions(+), 17 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index 62a3b655ad1..190473aa196 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1557,10 +1557,14 @@ class Project < ActiveRecord::Base end end + def import_export + @import_export ||= Gitlab::ImportExport::Shared.new(self) + end + def export_path return nil unless namespace.present? || hashed_storage?(:repository) - Gitlab::ImportExport::Shared.new(self).archive_path + import_export.archive_path end def export_project_path @@ -1578,7 +1582,7 @@ class Project < ActiveRecord::Base end def export_in_progress? - Gitlab::ImportExport::Shared.new(self).active_export_count > 0 + import_export.active_export_count > 0 end def remove_exports diff --git a/app/services/projects/import_export/export_service.rb b/app/services/projects/import_export/export_service.rb index e26ce3089bb..2af228eff05 100644 --- a/app/services/projects/import_export/export_service.rb +++ b/app/services/projects/import_export/export_service.rb @@ -2,7 +2,7 @@ module Projects module ImportExport class ExportService < BaseService def execute(_options = {}) - @shared = Gitlab::ImportExport::Shared.new(project) + @shared = project.import_export save_all end diff --git a/lib/gitlab/import_export/importer.rb b/lib/gitlab/import_export/importer.rb index 070dd630c10..8b780cbe3a1 100644 --- a/lib/gitlab/import_export/importer.rb +++ b/lib/gitlab/import_export/importer.rb @@ -9,7 +9,7 @@ module Gitlab @archive_file = project.import_source @current_user = project.creator @project = project - @shared = Gitlab::ImportExport::Shared.new(project) + @shared = project.import_export end def execute diff --git a/spec/lib/gitlab/import_export/avatar_restorer_spec.rb b/spec/lib/gitlab/import_export/avatar_restorer_spec.rb index 5b1d9cd1e6c..c981386ef3d 100644 --- a/spec/lib/gitlab/import_export/avatar_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/avatar_restorer_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Gitlab::ImportExport::AvatarRestorer do include UploadHelpers - let(:shared) { Gitlab::ImportExport::Shared.new(project) } + let(:shared) { project.import_export } let(:project) { create(:project) } before do diff --git a/spec/lib/gitlab/import_export/avatar_saver_spec.rb b/spec/lib/gitlab/import_export/avatar_saver_spec.rb index e688929b216..78055f3970a 100644 --- a/spec/lib/gitlab/import_export/avatar_saver_spec.rb +++ b/spec/lib/gitlab/import_export/avatar_saver_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::ImportExport::AvatarSaver do - let(:shared) { Gitlab::ImportExport::Shared.new(project) } + let(:shared) { project.import_export } let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" } let(:project_with_avatar) { create(:project, avatar: fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "image/png")) } let(:project) { create(:project) } diff --git a/spec/lib/gitlab/import_export/fork_spec.rb b/spec/lib/gitlab/import_export/fork_spec.rb index 49690c1f8b5..77aa37a1992 100644 --- a/spec/lib/gitlab/import_export/fork_spec.rb +++ b/spec/lib/gitlab/import_export/fork_spec.rb @@ -7,7 +7,7 @@ describe 'forked project import' do let!(:project_with_repo) { create(:project, :repository, name: 'test-repo-restorer', path: 'test-repo-restorer') } let!(:project) { create(:project, name: 'test-repo-restorer-no-repo', path: 'test-repo-restorer-no-repo') } let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" } - let(:shared) { Gitlab::ImportExport::Shared.new(project) } + let(:shared) { project.import_export } let(:forked_from_project) { create(:project, :repository) } let(:forked_project) { fork_project(project_with_repo, nil, repository: true) } let(:repo_saver) { Gitlab::ImportExport::RepoSaver.new(project: project_with_repo, shared: shared) } diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb index 974297fb078..ae4bdc7be00 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -7,9 +7,9 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do @user = create(:user) RSpec::Mocks.with_temporary_scope do - @shared = Gitlab::ImportExport::Shared.new(@project) - allow(@shared).to receive(:export_path).and_return('spec/lib/gitlab/import_export/') @project = create(:project, :builds_disabled, :issues_disabled, name: 'project', path: 'project') + @shared = @project.import_export + allow(@shared).to receive(:export_path).and_return('spec/lib/gitlab/import_export/') allow_any_instance_of(Repository).to receive(:fetch_ref).and_return(true) allow_any_instance_of(Gitlab::Git::Repository).to receive(:branch_exists?).and_return(false) @@ -259,7 +259,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do context 'Light JSON' do let(:user) { create(:user) } - let(:shared) { Gitlab::ImportExport::Shared.new(project) } + let(:shared) { project.import_export } let!(:project) { create(:project, :builds_disabled, :issues_disabled, name: 'project', path: 'project') } let(:project_tree_restorer) { described_class.new(user: user, shared: shared, project: project) } let(:restored_project_json) { project_tree_restorer.restore } diff --git a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb index a1b7a996ac5..4daef697c1a 100644 --- a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Gitlab::ImportExport::ProjectTreeSaver do describe 'saves the project tree into a json object' do - let(:shared) { Gitlab::ImportExport::Shared.new(project) } + let(:shared) { project.import_export } let(:project_tree_saver) { described_class.new(project: project, current_user: user, shared: shared) } let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" } let(:user) { create(:user) } diff --git a/spec/lib/gitlab/import_export/repo_restorer_spec.rb b/spec/lib/gitlab/import_export/repo_restorer_spec.rb index 16aa1413fd8..3fdb40996bf 100644 --- a/spec/lib/gitlab/import_export/repo_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/repo_restorer_spec.rb @@ -6,7 +6,7 @@ describe Gitlab::ImportExport::RepoRestorer do let!(:project_with_repo) { create(:project, :repository, name: 'test-repo-restorer', path: 'test-repo-restorer') } let!(:project) { create(:project) } let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" } - let(:shared) { Gitlab::ImportExport::Shared.new(project) } + let(:shared) { project.import_export } let(:bundler) { Gitlab::ImportExport::RepoSaver.new(project: project_with_repo, shared: shared) } let(:bundle_path) { File.join(shared.export_path, Gitlab::ImportExport.project_bundle_filename) } let(:restorer) do diff --git a/spec/lib/gitlab/import_export/repo_saver_spec.rb b/spec/lib/gitlab/import_export/repo_saver_spec.rb index 29f94906ec7..74859d81c71 100644 --- a/spec/lib/gitlab/import_export/repo_saver_spec.rb +++ b/spec/lib/gitlab/import_export/repo_saver_spec.rb @@ -5,7 +5,7 @@ describe Gitlab::ImportExport::RepoSaver do let(:user) { create(:user) } let!(:project) { create(:project, :public, name: 'searchable_project') } let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" } - let(:shared) { Gitlab::ImportExport::Shared.new(project) } + let(:shared) { project.import_export } let(:bundler) { described_class.new(project: project, shared: shared) } before do diff --git a/spec/lib/gitlab/import_export/uploads_restorer_spec.rb b/spec/lib/gitlab/import_export/uploads_restorer_spec.rb index d466bfc7667..e919a7d9cbb 100644 --- a/spec/lib/gitlab/import_export/uploads_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/uploads_restorer_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Gitlab::ImportExport::UploadsRestorer do describe 'bundle a project Git repo' do let(:export_path) { "#{Dir.tmpdir}/uploads_saver_spec" } - let(:shared) { Gitlab::ImportExport::Shared.new(project) } + let(:shared) { project.import_export } before do allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) diff --git a/spec/lib/gitlab/import_export/uploads_saver_spec.rb b/spec/lib/gitlab/import_export/uploads_saver_spec.rb index 1714badfc3d..8ac37850328 100644 --- a/spec/lib/gitlab/import_export/uploads_saver_spec.rb +++ b/spec/lib/gitlab/import_export/uploads_saver_spec.rb @@ -4,7 +4,7 @@ describe Gitlab::ImportExport::UploadsSaver do describe 'bundle a project Git repo' do let(:export_path) { "#{Dir.tmpdir}/uploads_saver_spec" } let(:file) { fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif') } - let(:shared) { Gitlab::ImportExport::Shared.new(project) } + let(:shared) { project.import_export } before do allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) diff --git a/spec/lib/gitlab/import_export/wiki_repo_saver_spec.rb b/spec/lib/gitlab/import_export/wiki_repo_saver_spec.rb index d445c69a897..fbd00cf5ce5 100644 --- a/spec/lib/gitlab/import_export/wiki_repo_saver_spec.rb +++ b/spec/lib/gitlab/import_export/wiki_repo_saver_spec.rb @@ -5,7 +5,7 @@ describe Gitlab::ImportExport::WikiRepoSaver do let(:user) { create(:user) } let!(:project) { create(:project, :public, name: 'searchable_project') } let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" } - let(:shared) { Gitlab::ImportExport::Shared.new(project) } + let(:shared) { project.import_export } let(:wiki_bundler) { described_class.new(project: project, shared: shared) } let!(:project_wiki) { ProjectWiki.new(project, user) } diff --git a/spec/lib/gitlab/import_export/wiki_restorer_spec.rb b/spec/lib/gitlab/import_export/wiki_restorer_spec.rb index 4fcb1a0e13f..575ae837d64 100644 --- a/spec/lib/gitlab/import_export/wiki_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/wiki_restorer_spec.rb @@ -6,7 +6,7 @@ describe Gitlab::ImportExport::WikiRestorer do let!(:project_without_wiki) { create(:project) } let!(:project) { create(:project) } let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" } - let(:shared) { Gitlab::ImportExport::Shared.new(project) } + let(:shared) { project.import_export } let(:bundler) { Gitlab::ImportExport::WikiRepoSaver.new(project: project_with_wiki, shared: shared) } let(:bundle_path) { File.join(shared.export_path, Gitlab::ImportExport.project_bundle_filename) } let(:restorer) do From 60642307c7bb0fe1917324966ed01393f3d25076 Mon Sep 17 00:00:00 2001 From: Travis Miller Date: Wed, 28 Feb 2018 21:30:47 -0600 Subject: [PATCH 12/16] review: add note about _links present when export is finished --- doc/api/project_import_export.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/api/project_import_export.md b/doc/api/project_import_export.md index c6923f2a439..677765368a8 100644 --- a/doc/api/project_import_export.md +++ b/doc/api/project_import_export.md @@ -44,6 +44,8 @@ curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/a Status can be one of `none`, `started`, or `finished`. +`_links` are only present when export has finished. + ```json { "id": 1, From 2f732a55e1172eac037efa183fb825ba1d533f3a Mon Sep 17 00:00:00 2001 From: Travis Miller Date: Wed, 28 Feb 2018 21:32:03 -0600 Subject: [PATCH 13/16] review: remove un-necessary current_application_settings --- lib/api/project_export.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/project_export.rb b/lib/api/project_export.rb index 4041faae971..6ec2626df1a 100644 --- a/lib/api/project_export.rb +++ b/lib/api/project_export.rb @@ -1,7 +1,7 @@ module API class ProjectExport < Grape::API before do - not_found! unless Gitlab::CurrentSettings.current_application_settings.project_export_enabled? + not_found! unless Gitlab::CurrentSettings.project_export_enabled? authorize_admin_project end From e20061218c039859ecff6f89d709c7132ad7d18f Mon Sep 17 00:00:00 2001 From: Travis Miller Date: Wed, 28 Feb 2018 21:33:14 -0600 Subject: [PATCH 14/16] review: specifying path in spec project creation is not necessary --- spec/requests/api/project_export_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/requests/api/project_export_spec.rb b/spec/requests/api/project_export_spec.rb index 146bf3fd648..fbed527963f 100644 --- a/spec/requests/api/project_export_spec.rb +++ b/spec/requests/api/project_export_spec.rb @@ -2,9 +2,9 @@ require 'spec_helper' describe API::ProjectExport do set(:project) { create(:project) } - set(:project_none) { create(:project, path: 'export-none') } - set(:project_started) { create(:project, path: 'export-started') } - set(:project_finished) { create(:project, path: 'export-finished') } + set(:project_none) { create(:project) } + set(:project_started) { create(:project) } + set(:project_finished) { create(:project) } set(:user) { create(:user) } set(:admin) { create(:admin) } From 6d1c5014e96d96ad44fed71dd99bf6c1d4af3cec Mon Sep 17 00:00:00 2001 From: Travis Miller Date: Mon, 5 Mar 2018 13:53:40 -0600 Subject: [PATCH 15/16] review: rename import_export to import_export_shared --- app/models/project.rb | 8 ++++---- app/services/projects/import_export/export_service.rb | 2 +- lib/gitlab/import_export/importer.rb | 2 +- spec/lib/gitlab/import_export/avatar_restorer_spec.rb | 2 +- spec/lib/gitlab/import_export/avatar_saver_spec.rb | 2 +- spec/lib/gitlab/import_export/fork_spec.rb | 2 +- .../gitlab/import_export/project_tree_restorer_spec.rb | 4 ++-- spec/lib/gitlab/import_export/project_tree_saver_spec.rb | 2 +- spec/lib/gitlab/import_export/repo_restorer_spec.rb | 2 +- spec/lib/gitlab/import_export/repo_saver_spec.rb | 2 +- spec/lib/gitlab/import_export/uploads_restorer_spec.rb | 2 +- spec/lib/gitlab/import_export/uploads_saver_spec.rb | 2 +- spec/lib/gitlab/import_export/wiki_repo_saver_spec.rb | 2 +- spec/lib/gitlab/import_export/wiki_restorer_spec.rb | 2 +- 14 files changed, 18 insertions(+), 18 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index 190473aa196..666614bb3d9 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1557,14 +1557,14 @@ class Project < ActiveRecord::Base end end - def import_export - @import_export ||= Gitlab::ImportExport::Shared.new(self) + def import_export_shared + @import_export_shared ||= Gitlab::ImportExport::Shared.new(self) end def export_path return nil unless namespace.present? || hashed_storage?(:repository) - import_export.archive_path + import_export_shared.archive_path end def export_project_path @@ -1582,7 +1582,7 @@ class Project < ActiveRecord::Base end def export_in_progress? - import_export.active_export_count > 0 + import_export_shared.active_export_count > 0 end def remove_exports diff --git a/app/services/projects/import_export/export_service.rb b/app/services/projects/import_export/export_service.rb index 2af228eff05..af41ce82f65 100644 --- a/app/services/projects/import_export/export_service.rb +++ b/app/services/projects/import_export/export_service.rb @@ -2,7 +2,7 @@ module Projects module ImportExport class ExportService < BaseService def execute(_options = {}) - @shared = project.import_export + @shared = project.import_export_shared save_all end diff --git a/lib/gitlab/import_export/importer.rb b/lib/gitlab/import_export/importer.rb index 8b780cbe3a1..c38df9102eb 100644 --- a/lib/gitlab/import_export/importer.rb +++ b/lib/gitlab/import_export/importer.rb @@ -9,7 +9,7 @@ module Gitlab @archive_file = project.import_source @current_user = project.creator @project = project - @shared = project.import_export + @shared = project.import_export_shared end def execute diff --git a/spec/lib/gitlab/import_export/avatar_restorer_spec.rb b/spec/lib/gitlab/import_export/avatar_restorer_spec.rb index c981386ef3d..4897d604bc1 100644 --- a/spec/lib/gitlab/import_export/avatar_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/avatar_restorer_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Gitlab::ImportExport::AvatarRestorer do include UploadHelpers - let(:shared) { project.import_export } + let(:shared) { project.import_export_shared } let(:project) { create(:project) } before do diff --git a/spec/lib/gitlab/import_export/avatar_saver_spec.rb b/spec/lib/gitlab/import_export/avatar_saver_spec.rb index 78055f3970a..f40d4bc2d08 100644 --- a/spec/lib/gitlab/import_export/avatar_saver_spec.rb +++ b/spec/lib/gitlab/import_export/avatar_saver_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::ImportExport::AvatarSaver do - let(:shared) { project.import_export } + let(:shared) { project.import_export_shared } let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" } let(:project_with_avatar) { create(:project, avatar: fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "image/png")) } let(:project) { create(:project) } diff --git a/spec/lib/gitlab/import_export/fork_spec.rb b/spec/lib/gitlab/import_export/fork_spec.rb index 77aa37a1992..17e06a6a83f 100644 --- a/spec/lib/gitlab/import_export/fork_spec.rb +++ b/spec/lib/gitlab/import_export/fork_spec.rb @@ -7,7 +7,7 @@ describe 'forked project import' do let!(:project_with_repo) { create(:project, :repository, name: 'test-repo-restorer', path: 'test-repo-restorer') } let!(:project) { create(:project, name: 'test-repo-restorer-no-repo', path: 'test-repo-restorer-no-repo') } let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" } - let(:shared) { project.import_export } + let(:shared) { project.import_export_shared } let(:forked_from_project) { create(:project, :repository) } let(:forked_project) { fork_project(project_with_repo, nil, repository: true) } let(:repo_saver) { Gitlab::ImportExport::RepoSaver.new(project: project_with_repo, shared: shared) } diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb index ae4bdc7be00..777f11ef7cf 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -8,7 +8,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do RSpec::Mocks.with_temporary_scope do @project = create(:project, :builds_disabled, :issues_disabled, name: 'project', path: 'project') - @shared = @project.import_export + @shared = @project.import_export_shared allow(@shared).to receive(:export_path).and_return('spec/lib/gitlab/import_export/') allow_any_instance_of(Repository).to receive(:fetch_ref).and_return(true) @@ -259,7 +259,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do context 'Light JSON' do let(:user) { create(:user) } - let(:shared) { project.import_export } + let(:shared) { project.import_export_shared } let!(:project) { create(:project, :builds_disabled, :issues_disabled, name: 'project', path: 'project') } let(:project_tree_restorer) { described_class.new(user: user, shared: shared, project: project) } let(:restored_project_json) { project_tree_restorer.restore } diff --git a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb index 4daef697c1a..4738d991e6c 100644 --- a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Gitlab::ImportExport::ProjectTreeSaver do describe 'saves the project tree into a json object' do - let(:shared) { project.import_export } + let(:shared) { project.import_export_shared } let(:project_tree_saver) { described_class.new(project: project, current_user: user, shared: shared) } let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" } let(:user) { create(:user) } diff --git a/spec/lib/gitlab/import_export/repo_restorer_spec.rb b/spec/lib/gitlab/import_export/repo_restorer_spec.rb index 3fdb40996bf..dc806d036ff 100644 --- a/spec/lib/gitlab/import_export/repo_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/repo_restorer_spec.rb @@ -6,7 +6,7 @@ describe Gitlab::ImportExport::RepoRestorer do let!(:project_with_repo) { create(:project, :repository, name: 'test-repo-restorer', path: 'test-repo-restorer') } let!(:project) { create(:project) } let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" } - let(:shared) { project.import_export } + let(:shared) { project.import_export_shared } let(:bundler) { Gitlab::ImportExport::RepoSaver.new(project: project_with_repo, shared: shared) } let(:bundle_path) { File.join(shared.export_path, Gitlab::ImportExport.project_bundle_filename) } let(:restorer) do diff --git a/spec/lib/gitlab/import_export/repo_saver_spec.rb b/spec/lib/gitlab/import_export/repo_saver_spec.rb index 74859d81c71..187ec8fcfa2 100644 --- a/spec/lib/gitlab/import_export/repo_saver_spec.rb +++ b/spec/lib/gitlab/import_export/repo_saver_spec.rb @@ -5,7 +5,7 @@ describe Gitlab::ImportExport::RepoSaver do let(:user) { create(:user) } let!(:project) { create(:project, :public, name: 'searchable_project') } let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" } - let(:shared) { project.import_export } + let(:shared) { project.import_export_shared } let(:bundler) { described_class.new(project: project, shared: shared) } before do diff --git a/spec/lib/gitlab/import_export/uploads_restorer_spec.rb b/spec/lib/gitlab/import_export/uploads_restorer_spec.rb index e919a7d9cbb..acef97459b8 100644 --- a/spec/lib/gitlab/import_export/uploads_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/uploads_restorer_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Gitlab::ImportExport::UploadsRestorer do describe 'bundle a project Git repo' do let(:export_path) { "#{Dir.tmpdir}/uploads_saver_spec" } - let(:shared) { project.import_export } + let(:shared) { project.import_export_shared } before do allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) diff --git a/spec/lib/gitlab/import_export/uploads_saver_spec.rb b/spec/lib/gitlab/import_export/uploads_saver_spec.rb index 8ac37850328..1304d8fabfc 100644 --- a/spec/lib/gitlab/import_export/uploads_saver_spec.rb +++ b/spec/lib/gitlab/import_export/uploads_saver_spec.rb @@ -4,7 +4,7 @@ describe Gitlab::ImportExport::UploadsSaver do describe 'bundle a project Git repo' do let(:export_path) { "#{Dir.tmpdir}/uploads_saver_spec" } let(:file) { fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif') } - let(:shared) { project.import_export } + let(:shared) { project.import_export_shared } before do allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) diff --git a/spec/lib/gitlab/import_export/wiki_repo_saver_spec.rb b/spec/lib/gitlab/import_export/wiki_repo_saver_spec.rb index fbd00cf5ce5..d2bd8ccdf3f 100644 --- a/spec/lib/gitlab/import_export/wiki_repo_saver_spec.rb +++ b/spec/lib/gitlab/import_export/wiki_repo_saver_spec.rb @@ -5,7 +5,7 @@ describe Gitlab::ImportExport::WikiRepoSaver do let(:user) { create(:user) } let!(:project) { create(:project, :public, name: 'searchable_project') } let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" } - let(:shared) { project.import_export } + let(:shared) { project.import_export_shared } let(:wiki_bundler) { described_class.new(project: project, shared: shared) } let!(:project_wiki) { ProjectWiki.new(project, user) } diff --git a/spec/lib/gitlab/import_export/wiki_restorer_spec.rb b/spec/lib/gitlab/import_export/wiki_restorer_spec.rb index 575ae837d64..5c01ee0ebb8 100644 --- a/spec/lib/gitlab/import_export/wiki_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/wiki_restorer_spec.rb @@ -6,7 +6,7 @@ describe Gitlab::ImportExport::WikiRestorer do let!(:project_without_wiki) { create(:project) } let!(:project) { create(:project) } let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" } - let(:shared) { project.import_export } + let(:shared) { project.import_export_shared } let(:bundler) { Gitlab::ImportExport::WikiRepoSaver.new(project: project_with_wiki, shared: shared) } let(:bundle_path) { File.join(shared.export_path, Gitlab::ImportExport.project_bundle_filename) } let(:restorer) do From 3e71955befba95f823ba92290dedc13a9bf332ff Mon Sep 17 00:00:00 2001 From: Travis Miller Date: Mon, 5 Mar 2018 13:55:20 -0600 Subject: [PATCH 16/16] review: prefix un-used argument with underscore --- lib/api/entities.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/entities.rb b/lib/api/entities.rb index f140563882f..99424f1d867 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -95,7 +95,7 @@ module API include ::API::Helpers::RelatedResourcesHelpers expose :export_status - expose :_links, if: lambda { |project, options| project.export_status == :finished } do + expose :_links, if: lambda { |project, _options| project.export_status == :finished } do expose :api_url do |project| expose_url(api_v4_projects_export_download_path(id: project.id)) end