Merge branch 'georgekoltsov/add-attribute-cleaner-transformer' into 'master'

Add Attributes cleaner to Group Migration

See merge request gitlab-org/gitlab!48374

changelogs/unreleased/georgekoltsov-add-attribute-cleaner-transformer.yml

+---
+title: Add Attributes cleaner to Group Migration
+merge_request: 48374
+author:
+type: changed

ee/lib/bulk_imports/ee/groups/pipelines/epics_pipeline.rb

@@ -13,6 +13,7 @@ module BulkImports
           transformer ::BulkImports::Common::Transformers::HashKeyDigger,
             key_path: %w[data group epics]
           transformer ::BulkImports::Common::Transformers::UnderscorifyKeysTransformer
+          transformer ::BulkImports::Common::Transformers::ProhibitedAttributesTransformer
 
           loader BulkImports::EE::Groups::Loaders::EpicsLoader
 

ee/spec/lib/bulk_imports/ee/groups/pipelines/epics_pipeline_spec.rb

@@ -61,7 +61,8 @@ RSpec.describe BulkImports::EE::Groups::Pipelines::EpicsPipeline do
       expect(described_class.transformers)
         .to contain_exactly(
           { klass: BulkImports::Common::Transformers::HashKeyDigger, options: { key_path: %w[data group epics] } },
-          { klass: BulkImports::Common::Transformers::UnderscorifyKeysTransformer, options: nil }
+          { klass: BulkImports::Common::Transformers::UnderscorifyKeysTransformer, options: nil },
+          { klass: BulkImports::Common::Transformers::ProhibitedAttributesTransformer, options: nil }
         )
     end
 

lib/bulk_imports/common/transformers/prohibited_attributes_transformer.rb

+# frozen_string_literal: true
+
+module BulkImports
+  module Common
+    module Transformers
+      class ProhibitedAttributesTransformer
+        PROHIBITED_REFERENCES = Regexp.union(
+          /\Acached_markdown_version\Z/,
+          /\Aid\Z/,
+          /_id\Z/,
+          /_ids\Z/,
+          /_html\Z/,
+          /attributes/,
+          /\Aremote_\w+_(url|urls|request_header)\Z/ # carrierwave automatically creates these attribute methods for uploads
+        ).freeze
+
+        def initialize(options = {})
+          @options = options
+        end
+
+        def transform(context, data)
+          data.each_with_object({}) do |(key, value), result|
+            prohibited = prohibited_key?(key)
+
+            unless prohibited
+              result[key] = value.is_a?(Hash) ? transform(context, value) : value
+            end
+          end
+        end
+
+        private
+
+        def prohibited_key?(key)
+          key.to_s =~ PROHIBITED_REFERENCES
+        end
+      end
+    end
+  end
+end

lib/bulk_imports/groups/pipelines/group_pipeline.rb

@@ -12,6 +12,7 @@ module BulkImports
 
         transformer Common::Transformers::HashKeyDigger, key_path: %w[data group]
         transformer Common::Transformers::UnderscorifyKeysTransformer
+        transformer Common::Transformers::ProhibitedAttributesTransformer
         transformer Groups::Transformers::GroupAttributesTransformer
 
         loader Groups::Loaders::GroupLoader

lib/bulk_imports/groups/pipelines/subgroup_entities_pipeline.rb

@@ -7,6 +7,7 @@ module BulkImports
         include Pipeline
 
         extractor BulkImports::Groups::Extractors::SubgroupsExtractor
+        transformer Common::Transformers::ProhibitedAttributesTransformer
         transformer BulkImports::Groups::Transformers::SubgroupToEntityTransformer
         loader BulkImports::Common::Loaders::EntityLoader
       end

spec/lib/bulk_imports/common/transformers/prohibited_attributes_transformer_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe BulkImports::Common::Transformers::ProhibitedAttributesTransformer do
+  describe '#transform' do
+    let_it_be(:hash) do
+      {
+        'id' => 101,
+        'service_id' => 99,
+        'moved_to_id' => 99,
+        'namespace_id' => 99,
+        'ci_id' => 99,
+        'random_project_id' => 99,
+        'random_id' => 99,
+        'milestone_id' => 99,
+        'project_id' => 99,
+        'user_id' => 99,
+        'random_id_in_the_middle' => 99,
+        'notid' => 99,
+        'import_source' => 'test',
+        'import_type' => 'test',
+        'non_existent_attr' => 'test',
+        'some_html' => '<p>dodgy html</p>',
+        'legit_html' => '<p>legit html</p>',
+        '_html' => '<p>perfectly ordinary html</p>',
+        'cached_markdown_version' => 12345,
+        'custom_attributes' => 'test',
+        'some_attributes_metadata' => 'test',
+        'group_id' => 99,
+        'commit_id' => 99,
+        'issue_ids' => [1, 2, 3],
+        'merge_request_ids' => [1, 2, 3],
+        'note_ids' => [1, 2, 3],
+        'remote_attachment_url' => 'http://something.dodgy',
+        'remote_attachment_request_header' => 'bad value',
+        'remote_attachment_urls' => %w(http://something.dodgy http://something.okay),
+        'attributes' => {
+          'issue_ids' => [1, 2, 3],
+          'merge_request_ids' => [1, 2, 3],
+          'note_ids' => [1, 2, 3]
+        },
+        'variables_attributes' => {
+          'id' => 1
+        },
+        'attr_with_nested_attrs' => {
+          'nested_id' => 1,
+          'nested_attr' => 2
+        }
+      }
+    end
+
+    let(:expected_hash) do
+      {
+        'random_id_in_the_middle' => 99,
+        'notid' => 99,
+        'import_source' => 'test',
+        'import_type' => 'test',
+        'non_existent_attr' => 'test',
+        'attr_with_nested_attrs' => {
+          'nested_attr' => 2
+        }
+      }
+    end
+
+    it 'removes prohibited attributes' do
+      transformed_hash = subject.transform(nil, hash)
+
+      expect(transformed_hash).to eq(expected_hash)
+    end
+  end
+end

spec/lib/bulk_imports/groups/pipelines/group_pipeline_spec.rb

@@ -91,6 +91,7 @@ RSpec.describe BulkImports::Groups::Pipelines::GroupPipeline do
         .to contain_exactly(
           { klass: BulkImports::Common::Transformers::HashKeyDigger, options: { key_path: %w[data group] } },
           { klass: BulkImports::Common::Transformers::UnderscorifyKeysTransformer, options: nil },
+          { klass: BulkImports::Common::Transformers::ProhibitedAttributesTransformer, options: nil },
           { klass: BulkImports::Groups::Transformers::GroupAttributesTransformer, options: nil }
         )
     end

spec/lib/bulk_imports/groups/pipelines/subgroup_entities_pipeline_spec.rb

@@ -66,8 +66,8 @@ RSpec.describe BulkImports::Groups::Pipelines::SubgroupEntitiesPipeline do
 
     it 'has transformers' do
       expect(described_class.transformers).to contain_exactly(
-        klass: BulkImports::Groups::Transformers::SubgroupToEntityTransformer,
-        options: nil
+        { klass: BulkImports::Common::Transformers::ProhibitedAttributesTransformer, options: nil },
+        { klass: BulkImports::Groups::Transformers::SubgroupToEntityTransformer, options: nil }
       )
     end