Commit ddb7083d authored by Kassio Borges's avatar Kassio Borges

Log invalid realtions build during Gitlab Project/Group import

Some imports are failing without clear reasons. To bring some light on
invalid relations will be logged. With these logs in place we'll be able
to check what validations need to be disabled for the import process.

Related to: https://gitlab.com/gitlab-org/gitlab/-/issues/285107
parent 32752f49
...@@ -71,7 +71,7 @@ module Gitlab ...@@ -71,7 +71,7 @@ module Gitlab
end end
def process_relation_item!(relation_key, relation_definition, relation_index, data_hash) def process_relation_item!(relation_key, relation_definition, relation_index, data_hash)
relation_object = build_relation(relation_key, relation_definition, data_hash) relation_object = build_relation(relation_key, relation_definition, relation_index, data_hash)
return unless relation_object return unless relation_object
return if importable_class == ::Project && group_model?(relation_object) return if importable_class == ::Project && group_model?(relation_object)
...@@ -139,23 +139,35 @@ module Gitlab ...@@ -139,23 +139,35 @@ module Gitlab
end end
end end
def build_relations(relation_key, relation_definition, data_hashes) def build_relations(relation_key, relation_definition, relation_index, data_hashes)
data_hashes data_hashes
.map { |data_hash| build_relation(relation_key, relation_definition, data_hash) } .map { |data_hash| build_relation(relation_key, relation_definition, relation_index, data_hash) }
.tap { |entries| entries.compact! } .tap { |entries| entries.compact! }
end end
def build_relation(relation_key, relation_definition, data_hash) def build_relation(relation_key, relation_definition, relation_index, data_hash)
# TODO: This is hack to not create relation for the author # TODO: This is hack to not create relation for the author
# Rather make `RelationFactory#set_note_author` to take care of that # Rather make `RelationFactory#set_note_author` to take care of that
return data_hash if relation_key == 'author' || already_restored?(data_hash) return data_hash if relation_key == 'author' || already_restored?(data_hash)
# create relation objects recursively for all sub-objects # create relation objects recursively for all sub-objects
relation_definition.each do |sub_relation_key, sub_relation_definition| relation_definition.each do |sub_relation_key, sub_relation_definition|
transform_sub_relations!(data_hash, sub_relation_key, sub_relation_definition) transform_sub_relations!(data_hash, sub_relation_key, sub_relation_definition, relation_index)
end end
@relation_factory.create(relation_factory_params(relation_key, data_hash)) relation = @relation_factory.create(relation_factory_params(relation_key, data_hash))
if relation && !relation.valid?
@shared.logger.warn(
message: "[Project/Group Import] Invalid object relation built",
relation_key: relation_key,
relation_index: relation_index,
relation_class: relation.class.name,
error_messages: relation.errors.full_messages.join(". ")
)
end
relation
end end
# Since we update the data hash in place as we restore relation items, # Since we update the data hash in place as we restore relation items,
...@@ -165,7 +177,7 @@ module Gitlab ...@@ -165,7 +177,7 @@ module Gitlab
!relation_item.is_a?(Hash) !relation_item.is_a?(Hash)
end end
def transform_sub_relations!(data_hash, sub_relation_key, sub_relation_definition) def transform_sub_relations!(data_hash, sub_relation_key, sub_relation_definition, relation_index)
sub_data_hash = data_hash[sub_relation_key] sub_data_hash = data_hash[sub_relation_key]
return unless sub_data_hash return unless sub_data_hash
...@@ -176,11 +188,13 @@ module Gitlab ...@@ -176,11 +188,13 @@ module Gitlab
build_relations( build_relations(
sub_relation_key, sub_relation_key,
sub_relation_definition, sub_relation_definition,
relation_index,
sub_data_hash).presence sub_data_hash).presence
else else
build_relation( build_relation(
sub_relation_key, sub_relation_key,
sub_relation_definition, sub_relation_definition,
relation_index,
sub_data_hash) sub_data_hash)
end end
......
{"description":"Nisi et repellendus ut enim quo accusamus vel magnam.","import_type":"gitlab_project","creator_id":2147483547,"visibility_level":10,"archived":false,"hooks":[]}
{"id":2,"title":"","color":"#428bca","project_id":8,"created_at":"2016-07-22T08:55:44.161Z","updated_at":"2016-07-22T08:55:44.161Z","template":false,"description":"","type":"","priorities":[]}
...@@ -113,12 +113,31 @@ RSpec.describe Gitlab::ImportExport::RelationTreeRestorer do ...@@ -113,12 +113,31 @@ RSpec.describe Gitlab::ImportExport::RelationTreeRestorer do
include_examples 'logging of relations creation' include_examples 'logging of relations creation'
end end
end
context 'using ndjson reader' do
let(:path) { 'spec/fixtures/lib/gitlab/import_export/complex/tree' }
let(:relation_reader) { Gitlab::ImportExport::JSON::NdjsonReader.new(path) }
it_behaves_like 'import project successfully'
end
context 'using ndjson reader' do context 'with invalid relations' do
let(:path) { 'spec/fixtures/lib/gitlab/import_export/complex/tree' } let(:path) { 'spec/fixtures/lib/gitlab/import_export/project_with_invalid_relations/tree' }
let(:relation_reader) { Gitlab::ImportExport::JSON::NdjsonReader.new(path) } let(:relation_reader) { Gitlab::ImportExport::JSON::NdjsonReader.new(path) }
it_behaves_like 'import project successfully' it 'logs the invalid relation and its errors' do
expect(relation_tree_restorer.shared.logger)
.to receive(:warn)
.with(
error_messages: "Title can't be blank. Title is invalid",
message: '[Project/Group Import] Invalid object relation built',
relation_class: 'ProjectLabel',
relation_index: 0,
relation_key: 'labels'
).once
relation_tree_restorer.restore
end end
end end
end end
......
...@@ -110,6 +110,7 @@ RSpec.describe Groups::ImportExport::ImportService do ...@@ -110,6 +110,7 @@ RSpec.describe Groups::ImportExport::ImportService do
allow(Gitlab::Import::Logger).to receive(:build).and_return(import_logger) allow(Gitlab::Import::Logger).to receive(:build).and_return(import_logger)
allow(import_logger).to receive(:error) allow(import_logger).to receive(:error)
allow(import_logger).to receive(:info) allow(import_logger).to receive(:info)
allow(import_logger).to receive(:warn)
allow(FileUtils).to receive(:rm_rf).and_call_original allow(FileUtils).to receive(:rm_rf).and_call_original
end end
...@@ -220,6 +221,7 @@ RSpec.describe Groups::ImportExport::ImportService do ...@@ -220,6 +221,7 @@ RSpec.describe Groups::ImportExport::ImportService do
allow(Gitlab::Import::Logger).to receive(:build).and_return(import_logger) allow(Gitlab::Import::Logger).to receive(:build).and_return(import_logger)
allow(import_logger).to receive(:error) allow(import_logger).to receive(:error)
allow(import_logger).to receive(:warn)
allow(import_logger).to receive(:info) allow(import_logger).to receive(:info)
allow(FileUtils).to receive(:rm_rf).and_call_original allow(FileUtils).to receive(:rm_rf).and_call_original
end end
......
...@@ -70,9 +70,12 @@ module ImportExport ...@@ -70,9 +70,12 @@ module ImportExport
) )
end end
def get_shared_env(path:) def get_shared_env(path:, logger: nil)
logger ||= double(info: true, warn: true, error: true)
instance_double(Gitlab::ImportExport::Shared).tap do |shared| instance_double(Gitlab::ImportExport::Shared).tap do |shared|
allow(shared).to receive(:export_path).and_return(path) allow(shared).to receive(:export_path).and_return(path)
allow(shared).to receive(:logger).and_return(logger)
end end
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