From e40c0085ef300aca38076af3ea2f227761084038 Mon Sep 17 00:00:00 2001 From: Bob Van Landuyt Date: Thu, 29 Mar 2018 13:57:21 +0200 Subject: [PATCH] Store override params as import data on projects This means import data doesn't necessarily have to have an import_url anymore. The `ProjectImportData` could just contain the override data in it's serialized data attribute. The import data is automatically cleaned up after it is finished by the state machine. --- app/models/project.rb | 2 +- .../gitlab_projects_import_service.rb | 5 ++-- lib/api/project_import.rb | 12 +++++++- spec/requests/api/project_import_spec.rb | 30 ++++++++++++++++++- .../gitlab_projects_import_service_spec.rb | 16 ++++++++-- 5 files changed, 57 insertions(+), 8 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index 714a15ade9c..71d2f30698e 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -630,7 +630,7 @@ class Project < ActiveRecord::Base end def create_or_update_import_data(data: nil, credentials: nil) - return unless import_url.present? && valid_import_url? + return if data.nil? && credentials.nil? project_import_data = import_data || build_import_data if data diff --git a/app/services/projects/gitlab_projects_import_service.rb b/app/services/projects/gitlab_projects_import_service.rb index a68ecb4abe1..fb4afb85588 100644 --- a/app/services/projects/gitlab_projects_import_service.rb +++ b/app/services/projects/gitlab_projects_import_service.rb @@ -5,8 +5,8 @@ module Projects class GitlabProjectsImportService attr_reader :current_user, :params - def initialize(user, params) - @current_user, @params = user, params.dup + def initialize(user, import_params, override_params = nil) + @current_user, @params, @override_params = user, import_params.dup, override_params end def execute @@ -17,6 +17,7 @@ module Projects params[:import_type] = 'gitlab_project' params[:import_source] = import_upload_path + params[:import_data] = { data: { override_params: @override_params } } if @override_params ::Projects::CreateService.new(current_user, params).execute end diff --git a/lib/api/project_import.rb b/lib/api/project_import.rb index a509c1f32c1..303b58a5942 100644 --- a/lib/api/project_import.rb +++ b/lib/api/project_import.rb @@ -1,6 +1,7 @@ module API class ProjectImport < Grape::API include PaginationParams + include Helpers::ProjectsHelpers helpers do def import_params @@ -25,6 +26,11 @@ module API requires :path, type: String, desc: 'The new project path and name' requires :file, type: File, desc: 'The project export file to be imported' optional :namespace, type: String, desc: "The ID or name of the namespace that the project will be imported into. Defaults to the current user's namespace." + optional :override_params, + type: Hash, + desc: 'New project params to override values in the export' do + use :optional_project_params + end end desc 'Create a new project import' do detail 'This feature was introduced in GitLab 10.6.' @@ -47,7 +53,11 @@ module API file: import_params[:file]['tempfile'] } - project = ::Projects::GitlabProjectsImportService.new(current_user, project_params).execute + override_params = import_params.delete(:override_params) + + project = ::Projects::GitlabProjectsImportService.new( + current_user, project_params, override_params + ).execute render_api_error!(project.errors.full_messages&.first, 400) unless project.saved? diff --git a/spec/requests/api/project_import_spec.rb b/spec/requests/api/project_import_spec.rb index 987f6e26971..cf0c2aa903a 100644 --- a/spec/requests/api/project_import_spec.rb +++ b/spec/requests/api/project_import_spec.rb @@ -40,7 +40,7 @@ describe API::ProjectImport do expect(response).to have_gitlab_http_status(201) end - it 'schedules an import at the user namespace level' do + it 'does not shedule an import for a nampespace that does not exist' do expect_any_instance_of(Project).not_to receive(:import_schedule) expect(::Projects::CreateService).not_to receive(:new) @@ -71,6 +71,34 @@ describe API::ProjectImport do expect(json_response['error']).to eq('file is invalid') end + it 'allows overriding project params' do + stub_import(namespace) + override_params = { 'description' => 'Hello world' } + + post api('/projects/import', user), + path: 'test-import', + file: fixture_file_upload(file), + namespace: namespace.id, + override_params: override_params + import_project = Project.find(json_response['id']) + + expect(import_project.import_data.data['override_params']).to eq(override_params) + end + + it 'does store params that are not allowed' do + stub_import(namespace) + override_params = { 'not_allowed' => 'Hello world' } + + post api('/projects/import', user), + path: 'test-import', + file: fixture_file_upload(file), + namespace: namespace.id, + override_params: override_params + import_project = Project.find(json_response['id']) + + expect(import_project.import_data.data['override_params']).to be_empty + end + def stub_import(namespace) expect_any_instance_of(Project).to receive(:import_schedule) expect(::Projects::CreateService).to receive(:new).with(user, hash_including(namespace_id: namespace.id)).and_call_original diff --git a/spec/services/projects/gitlab_projects_import_service_spec.rb b/spec/services/projects/gitlab_projects_import_service_spec.rb index 6b8f9619bc4..880b2aae66a 100644 --- a/spec/services/projects/gitlab_projects_import_service_spec.rb +++ b/spec/services/projects/gitlab_projects_import_service_spec.rb @@ -2,8 +2,10 @@ require 'spec_helper' describe Projects::GitlabProjectsImportService do set(:namespace) { create(:namespace) } + let(:path) { 'test-path' } let(:file) { fixture_file_upload(Rails.root + 'spec/fixtures/doc_sample.txt', 'text/plain') } - subject { described_class.new(namespace.owner, { namespace_id: namespace.id, path: path, file: file }) } + let(:import_params) { { namespace_id: namespace.id, path: path, file: file } } + subject { described_class.new(namespace.owner, import_params) } describe '#execute' do context 'with an invalid path' do @@ -18,8 +20,6 @@ describe Projects::GitlabProjectsImportService do end context 'with a valid path' do - let(:path) { 'test-path' } - it 'creates a project' do project = subject.execute @@ -27,5 +27,15 @@ describe Projects::GitlabProjectsImportService do expect(project).to be_valid end end + + context 'override params' do + it 'stores them as import data when passed' do + project = described_class + .new(namespace.owner, import_params, description: 'Hello') + .execute + + expect(project.import_data.data['override_params']['description']).to eq('Hello') + end + end end end -- GitLab