This instance will be upgraded to Heptapod 0.31.0 (final) on 2022-05-24 at 14:00 UTC+2 (a few minutes of down time)

Commit f09cf593 authored by Michael Kozono's avatar Michael Kozono
Browse files

Move temp table creation into the prepare job

* Hopefully fixes spec failures in which the table doesn’t exist
* Decouples the background migration from the post-deploy migration, e.g. we could easily run it again even though the table is dropped when finished.
parent f8093100411d
......@@ -10,8 +10,6 @@ class TrackUntrackedUploads < ActiveRecord::Migration
MIGRATION = 'PrepareUntrackedUploads'
def up
ensure_temporary_tracking_table_exists
BackgroundMigrationWorker.perform_async(MIGRATION)
end
......@@ -20,22 +18,4 @@ def down
drop_table :untracked_files_for_uploads
end
end
def ensure_temporary_tracking_table_exists
unless table_exists?(:untracked_files_for_uploads)
create_table :untracked_files_for_uploads do |t|
t.string :path, limit: 600, null: false
t.boolean :tracked, default: false, null: false
t.timestamps_with_timezone null: false
end
end
unless index_exists?(:untracked_files_for_uploads, :path)
add_index :untracked_files_for_uploads, :path, unique: true
end
unless index_exists?(:untracked_files_for_uploads, :tracked)
add_index :untracked_files_for_uploads, :tracked
end
end
end
......@@ -19,16 +19,29 @@ class UntrackedFile < ActiveRecord::Base
end
def perform
return unless migrate?
ensure_temporary_tracking_table_exists
store_untracked_file_paths
schedule_populate_untracked_uploads_jobs
end
private
def migrate?
UntrackedFile.table_exists?
def ensure_temporary_tracking_table_exists
unless UntrackedFile.connection.table_exists?(:untracked_files_for_uploads)
UntrackedFile.connection.create_table :untracked_files_for_uploads do |t|
t.string :path, limit: 600, null: false
t.boolean :tracked, default: false, null: false
t.timestamps_with_timezone null: false
end
end
unless UntrackedFile.connection.index_exists?(:untracked_files_for_uploads, :path)
UntrackedFile.connection.add_index :untracked_files_for_uploads, :path, unique: true
end
unless UntrackedFile.connection.index_exists?(:untracked_files_for_uploads, :tracked)
UntrackedFile.connection.add_index :untracked_files_for_uploads, :tracked
end
end
def store_untracked_file_paths
......
require 'spec_helper'
require Rails.root.join('db', 'post_migrate', '20171103140253_track_untracked_uploads')
describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :migration, :sidekiq, :temp_table_may_drop, schema: 20171103140253 do
describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :migration, :sidekiq, schema: 20171103140253 do
include TrackUntrackedUploadsHelpers
subject { described_class.new }
......@@ -10,8 +10,11 @@
let!(:uploads) { table(:uploads) }
before do
# Prevent the TrackUntrackedUploads migration from running PrepareUntrackedUploads job
allow(BackgroundMigrationWorker).to receive(:perform_async).and_return(true)
ensure_temporary_tracking_table_exists
end
after(:all) do
drop_temp_table_if_exists
end
context 'with untracked files and tracked files in untracked_files_for_uploads' do
......@@ -130,6 +133,14 @@
let(:upload_class) { Gitlab::BackgroundMigration::PopulateUntrackedUploads::Upload }
before(:all) do
ensure_temporary_tracking_table_exists
end
after(:all) do
drop_temp_table_if_exists
end
describe '#ensure_tracked!' do
let!(:user1) { create(:user, :with_avatar) }
let!(:untracked_file) { described_class.create!(path: user1.uploads.first.path) }
......
......@@ -17,6 +17,14 @@
end
end
before do
drop_temp_table_if_exists
end
after(:all) do
drop_temp_table_if_exists
end
around do |example|
# Especially important so the follow-up migration does not get run
Sidekiq::Testing.fake! do
......@@ -24,6 +32,27 @@
end
end
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
it 'has a path field long enough for really long paths' do
described_class.new.perform
component = 'a' * 255
long_path = [
'uploads',
component, # project.full_path
component # filename
].flatten.join('/')
record = untracked_files_for_uploads.create!(path: long_path)
expect(record.reload.path.size).to eq(519)
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!(:user) { create(:user, :with_avatar) }
......@@ -41,9 +70,9 @@
end
it 'adds unhashed files to the untracked_files_for_uploads table' do
expect do
described_class.new.perform
end.to change { untracked_files_for_uploads.count }.from(0).to(5)
described_class.new.perform
expect(untracked_files_for_uploads.count).to eq(5)
end
it 'adds files with paths relative to CarrierWave.root' do
......@@ -94,9 +123,9 @@
end
it 'does not add files from /uploads/tmp' do
expect do
described_class.new.perform
end.to change { untracked_files_for_uploads.count }.from(0).to(5)
described_class.new.perform
expect(untracked_files_for_uploads.count).to eq(5)
end
end
end
......@@ -105,9 +134,9 @@
# may not have an upload directory because they have no uploads.
context 'when no files were ever uploaded' do
it 'does not add to the untracked_files_for_uploads table (and does not raise error)' do
expect do
described_class.new.perform
end.not_to change { untracked_files_for_uploads.count }.from(0)
described_class.new.perform
expect(untracked_files_for_uploads.count).to eq(0)
end
end
end
require 'spec_helper'
require Rails.root.join('db', 'post_migrate', '20171103140253_track_untracked_uploads')
describe TrackUntrackedUploads, :migration, :sidekiq, :temp_table_may_drop do
describe TrackUntrackedUploads, :migration, :sidekiq do
include TrackUntrackedUploadsHelpers
let(:untracked_files_for_uploads) { table(:untracked_files_for_uploads) }
......@@ -18,41 +18,13 @@
end
end
context 'before running the background migration' do
around do |example|
# Especially important so the follow-up migration does not get run
Sidekiq::Testing.fake! do
example.run
end
end
it 'correctly schedules the follow-up background migration' do
it 'correctly schedules the follow-up background migration' do
Sidekiq::Testing.fake! do
migrate!
expect(described_class::MIGRATION).to be_scheduled_migration
expect(BackgroundMigrationWorker.jobs.size).to eq(1)
end
it 'ensures the untracked_files_for_uploads table exists' do
expect do
migrate!
end.to change { table_exists?(:untracked_files_for_uploads) }.from(false).to(true)
end
it 'has a path field long enough for really long paths' do
migrate!
component = 'a' * 255
long_path = [
'uploads',
component, # project.full_path
component # filename
].flatten.join('/')
record = untracked_files_for_uploads.create!(path: long_path)
expect(record.reload.path.size).to eq(519)
end
end
context 'with tracked and untracked uploads' do
......
......@@ -4,17 +4,11 @@ def uploaded_file
fixture_file_upload(fixture_path)
end
def recreate_temp_table_if_dropped
TrackUntrackedUploads.new.ensure_temporary_tracking_table_exists
def ensure_temporary_tracking_table_exists
Gitlab::BackgroundMigration::PrepareUntrackedUploads.new.send(:ensure_temporary_tracking_table_exists)
end
RSpec.configure do |config|
config.after(:each, :temp_table_may_drop) do
recreate_temp_table_if_dropped
end
config.after(:context, :temp_table_may_drop) do
recreate_temp_table_if_dropped
end
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
end
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment