Merge branch 'kassio/project-import-log-invalid-relations' into 'master'

Log invalid realtions build during Gitlab Project/Group import

See merge request gitlab-org/gitlab!49717

lib/gitlab/import_export/relation_tree_restorer.rb

@@ -71,7 +71,7 @@ module Gitlab
       end
 
       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 if importable_class == ::Project && group_model?(relation_object)
 
@@ -139,23 +139,35 @@ module Gitlab
         end
       end
 
-      def build_relations(relation_key, relation_definition, data_hashes)
+      def build_relations(relation_key, relation_definition, relation_index, 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! }
       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
         # Rather make `RelationFactory#set_note_author` to take care of that
         return data_hash if relation_key == 'author' || already_restored?(data_hash)
 
         # create relation objects recursively for all sub-objects
         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
 
-        @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
 
       # Since we update the data hash in place as we restore relation items,
@@ -165,7 +177,7 @@ module Gitlab
         !relation_item.is_a?(Hash)
       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]
         return unless sub_data_hash
 
@@ -176,11 +188,13 @@ module Gitlab
             build_relations(
               sub_relation_key,
               sub_relation_definition,
+              relation_index,
               sub_data_hash).presence
           else
             build_relation(
               sub_relation_key,
               sub_relation_definition,
+              relation_index,
               sub_data_hash)
           end
 

spec/fixtures/lib/gitlab/import_export/project_with_invalid_relations/tree/project.json

+{"description":"Nisi et repellendus ut enim quo accusamus vel magnam.","import_type":"gitlab_project","creator_id":2147483547,"visibility_level":10,"archived":false,"hooks":[]}

spec/fixtures/lib/gitlab/import_export/project_with_invalid_relations/tree/project/labels.ndjson

+{"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":[]}

spec/lib/gitlab/import_export/relation_tree_restorer_spec.rb

@@ -113,12 +113,31 @@ RSpec.describe Gitlab::ImportExport::RelationTreeRestorer do
 
         include_examples 'logging of relations creation'
       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
-        let(:path) { 'spec/fixtures/lib/gitlab/import_export/complex/tree' }
-        let(:relation_reader) { Gitlab::ImportExport::JSON::NdjsonReader.new(path) }
+    context 'with invalid relations' do
+      let(:path) { 'spec/fixtures/lib/gitlab/import_export/project_with_invalid_relations/tree' }
+      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

spec/services/groups/import_export/import_service_spec.rb

@@ -110,6 +110,7 @@ RSpec.describe Groups::ImportExport::ImportService do
         allow(Gitlab::Import::Logger).to receive(:build).and_return(import_logger)
         allow(import_logger).to receive(:error)
         allow(import_logger).to receive(:info)
+        allow(import_logger).to receive(:warn)
         allow(FileUtils).to receive(:rm_rf).and_call_original
       end
 
@@ -220,6 +221,7 @@ RSpec.describe Groups::ImportExport::ImportService do
 
         allow(Gitlab::Import::Logger).to receive(:build).and_return(import_logger)
         allow(import_logger).to receive(:error)
+        allow(import_logger).to receive(:warn)
         allow(import_logger).to receive(:info)
         allow(FileUtils).to receive(:rm_rf).and_call_original
       end

spec/support/import_export/common_util.rb

@@ -70,9 +70,12 @@ module ImportExport
       )
     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|
         allow(shared).to receive(:export_path).and_return(path)
+        allow(shared).to receive(:logger).and_return(logger)
       end
     end