diff --git a/changelogs/unreleased/backfill-routes-for-users-migration.yml b/changelogs/unreleased/backfill-routes-for-users-migration.yml new file mode 100644 index 0000000000000000000000000000000000000000..c883d95ca0d90c08066d5a85924174aa3c55e671 --- /dev/null +++ b/changelogs/unreleased/backfill-routes-for-users-migration.yml @@ -0,0 +1,5 @@ +--- +title: Backfill missing routes for Bot users +merge_request: 35960 +author: +type: fixed diff --git a/db/post_migrate/20200703064117_generate_missing_routes_for_bots.rb b/db/post_migrate/20200703064117_generate_missing_routes_for_bots.rb new file mode 100644 index 0000000000000000000000000000000000000000..85d62cbb6ddbe7abf65aab3f992e8e62cda48901 --- /dev/null +++ b/db/post_migrate/20200703064117_generate_missing_routes_for_bots.rb @@ -0,0 +1,92 @@ +# frozen_string_literal: true + +class GenerateMissingRoutesForBots < ActiveRecord::Migration[6.0] + DOWNTIME = false + + disable_ddl_transaction! + + class User < ActiveRecord::Base + self.table_name = 'users' + + USER_TYPES = { + human: nil, + support_bot: 1, + alert_bot: 2, + visual_review_bot: 3, + service_user: 4, + ghost: 5, + project_bot: 6, + migration_bot: 7 + }.with_indifferent_access.freeze + + BOT_USER_TYPES = %w[alert_bot project_bot support_bot visual_review_bot migration_bot].freeze + + scope :bots, -> { where(user_type: USER_TYPES.values_at(*BOT_USER_TYPES)) } + end + + class Route < ActiveRecord::Base + self.table_name = 'routes' + + validates :path, + uniqueness: { case_sensitive: false } + end + + class Namespace < ActiveRecord::Base + self.table_name = 'namespaces' + + belongs_to :owner, class_name: 'GenerateMissingRoutesForBots::User' + + scope :for_user, -> { where('type IS NULL') } + scope :for_bots, -> { for_user.joins(:owner).merge(GenerateMissingRoutesForBots::User.bots) } + + scope :without_routes, -> do + where( + 'NOT EXISTS ( + SELECT 1 + FROM routes + WHERE source_type = ? + AND source_id = namespaces.id + )', + self.source_type_for_route + ) + end + + def self.source_type_for_route + 'Namespace' + end + + def attributes_for_insert + { + source_type: self.class.source_type_for_route, + source_id: id, + name: name, + path: path + } + end + end + + def up + # Reset the column information of all the models that update the database + # to ensure the Active Record's knowledge of the table structure is current + Route.reset_column_information + + logger = Gitlab::BackgroundMigration::Logger.build + attributes_to_be_logged = %w(id path name) + + GenerateMissingRoutesForBots::Namespace.for_bots.without_routes.each do |namespace| + route = GenerateMissingRoutesForBots::Route.create(namespace.attributes_for_insert) + namespace_details = namespace.as_json.slice(*attributes_to_be_logged) + + if route.persisted? + logger.info namespace_details.merge(message: 'a new route was created for the namespace') + else + errors = route.errors.full_messages.join(',') + logger.info namespace_details.merge(message: 'route creation failed for the namespace', errors: errors) + end + end + end + + def down + # no op + end +end diff --git a/db/structure.sql b/db/structure.sql index 03f5b95313e1e101d4bead737d053cb474299aca..2f9a661f0a0b00c929eb258210e861d7df016416 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -23636,6 +23636,7 @@ COPY "schema_migrations" (version) FROM STDIN; 20200701093859 20200701205710 20200702123805 +20200703064117 20200703121557 20200703154822 20200704143633 diff --git a/spec/migrations/generate_missing_routes_for_bots_spec.rb b/spec/migrations/generate_missing_routes_for_bots_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..8af22042350750559510f512997ba59230c4279e --- /dev/null +++ b/spec/migrations/generate_missing_routes_for_bots_spec.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +require 'spec_helper' + +require Rails.root.join('db', 'post_migrate', '20200703064117_generate_missing_routes_for_bots.rb') + +RSpec.describe GenerateMissingRoutesForBots, :migration do + let(:users) { table(:users) } + let(:namespaces) { table(:namespaces) } + let(:routes) { table(:routes) } + + let(:visual_review_bot) do + users.create!(email: 'visual-review-bot@gitlab.com', name: 'GitLab Visual Review Bot', username: 'visual-review-bot', user_type: 3, projects_limit: 5) + end + + let(:migration_bot) do + users.create!(email: 'migration-bot@gitlab.com', name: 'GitLab Migration Bot', username: 'migration-bot', user_type: 7, projects_limit: 5) + end + + let!(:visual_review_bot_namespace) do + namespaces.create!(owner_id: visual_review_bot.id, name: visual_review_bot.name, path: visual_review_bot.username) + end + + let!(:migration_bot_namespace) do + namespaces.create!(owner_id: migration_bot.id, name: migration_bot.name, path: migration_bot.username) + end + + context 'for bot users without an existing route' do + it 'creates new routes' do + expect { migrate! }.to change { routes.count }.by(2) + end + + it 'creates new routes with the same path and name as their namespace' do + migrate! + + [visual_review_bot, migration_bot].each do |bot| + namespace = namespaces.find_by(owner_id: bot.id) + route = route_for(namespace: namespace) + + expect(route.path).to eq(namespace.path) + expect(route.name).to eq(namespace.name) + end + end + end + + it 'does not create routes for bot users with existing routes' do + create_route!(namespace: visual_review_bot_namespace) + create_route!(namespace: migration_bot_namespace) + + expect { migrate! }.not_to change { routes.count } + end + + it 'does not create routes for human users without an existing route' do + human_namespace = create_human_namespace!(name: 'GitLab Human', username: 'human') + + expect { migrate! }.not_to change { route_for(namespace: human_namespace) } + end + + it 'does not create route for a bot user with a missing route, if a human user with the same path already exists' do + human_namespace = create_human_namespace!(name: visual_review_bot.name, username: visual_review_bot.username) + create_route!(namespace: human_namespace) + + expect { migrate! }.not_to change { route_for(namespace: visual_review_bot_namespace) } + end + + private + + def create_human_namespace!(name:, username:) + human = users.create!(email: 'human@gitlab.com', name: name, username: username, user_type: nil, projects_limit: 5) + namespaces.create!(owner_id: human.id, name: human.name, path: human.username) + end + + def create_route!(namespace:) + routes.create!(path: namespace.path, name: namespace.name, source_id: namespace.id, source_type: 'Namespace') + end + + def route_for(namespace:) + routes.find_by(source_type: 'Namespace', source_id: namespace.id) + end +end