Commit 084520ba authored by Rémy Coutable's avatar Rémy Coutable

Merge branch '325800-doc-jh-features' into 'master'

Add `prepend_mod` and a new JH merge requests review guidance

See merge request gitlab-org/gitlab!57807
parents 6fcf2eed 12a1daf9
......@@ -83,5 +83,4 @@ module AppearancesHelper
end
end
AppearancesHelper.prepend_if_ee('EE::AppearancesHelper')
AppearancesHelper.prepend_if_jh('JH::AppearancesHelper')
AppearancesHelper.prepend_mod
......@@ -409,5 +409,4 @@ module ApplicationHelper
end
end
ApplicationHelper.prepend_if_ee('EE::ApplicationHelper')
ApplicationHelper.prepend_if_jh('JH::ApplicationHelper')
ApplicationHelper.prepend_mod
......@@ -3,42 +3,50 @@
require 'active_support/inflector'
module InjectEnterpriseEditionModule
def prepend_if_ee(constant, with_descendants: false)
return unless Gitlab.ee?
prepend_module(constant.constantize, with_descendants)
def prepend_if_ee(constant_with_prefix, namespace: Object, with_descendants: false)
prepend_mod_for(
constant_without_prefix(constant_with_prefix),
namespace: namespace,
with_descendants: with_descendants)
end
def extend_if_ee(constant)
extend(constant.constantize) if Gitlab.ee?
def extend_if_ee(constant_with_prefix, namespace: Object)
each_extension_for(
constant_without_prefix(constant_with_prefix),
namespace,
&method(:extend))
end
def include_if_ee(constant)
include(constant.constantize) if Gitlab.ee?
def include_if_ee(constant_with_prefix, namespace: Object)
each_extension_for(
constant_without_prefix(constant_with_prefix),
namespace,
&method(:include))
end
def prepend_ee_mod(with_descendants: false)
return unless Gitlab.ee?
prepend_module(ee_module, with_descendants)
def prepend_mod(with_descendants: false)
prepend_mod_for(name, with_descendants: with_descendants)
end
alias_method :prepend_ee_mod, :prepend_mod
def extend_ee_mod
extend(ee_module) if Gitlab.ee?
def extend_mod
each_extension_for(name, Object, &method(:extend))
end
alias_method :extend_ee_mod, :extend_mod
def include_ee_mod
include(ee_module) if Gitlab.ee?
def include_mod
each_extension_for(name, Object, &method(:include))
end
alias_method :include_ee_mod, :include_mod
def prepend_if_jh(constant, with_descendants: false)
return unless Gitlab.jh?
private
prepend_module(constant.constantize, with_descendants)
def prepend_mod_for(constant_name, namespace: Object, with_descendants: false)
each_extension_for(constant_name, namespace) do |constant|
prepend_module(constant, with_descendants)
end
end
private
def prepend_module(mod, with_descendants)
prepend(mod)
......@@ -47,8 +55,34 @@ module InjectEnterpriseEditionModule
end
end
def ee_module
::EE.const_get(name, false)
def each_extension_for(constant_name, namespace)
Gitlab.extensions.each do |extension_name|
extension_namespace =
const_get_maybe_false(namespace, extension_name.upcase)
extension_module =
const_get_maybe_false(extension_namespace, constant_name)
yield(extension_module) if extension_module
end
end
def const_get_maybe_false(mod, name)
# We're still heavily relying on Rails autoloading instead of zeitwerk,
# therefore this check: `mod.const_defined?(name, false)`
# Is not reliable, which may return false while it's defined.
# After we moved everything over to zeitwerk we can avoid rescuing
# NameError and just check if const_defined?
# mod && mod.const_defined?(name, false) && mod.const_get(name, false)
mod && mod.const_get(name, false)
rescue NameError
false
end
def constant_without_prefix(constant_with_prefix)
constant_with_prefix
.delete_prefix('::') # TODO: Some calling sites are passing this prefix
.delete_prefix('EE::')
end
end
......
---
stage: none
group: unassigned
info: https://gitlab.com/gitlab-jh/gitlab
---
# Guidelines for reviewing JiHu (JH) Edition related merge requests
We have two kinds of changes related to JH:
- Inside `jh/`
- This is beyond EE repository and not the intention for this documentation.
- Outside `jh/`
- These will have to sit in EE repository, so reviewers and maintainers for
EE repository will have to review and maintain. This includes codes like
`Gitlab.jh?`, and how it attempts to load codes under `jh/` just like we
have codes which will load codes under `ee/`.
- This documentation intended to guide how those codes should look like, so
we'll have better understanding what are the changes needed for looking up
codes under `jh/`.
- We will generalize this so both EE and JH can share the same mechanism,
then we wouldn't have to treat them differently.
If needed, review the corresponding JH merge request located at [JH repository](https://gitlab.com/gitlab-jh/gitlab)
## Act as EE when `jh/` does not exist
- In the case of EE repository, `jh/` does not exist so it should just act like EE (or CE when the license is absent)
- In the case of JH repository, `jh/` does exist but `EE_ONLY` environment variable can be set to force it run under EE mode.
- In the case of JH repository, `jh/` does exist but `FOSS_ONLY` environment variable can be set to force it run under CE mode.
## CI pipelines in a JH context
EE repository does not have `jh/` directory therefore there is no way to run
JH pipelines in the EE repository. All JH tests should go to [JH repository](https://gitlab.com/gitlab-jh/gitlab).
The top-level JH CI configuration is located at `jh/.gitlab-ci.yml` (which
does not exist in EE repository) and it'll include EE CI configurations
accordingly. Sometimes it's needed to update the EE CI configurations for JH
to customize more easily.
### JH features based on CE or EE features
For features that build on existing CE/EE features, a module in the `JH`
namespace injected in the CE/EE class/module is needed. This aligns with
what we're doing with EE features.
See [EE features based on CE features](ee_features.md#ee-features-based-on-ce-features) for more details.
For example, to prepend a module into the `User` class you would use
the following approach:
```ruby
class User < ActiveRecord::Base
# ... lots of code here ...
end
User.prepend_mod
```
Under EE, `User.prepend_mod` will attempt to:
- Load EE module
Under JH, `User.prepend_mod` will attempt to:
- Load EE module, and:
- Load JH module
Do not use methods such as `prepend`, `extend`, and `include`. Instead, use
`prepend_mod`, `extend_mod`, or `include_mod`. These methods will try to find
the relevant EE and JH modules by the name of the receiver module.
If reviewing the corresponding JH file is needed, it should be found at
[JH repository](https://gitlab.com/gitlab-jh/gitlab).
### General guidance for writing JH extensions
See [Guidelines for implementing Enterprise Edition features](ee_features.md)
for general guidance.
# frozen_string_literal: true
module Gitlab
module ImportExport
module GroupHelper
def group_wiki_repo_bundle_filename(group_id)
"#{group_id}.wiki.bundle"
end
def group_wiki_repo_bundle_path(shared, filename)
File.join(shared.export_path, 'repositories', filename)
end
def group_wiki_repo_bundle_full_path(shared, group_id)
group_wiki_repo_bundle_path(shared, group_wiki_repo_bundle_filename(group_id))
end
end
end
end
......@@ -92,6 +92,16 @@ module Gitlab
Rails.env.development? || Rails.env.test?
end
def self.extensions
if jh?
%w[ee jh]
elsif ee?
%w[ee]
else
%w[]
end
end
def self.ee?
@is_ee ||=
# We use this method when the Rails environment is not loaded. This
......
......@@ -99,11 +99,19 @@ module Gitlab
def group_config_file
Rails.root.join('lib/gitlab/import_export/group/import_export.yml')
end
def group_wiki_repo_bundle_filename(group_id)
"#{group_id}.wiki.bundle"
end
def group_wiki_repo_bundle_path(shared, filename)
File.join(shared.export_path, 'repositories', filename)
end
def group_wiki_repo_bundle_full_path(shared, group_id)
group_wiki_repo_bundle_path(shared, group_wiki_repo_bundle_filename(group_id))
end
end
end
Gitlab::ImportExport.prepend_if_ee('EE::Gitlab::ImportExport')
# The methods in `Gitlab::ImportExport::GroupHelper` should be available as both
# instance and class methods.
Gitlab::ImportExport.extend_if_ee('Gitlab::ImportExport::GroupHelper')
Gitlab::ImportExport.prepend_mod
......@@ -12,5 +12,5 @@ module Gitlab
end
end
Gitlab::SubscriptionPortal.prepend_if_jh('JH::Gitlab::SubscriptionPortal')
Gitlab::SubscriptionPortal.prepend_mod
Gitlab::SubscriptionPortal::SUBSCRIPTIONS_URL = Gitlab::SubscriptionPortal.subscriptions_url.freeze
......@@ -107,4 +107,4 @@ module QA
end
end
QA::Page::Admin::Menu.prepend_if_ee('QA::EE::Page::Admin::Menu')
QA::Page::Admin::Menu.prepend_if_ee('Page::Admin::Menu', namespace: QA)
......@@ -20,4 +20,4 @@ module QA
end
end
QA::Page::Admin::Overview::Groups::Edit.prepend_if_ee('QA::EE::Page::Admin::Overview::Groups::Edit')
QA::Page::Admin::Overview::Groups::Edit.prepend_if_ee('Page::Admin::Overview::Groups::Edit', namespace: QA)
......@@ -137,4 +137,4 @@ module QA
end
end
QA::Page::Component::IssueBoard::Show.prepend_if_ee('QA::EE::Page::Component::IssueBoard::Show')
QA::Page::Component::IssueBoard::Show.prepend_if_ee('Page::Component::IssueBoard::Show', namespace: QA)
......@@ -45,4 +45,4 @@ module QA
end
end
QA::Page::Dashboard::Projects.prepend_if_ee('QA::EE::Page::Dashboard::Projects')
QA::Page::Dashboard::Projects.prepend_if_ee('Page::Dashboard::Projects', namespace: QA)
......@@ -62,4 +62,4 @@ module QA
end
end
QA::Page::File::Show.prepend_if_ee('QA::EE::Page::File::Show')
QA::Page::File::Show.prepend_if_ee('Page::File::Show', namespace: QA)
......@@ -83,4 +83,4 @@ module QA
end
end
QA::Page::Group::Menu.prepend_if_ee('QA::EE::Page::Group::Menu')
QA::Page::Group::Menu.prepend_if_ee('Page::Group::Menu', namespace: QA)
......@@ -129,4 +129,4 @@ module QA
end
end
QA::Page::Group::Settings::General.prepend_if_ee('QA::EE::Page::Group::Settings::General')
QA::Page::Group::Settings::General.prepend_if_ee('Page::Group::Settings::General', namespace: QA)
......@@ -175,4 +175,4 @@ module QA
end
end
QA::Page::Main::Menu.prepend_if_ee('QA::EE::Page::Main::Menu')
QA::Page::Main::Menu.prepend_if_ee('Page::Main::Menu', namespace: QA)
......@@ -41,4 +41,4 @@ module QA
end
end
QA::Page::MergeRequest::New.prepend_if_ee('QA::EE::Page::MergeRequest::New')
QA::Page::MergeRequest::New.prepend_if_ee('Page::MergeRequest::New', namespace: QA)
......@@ -390,4 +390,4 @@ module QA
end
end
QA::Page::MergeRequest::Show.prepend_if_ee('QA::EE::Page::MergeRequest::Show')
QA::Page::MergeRequest::Show.prepend_if_ee('Page::MergeRequest::Show', namespace: QA)
......@@ -30,4 +30,4 @@ module QA
end
end
QA::Page::Milestone::Show.prepend_if_ee('QA::EE::Page::Milestone::Show')
QA::Page::Milestone::Show.prepend_if_ee('Page::Milestone::Show', namespace: QA)
......@@ -56,4 +56,4 @@ module QA
end
end
QA::Page::Profile::Menu.prepend_if_ee('QA::EE::Page::Profile::Menu')
QA::Page::Profile::Menu.prepend_if_ee('Page::Profile::Menu', namespace: QA)
......@@ -85,4 +85,4 @@ module QA
end
end
QA::Page::Project::Issue::Index.prepend_if_ee('QA::EE::Page::Project::Issue::Index')
QA::Page::Project::Issue::Index.prepend_if_ee('Page::Project::Issue::Index', namespace: QA)
......@@ -70,4 +70,4 @@ module QA
end
end
QA::Page::Project::Issue::Show.prepend_if_ee('QA::EE::Page::Project::Issue::Show')
QA::Page::Project::Issue::Show.prepend_if_ee('Page::Project::Issue::Show', namespace: QA)
......@@ -75,4 +75,4 @@ module QA
end
end
QA::Page::Project::Job::Show.prepend_if_ee('QA::EE::Page::Project::Job::Show')
QA::Page::Project::Job::Show.prepend_if_ee('Page::Project::Job::Show', namespace: QA)
......@@ -56,4 +56,4 @@ module QA
end
end
QA::Page::Project::Menu.prepend_if_ee('QA::EE::Page::Project::Menu')
QA::Page::Project::Menu.prepend_if_ee('Page::Project::Menu', namespace: QA)
......@@ -72,4 +72,4 @@ module QA
end
end
QA::Page::Project::New.prepend_if_ee('QA::EE::Page::Project::New')
QA::Page::Project::New.prepend_if_ee('Page::Project::New', namespace: QA)
......@@ -134,4 +134,4 @@ module QA
end
end
QA::Page::Project::Operations::Metrics::Show.prepend_if_ee('QA::EE::Page::Project::Operations::Metrics::Show')
QA::Page::Project::Operations::Metrics::Show.prepend_if_ee('Page::Project::Operations::Metrics::Show', namespace: QA)
......@@ -27,4 +27,4 @@ module QA
end
end
QA::Page::Project::Packages::Index.prepend_if_ee('QA::EE::Page::Project::Packages::Index')
QA::Page::Project::Packages::Index.prepend_if_ee('Page::Project::Packages::Index', namespace: QA)
......@@ -67,4 +67,4 @@ module QA
end
end
QA::Page::Project::Pipeline::Index.prepend_if_ee('QA::EE::Page::Project::Pipeline::Index')
QA::Page::Project::Pipeline::Index.prepend_if_ee('Page::Project::Pipeline::Index', namespace: QA)
......@@ -117,4 +117,4 @@ module QA
end
end
QA::Page::Project::Pipeline::Show.prepend_if_ee('QA::EE::Page::Project::Pipeline::Show')
QA::Page::Project::Pipeline::Show.prepend_if_ee('Page::Project::Pipeline::Show', namespace: QA)
......@@ -43,4 +43,4 @@ module QA
end
end
QA::Page::Project::Settings::CICD.prepend_if_ee("QA::EE::Page::Project::Settings::CICD")
QA::Page::Project::Settings::CICD.prepend_if_ee("Page::Project::Settings::CICD", namespace: QA)
......@@ -23,4 +23,4 @@ module QA
end
end
QA::Page::Project::Settings::Integrations.prepend_if_ee('QA::EE::Page::Project::Settings::Integrations')
QA::Page::Project::Settings::Integrations.prepend_if_ee('Page::Project::Settings::Integrations', namespace: QA)
......@@ -57,4 +57,4 @@ module QA
end
end
QA::Page::Project::Settings::Main.prepend_if_ee("QA::EE::Page::Project::Settings::Main")
QA::Page::Project::Settings::Main.prepend_if_ee("Page::Project::Settings::Main", namespace: QA)
......@@ -38,4 +38,4 @@ module QA
end
end
QA::Page::Project::Settings::MergeRequest.prepend_if_ee("QA::EE::Page::Project::Settings::MergeRequest")
QA::Page::Project::Settings::MergeRequest.prepend_if_ee("Page::Project::Settings::MergeRequest", namespace: QA)
......@@ -129,4 +129,4 @@ module QA
end
end
QA::Page::Project::Settings::MirroringRepositories.prepend_if_ee('QA::EE::Page::Project::Settings::MirroringRepositories')
QA::Page::Project::Settings::MirroringRepositories.prepend_if_ee('Page::Project::Settings::MirroringRepositories', namespace: QA)
......@@ -69,4 +69,4 @@ module QA
end
end
QA::Page::Project::Settings::ProtectedBranches.prepend_if_ee('QA::EE::Page::Project::Settings::ProtectedBranches')
QA::Page::Project::Settings::ProtectedBranches.prepend_if_ee('Page::Project::Settings::ProtectedBranches', namespace: QA)
......@@ -43,4 +43,4 @@ module QA
end
end
QA::Page::Project::Settings::ProtectedTags.prepend_if_ee('QA::EE::Page::Project::Settings::ProtectedTags')
QA::Page::Project::Settings::ProtectedTags.prepend_if_ee('Page::Project::Settings::ProtectedTags', namespace: QA)
......@@ -62,4 +62,4 @@ module QA
end
end
QA::Page::Project::Settings::Repository.prepend_if_ee('QA::EE::Page::Project::Settings::Repository')
QA::Page::Project::Settings::Repository.prepend_if_ee('Page::Project::Settings::Repository', namespace: QA)
......@@ -180,4 +180,4 @@ module QA
end
end
QA::Page::Project::Show.prepend_if_ee('QA::EE::Page::Project::Show')
QA::Page::Project::Show.prepend_if_ee('Page::Project::Show', namespace: QA)
......@@ -26,4 +26,4 @@ module QA
end
end
QA::Page::Project::Snippet::Index.prepend_if_ee('QA::EE::Page::Project::Snippet::Index')
QA::Page::Project::Snippet::Index.prepend_if_ee('Page::Project::Snippet::Index', namespace: QA)
......@@ -311,4 +311,4 @@ module QA
end
end
QA::Page::Project::WebIDE::Edit.prepend_if_ee('QA::EE::Page::Component::WebIDE::WebTerminalPanel')
QA::Page::Project::WebIDE::Edit.prepend_if_ee('Page::Component::WebIDE::WebTerminalPanel', namespace: QA)
......@@ -14,4 +14,4 @@ module QA
end
end
QA::Page::Project::Wiki::Show.prepend_if_ee('QA::EE::Page::Project::Wiki::Show')
QA::Page::Project::Wiki::Show.prepend_if_ee('Page::Project::Wiki::Show', namespace: QA)
......@@ -21,4 +21,4 @@ module QA
end
end
QA::Page::Registration::Welcome.prepend_if_ee('QA::EE::Page::Registration::Welcome')
QA::Page::Registration::Welcome.prepend_if_ee('Page::Registration::Welcome', namespace: QA)
......@@ -403,4 +403,4 @@ module QA
end
end
QA::Runtime::Env.extend_if_ee('QA::EE::Runtime::Env')
QA::Runtime::Env.extend_if_ee('Runtime::Env', namespace: QA)
......@@ -58,4 +58,4 @@ module QA
end
end
QA::Scenario::Test::Sanity::Selectors.prepend_if_ee('QA::EE::Scenario::Test::Sanity::Selectors')
QA::Scenario::Test::Sanity::Selectors.prepend_if_ee('Scenario::Test::Sanity::Selectors', namespace: QA)
# frozen_string_literal: true
require 'fast_spec_helper'
RSpec.describe InjectEnterpriseEditionModule do
let(:extension_name) { 'FF' }
let(:extension_namespace) { Module.new }
let(:fish_name) { 'Fish' }
let(:fish_class) { Class.new }
let(:fish_extension) { Module.new }
before do
# Make sure we're not relying on which mode we're running under
allow(Gitlab).to receive(:extensions).and_return([extension_name.downcase])
# Test on an imagined extension and imagined class
stub_const(fish_name, fish_class) # Fish
allow(fish_class).to receive(:name).and_return(fish_name)
end
shared_examples 'expand the extension with' do |method|
context 'when extension namespace is set at top-level' do
before do
stub_const(extension_name, extension_namespace) # FF
extension_namespace.const_set(fish_name, fish_extension) # FF::Fish
end
it "calls #{method} with the extension module" do
expect(fish_class).to receive(method).with(fish_extension)
fish_class.__send__("#{method}_if_ee", fish_name)
end
it "ignores EE prefix and calls #{method} with the extension module" do
expect(fish_class).to receive(method).with(fish_extension)
fish_class.__send__("#{method}_if_ee", "EE::#{fish_name}")
end
it "ignores ::EE prefix and calls #{method} with the extension module" do
expect(fish_class).to receive(method).with(fish_extension)
fish_class.__send__("#{method}_if_ee", "::EE::#{fish_name}")
end
end
context 'when extension namespace is set at another namespace' do
let(:another_namespace) { Module.new } # QA
before do
another_namespace.const_set(extension_name, extension_namespace) # QA::FF
extension_namespace.const_set(fish_name, fish_extension) # QA::FF::Fish
end
it "calls #{method} with the extension module from the additional namespace" do
expect(fish_class).to receive(method).with(fish_extension)
fish_class.__send__("#{method}_if_ee", fish_name, namespace: another_namespace)
end
end
context 'when extension namespace exists but not the extension' do
before do
stub_const(extension_name, extension_namespace) # FF
end
it "does not call #{method}" do
expect(fish_class).not_to receive(method).with(fish_extension)
fish_class.__send__("#{method}_if_ee", fish_name)
end
end
context 'when extension namespace does not exist' do
it "does not call #{method}" do
expect(fish_class).not_to receive(method).with(fish_extension)
fish_class.__send__("#{method}_if_ee", fish_name)
end
end
end
shared_examples 'expand the assumed extension with' do |method|
context 'when extension namespace is set at top-level' do
before do
stub_const(extension_name, extension_namespace) # FF
extension_namespace.const_set(fish_name, fish_extension) # FF::Fish
end
it "calls #{method} with the extension module" do
expect(fish_class).to receive(method).with(fish_extension)
fish_class.__send__("#{method}_mod")
end
end
context 'when extension namespace exists but not the extension' do
before do
stub_const(extension_name, extension_namespace) # FF
end
it "does not call #{method}" do
expect(fish_class).not_to receive(method).with(fish_extension)
fish_class.__send__("#{method}_mod")
end
end
context 'when extension namespace does not exist' do
it "does not call #{method}" do
expect(fish_class).not_to receive(method).with(fish_extension)
fish_class.__send__("#{method}_mod")
end
end
end
describe '#prepend_if_ee' do
it_behaves_like 'expand the extension with', :prepend
end
describe '#extend_if_ee' do
it_behaves_like 'expand the extension with', :extend
end
describe '#include_if_ee' do
it_behaves_like 'expand the extension with', :include
end
describe '#prepend_mod' do
it_behaves_like 'expand the assumed extension with', :prepend
end
describe '#extend_mod' do
it_behaves_like 'expand the assumed extension with', :extend
end
describe '#include_mod' do
it_behaves_like 'expand the assumed extension with', :include
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment