Namespace#actual_limits should always reference a persisted record
Overview
In !19438 (merged) we introduced a new table plan_limits
where we can store any application limits per plan. On Gitlab.com a namespace can be associated to any plan from free
to gold
. On self-hosted a namespace can only be associated to a default
plan.
Problem
- The current code references in-memory objects if namespace does not have an explicit subscription
-
actual_limits
defaults to an unlimitedPlanLimits.new
when there is no limits record associated to theactual_plan
. This should not happen in production code as each created plan has related plan limits. However, this can occur in specs as some factories do not create associated subscriptions, plans and limits.
While using these defaults helped introducing new behavior in a cheap way it can become difficult to debug an issue when explicit relationships are missing.
Solution
We should move from the current code:
def actual_plan
strong_memoize(:actual_plan) do
subscription = find_or_create_subscription
subscription&.hosted_plan || Plan.free || Plan.default
end
end
def actual_limits
actual_plan&.limits || PlanLimits.new
end
To something like this:
def actual_plan
strong_memoize(:actual_plan) do
# in find_or_create_subscription create a free plan
# if doesn't exist (on Gitlab.com) or default plan
# on self-hosted.
subscription = find_or_create_subscription
subscription.hosted_plan
end
end
def actual_limits
# create limits lazily if for some reason a plan
# was created without associated limits.
actual_plan.limits || actual_plan.create_limits
end