squashed - Fix DB exceptions raised importing some specific projects.

Better import of labels, milestones and protected branches. Updated relevant specs.
Loose pipeline validation on importing, so it does not fail when there are missing fields, which are not validated at DB level. Also, updated spec with relevant test.

10 files changed, 145 insertions, 25 deletions

diff --git a/CHANGELOG b/CHANGELOG
index e9445a18a18..8380a02b14b 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -158,6 +158,9 @@ v 8.11.6
   - Restore SSH Key title auto-population behavior. !6186
   - Fix DB schema to match latest migration. !6256
   - Exclude some pending or inactivated rows in Member scopes.
+  - Fix Import/Export issues importing protected branches and some specific models
+
+v 8.11.6 (unreleased)
 
 v 8.11.5
   - Optimize branch lookups and force a repository reload for Repository#find_branch. !6087
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 0b1df9f4294..70647b8532b 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -2,6 +2,7 @@ module Ci
   class Pipeline < ActiveRecord::Base
     extend Ci::Model
     include HasStatus
+    include Importable
 
     self.table_name = 'ci_commits'
 
@@ -12,12 +13,12 @@ module Ci
     has_many :builds, class_name: 'Ci::Build', foreign_key: :commit_id
     has_many :trigger_requests, dependent: :destroy, class_name: 'Ci::TriggerRequest', foreign_key: :commit_id
 
-    validates_presence_of :sha
-    validates_presence_of :ref
-    validates_presence_of :status
-    validate :valid_commit_sha
+    validates_presence_of :sha, unless: :importing?
+    validates_presence_of :ref, unless: :importing?
+    validates_presence_of :status, unless: :importing?
+    validate :valid_commit_sha, unless: :importing?
 
-    after_save :keep_around_commits
+    after_save :keep_around_commits, unless: :importing?
 
     delegate :stages, to: :statuses
 
diff --git a/lib/gitlab/import_export.rb b/lib/gitlab/import_export.rb
index bb562bdcd2c..b90651cbc2d 100644
--- a/lib/gitlab/import_export.rb
+++ b/lib/gitlab/import_export.rb
@@ -2,7 +2,7 @@ module Gitlab
   module ImportExport
     extend self
 
-    VERSION = '0.1.3'
+    VERSION = '0.1.4'
     FILENAME_LIMIT = 50
 
     def export_path(relative_path:)
diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml
index c2e8a1ca5dd..925a952156f 100644
--- a/lib/gitlab/import_export/import_export.yml
+++ b/lib/gitlab/import_export/import_export.yml
@@ -35,7 +35,9 @@ project_tree:
   - :deploy_keys
   - :services
   - :hooks
-  - :protected_branches
+  - protected_branches:
+    - :merge_access_levels
+    - :push_access_levels
   - :labels
   - milestones:
     - :events
diff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rb
index b0726268ca6..7b18d11ce0f 100644
--- a/lib/gitlab/import_export/relation_factory.rb
+++ b/lib/gitlab/import_export/relation_factory.rb
@@ -7,7 +7,9 @@ module Gitlab
                     variables: 'Ci::Variable',
                     triggers: 'Ci::Trigger',
                     builds: 'Ci::Build',
-                    hooks: 'ProjectHook' }.freeze
+                    hooks: 'ProjectHook',
+                    merge_access_levels: 'ProtectedBranch::MergeAccessLevel',
+                    push_access_levels: 'ProtectedBranch::PushAccessLevel' }.freeze
 
       USER_REFERENCES = %w[author_id assignee_id updated_by_id user_id].freeze
 
@@ -17,6 +19,8 @@ module Gitlab
 
       EXISTING_OBJECT_CHECK = %i[milestone milestones label labels].freeze
 
+      FINDER_ATTRIBUTES = %w[title color project_id].freeze
+
       def self.create(*args)
         new(*args).create
       end
@@ -149,7 +153,7 @@ module Gitlab
       end
 
       def parsed_relation_hash
-        @relation_hash.reject { |k, _v| !relation_class.attribute_method?(k) }
+        @parsed_relation_hash ||= @relation_hash.reject { |k, _v| !relation_class.attribute_method?(k) }
       end
 
       def set_st_diffs
@@ -161,14 +165,30 @@ module Gitlab
         # Otherwise always create the record, skipping the extra SELECT clause.
         @existing_or_new_object ||= begin
           if EXISTING_OBJECT_CHECK.include?(@relation_name)
