Merge branch 'fix/import-export-db-errors' into 'master'

Fix import/export database errors

Fixes protected branches errors when importing a project including them

Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/21295

Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/21799

See merge request !6099

11 files changed, 160 insertions, 27 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 6093cb09880..87cccec3e4e 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -150,6 +150,7 @@ v 8.12.0 (unreleased)
   - Fix Gitlab::Popen.popen thread-safety issue
   - Add specs to removing project (Katarzyna Kobierska Ula Budziszewska)
   - Clean environment variables when running git hooks
+  - Fix Import/Export issues importing protected branches and some specific models
   - Fix non-master branch readme display in tree view
 
 v 8.11.6
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/doc/user/project/settings/import_export.md b/doc/user/project/settings/import_export.md
index 08ff89ce6ae..445c0ee8333 100644
--- a/doc/user/project/settings/import_export.md
+++ b/doc/user/project/settings/import_export.md
@@ -3,8 +3,8 @@
 >**Notes:**
 >
 >  - [Introduced][ce-3050] in GitLab 8.9.
->  - Importing will not be possible if the import instance version is lower
->    than that of the exporter.
+>  - Importing will not be possible if the import instance version differs from
+>    that of the exporter.
 >  - For existing installations, the project import option has to be enabled in
 >    application settings (`/admin/application_settings`) under 'Import sources'.
 >    You will have to be an administrator to enable and use the import functionality.
@@ -17,6 +17,20 @@
 Existing projects running on any GitLab instance or GitLab.com can be exported
 with all their related data and be moved into a new GitLab instance.
 
+## Version history
+
+| GitLab version | Import/Export version |
+| -------- | -------- |
+| 8.12.0 to current  | 0.1.4    |
+| 8.10.3   | 0.1.3    |
+| 8.10.0   | 0.1.2    |
+| 8.9.5    | 0.1.1    |
+| 8.9.0    | 0.1.0    |
+ 
+ > The table reflects what GitLab version we updated the Import/Export version at.
+ > For instance, 8.10.3 and 8.11 will have the same Import/Export version (0.1.3)
+ > and the exports between them will be compatible.
+
 ## Exported contents
 
 The following items will be exported:
diff --git a/lib/gitlab/import_export.rb b/lib/gitlab/import_export.rb
index bb562bdcd2c..181e288a014 100644
--- a/lib/gitlab/import_export.rb
+++ b/lib/gitlab/import_export.rb
@@ -2,7 +2,8 @@ module Gitlab
   module ImportExport
     extend self
 
-    VERSION = '0.1.3'
+    # For every version update, the version history in import_export.md has to be kept up to date.
+    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..354ccd64696 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 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