Merge pull request doorkeeper-gem#734 from reidab/request-strategy-re…

…factor

Refactor request strategy classes

lib/doorkeeper/request.rb

@@ -9,22 +9,32 @@ module Doorkeeper
   module Request
     module_function
 
-    def authorization_strategy(strategy)
-      get_strategy strategy, Doorkeeper.configuration.authorization_response_types
+    def authorization_strategy(response_type)
+      get_strategy response_type, authorization_response_types
     rescue NameError
       raise Errors::InvalidAuthorizationStrategy
     end
 
-    def token_strategy(strategy)
-      get_strategy strategy, Doorkeeper.configuration.token_grant_types
+    def token_strategy(grant_type)
+      get_strategy grant_type, token_grant_types
     rescue NameError
       raise Errors::InvalidTokenStrategy
     end
 
-    def get_strategy(strategy, available)
-      fail Errors::MissingRequestStrategy unless strategy.present?
-      fail NameError unless available.include?(strategy.to_s)
-      "Doorkeeper::Request::#{strategy.to_s.camelize}".constantize
+    def get_strategy(grant_or_request_type, available)
+      fail Errors::MissingRequestStrategy unless grant_or_request_type.present?
+      fail NameError unless available.include?(grant_or_request_type.to_s)
+      "Doorkeeper::Request::#{grant_or_request_type.to_s.camelize}".constantize
     end
+
+    def authorization_response_types
+      Doorkeeper.configuration.authorization_response_types
+    end
+    private_class_method :authorization_response_types
+
+    def token_grant_types
+      Doorkeeper.configuration.token_grant_types
+    end
+    private_class_method :token_grant_types
   end
 end

lib/doorkeeper/request/authorization_code.rb

@@ -1,22 +1,17 @@
+require 'doorkeeper/request/strategy'
+
 module Doorkeeper
   module Request
-    class AuthorizationCode
-      def self.build(server)
-        new(server.grant, server.client, server)
-      end
-
-      attr_accessor :grant, :client, :server
-
-      def initialize(grant, client, server)
-        @grant, @client, @server = grant, client, server
-      end
+    class AuthorizationCode < Strategy
+      delegate :grant, :client, :parameters, to: :server
 
       def request
-        @request ||= OAuth::AuthorizationCodeRequest.new(Doorkeeper.configuration, grant, client, server.parameters)
-      end
-
-      def authorize
-        request.authorize
+        @request ||= OAuth::AuthorizationCodeRequest.new(
+          Doorkeeper.configuration,
+          grant,
+          client,
+          parameters
+        )
       end
     end
   end

lib/doorkeeper/request/client_credentials.rb

@@ -1,22 +1,16 @@
+require 'doorkeeper/request/strategy'
+
 module Doorkeeper
   module Request
-    class ClientCredentials
-      def self.build(server)
-        new(server.client, server)
-      end
-
-      attr_accessor :client, :server
-
-      def initialize(client, server)
-        @client, @server = client, server
-      end
+    class ClientCredentials < Strategy
+      delegate :client, :parameters, to: :server
 
       def request
-        @request ||= OAuth::ClientCredentialsRequest.new(Doorkeeper.configuration, client, server.parameters)
-      end
-
-      def authorize
-        request.authorize
+        @request ||= OAuth::ClientCredentialsRequest.new(
+          Doorkeeper.configuration,
+          client,
+          parameters
+        )
       end
     end
   end

lib/doorkeeper/request/code.rb

@@ -1,22 +1,16 @@
+require 'doorkeeper/request/strategy'
+
 module Doorkeeper
   module Request
-    class Code
-      def self.build(server)
-        new(server.context.send(:pre_auth), server)
-      end
-
-      attr_accessor :pre_auth, :server
+    class Code < Strategy
+      delegate :current_resource_owner, to: :server
 
-      def initialize(pre_auth, server)
-        @pre_auth, @server = pre_auth, server
+      def pre_auth
+        server.context.send(:pre_auth)
       end
 
       def request
-        @request ||= OAuth::CodeRequest.new(pre_auth, server.current_resource_owner)
-      end
-
-      def authorize
-        request.authorize
+        @request ||= OAuth::CodeRequest.new(pre_auth, current_resource_owner)
       end
     end
   end

lib/doorkeeper/request/password.rb

@@ -1,22 +1,17 @@
+require 'doorkeeper/request/strategy'
+
 module Doorkeeper
   module Request
-    class Password
-      def self.build(server)
-        new(server.credentials, server.resource_owner, server)
-      end
-
-      attr_accessor :credentials, :resource_owner, :server
-
-      def initialize(credentials, resource_owner, server)
-        @credentials, @resource_owner, @server = credentials, resource_owner, server
-      end
+    class Password < Strategy
+      delegate :credentials, :resource_owner, :parameters, to: :server
 
       def request
