Add disallowed fields to AttributeCleaner

3 files changed, 38 insertions, 2 deletions

diff --git a/lib/gitlab/import_export/attribute_cleaner.rb b/lib/gitlab/import_export/attribute_cleaner.rb
index 93b37b7bc5f..1faa3c1614f 100644
--- a/lib/gitlab/import_export/attribute_cleaner.rb
+++ b/lib/gitlab/import_export/attribute_cleaner.rb
@@ -24,7 +24,19 @@ module Gitlab
       private
 
       def prohibited_key?(key)
-        key.end_with?('_id') && !ALLOWED_REFERENCES.include?(key)
+        return false if allowed_reference?(key)
+
+        return true if 'cached_markdown_version'.equal?(key)
+
+        prohibited_suffices = %w(_id _html)
+        prohibited_suffices.each do |suffix|
+          return true if key.end_with?(suffix)
+        end
+        false
+      end
+
+      def allowed_reference?(key)
+        ALLOWED_REFERENCES.include?(key)
       end
 
       def excluded_key?(key)
diff --git a/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb b/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb
index 536cc359d39..99669285d5b 100644
--- a/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb
+++ b/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb
@@ -18,7 +18,11 @@ describe Gitlab::ImportExport::AttributeCleaner do
       'notid' => 99,
       'import_source' => 'whatever',
       'import_type' => 'whatever',
-      'non_existent_attr' => 'whatever'
+      'non_existent_attr' => 'whatever',
+      'some_html' => '<p>dodgy html</p>',
+      'legit_html' => '<p>legit html</p>',
+      '_html' => '<p>perfectly ordinary html</p>',
+      'cached_markdown_version' => 12345
     }
   end
 
diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb
index 6084dc96410..be090bfed25 100644
--- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb
+++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb
@@ -10,6 +10,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
         create(:user, username: 'bernard_willms'),
         create(:user, username: 'saul_will')
       ]
+      @markdown_classes = [AbuseReport, Appearance, ApplicationSetting, BroadcastMessage, Issue, Label, MergeRequest, Milestone, Namespace, Project, Release, ResourceLabelEvent, Snippet, UserStatus]
 
       RSpec::Mocks.with_temporary_scope do
         @project = create(:project, :builds_enabled, :issues_disabled, name: 'project', path: 'project')
@@ -21,6 +22,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
 
         expect_any_instance_of(Gitlab::Git::Repository).to receive(:create_branch).with('feature', 'DCBA')
         allow_any_instance_of(Gitlab::Git::Repository).to receive(:create_branch)
+        @markdown_classes.each {|klass| allow_any_instance_of(klass).to receive(:latest_cached_markdown_version).and_return(434343)}
 
         project_tree_restorer = described_class.new(user: @user, shared: @shared, project: @project)
 
@@ -58,6 +60,24 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
         expect(Milestone.find_by_description('test milestone').issues.count).to eq(2)
       end
 
+      context 'when importing a project with cached_markdown_version and note_html' do
+        let!(:issue) { Issue.find_by(description: 'Aliquam enim illo et possimus.') }
+        let(:note1) { issue.notes.select {|n| n.note.match(/Quo reprehenderit aliquam qui dicta impedit cupiditate eligendi/)}.first }
+        let(:note2) { issue.notes.select {|n| n.note.match(/Est reprehenderit quas aut aspernatur autem recusandae voluptatem/)}.first }
+
+        it 'does not import the note_html' do
+          expect(note1.note_html).to match(/Quo reprehenderit aliquam qui dicta impedit cupiditate eligendi/)
+        end
+
+        it 'does not set the old cached_markdown_version' do
+          expect(note2.cached_markdown_version).not_to eq(121212)
+        end
+
+        it 'does not import the note_html' do
+          expect(note2.note_html).to match(/Est reprehenderit quas aut aspernatur autem recusandae voluptatem/)
+        end
+      end
+
       it 'creates a valid pipeline note' do
         expect(Ci::Pipeline.find_by_sha('sha-notes').notes).not_to be_empty
       end