diff --git a/app/models/organizations/organization.rb b/app/models/organizations/organization.rb index f016e47ce110f9af909022c3893ebe0a2ad08682..f9b38d179cda7f36fffdc0d1f30a5bdcd9c2d483 100644 --- a/app/models/organizations/organization.rb +++ b/app/models/organizations/organization.rb @@ -25,6 +25,7 @@ class Organization < ApplicationRecord has_many :root_groups, -> { roots }, class_name: 'Group', inverse_of: :organization has_many :projects has_many :snippets + has_many :snippet_repositories, inverse_of: :organization has_many :topics, class_name: "Projects::Topic" has_many :integrations diff --git a/app/models/project.rb b/app/models/project.rb index 74a648a44305a53cf5d8d6c90ec2f444df41b6c8..6cbc91041b50a308e983854902f2f9c7f144f6fc 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -347,6 +347,7 @@ def self.integration_association_name(name) has_many :tag_push_hooks_integrations, -> { tag_push_hooks }, class_name: 'Integration' has_many :wiki_page_hooks_integrations, -> { wiki_page_hooks }, class_name: 'Integration' has_many :snippets, class_name: 'ProjectSnippet' + has_many :snippet_repositories, inverse_of: :project has_many :hooks, class_name: 'ProjectHook' has_many :protected_branches has_many :exported_protected_branches diff --git a/app/models/snippet_repository.rb b/app/models/snippet_repository.rb index 6e0e94379037544756163472459816a971562b3a..ec5f18ca23e7731f425b40670a2fdc7ee75d60f3 100644 --- a/app/models/snippet_repository.rb +++ b/app/models/snippet_repository.rb @@ -13,8 +13,24 @@ class SnippetRepository < ApplicationRecord belongs_to :snippet, inverse_of: :snippet_repository + belongs_to :organization, + class_name: 'Organizations::Organization', + foreign_key: 'snippet_organization_id', + inverse_of: :snippet_repositories, + optional: true + + belongs_to :project, + class_name: 'Project', + foreign_key: 'snippet_project_id', + inverse_of: :snippet_repositories, + optional: true + delegate :repository, :repository_storage, to: :snippet + before_validation :ensure_sharding_keys + + validate :validate_has_exactly_one_sharding_key + class << self def find_snippet(disk_path) find_by(disk_path: disk_path)&.snippet @@ -43,6 +59,25 @@ def multi_files_action(user, files = [], **options) private + def compact_sharding_keys + [organization, project].compact + end + + def ensure_sharding_keys + return if compact_sharding_keys.size == 1 + + self.organization = snippet&.organization if snippet&.organization_id.present? + self.project = snippet&.project if snippet&.project_id.present? + end + + def validate_has_exactly_one_sharding_key + if compact_sharding_keys.empty? + errors.add(:base, _('must belong to either an organization or a project')) + elsif compact_sharding_keys.size > 1 + errors.add(:base, _('cannot belong to both an organization and a project')) + end + end + def capture_git_error(&block) yield block rescue Gitlab::Git::Index::IndexError, diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 5596a4236072c4f79f618b218d3ea346a0c932e4..6de352ce889ba658ce97176bf5dd356d09d809cf 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -76910,6 +76910,9 @@ msgstr "" msgid "cannot be used for user namespace" msgstr "" +msgid "cannot belong to both an organization and a project" +msgstr "" + msgid "cannot contain HTML/XML tags, including any word between angle brackets (<,>)." msgstr "" @@ -78415,6 +78418,9 @@ msgstr "" msgid "must belong to either a project or an organization" msgstr "" +msgid "must belong to either an organization or a project" +msgstr "" + msgid "must belong to same project of its requirement object." msgstr "" diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 78c0b417a17e9c7a3093cc8131607b147fb1dade..0946541d5dba24526a5e1ace016c6a3076d6a8ae 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -715,6 +715,7 @@ project: - iterations - notes - snippets +- snippet_repositories - hooks - all_protected_branches - protected_branches diff --git a/spec/models/organizations/organization_spec.rb b/spec/models/organizations/organization_spec.rb index c37da0b7344822ec9dfad537aef909a0bd6d7fe9..a1a594979ae1a4fe46789fa95986981ae7da6619 100644 --- a/spec/models/organizations/organization_spec.rb +++ b/spec/models/organizations/organization_spec.rb @@ -26,6 +26,7 @@ it { is_expected.to have_many(:organization_users).inverse_of(:organization) } it { is_expected.to have_many :projects } it { is_expected.to have_many :snippets } + it { is_expected.to have_many :snippet_repositories } it { is_expected.to have_many :topics } end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 0febda10dfa1e45c0a72b46f181949228f9d7957..7f8c277b3e8a07b88e22db99d0b7ebace2bee215 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -48,6 +48,7 @@ it { is_expected.to have_many(:namespace_requesters) } it { is_expected.to have_many(:notes).dependent(:destroy) } it { is_expected.to have_many(:snippets).class_name('ProjectSnippet') } + it { is_expected.to have_many(:snippet_repositories) } it { is_expected.to have_many(:deploy_keys_projects) } it { is_expected.to have_many(:deploy_keys) } it { is_expected.to have_many(:hooks) } diff --git a/spec/models/snippet_repository_spec.rb b/spec/models/snippet_repository_spec.rb index 70fed43624e37b8f4136ab0e3751bc20d916bd69..657f66d4184a2c8e7a6d8a79b04e5f19ad164af0 100644 --- a/spec/models/snippet_repository_spec.rb +++ b/spec/models/snippet_repository_spec.rb @@ -12,6 +12,128 @@ describe 'associations' do it { is_expected.to belong_to(:shard) } it { is_expected.to belong_to(:snippet) } + it { is_expected.to belong_to(:organization) } + it { is_expected.to belong_to(:project) } + end + + # This is for the cells related sharding keys: + # snippet_project_id and snippet_organization_id + describe 'sharding key validations' do + let_it_be(:organization) { create(:organization) } + let_it_be(:project) { create(:project, organization: organization) } + let_it_be(:personal_snippet) { create(:personal_snippet, organization: organization) } + let_it_be(:project_snippet) { create(:project_snippet, project: project) } + + describe 'validations' do + context 'when both sharding keys are present' do + it 'is invalid' do + snippet_repo = build(:snippet_repository, + snippet: personal_snippet, + organization: organization, + project: project) + + expect(snippet_repo).not_to be_valid + expect(snippet_repo.errors[:base]).to include('cannot belong to both an organization and a project') + end + end + + context 'when neither sharding key is present' do + it 'is invalid' do + personal_snippet.organization_id = nil + + snippet_repo = build(:snippet_repository, + snippet: personal_snippet, + organization: nil, + project: nil) + + expect(snippet_repo).not_to be_valid + expect(snippet_repo.errors[:base]).to include('must belong to either an organization or a project') + end + end + + context 'when only organization is present' do + it 'is valid' do + snippet_repo = build(:snippet_repository, + snippet: personal_snippet, + organization: organization, + project: nil) + + expect(snippet_repo).to be_valid + end + end + + context 'when only project is present' do + it 'is valid' do + snippet_repo = build(:snippet_repository, + snippet: project_snippet, + organization: nil, + project: project) + + expect(snippet_repo).to be_valid + end + end + end + + describe 'before_validation callbacks' do + let_it_be(:other_organization) { create(:organization) } + let_it_be(:other_project) { create(:project) } + + context 'for personal snippet' do + it 'automatically assigns organization from snippet' do + snippet_repo = build(:snippet_repository, snippet: personal_snippet) + + expect(snippet_repo.organization).to be_nil + snippet_repo.valid? + expect(snippet_repo.organization).to eq(personal_snippet.organization) + end + + # This is an edge case because the key would initially be based on the + # snippet's key, but the test exists to prove that we early return if + # the key exists. + context 'when one sharding key is already set' do + it 'retains the same key' do + snippet_repo = build( + :snippet_repository, + snippet: personal_snippet, + organization: other_organization + ) + + expect(snippet_repo.organization).not_to be_nil + snippet_repo.valid? + expect(snippet_repo.organization).not_to eq(personal_snippet.organization) + expect(snippet_repo.organization).to eq(other_organization) + end + end + end + + context 'for project snippet' do + it 'automatically assigns project from snippet' do + snippet_repo = build(:snippet_repository, snippet: project_snippet) + + expect(snippet_repo.project).to be_nil + snippet_repo.valid? + expect(snippet_repo.project).to eq(project_snippet.project) + end + end + + # This is an edge case because the key would initially be based on the + # snippet's key, but the test exists to prove that we early return if + # the key exists. + context 'when one sharding key is already set' do + it 'retains the same key' do + snippet_repo = build( + :snippet_repository, + snippet: project_snippet, + project: other_project + ) + + expect(snippet_repo.project).not_to be_nil + snippet_repo.valid? + expect(snippet_repo.project).not_to eq(project_snippet.project) + expect(snippet_repo.project).to eq(other_project) + end + end + end end it_behaves_like 'shardable scopes' do