-            existing_object = relation_class.find_or_initialize_by(parsed_relation_hash.slice('title', 'project_id'))
-            existing_object.assign_attributes(parsed_relation_hash)
+            events = parsed_relation_hash.delete('events')
+
+            unless events.blank?
+              existing_object.assign_attributes(events: events)
+            end
+
             existing_object
           else
             relation_class.new(parsed_relation_hash)
           end
         end
       end
+
+      def existing_object
+        @existing_object ||=
+          begin
+            finder_hash = parsed_relation_hash.slice(*FINDER_ATTRIBUTES)
+            existing_object = relation_class.find_or_create_by(finder_hash)
+            # Done in two steps, as MySQL behaves differently than PostgreSQL using
+            # the +find_or_create_by+ method and does not return the ID the second time.
+            existing_object.update(parsed_relation_hash)
+            existing_object
+          end
+      end
     end
   end
 end
diff --git a/lib/gitlab/import_export/version_checker.rb b/lib/gitlab/import_export/version_checker.rb
index de3fe6d822e..fc08082fc86 100644
--- a/lib/gitlab/import_export/version_checker.rb
+++ b/lib/gitlab/import_export/version_checker.rb
@@ -24,8 +24,8 @@ module Gitlab
       end
 
       def verify_version!(version)
-        if Gem::Version.new(version) > Gem::Version.new(Gitlab::ImportExport.version)
-          raise Gitlab::ImportExport::Error.new("Import version mismatch: Required <= #{Gitlab::ImportExport.version} but was #{version}")
+        if Gem::Version.new(version) != Gem::Version.new(Gitlab::ImportExport.version)
+          raise Gitlab::ImportExport::Error.new("Import version mismatch: Required #{Gitlab::ImportExport.version} but was #{version}")
         else
           true
         end
diff --git a/spec/features/projects/import_export/test_project_export.tar.gz b/spec/features/projects/import_export/test_project_export.tar.gz
index e14b2705704..d04bdea0fe4 100644
--- a/spec/features/projects/import_export/test_project_export.tar.gz
+++ b/spec/features/projects/import_export/test_project_export.tar.gz
diff --git a/spec/lib/gitlab/import_export/project.json b/spec/lib/gitlab/import_export/project.json
index 5114f9c55e1..281f6cf1177 100644
--- a/spec/lib/gitlab/import_export/project.json
+++ b/spec/lib/gitlab/import_export/project.json
@@ -24,7 +24,7 @@
       "test_ee_field": "test",
       "milestone": {
         "id": 1,
-        "title": "v0.0",
+        "title": "test milestone",
         "project_id": 8,
         "description": "test milestone",
         "due_date": null,
@@ -51,7 +51,7 @@
         {
           "id": 2,
           "label_id": 2,
-          "target_id": 3,
+          "target_id": 40,
           "target_type": "Issue",
           "created_at": "2016-07-22T08:57:02.840Z",
           "updated_at": "2016-07-22T08:57:02.840Z",
@@ -281,6 +281,31 @@
       "deleted_at": null,
       "due_date": null,
       "moved_to_id": null,
+      "milestone": {
+        "id": 1,
+        "title": "test milestone",
+        "project_id": 8,
+        "description": "test milestone",
+        "due_date": null,
+        "created_at": "2016-06-14T15:02:04.415Z",
+        "updated_at": "2016-06-14T15:02:04.415Z",
+        "state": "active",
+        "iid": 1,
+        "events": [
+          {
+            "id": 487,
+            "target_type": "Milestone",
+            "target_id": 1,
+            "title": null,
+            "data": null,
+            "project_id": 46,
+            "created_at": "2016-06-14T15:02:04.418Z",
+            "updated_at": "2016-06-14T15:02:04.418Z",
+            "action": 1,
+            "author_id": 18
+          }
+        ]
+      },
       "notes": [
         {
           "id": 359,
@@ -494,6 +519,27 @@
       "deleted_at": null,
       "due_date": null,
       "moved_to_id": null,
+      "label_links": [
+        {
+          "id": 99,
+          "label_id": 2,
+          "target_id": 38,
+          "target_type": "Issue",
+          "created_at": "2016-07-22T08:57:02.840Z",
+          "updated_at": "2016-07-22T08:57:02.840Z",
+          "label": {
+            "id": 2,
+            "title": "test2",
+            "color": "#428bca",
+            "project_id": 8,
+            "created_at": "2016-07-22T08:55:44.161Z",
+            "updated_at": "2016-07-22T08:55:44.161Z",
+            "template": false,
+            "description": "",
+            "priority": null
+          }
+        }
+      ],
       "notes": [
         {
           "id": 367,
@@ -6478,7 +6524,7 @@
     {
       "id": 37,
       "project_id": 5,
-      "ref": "master",
+      "ref": null,
       "sha": "048721d90c449b244b7b4c53a9186b04330174ec",
       "before_sha": null,
       "push_data": null,
@@ -7301,6 +7347,30 @@
 
   ],
   "protected_branches": [
-
+    {
+      "id": 1,
+      "project_id": 9,
+      "name": "master",
+      "created_at": "2016-08-30T07:32:52.426Z",
+      "updated_at": "2016-08-30T07:32:52.426Z",
+      "merge_access_levels": [
+        {
+          "id": 1,
+          "protected_branch_id": 1,
+          "access_level": 40,
+          "created_at": "2016-08-30T07:32:52.458Z",
+          "updated_at": "2016-08-30T07:32:52.458Z"
+        }
+      ],
+      "push_access_levels": [
+        {
+          "id": 1,
+          "protected_branch_id": 1,
+          "access_level": 40,
+          "created_at": "2016-08-30T07:32:52.490Z",
+          "updated_at": "2016-08-30T07:32:52.490Z"
+        }
+      ]
+    }
   ]
-}
+}
\ No newline at end of file
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 a07ef279e68..feacb295231 100644
--- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb
+++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb
@@ -29,12 +29,30 @@ describe Gitlab::ImportExport::ProjectTreeRestorer, services: true do
         expect(project.project_feature.merge_requests_access_level).to eq(ProjectFeature::ENABLED)
       end
 
+      it 'has the same label associated to two issues' do
+        restored_project_json
+
+        expect(Label.first.issues.count).to eq(2)
+      end
+
+      it 'has milestones associated to two separate issues' do
+        restored_project_json
+
+        expect(Milestone.find_by_description('test milestone').issues.count).to eq(2)
+      end
+
       it 'creates a valid pipeline note' do
         restored_project_json
 
         expect(Ci::Pipeline.first.notes).not_to be_empty
       end
 
+      it 'restores pipelines with missing ref' do
+        restored_project_json
+
+        expect(Ci::Pipeline.where(ref: nil)).not_to be_empty
+      end
+
       it 'restores the correct event with symbolised data' do
         restored_project_json
 
@@ -49,6 +67,18 @@ describe Gitlab::ImportExport::ProjectTreeRestorer, services: true do
         expect(issue.reload.updated_at.to_s).to eq('2016-06-14 15:02:47 UTC')
       end
 
+      it 'contains the merge access levels on a protected branch' do
+        restored_project_json
+
+        expect(ProtectedBranch.first.merge_access_levels).not_to be_empty
+      end
+
+      it 'contains the push access levels on a protected branch' do
+        restored_project_json
+
+        expect(ProtectedBranch.first.push_access_levels).not_to be_empty
+      end
+
       context 'event at forth level of the tree' do
         let(:event) { Event.where(title: 'test levels').first }
 
@@ -77,12 +107,6 @@ describe Gitlab::ImportExport::ProjectTreeRestorer, services: true do
         expect(Label.first.label_links.first.target).not_to be_nil
       end
 
-      it 'has milestones associated to issues' do
-        restored_project_json
-
-        expect(Milestone.find_by_description('test milestone').issues).not_to be_empty
-      end
-
       context 'Merge requests' do
         before do
           restored_project_json
diff --git a/spec/lib/gitlab/import_export/version_checker_spec.rb b/spec/lib/gitlab/import_export/version_checker_spec.rb
index 90c6d1c67f6..c680e712b59 100644
--- a/spec/lib/gitlab/import_export/version_checker_spec.rb
+++ b/spec/lib/gitlab/import_export/version_checker_spec.rb
@@ -23,7 +23,7 @@ describe Gitlab::ImportExport::VersionChecker, services: true do
       it 'shows the correct error message' do
         described_class.check!(shared: shared)
 
-        expect(shared.errors.first).to eq("Import version mismatch: Required <= #{Gitlab::ImportExport.version} but was #{version}")
+        expect(shared.errors.first).to eq("Import version mismatch: Required #{Gitlab::ImportExport.version} but was #{version}")
       end
     end
   end