diff options
author | Michael Kozono <mkozono@gmail.com> | 2017-11-14 16:11:53 -0800 |
---|---|---|
committer | Michael Kozono <mkozono@gmail.com> | 2017-12-01 15:26:41 -0800 |
commit | 3dc0b118eca3716c0cda841232e0789739627957 (patch) | |
tree | ea9217d3108a11557f254edaada6bd924f5e23f8 | |
parent | 81f061d5a4ebefefe0efbc3c1f28b86b665a2b92 (diff) | |
download | gitlab-ce-3dc0b118eca3716c0cda841232e0789739627957.tar.gz |
Store paths relative to CarrierWave.root
So the path on source installs cannot be too long for our column.
And fix the column length test since Route.path is limited to 255 chars, it doesn’t matter how many nested groups there are.
5 files changed, 79 insertions, 68 deletions
diff --git a/lib/gitlab/background_migration/populate_untracked_uploads.rb b/lib/gitlab/background_migration/populate_untracked_uploads.rb index bd0f2f591a4..c5f9d998d9e 100644 --- a/lib/gitlab/background_migration/populate_untracked_uploads.rb +++ b/lib/gitlab/background_migration/populate_untracked_uploads.rb @@ -90,7 +90,7 @@ module Gitlab matchd = path_relative_to_upload_dir.match(FILE_UPLOADER_PATH_PATTERN) matchd[0].sub(%r{\A/}, '') # remove leading slash else - path_relative_to_carrierwave_root + path end end @@ -113,20 +113,16 @@ module Gitlab end def file_size - File.size(path) + absolute_path = File.join(CarrierWave.root, path) + File.size(absolute_path) end # Not including a leading slash def path_relative_to_upload_dir - base = %r{\A#{Regexp.escape(Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR)}/} + base = %r{\A#{Regexp.escape(Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR)}/} @path_relative_to_upload_dir ||= path.sub(base, '') end - # Not including a leading slash - def path_relative_to_carrierwave_root - "uploads/#{path_relative_to_upload_dir}" - end - private def matching_pattern_map diff --git a/lib/gitlab/background_migration/prepare_untracked_uploads.rb b/lib/gitlab/background_migration/prepare_untracked_uploads.rb index 6aba3011720..e9d162661d1 100644 --- a/lib/gitlab/background_migration/prepare_untracked_uploads.rb +++ b/lib/gitlab/background_migration/prepare_untracked_uploads.rb @@ -5,8 +5,12 @@ module Gitlab include Database::MigrationHelpers FILE_PATH_BATCH_SIZE = 500 - UPLOAD_DIR = "#{CarrierWave.root}/uploads".freeze + RELATIVE_UPLOAD_DIR = "uploads".freeze + ABSOLUTE_UPLOAD_DIR = "#{CarrierWave.root}/#{RELATIVE_UPLOAD_DIR}".freeze FOLLOW_UP_MIGRATION = 'PopulateUntrackedUploads'.freeze + START_WITH_CARRIERWAVE_ROOT_REGEX = %r{\A#{CarrierWave.root}/} + EXCLUDED_HASHED_UPLOADS_PATH = "#{ABSOLUTE_UPLOAD_DIR}/@hashed/*".freeze + EXCLUDED_TMP_UPLOADS_PATH = "#{ABSOLUTE_UPLOAD_DIR}/tmp/*".freeze class UntrackedFile < ActiveRecord::Base include EachBatch @@ -28,9 +32,9 @@ module Gitlab end def store_untracked_file_paths - return unless Dir.exist?(UPLOAD_DIR) + return unless Dir.exist?(ABSOLUTE_UPLOAD_DIR) - each_file_batch(UPLOAD_DIR, FILE_PATH_BATCH_SIZE) do |file_paths| + each_file_batch(ABSOLUTE_UPLOAD_DIR, FILE_PATH_BATCH_SIZE) do |file_paths| insert_file_paths(file_paths) end end @@ -49,7 +53,7 @@ module Gitlab paths = [] stdout.each_line("\0") do |line| - paths << line.chomp("\0") + paths << line.chomp("\0").sub(START_WITH_CARRIERWAVE_ROOT_REGEX, '') if paths.size >= batch_size yield(paths) @@ -61,9 +65,7 @@ module Gitlab end def build_find_command(search_dir) - hashed_path = "#{UPLOAD_DIR}/@hashed/*" - tmp_path = "#{UPLOAD_DIR}/tmp/*" - cmd = %W[find #{search_dir} -type f ! ( -path #{hashed_path} -prune ) ! ( -path #{tmp_path} -prune ) -print0] + cmd = %W[find #{search_dir} -type f ! ( -path #{EXCLUDED_HASHED_UPLOADS_PATH} -prune ) ! ( -path #{EXCLUDED_TMP_UPLOADS_PATH} -prune ) -print0] cmd = %w[ionice -c Idle] + cmd if ionice_is_available? diff --git a/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb b/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb index b3122e90c83..953793a353c 100644 --- a/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb +++ b/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb @@ -29,14 +29,14 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :migration, :sid appearance.update!(header_logo: uploaded_file) # File records created by PrepareUntrackedUploads - untracked_files_for_uploads.create!(path: appearance.logo.file.file) - untracked_files_for_uploads.create!(path: appearance.header_logo.file.file) - untracked_files_for_uploads.create!(path: user1.avatar.file.file) - untracked_files_for_uploads.create!(path: user2.avatar.file.file) - untracked_files_for_uploads.create!(path: project1.avatar.file.file) - untracked_files_for_uploads.create!(path: project2.avatar.file.file) - untracked_files_for_uploads.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/#{project1.full_path}/#{project1.uploads.last.path}") - untracked_files_for_uploads.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/#{project2.full_path}/#{project2.uploads.last.path}") + untracked_files_for_uploads.create!(path: found_path(appearance.logo.file.file)) + untracked_files_for_uploads.create!(path: found_path(appearance.header_logo.file.file)) + untracked_files_for_uploads.create!(path: found_path(user1.avatar.file.file)) + untracked_files_for_uploads.create!(path: found_path(user2.avatar.file.file)) + untracked_files_for_uploads.create!(path: found_path(project1.avatar.file.file)) + untracked_files_for_uploads.create!(path: found_path(project2.avatar.file.file)) + untracked_files_for_uploads.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/#{project1.full_path}/#{project1.uploads.last.path}") + untracked_files_for_uploads.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/#{project2.full_path}/#{project2.uploads.last.path}") user2.uploads.delete_all project2.uploads.delete_all @@ -122,7 +122,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do let(:user1) { create(:user) } context 'when the file is already in the uploads table' do - let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/-/system/user/avatar/#{user1.id}/avatar.jpg") } + let(:untracked_file) { described_class.create!(path: "uploads/-/system/user/avatar/#{user1.id}/avatar.jpg") } before do upload_class.create!(path: "uploads/-/system/user/avatar/#{user1.id}/avatar.jpg", uploader: 'AvatarUploader', model_type: 'User', model_id: user1.id, size: 1234) @@ -142,7 +142,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do context 'for an appearance logo file path' do let(:model) { create(:appearance) } - let(:untracked_file) { described_class.create!(path: model.logo.file.file) } + let(:untracked_file) { described_class.create!(path: found_path(model.logo.file.file)) } before do model.update!(logo: uploaded_file) @@ -163,7 +163,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do context 'for an appearance header_logo file path' do let(:model) { create(:appearance) } - let(:untracked_file) { described_class.create!(path: model.header_logo.file.file) } + let(:untracked_file) { described_class.create!(path: found_path(model.header_logo.file.file)) } before do model.update!(header_logo: uploaded_file) @@ -184,7 +184,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do context 'for a pre-Markdown Note attachment file path' do let(:model) { create(:note) } - let(:untracked_file) { described_class.create!(path: model.attachment.file.file) } + let(:untracked_file) { described_class.create!(path: found_path(model.attachment.file.file)) } before do model.update!(attachment: uploaded_file) @@ -207,7 +207,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do context 'for a user avatar file path' do let(:model) { create(:user) } - let(:untracked_file) { described_class.create!(path: model.avatar.file.file) } + let(:untracked_file) { described_class.create!(path: found_path(model.avatar.file.file)) } before do model.update!(avatar: uploaded_file) @@ -228,7 +228,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do context 'for a group avatar file path' do let(:model) { create(:group) } - let(:untracked_file) { described_class.create!(path: model.avatar.file.file) } + let(:untracked_file) { described_class.create!(path: found_path(model.avatar.file.file)) } before do model.update!(avatar: uploaded_file) @@ -251,7 +251,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do context 'for a project avatar file path' do let(:model) { create(:project) } - let(:untracked_file) { described_class.create!(path: model.avatar.file.file) } + let(:untracked_file) { described_class.create!(path: found_path(model.avatar.file.file)) } before do model.update!(avatar: uploaded_file) @@ -272,7 +272,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do context 'for a project Markdown attachment (notes, issues, MR descriptions) file path' do let(:model) { create(:project) } - let(:untracked_file) { described_class.new(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/#{model.full_path}/#{model.uploads.first.path}") } + let(:untracked_file) { described_class.new(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/#{model.full_path}/#{model.uploads.first.path}") } before do UploadService.new(model, uploaded_file, FileUploader).execute # Markdown upload @@ -295,7 +295,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do end describe '#mark_as_tracked' do - let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/-/system/appearance/logo/1/some_logo.jpg") } + let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/-/system/appearance/logo/1/some_logo.jpg") } it 'saves the record with tracked set to true' do expect do @@ -308,7 +308,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do describe '#upload_path' do context 'for an appearance logo file path' do - let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/-/system/appearance/logo/1/some_logo.jpg") } + let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/-/system/appearance/logo/1/some_logo.jpg") } it 'returns the file path relative to the CarrierWave root' do expect(untracked_file.upload_path).to eq('uploads/-/system/appearance/logo/1/some_logo.jpg') @@ -316,7 +316,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do end context 'for an appearance header_logo file path' do - let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/-/system/appearance/header_logo/1/some_logo.jpg") } + let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/-/system/appearance/header_logo/1/some_logo.jpg") } it 'returns the file path relative to the CarrierWave root' do expect(untracked_file.upload_path).to eq('uploads/-/system/appearance/header_logo/1/some_logo.jpg') @@ -324,7 +324,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do end context 'for a pre-Markdown Note attachment file path' do - let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/-/system/note/attachment/1234/some_attachment.pdf") } + let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/-/system/note/attachment/1234/some_attachment.pdf") } it 'returns the file path relative to the CarrierWave root' do expect(untracked_file.upload_path).to eq('uploads/-/system/note/attachment/1234/some_attachment.pdf') @@ -332,7 +332,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do end context 'for a user avatar file path' do - let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/-/system/user/avatar/1234/avatar.jpg") } + let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/-/system/user/avatar/1234/avatar.jpg") } it 'returns the file path relative to the CarrierWave root' do expect(untracked_file.upload_path).to eq('uploads/-/system/user/avatar/1234/avatar.jpg') @@ -340,7 +340,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do end context 'for a group avatar file path' do - let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/-/system/group/avatar/1234/avatar.jpg") } + let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/-/system/group/avatar/1234/avatar.jpg") } it 'returns the file path relative to the CarrierWave root' do expect(untracked_file.upload_path).to eq('uploads/-/system/group/avatar/1234/avatar.jpg') @@ -348,7 +348,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do end context 'for a project avatar file path' do - let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/-/system/project/avatar/1234/avatar.jpg") } + let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/-/system/project/avatar/1234/avatar.jpg") } it 'returns the file path relative to the CarrierWave root' do expect(untracked_file.upload_path).to eq('uploads/-/system/project/avatar/1234/avatar.jpg') @@ -358,7 +358,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do context 'for a project Markdown attachment (notes, issues, MR descriptions) file path' do let(:project) { create(:project) } let(:random_hex) { SecureRandom.hex } - let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/#{project.full_path}/#{random_hex}/Some file.jpg") } + let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/#{project.full_path}/#{random_hex}/Some file.jpg") } it 'returns the file path relative to the project directory in uploads' do expect(untracked_file.upload_path).to eq("#{random_hex}/Some file.jpg") @@ -368,7 +368,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do describe '#uploader' do context 'for an appearance logo file path' do - let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/-/system/appearance/logo/1/some_logo.jpg") } + let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/-/system/appearance/logo/1/some_logo.jpg") } it 'returns AttachmentUploader as a string' do expect(untracked_file.uploader).to eq('AttachmentUploader') @@ -376,7 +376,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do end context 'for an appearance header_logo file path' do - let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/-/system/appearance/header_logo/1/some_logo.jpg") } + let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/-/system/appearance/header_logo/1/some_logo.jpg") } it 'returns AttachmentUploader as a string' do expect(untracked_file.uploader).to eq('AttachmentUploader') @@ -384,7 +384,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do end context 'for a pre-Markdown Note attachment file path' do - let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/-/system/note/attachment/1234/some_attachment.pdf") } + let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/-/system/note/attachment/1234/some_attachment.pdf") } it 'returns AttachmentUploader as a string' do expect(untracked_file.uploader).to eq('AttachmentUploader') @@ -392,7 +392,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do end context 'for a user avatar file path' do - let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/-/system/user/avatar/1234/avatar.jpg") } + let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/-/system/user/avatar/1234/avatar.jpg") } it 'returns AvatarUploader as a string' do expect(untracked_file.uploader).to eq('AvatarUploader') @@ -400,7 +400,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do end context 'for a group avatar file path' do - let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/-/system/group/avatar/1234/avatar.jpg") } + let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/-/system/group/avatar/1234/avatar.jpg") } it 'returns AvatarUploader as a string' do expect(untracked_file.uploader).to eq('AvatarUploader') @@ -408,7 +408,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do end context 'for a project avatar file path' do - let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/-/system/project/avatar/1234/avatar.jpg") } + let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/-/system/project/avatar/1234/avatar.jpg") } it 'returns AvatarUploader as a string' do expect(untracked_file.uploader).to eq('AvatarUploader') @@ -417,7 +417,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do context 'for a project Markdown attachment (notes, issues, MR descriptions) file path' do let(:project) { create(:project) } - let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/#{project.full_path}/#{SecureRandom.hex}/Some file.jpg") } + let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/#{project.full_path}/#{SecureRandom.hex}/Some file.jpg") } it 'returns FileUploader as a string' do expect(untracked_file.uploader).to eq('FileUploader') @@ -427,7 +427,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do describe '#model_type' do context 'for an appearance logo file path' do - let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/-/system/appearance/logo/1/some_logo.jpg") } + let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/-/system/appearance/logo/1/some_logo.jpg") } it 'returns Appearance as a string' do expect(untracked_file.model_type).to eq('Appearance') @@ -435,7 +435,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do end context 'for an appearance header_logo file path' do - let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/-/system/appearance/header_logo/1/some_logo.jpg") } + let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/-/system/appearance/header_logo/1/some_logo.jpg") } it 'returns Appearance as a string' do expect(untracked_file.model_type).to eq('Appearance') @@ -443,7 +443,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do end context 'for a pre-Markdown Note attachment file path' do - let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/-/system/note/attachment/1234/some_attachment.pdf") } + let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/-/system/note/attachment/1234/some_attachment.pdf") } it 'returns Note as a string' do expect(untracked_file.model_type).to eq('Note') @@ -451,7 +451,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do end context 'for a user avatar file path' do - let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/-/system/user/avatar/1234/avatar.jpg") } + let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/-/system/user/avatar/1234/avatar.jpg") } it 'returns User as a string' do expect(untracked_file.model_type).to eq('User') @@ -459,7 +459,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do end context 'for a group avatar file path' do - let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/-/system/group/avatar/1234/avatar.jpg") } + let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/-/system/group/avatar/1234/avatar.jpg") } it 'returns Namespace as a string' do expect(untracked_file.model_type).to eq('Namespace') @@ -467,7 +467,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do end context 'for a project avatar file path' do - let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/-/system/project/avatar/1234/avatar.jpg") } + let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/-/system/project/avatar/1234/avatar.jpg") } it 'returns Project as a string' do expect(untracked_file.model_type).to eq('Project') @@ -476,7 +476,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do context 'for a project Markdown attachment (notes, issues, MR descriptions) file path' do let(:project) { create(:project) } - let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/#{project.full_path}/#{SecureRandom.hex}/Some file.jpg") } + let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/#{project.full_path}/#{SecureRandom.hex}/Some file.jpg") } it 'returns Project as a string' do expect(untracked_file.model_type).to eq('Project') @@ -486,7 +486,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do describe '#model_id' do context 'for an appearance logo file path' do - let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/-/system/appearance/logo/1/some_logo.jpg") } + let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/-/system/appearance/logo/1/some_logo.jpg") } it 'returns the ID as a string' do expect(untracked_file.model_id).to eq('1') @@ -494,7 +494,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do end context 'for an appearance header_logo file path' do - let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/-/system/appearance/header_logo/1/some_logo.jpg") } + let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/-/system/appearance/header_logo/1/some_logo.jpg") } it 'returns the ID as a string' do expect(untracked_file.model_id).to eq('1') @@ -502,7 +502,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do end context 'for a pre-Markdown Note attachment file path' do - let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/-/system/note/attachment/1234/some_attachment.pdf") } + let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/-/system/note/attachment/1234/some_attachment.pdf") } it 'returns the ID as a string' do expect(untracked_file.model_id).to eq('1234') @@ -510,7 +510,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do end context 'for a user avatar file path' do - let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/-/system/user/avatar/1234/avatar.jpg") } + let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/-/system/user/avatar/1234/avatar.jpg") } it 'returns the ID as a string' do expect(untracked_file.model_id).to eq('1234') @@ -518,7 +518,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do end context 'for a group avatar file path' do - let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/-/system/group/avatar/1234/avatar.jpg") } + let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/-/system/group/avatar/1234/avatar.jpg") } it 'returns the ID as a string' do expect(untracked_file.model_id).to eq('1234') @@ -526,7 +526,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do end context 'for a project avatar file path' do - let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/-/system/project/avatar/1234/avatar.jpg") } + let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/-/system/project/avatar/1234/avatar.jpg") } it 'returns the ID as a string' do expect(untracked_file.model_id).to eq('1234') @@ -535,7 +535,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do context 'for a project Markdown attachment (notes, issues, MR descriptions) file path' do let(:project) { create(:project) } - let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/#{project.full_path}/#{SecureRandom.hex}/Some file.jpg") } + let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/#{project.full_path}/#{SecureRandom.hex}/Some file.jpg") } it 'returns the ID as a string' do expect(untracked_file.model_id).to eq(project.id.to_s) @@ -549,7 +549,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do context 'for an appearance logo file path' do let(:appearance) { create(:appearance) } - let(:untracked_file) { described_class.create!(path: appearance.logo.file.file) } + let(:untracked_file) { described_class.create!(path: found_path(appearance.logo.file.file)) } before do appearance.update!(logo: uploaded_file) @@ -566,7 +566,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do context 'for a project avatar file path' do let(:project) { create(:project) } - let(:untracked_file) { described_class.create!(path: project.avatar.file.file) } + let(:untracked_file) { described_class.create!(path: found_path(project.avatar.file.file)) } before do project.update!(avatar: uploaded_file) @@ -583,7 +583,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do context 'for a project Markdown attachment (notes, issues, MR descriptions) file path' do let(:project) { create(:project) } - let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::UPLOAD_DIR}/#{project.full_path}/#{project.uploads.first.path}") } + let(:untracked_file) { described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/#{project.full_path}/#{project.uploads.first.path}") } before do UploadService.new(project, uploaded_file, FileUploader).execute @@ -599,3 +599,9 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do end end end + +# The path returned by the find command in PrepareUntrackedUploads +# AKA the path relative to CarrierWave.root, without a leading slash. +def found_path(absolute_path) + absolute_path.sub(%r{\A#{CarrierWave.root}/}, '') +end diff --git a/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb b/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb index 913622cdb95..d0dd6b7c157 100644 --- a/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb +++ b/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb @@ -47,6 +47,15 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :migration, :side end end + it 'adds files with paths relative to CarrierWave.root' do + Sidekiq::Testing.fake! do + described_class.new.perform + untracked_files_for_uploads.all.each do |file| + expect(file.path.start_with?('uploads/')).to be_truthy + end + end + end + it 'does not add hashed files to the untracked_files_for_uploads table' do Sidekiq::Testing.fake! do described_class.new.perform @@ -83,7 +92,7 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :migration, :side # E.g. The installation is in use at the time of migration, and someone has # just uploaded a file context 'when there are files in /uploads/tmp' do - let(:tmp_file) { Rails.root.join(described_class::UPLOAD_DIR, 'tmp', 'some_file.jpg') } + let(:tmp_file) { Rails.root.join(described_class::ABSOLUTE_UPLOAD_DIR, 'tmp', 'some_file.jpg') } before do FileUtils.touch(tmp_file) diff --git a/spec/migrations/track_untracked_uploads_spec.rb b/spec/migrations/track_untracked_uploads_spec.rb index 95496696bc6..83eb6bbd537 100644 --- a/spec/migrations/track_untracked_uploads_spec.rb +++ b/spec/migrations/track_untracked_uploads_spec.rb @@ -41,15 +41,13 @@ describe TrackUntrackedUploads, :migration, :sidekiq do component = 'a'*255 long_path = [ - CarrierWave.root, 'uploads', - [component] * Namespace::NUMBER_OF_ANCESTORS_ALLOWED, # namespaces - component, # project + component, # project.full_path component # filename ].flatten.join('/') record = UntrackedFile.create!(path: long_path) - expect(record.reload.path.size).to eq(5711) + expect(record.reload.path.size).to eq(519) end context 'with tracked and untracked uploads' do |