-        @request ||= OAuth::PasswordAccessTokenRequest.new(Doorkeeper.configuration, credentials, resource_owner, server.parameters)
-      end
-
-      def authorize
-        request.authorize
+        @request ||= OAuth::PasswordAccessTokenRequest.new(
+          Doorkeeper.configuration,
+          credentials,
+          resource_owner,
+          parameters
+        )
       end
     end
   end

lib/doorkeeper/request/refresh_token.rb

@@ -1,22 +1,20 @@
+require 'doorkeeper/request/strategy'
+
 module Doorkeeper
   module Request
-    class RefreshToken
-      def self.build(server)
-        new(server.current_refresh_token, server.credentials, server)
-      end
-
-      attr_accessor :refresh_token, :credentials, :server
+    class RefreshToken < Strategy
+      delegate :credentials, :parameters, to: :server
 
-      def initialize(refresh_token, credentials, server)
-        @refresh_token, @credentials, @server = refresh_token, credentials, server
+      def refresh_token
+        server.current_refresh_token
       end
 
       def request
-        @request ||= OAuth::RefreshTokenRequest.new(Doorkeeper.configuration, refresh_token, credentials, server.parameters)
-      end
-
-      def authorize
-        request.authorize
+        @request ||= OAuth::RefreshTokenRequest.new(
+          Doorkeeper.configuration,
+          refresh_token, credentials,
+          parameters
+        )
       end
     end
   end

lib/doorkeeper/request/strategy.rb

@@ -0,0 +1,17 @@
+module Doorkeeper
+  module Request
+    class Strategy
+      attr_accessor :server
+
+      delegate :authorize, to: :request
+
+      def initialize(server)
+        self.server = server
+      end
+
+      def request
+        raise NotImplementedError, "request strategies must define #request"
+      end
+    end
+  end
+end

lib/doorkeeper/request/token.rb

@@ -1,22 +1,16 @@
+require 'doorkeeper/request/strategy'
+
 module Doorkeeper
   module Request
-    class Token
-      def self.build(server)
-        new(server.context.send(:pre_auth), server)
-      end
-
-      attr_accessor :pre_auth, :server
+    class Token < Strategy
+      delegate :current_resource_owner, to: :server
 
-      def initialize(pre_auth, server)
-        @pre_auth, @server = pre_auth, server
+      def pre_auth
+        server.context.send(:pre_auth)
       end
 
       def request
-        @request ||= OAuth::TokenRequest.new(pre_auth, server.current_resource_owner)
-      end
-
-      def authorize
-        request.authorize
+        @request ||= OAuth::TokenRequest.new(pre_auth, current_resource_owner)
       end
     end
   end

lib/doorkeeper/server.rb

@@ -8,12 +8,12 @@ def initialize(context = nil)
 
     def authorization_request(strategy)
       klass = Request.authorization_strategy strategy
-      klass.build self
+      klass.new self
     end
 
     def token_request(strategy)
       klass = Request.token_strategy strategy
-      klass.build self
+      klass.new self
     end
 
     # TODO: context should be the request

spec/lib/request/strategy_spec.rb

@@ -0,0 +1,53 @@
+require 'spec_helper'
+require 'doorkeeper/request/strategy'
+
+module Doorkeeper
+  module Request
+    describe Strategy do
+      let(:server) { double }
+      subject(:strategy) { Strategy.new(server) }
+
+      describe :initialize do
+        it "sets the server attribute" do
+          expect(strategy.server).to eq server
+        end
+      end
+
+      describe :request do
+        it "requires an implementation" do
+          expect { strategy.request }.to raise_exception NotImplementedError
+        end
+      end
+
+      describe "a sample Strategy subclass" do
+        let(:fake_request) { double }
+
+        let(:strategy_class) do
+          subclass = Class.new(Strategy) do
+            class << self
+              attr_accessor :fake_request
+            end
+
+            def request
+              self.class.fake_request
+            end
+          end
+
+          subclass.fake_request = fake_request
+          subclass
+        end
+
+        subject(:strategy) { strategy_class.new(server) }
+
+        it "provides a request implementation" do
+          expect(strategy.request).to eq fake_request
+        end
+
+        it "authorizes the request" do
+          expect(fake_request).to receive :authorize
+          strategy.authorize
+        end
+      end
+    end
+  end
+end

spec/lib/server_spec.rb

@@ -45,7 +45,7 @@
 
     it 'builds the request with selected strategy' do
       stub_const 'Doorkeeper::Request::Code', fake_class
-      expect(fake_class).to receive(:build).with(subject)
+      expect(fake_class).to receive(:new).with(subject)
       subject.authorization_request :code
     end
   end