Merge branch '7864-ee-routes' into 'master'

CE: Put EE routes in EE files under EE directories

See merge request gitlab-org/gitlab-ce!22376

.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'

config/initializers/routing_draw.rb

@@ -1,7 +1,3 @@
 # Adds draw method into Rails routing
-# It allows us to keep routing splitted into files
+# It allows us to keep routing split into files
-class ActionDispatch::Routing::Mapper
+ActionDispatch::Routing::Mapper.prepend Gitlab::Patch::DrawRoute
-  def draw(routes_name)
-    instance_eval(File.read(Rails.root.join("config/routes/#{routes_name}.rb")))
-  end
-end

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

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

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

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

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

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'

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