Skip to content

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

  1. The current code references in-memory objects if namespace does not have an explicit subscription
  2. actual_limits defaults to an unlimited PlanLimits.new when there is no limits record associated to the actual_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