diff --git a/.rubocop.yml b/.rubocop.yml index 0f4018326a1..5d2c5c7cf49 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -76,6 +76,7 @@ Naming/FileName: - 'qa/qa/specs/**/*' - 'qa/bin/*' - 'config/**/*' + - 'ee/config/**/*' - 'lib/generators/**/*' - 'locale/unfound_translations.rb' - 'ee/locale/unfound_translations.rb' diff --git a/config/initializers/routing_draw.rb b/config/initializers/routing_draw.rb index 25003cf0239..f0f74954eef 100644 --- a/config/initializers/routing_draw.rb +++ b/config/initializers/routing_draw.rb @@ -1,7 +1,3 @@ # Adds draw method into Rails routing -# It allows us to keep routing splitted into files -class ActionDispatch::Routing::Mapper - def draw(routes_name) - instance_eval(File.read(Rails.root.join("config/routes/#{routes_name}.rb"))) - end -end +# It allows us to keep routing split into files +ActionDispatch::Routing::Mapper.prepend Gitlab::Patch::DrawRoute diff --git a/config/routes.rb b/config/routes.rb index c081ca9672a..8723a928cc3 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -34,6 +34,8 @@ Rails.application.routes.draw do match '*all', via: [:get, :post], to: proc { [404, {}, ['']] } end + draw :oauth + use_doorkeeper_openid_connect # Autocomplete diff --git a/config/routes/admin.rb b/config/routes/admin.rb index fb29c4748c1..af333bdc748 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -71,6 +71,7 @@ namespace :admin do resource :logs, only: [:show] resource :health_check, controller: 'health_check', only: [:show] resource :background_jobs, controller: 'background_jobs', only: [:show] + resource :system_info, controller: 'system_info', only: [:show] resources :requests_profiles, only: [:index, :show], param: :name, constraints: { name: /.+\.html/ } @@ -104,6 +105,7 @@ namespace :admin do resource :application_settings, only: [:show, :update] do resources :services, only: [:index, :edit, :update] + get :usage_data put :reset_registration_token put :reset_health_check_token diff --git a/config/routes/group.rb b/config/routes/group.rb index 602bbe837cf..2328b50b760 100644 --- a/config/routes/group.rb +++ b/config/routes/group.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + resources :groups, only: [:index, :new, :create] do post :preview_markdown end @@ -63,7 +65,6 @@ constraints(::Constraints::GroupUrlConstrainer.new) do end end - # On CE only index and show actions are needed resources :boards, only: [:index, :show] resources :runners, only: [:index, :edit, :update, :destroy, :show] do diff --git a/doc/development/ee_features.md b/doc/development/ee_features.md index f9e6efa2c30..b6f053ff0e9 100644 --- a/doc/development/ee_features.md +++ b/doc/development/ee_features.md @@ -171,7 +171,7 @@ There are a few gotchas with it: class Base def execute return unless enabled? - + # ... # ... end @@ -185,12 +185,12 @@ There are a few gotchas with it: class Base def execute return unless enabled? - + do_something end - + private - + def do_something # ... # ... @@ -204,14 +204,14 @@ There are a few gotchas with it: ```ruby module EE::Base extend ::Gitlab::Utils::Override - + override :do_something def do_something # Follow the above pattern to call super and extend it end end ``` - + This would require updating CE first, or make sure this is back ported to CE. When prepending, place them in the `ee/` specific sub-directory, and @@ -332,6 +332,21 @@ full implementation details. [ce-mr-full-private]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12373 [ee-mr-full-private]: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2199 +### Code in `config/routes` + +When we add `draw :admin` in `config/routes.rb`, the application will try to +load the file located in `config/routes/admin.rb`, and also try to load the +file located in `ee/config/routes/admin.rb`. + +In EE, it should at least load one file, at most two files. If it cannot find +any files, an error will be raised. In CE, since we don't know if there will +be an EE route, it will not raise any errors even if it cannot find anything. + +This means if we want to extend a particular CE route file, just add the same +file located in `ee/config/routes`. If we want to add an EE only route, we +could still put `draw :ee_only` in both CE and EE, and add +`ee/config/routes/ee_only.rb` in EE, similar to `render_if_exists`. + ### Code in `app/controllers/` In controllers, the most common type of conflict is with `before_action` that diff --git a/lib/gitlab/patch/draw_route.rb b/lib/gitlab/patch/draw_route.rb new file mode 100644 index 00000000000..b00244a6e04 --- /dev/null +++ b/lib/gitlab/patch/draw_route.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +# We're patching `ActionDispatch::Routing::Mapper` in +# config/initializers/routing_draw.rb +module Gitlab + module Patch + module DrawRoute + RoutesNotFound = Class.new(StandardError) + + def draw(routes_name) + drawn_any = draw_ce(routes_name) | draw_ee(routes_name) + + drawn_any || raise(RoutesNotFound.new("Cannot find #{routes_name}")) + end + + def draw_ce(routes_name) + draw_route(route_path("config/routes/#{routes_name}.rb")) + end + + def draw_ee(_) + true + end + + def route_path(routes_name) + Rails.root.join(routes_name) + end + + def draw_route(path) + if File.exist?(path) + instance_eval(File.read(path)) + true + else + false + end + end + end + end +end diff --git a/spec/fast_spec_helper.rb b/spec/fast_spec_helper.rb index 134eb25e4b1..fe475e1f7a0 100644 --- a/spec/fast_spec_helper.rb +++ b/spec/fast_spec_helper.rb @@ -8,3 +8,4 @@ require_relative 'support/rspec' require 'active_support/all' ActiveSupport::Dependencies.autoload_paths << 'lib' +ActiveSupport::Dependencies.autoload_paths << 'ee/lib' diff --git a/spec/lib/gitlab/patch/draw_route_spec.rb b/spec/lib/gitlab/patch/draw_route_spec.rb new file mode 100644 index 00000000000..4009b903dc3 --- /dev/null +++ b/spec/lib/gitlab/patch/draw_route_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' + +describe Gitlab::Patch::DrawRoute do + subject do + Class.new do + include Gitlab::Patch::DrawRoute + + def route_path(route_name) + File.expand_path("../../../../#{route_name}", __dir__) + end + end.new + end + + before do + allow(subject).to receive(:instance_eval) + end + + it 'evaluates CE only route' do + subject.draw(:help) + + expect(subject).to have_received(:instance_eval) + .with(File.read(subject.route_path('config/routes/help.rb'))) + .once + + expect(subject).to have_received(:instance_eval) + .once + end +end