Fix a few places where autoloading would failstanhu/gitlab-ce-add-eager-load-lib

- Fix naming of API::CommitStatuses
- Ensure we use require_dependency instead of require
- Ensure the namespace is right in lib/api/api.rb, otherwise, we
  might require Grape::API::Helpers which defines the `#params` method.
  This is to avoid requiring a file multiple times and getting an "Already
  initialized constant" error.

Signed-off-by: Rémy Coutable <remy@rymai.me>

12 files changed, 199 insertions, 201 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 5c3c9efefdb..e79508e8efc 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -17,6 +17,7 @@ v 8.8.0 (unreleased)
   - Files over 5MB can only be viewed in their raw form, files over 1MB without highlighting !3718
   - Add support for supressing text diffs using .gitattributes on the default branch (Matt Oakes)
   - Added multiple colors for labels in dropdowns when dups happen.
+  - Add eager load paths to help prevent dependency load issues in Sidekiq workers
 
 v 8.7.2 (unreleased)
   - The "New Branch" button is now loaded asynchronously
@@ -60,8 +61,6 @@ v 8.7.0
   - Fix `signed_in_ip` being set to 127.0.0.1 when using a reverse proxy !3524
   - Improved Markdown rendering performance !3389
   - Make shared runners text in box configurable
-  - Add eager load paths to help prevent dependency load issues with Sidekiq workers (Stan Hu)
-  - Improved Markdown rendering performance !3389 (Yorick Peterse)
   - Don't attempt to look up an avatar in repo if repo directory does not exist (Stan Hu)
   - API: Ability to subscribe and unsubscribe from issues and merge requests (Robert Schilling)
   - Expose project badges in project settings
diff --git a/Gemfile b/Gemfile
index 25c13fda575..cfeab76752d 100644
--- a/Gemfile
+++ b/Gemfile
@@ -267,16 +267,6 @@ group :development, :test do
   gem 'awesome_print', '~> 1.2.0', require: false
   gem 'fuubar', '~> 2.0.0'
 
-  gem 'database_cleaner',   '~> 1.4.0'
-  gem 'factory_girl_rails', '~> 4.6.0'
-  gem 'rspec-rails',        '~> 3.3.0'
-  gem 'rspec-retry'
-  gem 'spinach-rails',      '~> 0.2.1'
-  gem 'spinach-rerun-reporter', '~> 0.0.2'
-
-  # Prevent occasions where minitest is not bundled in packaged versions of ruby (see #3826)
-  gem 'minitest', '~> 5.7.0'
-
   # Generate Fake data
   gem 'ffaker', '~> 2.0.0'
 
@@ -304,6 +294,16 @@ group :development, :test do
 end
 
 group :test do
+  gem 'database_cleaner',   '~> 1.4.0'
+  gem 'factory_girl_rails', '~> 4.6.0'
+  gem 'rspec-rails',        '~> 3.3.0'
+  gem 'rspec-retry'
+  gem 'spinach-rails',      '~> 0.2.1'
+  gem 'spinach-rerun-reporter', '~> 0.0.2'
+
+  # Prevent occasions where minitest is not bundled in packaged versions of ruby (see #3826)
+  gem 'minitest', '~> 5.7.0'
+
   gem 'shoulda-matchers', '~> 2.8.0', require: false
   gem 'email_spec', '~> 1.6.0'
   gem 'webmock', '~> 1.21.0'
diff --git a/app/models/repository.rb b/app/models/repository.rb
index b4319297e43..80d9fb37da5 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -81,7 +81,7 @@ class Repository
   def commit(id = 'HEAD')
     return nil unless exists?
     commit = Gitlab::Git::Commit.find(raw_repository, id)
-    commit = Commit.new(commit, @project) if commit
+    commit = ::Commit.new(commit, @project) if commit
     commit
   rescue Rugged::OdbError
     nil
diff --git a/config/application.rb b/config/application.rb
index b33e57f5fcd..cba80f38f1f 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -1,13 +1,13 @@
 require File.expand_path('../boot', __FILE__)
 
 require 'rails/all'
-require 'devise'
-I18n.config.enforce_available_locales = false
+
 Bundler.require(:default, Rails.env)
-require_relative '../lib/gitlab/redis'
 
 module Gitlab
   class Application < Rails::Application
