diff options
| -rw-r--r-- | .gitlab/ci/review.gitlab-ci.yml | 33 | ||||
| -rw-r--r-- | changelogs/unreleased/39113-project-user-grafana-integrations.yml | 5 | ||||
| -rw-r--r-- | doc/development/foreign_keys.md | 26 | ||||
| -rw-r--r-- | lib/gitlab/import_export/relation_factory.rb | 25 | ||||
| -rw-r--r-- | spec/lib/gitlab/import_export/relation_factory_spec.rb | 99 |
5 files changed, 171 insertions, 17 deletions
diff --git a/.gitlab/ci/review.gitlab-ci.yml b/.gitlab/ci/review.gitlab-ci.yml index 7e0759b3c85..53311ab08d3 100644 --- a/.gitlab/ci/review.gitlab-ci.yml +++ b/.gitlab/ci/review.gitlab-ci.yml @@ -116,7 +116,6 @@ schedule:review-build-cng: extends: - .default-tags - .default-retry - - .default-only image: registry.gitlab.com/gitlab-org/gitlab-build-images:gitlab-charts-build-base dependencies: [] variables: @@ -156,10 +155,11 @@ schedule:review-build-cng: when: always review-deploy: - extends: - - .review-deploy-base - - .only-review - - .only:changes-code-qa + extends: .review-deploy-base + rules: + - <<: *if-canonical-gitlab-and-merge-request + changes: *code-qa-patterns + when: on_success schedule:review-deploy: extends: @@ -167,10 +167,7 @@ schedule:review-deploy: - .only-review-schedules .base-review-stop: - extends: - - .review-workflow-base - - .only-review - - .only:changes-code-qa + extends: .review-workflow-base environment: action: stop variables: @@ -187,13 +184,20 @@ schedule:review-deploy: review-stop-failed-deployment: extends: .base-review-stop stage: prepare + rules: + - <<: *if-canonical-gitlab-and-merge-request + changes: *code-qa-patterns + when: on_success script: - delete_failed_release review-stop: extends: .base-review-stop stage: review - when: manual + rules: + - <<: *if-canonical-gitlab-and-merge-request + changes: *code-qa-patterns + when: manual allow_failure: true script: - delete_release @@ -271,10 +275,11 @@ review-qa-all: performance: performance.json review-performance: - extends: - - .review-performance-base - - .only-review - - .only:changes-code-qa + extends: .review-performance-base + rules: + - <<: *if-canonical-gitlab-and-merge-request + changes: *code-qa-patterns + when: on_success needs: ["review-deploy"] dependencies: ["review-deploy"] before_script: diff --git a/changelogs/unreleased/39113-project-user-grafana-integrations.yml b/changelogs/unreleased/39113-project-user-grafana-integrations.yml new file mode 100644 index 00000000000..9b0e470c0ed --- /dev/null +++ b/changelogs/unreleased/39113-project-user-grafana-integrations.yml @@ -0,0 +1,5 @@ +--- +title: Preload project, user and group to reuse objects during project import +merge_request: 21853 +author: +type: performance diff --git a/doc/development/foreign_keys.md b/doc/development/foreign_keys.md index 0ab0deb156f..38b60ce6f0b 100644 --- a/doc/development/foreign_keys.md +++ b/doc/development/foreign_keys.md @@ -61,3 +61,29 @@ introduces non database logic to a model, and means we can no longer rely on foreign keys to remove the data as this would result in the filesystem data being left behind. In such a case you should use a service class instead that takes care of removing non database data. + +## Alternative primary keys with has_one associations + +Sometimes a `has_one` association is used to create a one-to-one relationship: + +```ruby +class User < ActiveRecord::Base + has_one :user_config +end + +class UserConfig < ActiveRecord::Base + belongs_to :user +end +``` + +In these cases, there may be an opportunity to remove the unnecessary `id` +column on the associated table, `user_config.id` in this example. Instead, +the originating table ID can be used as the primary key for the associated +table: + +```ruby +create_table :user_configs, id: false do |t| + t.references :users, primary_key: true, default: nil, index: false, foreign_key: { on_delete: :cascade } + ... +end +``` diff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rb index 1438a7db001..9dd30803196 100644 --- a/lib/gitlab/import_export/relation_factory.rb +++ b/lib/gitlab/import_export/relation_factory.rb @@ -34,6 +34,8 @@ module Gitlab PROJECT_REFERENCES = %w[project_id source_project_id target_project_id].freeze + GROUP_REFERENCES = %w[group_id].freeze + BUILD_MODELS = %i[Ci::Build commit_status].freeze IMPORTED_OBJECT_MAX_RETRIES = 5.freeze @@ -89,7 +91,13 @@ module Gitlab setup_models - generate_imported_object + object = generate_imported_object + + # We preload the project, user, and group to re-use objects + object = preload_keys(object, PROJECT_REFERENCES, @project) + object = preload_keys(object, GROUP_REFERENCES, @project.group) + object = preload_keys(object, USER_REFERENCES, @user) + object end def self.overrides @@ -122,6 +130,21 @@ module Gitlab remove_encrypted_attributes! end + def preload_keys(object, references, value) + return object unless value + + references.each do |key| + attribute = "#{key.delete_suffix('_id')}=".to_sym + next unless object.respond_to?(key) && object.respond_to?(attribute) + + if object.read_attribute(key) == value&.id + object.public_send(attribute, value) # rubocop:disable GitlabSecurity/PublicSend + end + end + + object + end + def update_user_references USER_REFERENCES.each do |reference| if @relation_hash[reference] diff --git a/spec/lib/gitlab/import_export/relation_factory_spec.rb b/spec/lib/gitlab/import_export/relation_factory_spec.rb index 41d6e6f24fc..eb071d38eb7 100644 --- a/spec/lib/gitlab/import_export/relation_factory_spec.rb +++ b/spec/lib/gitlab/import_export/relation_factory_spec.rb @@ -3,7 +3,8 @@ require 'spec_helper' describe Gitlab::ImportExport::RelationFactory do - let(:project) { create(:project) } + let(:group) { create(:group) } + let(:project) { create(:project, :repository, group: group) } let(:members_mapper) { double('members_mapper').as_null_object } let(:merge_requests_mapping) { {} } let(:user) { create(:admin) } @@ -59,7 +60,7 @@ describe Gitlab::ImportExport::RelationFactory do end it 'has the new project_id' do - expect(created_object.project_id).to eq(project.id) + expect(created_object.project_id).to eql(project.id) end it 'has a nil token' do @@ -96,6 +97,100 @@ describe Gitlab::ImportExport::RelationFactory do end end + context 'merge_requset object' do + let(:relation_sym) { :merge_requests } + + let(:exported_member) do + { + "id" => 111, + "access_level" => 30, + "source_id" => 1, + "source_type" => "Project", + "user_id" => 3, + "notification_level" => 3, + "created_at" => "2016-11-18T09:29:42.634Z", + "updated_at" => "2016-11-18T09:29:42.634Z", + "user" => { + "id" => user.id, + "email" => user.email, + "username" => user.username + } + } + end + + let(:members_mapper) do + Gitlab::ImportExport::MembersMapper.new( + exported_members: [exported_member], + user: user, + importable: project) + end + + let(:relation_hash) do + { + 'id' => 27, + 'target_branch' => "feature", + 'source_branch' => "feature_conflict", + 'source_project_id' => project.id, + 'target_project_id' => project.id, + 'author_id' => user.id, + 'assignee_id' => user.id, + 'updated_by_id' => user.id, + 'title' => "MR1", + 'created_at' => "2016-06-14T15:02:36.568Z", + 'updated_at' => "2016-06-14T15:02:56.815Z", + 'state' => "opened", + 'merge_status' => "unchecked", + 'description' => "Description", + 'position' => 0, + 'source_branch_sha' => "ABCD", + 'target_branch_sha' => "DCBA", + 'merge_when_pipeline_succeeds' => true + } + end + + it 'has preloaded author' do + expect(created_object.author).to equal(user) + end + + it 'has preloaded updated_by' do + expect(created_object.updated_by).to equal(user) + end + + it 'has preloaded source project' do + expect(created_object.source_project).to equal(project) + end + + it 'has preloaded target project' do + expect(created_object.source_project).to equal(project) + end + end + + context 'label object' do + let(:relation_sym) { :labels } + let(:relation_hash) do + { + "id": 3, + "title": "test3", + "color": "#428bca", + "group_id": project.group.id, + "created_at": "2016-07-22T08:55:44.161Z", + "updated_at": "2016-07-22T08:55:44.161Z", + "template": false, + "description": "", + "project_id": project.id, + "type": "GroupLabel" + } + end + + it 'has preloaded project' do + expect(created_object.project).to equal(project) + end + + it 'has preloaded group' do + expect(created_object.group).to equal(project.group) + end + end + # `project_id`, `described_class.USER_REFERENCES`, noteable_id, target_id, and some project IDs are already # re-assigned by described_class. context 'Potentially hazardous foreign keys' do |