4b9c17f196
This moves the logic of Project#rename_repo and all methods _only_ used by this method into a new service class: Projects::AfterRenameService. By moving this code into a separate service class we can more easily refactor it, and we also get rid of some RuboCop "disable" statements automatically. During the refactoring of this code, I removed most of the explicit logging using Gitlab::AppLogger. The data that was logged would not be useful when debugging renaming issues, as it does not add any value on top of data provided by users. I also removed a variety of comments that either mentioned something the code does in literal form, or contained various grammatical errors. Instead we now resort to more clearly named methods, removing the need for code comments. This method was chosen based on analysis in https://gitlab.com/gitlab-org/release/framework/issues/28. In this issue we determined this method has seen a total of 293 lines being changed in it. We also noticed that RuboCop determined the ABC size (https://www.softwarerenovation.com/ABCMetric.pdf) was too great.
62 lines
1.9 KiB
Ruby
62 lines
1.9 KiB
Ruby
# encoding: utf-8
|
|
|
|
require 'spec_helper'
|
|
require Rails.root.join('db', 'post_migrate', '20161221153951_rename_reserved_project_names.rb')
|
|
|
|
# This migration is using factories, which set fields that don't actually
|
|
# exist in the DB schema previous to 20161221153951. Thus we just use the
|
|
# latest schema when testing this migration.
|
|
# This is ok-ish because:
|
|
# 1. This migration is a data migration
|
|
# 2. It only relies on very stable DB fields: routes.id, routes.path, namespaces.id, projects.namespace_id
|
|
# Ideally, the test should not use factories and rely on the `table` helper instead.
|
|
describe RenameReservedProjectNames, :migration, schema: :latest do
|
|
let(:migration) { described_class.new }
|
|
let!(:project) { create(:project) } # rubocop:disable RSpec/FactoriesInMigrationSpecs
|
|
|
|
before do
|
|
project.path = 'projects'
|
|
project.save!(validate: false)
|
|
end
|
|
|
|
describe '#up' do
|
|
context 'when project repository exists' do
|
|
before do
|
|
project.create_repository
|
|
end
|
|
|
|
context 'when no exception is raised' do
|
|
it 'renames project with reserved names' do
|
|
migration.up
|
|
|
|
expect(project.reload.path).to eq('projects0')
|
|
end
|
|
end
|
|
|
|
context 'when exception is raised during rename' do
|
|
before do
|
|
service = instance_double('service')
|
|
|
|
allow(service)
|
|
.to receive(:execute)
|
|
.and_raise(Projects::AfterRenameService::RenameFailedError)
|
|
|
|
allow(Projects::AfterRenameService)
|
|
.to receive(:new)
|
|
.with(project)
|
|
.and_return(service)
|
|
end
|
|
|
|
it 'captures exception from project rename' do
|
|
expect { migration.up }.not_to raise_error
|
|
end
|
|
end
|
|
end
|
|
|
|
context 'when project repository does not exist' do
|
|
it 'does not raise error' do
|
|
expect { migration.up }.not_to raise_error
|
|
end
|
|
end
|
|
end
|
|
end
|