From a7e7b59cde4bac7dacd754ac83afd3a08ba54783 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 11 Sep 2018 10:39:36 +0000 Subject: Merge branch '51318-project-export-broken-when-avatar-is-set' into 'master' Resolve "Project export broken when avatar is set" Closes #51318 See merge request gitlab-org/gitlab-ce!21649 --- app/uploaders/avatar_uploader.rb | 2 +- ...18-project-export-broken-when-avatar-is-set.yml | 5 ++++ lib/gitlab/import_export/avatar_restorer.rb | 2 +- .../gitlab/import_export/avatar_restorer_spec.rb | 33 +++++++++++++++------- spec/uploaders/avatar_uploader_spec.rb | 8 ++++++ 5 files changed, 38 insertions(+), 12 deletions(-) create mode 100644 changelogs/unreleased/51318-project-export-broken-when-avatar-is-set.yml diff --git a/app/uploaders/avatar_uploader.rb b/app/uploaders/avatar_uploader.rb index 8526bc16390..c0165759203 100644 --- a/app/uploaders/avatar_uploader.rb +++ b/app/uploaders/avatar_uploader.rb @@ -19,7 +19,7 @@ class AvatarUploader < GitlabUploader end def absolute_path - self.class.absolute_path(model.avatar) + self.class.absolute_path(model.avatar.upload) end private diff --git a/changelogs/unreleased/51318-project-export-broken-when-avatar-is-set.yml b/changelogs/unreleased/51318-project-export-broken-when-avatar-is-set.yml new file mode 100644 index 00000000000..c0f7e88f2b7 --- /dev/null +++ b/changelogs/unreleased/51318-project-export-broken-when-avatar-is-set.yml @@ -0,0 +1,5 @@ +--- +title: Fix broken exports when they include a projet avatar +merge_request: 21649 +author: +type: fixed diff --git a/lib/gitlab/import_export/avatar_restorer.rb b/lib/gitlab/import_export/avatar_restorer.rb index ded05f73cf8..17796430811 100644 --- a/lib/gitlab/import_export/avatar_restorer.rb +++ b/lib/gitlab/import_export/avatar_restorer.rb @@ -19,7 +19,7 @@ module Gitlab private def avatar_export_file - @avatar_export_file ||= Dir["#{avatar_export_path}/**/*"].first + @avatar_export_file ||= Dir["#{avatar_export_path}/**/*"].find { |f| File.file?(f) } end def avatar_export_path diff --git a/spec/lib/gitlab/import_export/avatar_restorer_spec.rb b/spec/lib/gitlab/import_export/avatar_restorer_spec.rb index 4897d604bc1..e44ff6bbcbd 100644 --- a/spec/lib/gitlab/import_export/avatar_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/avatar_restorer_spec.rb @@ -6,22 +6,35 @@ describe Gitlab::ImportExport::AvatarRestorer do let(:shared) { project.import_export_shared } let(:project) { create(:project) } - before do - allow_any_instance_of(described_class).to receive(:avatar_export_file) - .and_return(uploaded_image_temp_path) - end - after do project.remove_avatar! end - it 'restores a project avatar' do - expect(described_class.new(project: project, shared: shared).restore).to be true + context 'with avatar' do + before do + allow_any_instance_of(described_class).to receive(:avatar_export_file) + .and_return(uploaded_image_temp_path) + end + + it 'restores a project avatar' do + expect(described_class.new(project: project, shared: shared).restore).to be true + end + + it 'saves the avatar into the project' do + described_class.new(project: project, shared: shared).restore + + expect(project.reload.avatar.file.exists?).to be true + end end - it 'saves the avatar into the project' do - described_class.new(project: project, shared: shared).restore + it 'does not break if there is just a directory' do + Dir.mktmpdir do |tmpdir| + FileUtils.mkdir_p("#{tmpdir}/a/b") + + allow_any_instance_of(described_class).to receive(:avatar_export_path) + .and_return("#{tmpdir}/a") - expect(project.reload.avatar.file.exists?).to be true + expect(described_class.new(project: project, shared: shared).restore).to be true + end end end diff --git a/spec/uploaders/avatar_uploader_spec.rb b/spec/uploaders/avatar_uploader_spec.rb index b0468bc35ff..6aaec7a4fef 100644 --- a/spec/uploaders/avatar_uploader_spec.rb +++ b/spec/uploaders/avatar_uploader_spec.rb @@ -35,5 +35,13 @@ describe AvatarUploader do it_behaves_like "migrates", to_store: described_class::Store::REMOTE it_behaves_like "migrates", from_store: described_class::Store::REMOTE, to_store: described_class::Store::LOCAL + + it 'sets the right absolute path' do + storage_path = Gitlab.config.uploads.storage_path + absolute_path = File.join(storage_path, upload.path) + + expect(uploader.absolute_path.scan(storage_path).size).to eq(1) + expect(uploader.absolute_path).to eq(absolute_path) + end end end -- cgit v1.2.1