Fix inconsistent association between Project and ProjectNamespace
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:
- https://guides.rubyonrails.org/association_basics.html#options-for-belongs-to-inverse-of
- https://www.viget.com/articles/exploring-the-inverse-of-option-on-rails-model-associations/
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.
-
I have evaluated the MR acceptance checklist for this MR.
Related to #364277 (closed)