Skip to content

Fix inconsistent association between Project and ProjectNamespace

Abdul Wadood requested to merge 364277-inverse-project-namespace into master

Prerequisites

ProjectNamespace was introduced to allow us to have a namespace associated with every Project. This was required to consolidate Projects and Groups. To understand more about this project you can go through &6473.

We can only stop syncing from Project to ProjectNamespace when we start cleaning up Project after we have migrated all the functionality from Project to ProjectNamespace.

What does this MR do and why?

inverse_of tells Rails that the associated objects (belongs_to and has_one associated object) point to the same in-memory objects. Rails automatically adds inverse_of but it falls apart when using class_name or foreign_key.

To understand more about inverse_of go through these links:

The circular dependency between the project and the project namespace was removed in 1dc44ca6 by removing inverse_of from the project_namespace association to fix some specs. Adding back the inverse_of option to the project_namespace association in app/models/project.rb was resulting in the project getting saved twice when project.save is called. This was detected through spec failures. To reproduce the same issue I created this fresh rails project and the issue was reproducible. I later found that there's an open issue in Rails for the same.

Saving the parent (model with has_one) before the child (model with belongs_to) doesn't cause multiple saves as can be seen here as well. Also, saving the parent (ProjectNamespace) automatically saves the child (Project). As a workaround for the above, we now save the parent before the child in Projects::CreateService which doesn't cause the child to get saved twice. But if the child is invalid the saving of the parent is not prevented. Therefore, we're checking if the child is valid in the Projects::CreateService using if @project.valid?. Another way to validate the child was to use the validate: true option but that was triggering the validations multiple times.

See !91030 (closed) and !89311 (closed) for prior attempts.

Screenshots or screen recordings

There should be no user-facing changes.

How to set up and validate locally

This was initially detected in specs with the following code:

p = create(:project)

p.project_namespace.project == nil # this returns true

However, loading the project does not result in the same issue:

p = Project.find(project.id)
p.project_namespace.project == nil # this returns false

This can also be reproduced from a rails console through Projects::CreateService:

Before changes

group = Group.last
params = {
  namespace_id: group.id,
  name: 'test1'.titleize,
  path: 'test1',
  description: FFaker::Lorem.sentence,
  visibility_level: Gitlab::VisibilityLevel::PRIVATE,
  skip_disk_validation: true
}
# {:namespace_id=>22, :name=>"Test1", :path=>"test1", :description=>"Omnis distinctio omnis nemo sequi atque voluptate non.", :visibility_level=>0, :skip_disk_validation=>true}
project = ::Projects::CreateService.new(User.first, params).execute
project.project_namespace.project
# nil

After changes

group = Group.last
params = {
  namespace_id: group.id,
  name: 'test1'.titleize,
  path: 'test1',
  description: FFaker::Lorem.sentence,
  visibility_level: Gitlab::VisibilityLevel::PRIVATE,
  skip_disk_validation: true
}
# {:namespace_id=>22, :name=>"Test1", :path=>"test1", :description=>"Omnis distinctio omnis nemo sequi atque voluptate non.", :visibility_level=>0, :skip_disk_validation=>true}
project = ::Projects::CreateService.new(User.first, params).execute
project.project_namespace.project
# <Project id:19 twitter/test1>>

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #364277 (closed)

Edited by Abdul Wadood

Merge request reports