Merge pull request #1627 from doorkeeper-gem/fixes/lazy-load-associat…

…ions

Lazy evaluate Doorkeeper config when loading files and executing initializers

CHANGELOG.md

@@ -14,6 +14,7 @@ User-visible changes worth mentioning.
 - [#1626] Remove deprecated `active_record_options` config option.
 - [#1631] Fix regression with redirect behavior after token lookup optimizations (redirect to app URI when found).
 - [#1630] Special case unique index creation for refresh_token on SQL Server.
+- [#1627] Lazy evaluate Doorkeeper config when loading files and executing initializers.
 
 ## 5.6.2
 

bin/console

@@ -32,8 +32,5 @@ ActiveRecord::Base.establish_connection(
 # Load database schema
 load File.expand_path("../spec/dummy/db/schema.rb", __dir__)
 
-# Call engine #to_prepare block
-Doorkeeper.setup
-
 require "irb"
 IRB.start(__FILE__)

lib/doorkeeper.rb

@@ -90,6 +90,7 @@ module Models
     autoload :Expirable, "doorkeeper/models/concerns/expirable"
     autoload :ExpirationTimeSqlMath, "doorkeeper/models/concerns/expiration_time_sql_math"
     autoload :Orderable, "doorkeeper/models/concerns/orderable"
+    autoload :PolymorphicResourceOwner, "doorkeeper/models/concerns/polymorphic_resource_owner"
     autoload :Scopes, "doorkeeper/models/concerns/scopes"
     autoload :Reusable, "doorkeeper/models/concerns/reusable"
     autoload :ResourceOwnerable, "doorkeeper/models/concerns/resource_ownerable"
@@ -113,11 +114,77 @@ module SecretStoring
     autoload :BCrypt, "doorkeeper/secret_storing/bcrypt"
   end
 
-  def self.authenticate(request, methods = Doorkeeper.config.access_token_methods)
-    OAuth::Token.authenticate(request, *methods)
-  end
+  class << self
+    attr_reader :orm_adapter
+
+    def configure(&block)
+      @config = Config::Builder.new(&block).build
+      setup
+      @config
+    end
+
+    # @return [Doorkeeper::Config] configuration instance
+    #
+    def configuration
+      @config || configure
+    end
+
+    def configured?
+      !@config.nil?
+    end
+
+    alias config configuration
+
+    def setup
+      setup_orm_adapter
+      run_orm_hooks
+      config.clear_cache!
+
+      # Deprecated, will be removed soon
+      unless configuration.orm == :active_record
+        setup_orm_models
+        setup_application_owner
+      end
+    end
+
+    def setup_orm_adapter
+      @orm_adapter = "doorkeeper/orm/#{configuration.orm}".classify.constantize
+    rescue NameError => e
+      raise e, "ORM adapter not found (#{configuration.orm})", <<-ERROR_MSG.strip_heredoc
+        [DOORKEEPER] ORM adapter not found (#{configuration.orm}), or there was an error
+        trying to load it.
 
-  def self.gem_version
-    ::Gem::Version.new(::Doorkeeper::VERSION::STRING)
+        You probably need to add the related gem for this adapter to work with
+        doorkeeper.
+      ERROR_MSG
+    end
+
+    def run_orm_hooks
+      if @orm_adapter.respond_to?(:run_hooks)
+        @orm_adapter.run_hooks
+      else
+        ::Kernel.warn <<~MSG.strip_heredoc
+          [DOORKEEPER] ORM "#{configuration.orm}" should move all it's setup logic under `#run_hooks` method for
+          the #{@orm_adapter.name}. Later versions of Doorkeeper will no longer support `setup_orm_models` and
+          `setup_application_owner` API.
+        MSG
+      end
+    end
+
+    def setup_orm_models
+      @orm_adapter.initialize_models!
+    end
+
+    def setup_application_owner
+      @orm_adapter.initialize_application_owner!
+    end
+
+    def authenticate(request, methods = Doorkeeper.config.access_token_methods)
+      OAuth::Token.authenticate(request, *methods)
+    end
+
+    def gem_version
+      ::Gem::Version.new(::Doorkeeper::VERSION::STRING)
+    end
   end
 end

lib/doorkeeper/config.rb

@@ -5,81 +5,11 @@
 require "doorkeeper/config/validations"
 
 module Doorkeeper
-  # Defines a MissingConfiguration error for a missing Doorkeeper configuration
-  #
-  class MissingConfiguration < StandardError
-    def initialize
-      super("Configuration for doorkeeper missing. Do you have doorkeeper initializer?")
-    end
-  end
-
   # Doorkeeper option DSL could be reused in extensions to build their own
   # configurations. To use the Option DSL gems need to define `builder_class` method
   # that returns configuration Builder class. This exception raises when they don't
   # define it.
   #
-  class MissingConfigurationBuilderClass < StandardError; end
-
-  class << self
-    attr_reader :orm_adapter
-
-    def configure(&block)
-      @config = Config::Builder.new(&block).build
-    end
-
-    # @return [Doorkeeper::Config] configuration instance
-    #
-    def configuration
-      @config || (raise MissingConfiguration)
-    end
-
-    alias config configuration
-
-    def setup
-      setup_orm_adapter
-      run_orm_hooks
-      config.clear_cache!
-
-      # Deprecated, will be removed soon
-      unless configuration.orm == :active_record
-        setup_orm_models
-        setup_application_owner
-      end
-    end
-
-    def setup_orm_adapter
-      @orm_adapter = "doorkeeper/orm/#{configuration.orm}".classify.constantize
-    rescue NameError => e
-      raise e, "ORM adapter not found (#{configuration.orm})", <<-ERROR_MSG.strip_heredoc
-        [DOORKEEPER] ORM adapter not found (#{configuration.orm}), or there was an error
-        trying to load it.
-
-        You probably need to add the related gem for this adapter to work with
-        doorkeeper.
-      ERROR_MSG
-    end
-
-    def run_orm_hooks
-      if @orm_adapter.respond_to?(:run_hooks)
-        @orm_adapter.run_hooks
-      else
-        ::Kernel.warn <<~MSG.strip_heredoc
-          [DOORKEEPER] ORM "#{configuration.orm}" should move all it's setup logic under `#run_hooks` method for
-          the #{@orm_adapter.name}. Later versions of Doorkeeper will no longer support `setup_orm_models` and
-          `setup_application_owner` API.
-        MSG
-      end
-    end
-
-    def setup_orm_models
-      @orm_adapter.initialize_models!
-    end
-
-    def setup_application_owner
-      @orm_adapter.initialize_application_owner!
-    end
-  end
-
   class Config
     # Default Doorkeeper configuration builder
     class Builder < AbstractBuilder

lib/doorkeeper/config/abstract_builder.rb

@@ -12,7 +12,7 @@ class AbstractBuilder
       #
       def initialize(config = Config.new, &block)
         @config = config
-        instance_eval(&block)
+        instance_eval(&block) if block_given?
       end
 
       # Builds and validates configuration.

lib/doorkeeper/engine.rb

@@ -2,10 +2,12 @@
 
 module Doorkeeper
   class Engine < Rails::Engine
-    initializer "doorkeeper.params.filter" do |app|
-      parameters = %w[client_secret authentication_token access_token refresh_token]
-      parameters << "code" if Doorkeeper.config.grant_flows.include?("authorization_code")
-      app.config.filter_parameters << /^(#{Regexp.union(parameters)})$/
+    initializer "doorkeeper.params.filter", after: :load_config_initializers do |app|
+      if Doorkeeper.configured?
+        parameters = %w[client_secret authentication_token access_token refresh_token]
+        parameters << "code" if Doorkeeper.config.grant_flows.include?("authorization_code")
+        app.config.filter_parameters << /^(#{Regexp.union(parameters)})$/
+      end
     end
 
     initializer "doorkeeper.routes" do

lib/doorkeeper/models/concerns/polymorphic_resource_owner.rb

@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+module Doorkeeper
+  module Models
+    module PolymorphicResourceOwner
+      module ForAccessGrant
+        extend ActiveSupport::Concern
+
+        included do
+          if Doorkeeper.config.polymorphic_resource_owner?
+            belongs_to :resource_owner, polymorphic: true, optional: false
+          else
+            validates :resource_owner_id, presence: true
+          end
+        end
+      end
+
+      module ForAccessToken
+        extend ActiveSupport::Concern
+
+        included do
+          if Doorkeeper.config.polymorphic_resource_owner?
+            belongs_to :resource_owner, polymorphic: true, optional: true
+          end
+        end
+      end
+    end
+  end
+end
+

lib/doorkeeper/orm/active_record.rb

@@ -29,7 +29,16 @@ module Mixins
       end
 
       def self.run_hooks
-        # nop
+        initialize_configured_associations
+      end
+
+      def self.initialize_configured_associations
+        if Doorkeeper.config.enable_application_owner?
+          Doorkeeper.config.application_model.include ::Doorkeeper::Models::Ownership
+        end
+
+        Doorkeeper.config.access_grant_model.include ::Doorkeeper::Models::PolymorphicResourceOwner::ForAccessGrant
+        Doorkeeper.config.access_token_model.include ::Doorkeeper::Models::PolymorphicResourceOwner::ForAccessToken
       end
     end
   end

lib/doorkeeper/orm/active_record/mixins/access_grant.rb

@@ -14,12 +14,6 @@ module AccessGrant
                                optional: true,
                                inverse_of: :access_grants
 
-      if Doorkeeper.config.polymorphic_resource_owner?
-        belongs_to :resource_owner, polymorphic: true, optional: false
-      else
-        validates :resource_owner_id, presence: true
-      end
-
       validates :application_id,
                 :token,
                 :expires_in,

lib/doorkeeper/orm/active_record/mixins/access_token.rb

@@ -14,10 +14,6 @@ module AccessToken
                                inverse_of: :access_tokens,
                                optional: true
 
-      if Doorkeeper.config.polymorphic_resource_owner?
-        belongs_to :resource_owner, polymorphic: true, optional: true
-      end
-
       validates :token, presence: true, uniqueness: { case_sensitive: true }
       validates :refresh_token, uniqueness: { case_sensitive: true }, if: :use_refresh_token?
 

lib/doorkeeper/orm/active_record/mixins/application.rb

@@ -10,10 +10,6 @@ module Application
 
       include ::Doorkeeper::ApplicationMixin
 
-      if Doorkeeper.config.enable_application_owner?
-        include ::Doorkeeper::Models::Ownership
-      end
-
       has_many :access_grants,
                foreign_key: :application_id,
                dependent: :delete_all,

lib/doorkeeper/rails/routes.rb

@@ -36,7 +36,7 @@ def generate_routes!(options)
           map_route(:authorizations, :authorization_routes)
           map_route(:tokens, :token_routes)
           map_route(:tokens, :revoke_routes)
-          map_route(:tokens, :introspect_routes) unless Doorkeeper.config.allow_token_introspection.is_a?(FalseClass)
+          map_route(:tokens, :introspect_routes) if introspection_routes?
           map_route(:applications, :application_routes)
           map_route(:authorized_applications, :authorized_applications_routes)
           map_route(:token_info, :token_info_routes)
@@ -100,6 +100,11 @@ def authorized_applications_routes(mapping)
       def native_authorization_code_route
         Doorkeeper.configuration.native_authorization_code_route
       end
+
+      def introspection_routes?
+        Doorkeeper.configured? &&
+          !Doorkeeper.config.allow_token_introspection.is_a?(FalseClass)
+      end
     end
   end
 end

spec/factories.rb

@@ -22,8 +22,6 @@
   factory :application, class: "Doorkeeper::Application" do
     sequence(:name) { |n| "Application #{n}" }
     redirect_uri { "https://app.com/callback" }
-
-    factory :application_with_owner, class: "ApplicationWithOwner"
   end
 
   # do not name this factory :user, otherwise it will conflict with factories