Address cyclical dependency of `custom_project_templates` feature check with ApplicationSetting
Problem
The feature custom_project_templates
is marked as incompatible with Usage Ping because the License.feature_available?
has a cyclical dependency with ApplicationSetting
and a test will fail if we try to move it to Registration Features (a.k.a Usage Ping features).
There is plan to move this feature to Registration Features but currently this feature is blocked by the cyclical dependency.
Reproduction steps
- Remove the feature from the list of blocked features https://gitlab.com/gitlab-org/gitlab/-/blob/0daa71593c64e855845237ac590fe12437906976/ee/app/models/gitlab_subscriptions/features.rb#L322 - this blocklist prevents the cyclical dependency from happening.
- Run
bundle exec spring rspec spec/lib/gitlab/current_settings_spec.rb
- you should see some specs taking very long time due to the cyclical dependency. Eventually the spec examples timeout withSystemStackError
(stack level too deep).
Proposal
Refactor the code (possibly from ApplicationSetting
) not to rely so early in the app setup / startup on License.feature_available?
for the given feature.
The problem is likely occurring during the validation of AppliactionSetting#import_sources
because Gitlab::ImportSources.options
depends on a [licensed feature check](https://gitlab.com/gitlab-org/gitlab/-/blob/b3bbd7aebb618c3610eff8f7427523415bcd0eaf/ee/lib/ee/gitlab/import_sources.rb#L18) which in turn needs an
ApplicationSetting` record to exist, hence the cyclical dependency.
To fix that we could append on: :update
to the validates_each :import_sources
so that we don't validate that at creation as ApplicationSetting.create_from_defaults
already provides valid initial values.
Another place where it would be a good change to prevent further cyclical dependencies would be here. We can fix that by inverting the condition as super && custom_project_templates_enabled?
. The default value for custom_project_templates_group_id
is nil
which would shortcircuit the condition and not call custom_project_templates_enabled?
during the creation of ApplicationSetting
record.
Once the refactoring solves the cyclical dependency, remove the feature from FEATURES_NOT_SUPPORTING_USAGE_PING
list.