+    require_dependency Rails.root.join('lib/gitlab/redis')
+
     # Settings in config/environments/* take precedence over those specified here.
     # Application configuration should go into files in config/initializers
     # -- all .rb files in that directory are automatically loaded.
@@ -21,10 +21,10 @@ module Gitlab
     # This is a nice reference article on autoloading/eager loading:
     # http://blog.arkency.com/2014/11/dont-forget-about-eager-load-when-extending-autoload
     config.eager_load_paths.push(*%W(#{config.root}/lib
+                                     #{config.root}/app/models/ci
                                      #{config.root}/app/models/hooks
-                                     #{config.root}/app/models/concerns
-                                     #{config.root}/app/models/project_services
-                                     #{config.root}/app/models/members))
+                                     #{config.root}/app/models/members
+                                     #{config.root}/app/models/project_services))
 
     # Only load the plugins named here, in the order given (default is alphabetical).
     # :all can be used as a placeholder for all plugins not explicitly named.
diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb
index 8db2c05fe45..23c8cea038a 100644
--- a/config/initializers/1_settings.rb
+++ b/config/initializers/1_settings.rb
@@ -1,4 +1,4 @@
-require 'gitlab' # Load lib/gitlab.rb as soon as possible
+require_dependency Rails.root.join('lib/gitlab') # Load Gitlab as soon as possible
 
 class Settings < Settingslogic
   source ENV.fetch('GITLAB_CONFIG') { "#{Rails.root}/config/gitlab.yml" }
diff --git a/config/initializers/5_backend.rb b/config/initializers/5_backend.rb
index 80d641d73a3..e026151a032 100644
--- a/config/initializers/5_backend.rb
+++ b/config/initializers/5_backend.rb
@@ -1,11 +1,11 @@
 # GIT over HTTP
-require Rails.root.join("lib", "gitlab", "backend", "grack_auth")
+require_dependency Rails.root.join('lib/gitlab/backend/grack_auth')
 
 # GIT over SSH
-require Rails.root.join("lib", "gitlab", "backend", "shell")
+require_dependency Rails.root.join('lib/gitlab/backend/shell')
 
 # GitLab shell adapter
-require Rails.root.join("lib", "gitlab", "backend", "shell_adapter")
+require_dependency Rails.root.join('lib/gitlab/backend/shell_adapter')
 
 required_version = Gitlab::VersionInfo.parse(Gitlab::Shell.version_required)
 current_version = Gitlab::VersionInfo.parse(Gitlab::Shell.new.version)
diff --git a/lib/api/api.rb b/lib/api/api.rb
index cc1004f8005..5fd9c30cb42 100644
--- a/lib/api/api.rb
+++ b/lib/api/api.rb
@@ -1,5 +1,3 @@
-Dir["#{Rails.root}/lib/api/*.rb"].each {|file| require file}
-
 module API
   class API < Grape::API
     include APIGuard
@@ -25,38 +23,39 @@ module API
     format :json
     content_type :txt, "text/plain"
 
-    helpers Helpers
-
-    mount Groups
-    mount GroupMembers
-    mount Users
-    mount Projects
-    mount Repositories
-    mount Issues
-    mount Milestones
-    mount Session
-    mount MergeRequests
-    mount Notes
-    mount Internal
-    mount SystemHooks
-    mount ProjectSnippets
-    mount ProjectMembers
-    mount DeployKeys
-    mount ProjectHooks
-    mount Services
-    mount Files
-    mount Commits
-    mount CommitStatus
-    mount Namespaces
-    mount Branches
-    mount Labels
-    mount Settings
-    mount Keys
-    mount Tags
-    mount Triggers
-    mount Builds
-    mount Variables
-    mount Runners
-    mount Licenses
+    # Ensure the namespace is right, otherwise we might load Grape::API::Helpers
+    helpers ::API::Helpers
+
+    mount ::API::Groups
+    mount ::API::GroupMembers
+    mount ::API::Users
+    mount ::API::Projects
+    mount ::API::Repositories
+    mount ::API::Issues
+    mount ::API::Milestones
+    mount ::API::Session
+    mount ::API::MergeRequests
+    mount ::API::Notes
+    mount ::API::Internal
+    mount ::API::SystemHooks
+    mount ::API::ProjectSnippets
+    mount ::API::ProjectMembers
+    mount ::API::DeployKeys
+    mount ::API::ProjectHooks
+    mount ::API::Services
+    mount ::API::Files
+    mount ::API::Commits
+    mount ::API::CommitStatuses
+    mount ::API::Namespaces
+    mount ::API::Branches
+    mount ::API::Labels
+    mount ::API::Settings
+    mount ::API::Keys
+    mount ::API::Tags
+    mount ::API::Triggers
+    mount ::API::Builds
+    mount ::API::Variables
+    mount ::API::Runners
+    mount ::API::Licenses
   end
 end
diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb
index b9994fcefda..a7123d61102 100644
--- a/lib/api/api_guard.rb
+++ b/lib/api/api_guard.rb
@@ -2,171 +2,173 @@
 
 require 'rack/oauth2'
 
-module APIGuard
-  extend ActiveSupport::Concern
+module API
+  module APIGuard
+    extend ActiveSupport::Concern
 
-  included do |base|
-    # OAuth2 Resource Server Authentication
-    use Rack::OAuth2::Server::Resource::Bearer, 'The API' do |request|
-      # The authenticator only fetches the raw token string
+    included do |base|
+      # OAuth2 Resource Server Authentication
+      use Rack::OAuth2::Server::Resource::Bearer, 'The API' do |request|
+        # The authenticator only fetches the raw token string
 
-      # Must yield access token to store it in the env
-      request.access_token
-    end
+        # Must yield access token to store it in the env
+        request.access_token
+      end
 
-    helpers HelperMethods
+      helpers HelperMethods
 
-    install_error_responders(base)
-  end
+      install_error_responders(base)
+    end
 
-  # Helper Methods for Grape Endpoint
-  module HelperMethods
-    # Invokes the doorkeeper guard.
-    #
-    # If token is presented and valid, then it sets @current_user.
-    #
-    # If the token does not have sufficient scopes to cover the requred scopes,
-    # then it raises InsufficientScopeError.
-    #
-    # If the token is expired, then it raises ExpiredError.
-    #
-    # If the token is revoked, then it raises RevokedError.
-    #
-    # If the token is not found (nil), then it raises TokenNotFoundError.
-    #
-    # Arguments:
-    #
-    #   scopes: (optional) scopes required for this guard.
-    #           Defaults to empty array.
-    #
-    def doorkeeper_guard!(scopes: [])
-      if (access_token = find_access_token).nil?
-        raise TokenNotFoundError
-
-      else
-        case validate_access_token(access_token, scopes)
-        when Oauth2::AccessTokenValidationService::INSUFFICIENT_SCOPE
-          raise InsufficientScopeError.new(scopes)
-        when Oauth2::AccessTokenValidationService::EXPIRED
-          raise ExpiredError
-        when Oauth2::AccessTokenValidationService::REVOKED
-          raise RevokedError
-        when Oauth2::AccessTokenValidationService::VALID
-          @current_user = User.find(access_token.resource_owner_id)
+    # Helper Methods for Grape Endpoint
+    module HelperMethods
+      # Invokes the doorkeeper guard.
+      #
+      # If token is presented and valid, then it sets @current_user.
+      #
+      # If the token does not have sufficient scopes to cover the requred scopes,
+      # then it raises InsufficientScopeError.
+      #
+      # If the token is expired, then it raises ExpiredError.
+      #
+      # If the token is revoked, then it raises RevokedError.
+      #
+      # If the token is not found (nil), then it raises TokenNotFoundError.
+      #
+      # Arguments:
+      #
+      #   scopes: (optional) scopes required for this guard.
+      #           Defaults to empty array.
+      #
+      def doorkeeper_guard!(scopes: [])
+        if (access_token = find_access_token).nil?
+          raise TokenNotFoundError
+
+        else
+          case validate_access_token(access_token, scopes)
+          when Oauth2::AccessTokenValidationService::INSUFFICIENT_SCOPE
+            raise InsufficientScopeError.new(scopes)
+          when Oauth2::AccessTokenValidationService::EXPIRED
+            raise ExpiredError
+          when Oauth2::AccessTokenValidationService::REVOKED
+            raise RevokedError
+          when Oauth2::AccessTokenValidationService::VALID
+            @current_user = User.find(access_token.resource_owner_id)
+          end
         end
       end
-    end
 
-    def doorkeeper_guard(scopes: [])
-      if access_token = find_access_token
-        case validate_access_token(access_token, scopes)
-        when Oauth2::AccessTokenValidationService::INSUFFICIENT_SCOPE
-          raise InsufficientScopeError.new(scopes)
+      def doorkeeper_guard(scopes: [])
+        if access_token = find_access_token
+          case validate_access_token(access_token, scopes)
+          when Oauth2::AccessTokenValidationService::INSUFFICIENT_SCOPE
+            raise InsufficientScopeError.new(scopes)
 
-        when Oauth2::AccessTokenValidationService::EXPIRED
-          raise ExpiredError
+          when Oauth2::AccessTokenValidationService::EXPIRED
+            raise ExpiredError
 
-        when Oauth2::AccessTokenValidationService::REVOKED
-          raise RevokedError
+          when Oauth2::AccessTokenValidationService::REVOKED
+            raise RevokedError
 
-        when Oauth2::AccessTokenValidationService::VALID
-          @current_user = User.find(access_token.resource_owner_id)
+          when Oauth2::AccessTokenValidationService::VALID
+            @current_user = User.find(access_token.resource_owner_id)
+          end
         end
       end
-    end
-
-    def current_user
-      @current_user
-    end
 
-    private
-    def find_access_token
-      @access_token ||= Doorkeeper.authenticate(doorkeeper_request, Doorkeeper.configuration.access_token_methods)
-    end
+      def current_user
+        @current_user
+      end
 
-    def doorkeeper_request
-      @doorkeeper_request ||= ActionDispatch::Request.new(env)
-    end
+      private
+      def find_access_token
+        @access_token ||= Doorkeeper.authenticate(doorkeeper_request, Doorkeeper.configuration.access_token_methods)
+      end
 
-    def validate_access_token(access_token, scopes)
-      Oauth2::AccessTokenValidationService.validate(access_token, scopes: scopes)
-    end
-  end
+      def doorkeeper_request
+        @doorkeeper_request ||= ActionDispatch::Request.new(env)
+      end
 
-  module ClassMethods
-    # Installs the doorkeeper guard on the whole Grape API endpoint.
-    #
-    # Arguments:
-    #
-    #   scopes: (optional) scopes required for this guard.
-    #           Defaults to empty array.
-    #
-    def guard_all!(scopes: [])
-      before do
-        guard! scopes: scopes
+      def validate_access_token(access_token, scopes)
+        Oauth2::AccessTokenValidationService.validate(access_token, scopes: scopes)
       end
     end
 
-    private
-    def install_error_responders(base)
-      error_classes = [ MissingTokenError, TokenNotFoundError,
-                        ExpiredError, RevokedError, InsufficientScopeError]
+    module ClassMethods
+      # Installs the doorkeeper guard on the whole Grape API endpoint.
+      #
+      # Arguments:
+      #
+      #   scopes: (optional) scopes required for this guard.
+      #           Defaults to empty array.
+      #
+      def guard_all!(scopes: [])
+        before do
+          guard! scopes: scopes
+        end
+      end
 
-      base.send :rescue_from, *error_classes, oauth2_bearer_token_error_handler
-    end
+      private
+      def install_error_responders(base)
+        error_classes = [ MissingTokenError, TokenNotFoundError,
+                          ExpiredError, RevokedError, InsufficientScopeError]
 
-    def oauth2_bearer_token_error_handler
-      Proc.new do |e|
-        response =
-          case e
-          when MissingTokenError
-            Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new
-
-          when TokenNotFoundError
-            Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new(
-              :invalid_token,
-              "Bad Access Token.")
-
-          when ExpiredError
-            Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new(
-              :invalid_token,
-              "Token is expired. You can either do re-authorization or token refresh.")
-
-          when RevokedError
-            Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new(
-              :invalid_token,
-              "Token was revoked. You have to re-authorize from the user.")
-
-          when InsufficientScopeError
-            # FIXME: ForbiddenError (inherited from Bearer::Forbidden of Rack::Oauth2)
-            # does not include WWW-Authenticate header, which breaks the standard.
-            Rack::OAuth2::Server::Resource::Bearer::Forbidden.new(
-              :insufficient_scope,
-              Rack::OAuth2::Server::Resource::ErrorMethods::DEFAULT_DESCRIPTION[:insufficient_scope],
-              { scope: e.scopes })
-          end
+        base.send :rescue_from, *error_classes, oauth2_bearer_token_error_handler
+      end
 
-        response.finish
+      def oauth2_bearer_token_error_handler
+        Proc.new do |e|
+          response =
+            case e
+            when MissingTokenError
+              Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new
+
+            when TokenNotFoundError
+              Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new(
+                :invalid_token,
+                "Bad Access Token.")
+
+            when ExpiredError
+              Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new(
+                :invalid_token,
+                "Token is expired. You can either do re-authorization or token refresh.")
+
+            when RevokedError
+              Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new(
+                :invalid_token,
+                "Token was revoked. You have to re-authorize from the user.")
+
+            when InsufficientScopeError
+              # FIXME: ForbiddenError (inherited from Bearer::Forbidden of Rack::Oauth2)
+              # does not include WWW-Authenticate header, which breaks the standard.
+              Rack::OAuth2::Server::Resource::Bearer::Forbidden.new(
+                :insufficient_scope,
+                Rack::OAuth2::Server::Resource::ErrorMethods::DEFAULT_DESCRIPTION[:insufficient_scope],
+                { scope: e.scopes })
+            end
+
+          response.finish
+        end
       end
     end
-  end
 
-  #
-  # Exceptions
-  #
+    #
+    # Exceptions
+    #
 
-  class MissingTokenError < StandardError; end
+    class MissingTokenError < StandardError; end
 
-  class TokenNotFoundError < StandardError; end
+    class TokenNotFoundError < StandardError; end
 
-  class ExpiredError < StandardError; end
+    class ExpiredError < StandardError; end
 
-  class RevokedError < StandardError; end
+    class RevokedError < StandardError; end
 
-  class InsufficientScopeError < StandardError
-    attr_reader :scopes
-    def initialize(scopes)
-      @scopes = scopes
+    class InsufficientScopeError < StandardError
+      attr_reader :scopes
+      def initialize(scopes)
+        @scopes = scopes
+      end
     end
   end
 end
diff --git a/lib/api/commit_statuses.rb b/lib/api/commit_statuses.rb
index 7388ed2f4ea..9bcd33ff19e 100644
--- a/lib/api/commit_statuses.rb
+++ b/lib/api/commit_statuses.rb
@@ -2,7 +2,7 @@ require 'mime/types'
 
 module API
   # Project commit statuses API
-  class CommitStatus < Grape::API
+  class CommitStatuses < Grape::API
     resource :projects do
       before { authenticate! }
 
diff --git a/lib/ci/api/api.rb b/lib/ci/api/api.rb
index 353c4ddebf8..17bb99a2ae5 100644
--- a/lib/ci/api/api.rb
+++ b/lib/ci/api/api.rb
@@ -1,9 +1,7 @@
-Dir["#{Rails.root}/lib/ci/api/*.rb"].each {|file| require file}
-
 module Ci
   module API
     class API < Grape::API
-      include APIGuard
+      include ::API::APIGuard
       version 'v1', using: :path
 
       rescue_from ActiveRecord::RecordNotFound do
@@ -31,9 +29,9 @@ module Ci
       helpers ::API::Helpers
       helpers Gitlab::CurrentSettings
 
-      mount Builds
-      mount Runners
-      mount Triggers
+      mount ::Ci::API::Builds
+      mount ::Ci::API::Runners
+      mount ::Ci::API::Triggers
     end
   end
 end
diff --git a/lib/gitlab.rb b/lib/gitlab.rb
index 7479e729db1..37f4c34054f 100644
--- a/lib/gitlab.rb
+++ b/lib/gitlab.rb
@@ -1,4 +1,4 @@
-require 'gitlab/git'
+require_dependency 'gitlab/git'
 
 module Gitlab
   def self.com?
diff --git a/spec/requests/api/commit_status_spec.rb b/spec/requests/api/commit_statuses_spec.rb
index f3785b19362..633927c8c3e 100644
--- a/spec/requests/api/commit_status_spec.rb
+++ b/spec/requests/api/commit_statuses_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe API::CommitStatus, api: true do
+describe API::CommitStatuses, api: true do
   include ApiHelpers
 
   let!(:project) { create(:project) }