kotovalexarian-likes-gitlab/gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

squashed - Fix DB exceptions raised importing some specific projects.

Browse source 
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.
This commit is contained in:
James Lopez 2016-08-30 10:09:41 +02:00
parent 1d51bc7dfd
commit e74b7d665b
10 changed files with 145 additions and 25 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
CHANGELOG
View file

@ -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

11
app/models/ci/pipeline.rb
View file

@ -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

2
lib/gitlab/import_export.rb
View file

@ -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:)

4
lib/gitlab/import_export/import_export.yml
View file

@ -35,7 +35,9 @@ project_tree:
- :deploy_keys
- :services
- :hooks
- :protected_branches
- protected_branches:
- :merge_access_levels
- :push_access_levels
- :labels
- milestones:
- :events

28
lib/gitlab/import_export/relation_factory.rb
View file

@ -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

4
lib/gitlab/import_export/version_checker.rb
View file

@ -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

BIN
spec/features/projects/import_export/test_project_export.tar.gz
View file

Binary file not shown.

80
spec/lib/gitlab/import_export/project.json
View file

@ -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"
}
]
}
]
}
}

36
spec/lib/gitlab/import_export/project_tree_restorer_spec.rb
View file

@ -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

2
spec/lib/gitlab/import_export/version_checker_spec.rb
View file

@ -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