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 e1a5a17a60c..9c1261881df 100644 --- a/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb +++ b/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb @@ -1,13 +1,12 @@ require 'spec_helper' -require Rails.root.join('db', 'post_migrate', '20171103140253_track_untracked_uploads') -describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :migration, :sidekiq, schema: 20171103140253 do +describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :sidekiq do include TrackUntrackedUploadsHelpers subject { described_class.new } - let!(:untracked_files_for_uploads) { table(:untracked_files_for_uploads) } - let!(:uploads) { table(:uploads) } + let!(:untracked_files_for_uploads) { described_class::UntrackedFile } + let!(:uploads) { described_class::Upload } before do ensure_temporary_tracking_table_exists @@ -18,7 +17,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :migration, :sid end context 'with untracked files and tracked files in untracked_files_for_uploads' do - let!(:appearance) { create(:appearance, logo: uploaded_file, header_logo: uploaded_file) } + let!(:appearance) { create_or_update_appearance(logo: uploaded_file, header_logo: uploaded_file) } let!(:user1) { create(:user, :with_avatar) } let!(:user2) { create(:user, :with_avatar) } let!(:project1) { create(:project, :with_avatar) } @@ -111,13 +110,13 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :migration, :sid it 'does not drop the temporary tracking table after processing the batch, if there are still untracked rows' do subject.perform(1, untracked_files_for_uploads.last.id - 1) - expect(table_exists?(:untracked_files_for_uploads)).to be_truthy + expect(ActiveRecord::Base.connection.table_exists?(:untracked_files_for_uploads)).to be_truthy end it 'drops the temporary tracking table after processing the batch, if there are no untracked rows left' do subject.perform(1, untracked_files_for_uploads.last.id) - expect(table_exists?(:untracked_files_for_uploads)).to be_falsey + expect(ActiveRecord::Base.connection.table_exists?(:untracked_files_for_uploads)).to be_falsey end it 'does not block a whole batch because of one bad path' do @@ -168,13 +167,13 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :migration, :sid end context 'for an appearance logo file path' do - let(:model) { create(:appearance, logo: uploaded_file) } + let(:model) { create_or_update_appearance(logo: uploaded_file) } it_behaves_like 'non_markdown_file' end context 'for an appearance header_logo file path' do - let(:model) { create(:appearance, header_logo: uploaded_file) } + let(:model) { create_or_update_appearance(header_logo: uploaded_file) } it_behaves_like 'non_markdown_file' end @@ -459,7 +458,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do describe '#file_size' do context 'for an appearance logo file path' do - let(:appearance) { create(:appearance, logo: uploaded_file) } + let(:appearance) { create_or_update_appearance(logo: uploaded_file) } let(:untracked_file) { described_class.create!(path: appearance.uploads.first.path) } it 'returns the file size' do 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 f1eb7173717..dfe37e43751 100644 --- a/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb +++ b/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb @@ -1,9 +1,9 @@ require 'spec_helper' -describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :migration, :sidekiq, schema: 20171103140253 do +describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do include TrackUntrackedUploadsHelpers - let!(:untracked_files_for_uploads) { table(:untracked_files_for_uploads) } + let!(:untracked_files_for_uploads) { described_class::UntrackedFile } matcher :be_scheduled_migration do |*expected| match do |migration| @@ -35,7 +35,7 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :migration, :side it 'ensures the untracked_files_for_uploads table exists' do expect do described_class.new.perform - end.to change { table_exists?(:untracked_files_for_uploads) }.from(false).to(true) + end.to change { ActiveRecord::Base.connection.table_exists?(:untracked_files_for_uploads) }.from(false).to(true) end it 'has a path field long enough for really long paths' do @@ -63,7 +63,7 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :migration, :side end context 'when files were uploaded before and after hashed storage was enabled' do - let!(:appearance) { create(:appearance, logo: uploaded_file, header_logo: uploaded_file) } + let!(:appearance) { create_or_update_appearance(logo: uploaded_file, header_logo: uploaded_file) } let!(:user) { create(:user, :with_avatar) } let!(:project1) { create(:project, :with_avatar) } let(:project2) { create(:project) } # instantiate after enabling hashed_storage @@ -151,7 +151,7 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :migration, :side end context 'when files were uploaded before and after hashed storage was enabled' do - let!(:appearance) { create(:appearance, logo: uploaded_file, header_logo: uploaded_file) } + let!(:appearance) { create_or_update_appearance(logo: uploaded_file, header_logo: uploaded_file) } let!(:user) { create(:user, :with_avatar) } let!(:project1) { create(:project, :with_avatar) } let(:project2) { create(:project) } # instantiate after enabling hashed_storage diff --git a/spec/migrations/track_untracked_uploads_spec.rb b/spec/migrations/track_untracked_uploads_spec.rb index c9bbc09cc49..7fe7a140e2f 100644 --- a/spec/migrations/track_untracked_uploads_spec.rb +++ b/spec/migrations/track_untracked_uploads_spec.rb @@ -4,9 +4,6 @@ require Rails.root.join('db', 'post_migrate', '20171103140253_track_untracked_up describe TrackUntrackedUploads, :migration, :sidekiq do include TrackUntrackedUploadsHelpers - let(:untracked_files_for_uploads) { table(:untracked_files_for_uploads) } - let(:uploads) { table(:uploads) } - matcher :be_scheduled_migration do match do |migration| BackgroundMigrationWorker.jobs.any? do |job| @@ -27,75 +24,4 @@ describe TrackUntrackedUploads, :migration, :sidekiq do expect(BackgroundMigrationWorker.jobs.size).to eq(1) end end - - context 'with tracked and untracked uploads' do - let!(:appearance) { create(:appearance, logo: uploaded_file, header_logo: uploaded_file) } - let!(:user1) { create(:user, :with_avatar) } - let!(:user2) { create(:user, :with_avatar) } - let!(:project1) { create(:project, :with_avatar) } - let!(:project2) { create(:project, :with_avatar) } - - before do - UploadService.new(project1, uploaded_file, FileUploader).execute # Markdown upload - UploadService.new(project2, uploaded_file, FileUploader).execute # Markdown upload - - # Save expected Upload attributes - @appearance_logo_attributes = appearance.uploads.where("path like '%/logo/%'").first.attributes.slice('path', 'uploader', 'size', 'checksum') - @appearance_header_logo_attributes = appearance.uploads.where("path like '%/header_logo/%'").first.attributes.slice('path', 'uploader', 'size', 'checksum') - @user1_avatar_attributes = user1.uploads.first.attributes.slice('path', 'uploader', 'size', 'checksum') - @user2_avatar_attributes = user2.uploads.first.attributes.slice('path', 'uploader', 'size', 'checksum') - @project1_avatar_attributes = project1.uploads.first.attributes.slice('path', 'uploader', 'size', 'checksum') - @project2_avatar_attributes = project2.uploads.first.attributes.slice('path', 'uploader', 'size', 'checksum') - @project1_markdown_attributes = project1.uploads.last.attributes.slice('path', 'uploader', 'size', 'checksum') - @project2_markdown_attributes = project2.uploads.last.attributes.slice('path', 'uploader', 'size', 'checksum') - - # Untrack 4 files - user2.uploads.delete_all - project2.uploads.delete_all # 2 files: avatar and a Markdown upload - appearance.uploads.where("path like '%header_logo%'").delete_all - end - - it 'tracks untracked uploads' do - expect do - migrate! - end.to change { uploads.count }.from(4).to(8) - - expect(appearance.reload.uploads.where("path like '%/header_logo/%'").first.attributes).to include(@appearance_header_logo_attributes) - expect(user2.reload.uploads.first.attributes).to include(@user2_avatar_attributes) - expect(project2.reload.uploads.where(uploader: 'AvatarUploader').first.attributes).to include(@project2_avatar_attributes) - expect(project2.uploads.where(uploader: 'FileUploader').first.attributes).to include(@project2_markdown_attributes) - end - - it 'ignores already-tracked uploads' do - migrate! - - expect(appearance.reload.uploads.where("path like '%/logo/%'").first.attributes).to include(@appearance_logo_attributes) - expect(user1.reload.uploads.first.attributes).to include(@user1_avatar_attributes) - expect(project1.reload.uploads.where(uploader: 'AvatarUploader').first.attributes).to include(@project1_avatar_attributes) - expect(project1.uploads.where(uploader: 'FileUploader').first.attributes).to include(@project1_markdown_attributes) - end - - it 'ignores uploads for deleted models' do - user2.destroy - project2.destroy - - expect do - migrate! - end.to change { uploads.count }.from(4).to(5) - end - - it 'the temporary table untracked_files_for_uploads no longer exists' do - migrate! - - expect(table_exists?(:untracked_files_for_uploads)).to be_falsey - end - end - - context 'without any uploads ever' do - it 'does not add any upload records' do - expect do - migrate! - end.not_to change { uploads.count }.from(0) - end - end end diff --git a/spec/support/track_untracked_uploads_helpers.rb b/spec/support/track_untracked_uploads_helpers.rb index 4d4745fd7f4..d05eda08201 100644 --- a/spec/support/track_untracked_uploads_helpers.rb +++ b/spec/support/track_untracked_uploads_helpers.rb @@ -11,4 +11,10 @@ module TrackUntrackedUploadsHelpers def drop_temp_table_if_exists ActiveRecord::Base.connection.drop_table(:untracked_files_for_uploads) if ActiveRecord::Base.connection.table_exists?(:untracked_files_for_uploads) end + + def create_or_update_appearance(attrs) + a = Appearance.first_or_initialize(title: 'foo', description: 'bar') + a.update!(attrs) + a + end end