From 7c8359b7441c2631ceec3bd3783a9761646a9f99 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Wed, 1 Jun 2016 18:03:51 +0200 Subject: started refactoring some stuff based on MR feedback --- app/models/concerns/importable.rb | 6 ++++ app/models/member.rb | 10 +++---- app/models/merge_request.rb | 7 ++--- app/models/merge_request_diff.rb | 5 ++-- db/schema.rb | 16 +++++------ lib/gitlab/import_export/members_mapper.rb | 34 +++++++++++++---------- lib/gitlab/import_export/project_tree_restorer.rb | 34 ++++++++--------------- lib/gitlab/import_export/relation_factory.rb | 17 +++++------- lib/gitlab/import_export/repo_restorer.rb | 1 - 9 files changed, 61 insertions(+), 69 deletions(-) create mode 100644 app/models/concerns/importable.rb diff --git a/app/models/concerns/importable.rb b/app/models/concerns/importable.rb new file mode 100644 index 00000000000..019ef755849 --- /dev/null +++ b/app/models/concerns/importable.rb @@ -0,0 +1,6 @@ +module Importable + extend ActiveSupport::Concern + + attr_accessor :importing + alias_method :importing?, :importing +end diff --git a/app/models/member.rb b/app/models/member.rb index bef0b545c70..9a56d3d2181 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -19,10 +19,10 @@ class Member < ActiveRecord::Base include Sortable + include Importable include Gitlab::Access attr_accessor :raw_invite_token - attr_accessor :importing belongs_to :created_by, class_name: "User" belongs_to :user @@ -55,10 +55,10 @@ class Member < ActiveRecord::Base scope :owners, -> { where(access_level: OWNER) } before_validation :generate_invite_token, on: :create, if: -> (member) { member.invite_email.present? } - after_create :send_invite, if: :invite?, unless: :importing - after_create :create_notification_setting, unless: [:invite?, :importing] - after_create :post_create_hook, unless: [:invite?, :importing] - after_update :post_update_hook, unless: [:invite?, :importing] + after_create :send_invite, if: :invite?, unless: :importing? + after_create :create_notification_setting, unless: [:invite?, :importing?] + after_create :post_create_hook, unless: [:invite?, :importing?] + after_update :post_update_hook, unless: [:invite?, :importing?] after_destroy :post_destroy_hook, unless: :invite? delegate :name, :username, :email, to: :user, prefix: true diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 916329e04d6..c771968627c 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -34,6 +34,7 @@ class MergeRequest < ActiveRecord::Base include Referable include Sortable include Taskable + include Importable belongs_to :target_project, foreign_key: :target_project_id, class_name: "Project" belongs_to :source_project, foreign_key: :source_project_id, class_name: "Project" @@ -48,8 +49,6 @@ class MergeRequest < ActiveRecord::Base delegate :commits, :diffs, :real_size, to: :merge_request_diff, prefix: nil - attr_accessor :importing - # When this attribute is true some MR validation is ignored # It allows us to close or modify broken merge requests attr_accessor :allow_broken @@ -123,12 +122,12 @@ class MergeRequest < ActiveRecord::Base end end - validates :source_project, presence: true, unless: [:allow_broken, :importing] + validates :source_project, presence: true, unless: [:allow_broken, :importing?] validates :source_branch, presence: true validates :target_project, presence: true validates :target_branch, presence: true validates :merge_user, presence: true, if: :merge_when_build_succeeds? - validate :validate_branches, unless: [:allow_broken, :importing] + validate :validate_branches, unless: [:allow_broken, :importing?] validate :validate_fork scope :by_branch, ->(branch_name) { where("(source_branch LIKE :branch) OR (target_branch LIKE :branch)", branch: branch_name) } diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index 4278c7daa41..2c9eaf3849f 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -15,6 +15,7 @@ class MergeRequestDiff < ActiveRecord::Base include Sortable + include Importable # Prevent store of diff if commits amount more then 500 COMMITS_SAFE_SIZE = 100 @@ -37,9 +38,7 @@ class MergeRequestDiff < ActiveRecord::Base serialize :st_commits serialize :st_diffs - after_create :reload_content, unless: :importing - - attr_accessor :importing + after_create :reload_content, unless: :importing? def reload_content reload_commits diff --git a/db/schema.rb b/db/schema.rb index 71d953afe30..89dba1318fa 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -70,11 +70,11 @@ ActiveRecord::Schema.define(version: 20160508194200) do t.string "recaptcha_site_key" t.string "recaptcha_private_key" t.integer "metrics_port", default: 8089 - t.boolean "akismet_enabled", default: false - t.string "akismet_api_key" t.integer "metrics_sample_interval", default: 15 t.boolean "sentry_enabled", default: false t.string "sentry_dsn" + t.boolean "akismet_enabled", default: false + t.string "akismet_api_key" t.boolean "email_author_in_body", default: false t.integer "default_group_visibility" t.boolean "repository_checks_enabled", default: false @@ -428,8 +428,8 @@ ActiveRecord::Schema.define(version: 20160508194200) do t.integer "updated_by_id" t.boolean "confidential", default: false t.datetime "deleted_at" - t.date "due_date" t.integer "moved_to_id" + t.date "due_date" end add_index "issues", ["assignee_id"], name: "index_issues_on_assignee_id", using: :btree @@ -716,8 +716,8 @@ ActiveRecord::Schema.define(version: 20160508194200) do t.integer "project_id" t.text "data" t.text "encrypted_credentials" - t.string "encrypted_credentials_iv" - t.string "encrypted_credentials_salt" + t.text "encrypted_credentials_iv" + t.text "encrypted_credentials_salt" end create_table "projects", force: :cascade do |t| @@ -815,9 +815,9 @@ ActiveRecord::Schema.define(version: 20160508194200) do t.string "type" t.string "title" t.integer "project_id" - t.datetime "created_at" - t.datetime "updated_at" - t.boolean "active", default: false, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.boolean "active", null: false t.text "properties" t.boolean "template", default: false t.boolean "push_events", default: true diff --git a/lib/gitlab/import_export/members_mapper.rb b/lib/gitlab/import_export/members_mapper.rb index cd1e174f180..9c47216eb8d 100644 --- a/lib/gitlab/import_export/members_mapper.rb +++ b/lib/gitlab/import_export/members_mapper.rb @@ -2,7 +2,7 @@ module Gitlab module ImportExport class MembersMapper - attr_reader :map, :note_member_list + attr_reader :note_member_list def initialize(exported_members:, user:, project:) @exported_members = exported_members @@ -19,33 +19,37 @@ module Gitlab default_project_member end - @map = generate_map end def default_project_member @default_project_member ||= begin default_member = ProjectMember.new(default_project_member_hash) - default_member.save! + default_member.create! default_member.user.id end end - private - - def generate_map - @exported_members.each do |member| - existing_user = User.where(find_project_user_query(member)).first - assign_member(existing_user, member) if existing_user - end - @project_member_map + def map + @map ||= + begin + @exported_members.inject(@project_member_map) do |hash, member| + existing_user = User.where(find_project_user_query(member)).first + if existing_user + old_user_id = member['user']['id'] + add_user_as_team_member(existing_user, member) + hash[old_user_id] = existing_user.id + end + hash + end + end end - def assign_member(existing_user, member) - old_user_id = member['user']['id'] + private + + def add_user_as_team_member(existing_user, member) member['user'] = existing_user - project_member = ProjectMember.new(member_hash(member)) - @project_member_map[old_user_id] = project_member.user.id if project_member.save + ProjectMember.create!(member_hash(member)) end def member_hash(member) diff --git a/lib/gitlab/import_export/project_tree_restorer.rb b/lib/gitlab/import_export/project_tree_restorer.rb index 911ba06e748..e8f5fb88382 100644 --- a/lib/gitlab/import_export/project_tree_restorer.rb +++ b/lib/gitlab/import_export/project_tree_restorer.rb @@ -32,14 +32,15 @@ module Gitlab project: project) end - def create_relations(relation_list = default_relation_list, tree_hash = @tree_hash) + def create_relations saved = [] - relation_list.each do |relation| - next if !relation.is_a?(Hash) && tree_hash[relation.to_s].blank? - create_sub_relations(relation, tree_hash) if relation.is_a?(Hash) + default_relation_list.each do |relation| + next unless relation.is_a?(Hash) || @tree_hash[relation.to_s].present? + + create_sub_relations(relation, @tree_hash) if relation.is_a?(Hash) relation_key = relation.is_a?(Hash) ? relation.keys.first : relation - relation_hash = create_relation(relation_key, tree_hash[relation_key.to_s]) + relation_hash = create_relation(relation_key, @tree_hash[relation_key.to_s]) saved << project.update_attribute(relation_key, relation_hash) end saved.all? @@ -73,32 +74,19 @@ module Gitlab relation_hash = relation_item[sub_relation.to_s] end - process_sub_relation(relation_hash, relation_item, sub_relation) unless relation_hash.blank? + relation_item[sub_relation.to_s] = create_relation(sub_relation, relation_hash) unless relation_hash.blank? end end end - def process_sub_relation(relation_hash, relation_item, sub_relation) - if relation_hash.is_a?(Array) - sub_relation_object = create_relation(sub_relation, relation_hash) - else - sub_relation_object = relation_from_factory(sub_relation, relation_hash) - end - relation_item[sub_relation.to_s] = sub_relation_object - end - def create_relation(relation, relation_hash_list) [relation_hash_list].flatten.map do |relation_hash| - relation_from_factory(relation, relation_hash) + Gitlab::ImportExport::RelationFactory.create(relation_sym: relation.to_sym, + relation_hash: relation_hash.merge('project_id' => project.id), + members_mapper: members_mapper, + user_admin: @user.is_admin?) end end - - def relation_from_factory(relation, relation_hash) - Gitlab::ImportExport::RelationFactory.create(relation_sym: relation.to_sym, - relation_hash: relation_hash.merge('project_id' => project.id), - members_mapper: members_mapper, - user_admin: @user.is_admin?) - end end end end diff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rb index 0fdf208cfa0..2794eafe4b6 100644 --- a/lib/gitlab/import_export/relation_factory.rb +++ b/lib/gitlab/import_export/relation_factory.rb @@ -11,11 +11,14 @@ module Gitlab builds: 'Ci::Build', hooks: 'ProjectHook' }.freeze - USER_REFERENCES = %w(author_id assignee_id updated_by_id user_id).freeze + USER_REFERENCES = %w[author_id assignee_id updated_by_id user_id].freeze + # Guesses a model and saves it to the DB given its name `relation_sym` def create(relation_sym:, relation_hash:, members_mapper:, user_admin:) relation_sym = parse_relation_sym(relation_sym) - klass = parse_relation(relation_hash, relation_sym) + + klass = relation_class(relation_sym) + relation_hash.delete('id') update_missing_author(relation_hash, members_mapper, user_admin) if relation_sym == :notes update_user_references(relation_hash, members_mapper.map) @@ -49,8 +52,8 @@ module Gitlab return unless user_admin && members_map.note_member_list.include?(old_author_id) - relation_hash['note'] = ('*Blank note*') if relation_hash['note'].blank? - relation_hash['note'] += (missing_author_note(relation_hash['updated_at'], author['name'])) + relation_hash['note'] = '*Blank note*' if relation_hash['note'].blank? + relation_hash['note'] += missing_author_note(relation_hash['updated_at'], author['name']) end def missing_author_note(updated_at, author_name) @@ -109,12 +112,6 @@ module Gitlab imported_object.importing = true if imported_object.respond_to?(:importing) imported_object end - - def parse_relation(relation_hash, relation_sym) - klass = relation_class(relation_sym) - relation_hash.delete('id') - klass - end end end end diff --git a/lib/gitlab/import_export/repo_restorer.rb b/lib/gitlab/import_export/repo_restorer.rb index 36094b95aa4..9fac58e2242 100644 --- a/lib/gitlab/import_export/repo_restorer.rb +++ b/lib/gitlab/import_export/repo_restorer.rb @@ -12,7 +12,6 @@ module Gitlab def restore return true unless File.exists?(@path_to_bundle) - FileUtils.mkdir_p(repos_path) FileUtils.mkdir_p(path_to_repo) git_unbundle(repo_path: path_to_repo, bundle_path: @path_to_bundle) -- cgit v1.2.3