Merge pull request #108 from omniauth/define-api

pboling · web-flow · commit 38fa977c19c1 · 2021-03-19T21:46:34.000-06:00
Define API for classes utilizing OmniAuth::Identity::Model
diff --git a/.rubocop.yml b/.rubocop.yml
@@ -7,6 +7,7 @@ require:
   - 'rubocop-performance'
   - 'rubocop-rake'
   - 'rubocop-rspec'
+  - 'rubocop-sequel'
 
 AllCops:
   NewCops: enable
@@ -25,3 +26,9 @@ Metrics/BlockLength:
     - shared_examples_for
     - namespace
     - draw
+
+Sequel/SaveChanges:
+  Enabled: false
+
+Lint/UselessMethodDefinition:
+  Enabled: false
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
@@ -40,12 +40,12 @@ Metrics/AbcSize:
 # Configuration parameters: CountComments, CountAsOne, ExcludedMethods, IgnoredMethods.
 # IgnoredMethods: refine
 Metrics/BlockLength:
-  Max: 27
+  Max: 28
 
 # Offense count: 1
 # Configuration parameters: CountComments, CountAsOne.
 Metrics/ClassLength:
-  Max: 117
+  Max: 140
 
 # Offense count: 1
 # Configuration parameters: IgnoredMethods.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [3.0.5] - 2021-03-19
+
+### Fixed
+
+- Fix breaking changes introduced by [#86's](https://github.com/omniauth/omniauth-identity/pull/86) introduction of `:on_validation`
+
+### Added
+
+- Define `#save`, `#persisted?` and `::create` on `Omniauth::Identity::Model`
+- Add `@since` YARD tags to interface methods
+- Refactor `Omniauth::Strategies::Identity.registration_phase` to support `Omniauth::Identity::Model`-inheriting classes that do not define `#save`.
+  - This support will be dropped in v4.0.
+
 ## [3.0.4] - 2021-02-14
 
 ### Added
diff --git a/Gemfile b/Gemfile
@@ -39,6 +39,7 @@ group :development, :test do
     gem 'rubocop-performance', platform: :mri
     gem 'rubocop-rake', platform: :mri
     gem 'rubocop-rspec', platform: :mri
+    gem 'rubocop-sequel', platform: :mri
 
     gem 'simplecov', '~> 0.21', platform: :mri
   end
diff --git a/README.md b/README.md
@@ -249,21 +249,21 @@ always break things!
 
 From the code - here are the options we have for you, a couple of which are documented above, and the rest are documented... in the specs we hope!?
 ```
-      option :fields, %i[name email]
+option :fields, %i[name email]
 
       # Primary Feature Switches:
-      option :enable_registration, true   # See #other_phase and #request_phase
-      option :enable_login, true          # See #other_phase
+option :enable_registration, true   # See #other_phase and #request_phase
+option :enable_login, true          # See #other_phase
 
       # Customization Options:
-      option :on_login, nil               # See #request_phase
-      option :on_validation, nil          # See #registration_phase
-      option :on_registration, nil        # See #registration_phase
-      option :on_failed_registration, nil # See #registration_phase
-      option :locate_conditions, ->(req) { { model.auth_key => req['auth_key'] } }
+option :on_login, nil               # See #request_phase
+option :on_validation, nil          # See #registration_phase
+option :on_registration, nil        # See #registration_phase
+option :on_failed_registration, nil # See #registration_phase
+option :locate_conditions, ->(req) { { model.auth_key => req['auth_key'] } }
 ```
 
-Please contribute some documentation if you have the gumption!  The maintainer's time is limited, and sometimes the authors of PRs with new options don't update the _this_ readme. 😭 
+Please contribute some documentation if you have the gumption!  The maintainer's time is limited, and sometimes the authors of PRs with new options don't update the _this_ readme. 😭
 
 ## License
 
diff --git a/lib/omniauth-identity/version.rb b/lib/omniauth-identity/version.rb
@@ -2,6 +2,6 @@
 
 module OmniAuth
   module Identity
-    VERSION = '3.0.4'
+    VERSION = '3.0.5'
   end
 end
diff --git a/lib/omniauth/identity/model.rb b/lib/omniauth/identity/model.rb
@@ -4,23 +4,31 @@ module OmniAuth
   module Identity
     # This module provides an includable interface for implementing the
     # necessary API for OmniAuth Identity to properly locate identities
-    # and provide all necessary information. All methods marked as
-    # abstract must be implemented in the including class for things to
-    # work properly.
+    # and provide all necessary information.
+    #
+    # All methods marked as abstract must be implemented in the
+    # including class for things to work properly.
+    #
+    ### Singleton API
+    #
+    # * locate(key)
+    # * create(*args) - Deprecated in v3.0.5; Will be removed in v4.0
+    #
+    ### Instance API
+    #
+    # * save
+    # * persisted?
+    # * authenticate(password)
+    #
     module Model
+      SCHEMA_ATTRIBUTES = %w[name email nickname first_name last_name location description image phone].freeze
+
       def self.included(base)
         base.extend ClassMethods
       end
 
       module ClassMethods
-        # Locate an identity given its unique login key.
-        #
-        # @abstract
-        # @param [String] key The unique login key.
-        # @return [Model] An instance of the identity model class.
-        def locate(key)
-          raise NotImplementedError
-        end
+        extend Gem::Deprecate
 
         # Authenticate a user with the given key and password.
         #
@@ -43,6 +51,53 @@ def auth_key(method = false)
 
           @auth_key || 'email'
         end
+
+        # Persists a new Identity object to the ORM.
+        # Defaults to calling super.  Override as needed per ORM.
+        #
+        # @deprecated v4.0 will begin using {#new} with {#save} instead.
+        # @abstract
+        # @param [Hash] args Attributes of the new instance.
+        # @return [Model] An instance of the identity model class.
+        # @since 3.0.5
+        def create(*args)
+          raise NotImplementedError unless defined?(super)
+
+          super
+        end
+
+        # Locate an identity given its unique login key.
+        #
+        # @abstract
+        # @param [String] key The unique login key.
+        # @return [Model] An instance of the identity model class.
+        def locate(key)
+          raise NotImplementedError
+        end
+      end
+
+      # Persists a new Identity object to the ORM.
+      # Default raises an error.  Override as needed per ORM.
+      #
+      # @abstract
+      # @return [Model] An instance of the identity model class.
+      # @since 3.0.5
+      def save
+        raise NotImplementedError unless defined?(super)
+
+        super
+      end
+
+      # Checks if the Identity object is persisted in the ORM.
+      # Defaults to calling super.  Override as needed per ORM.
+      #
+      # @abstract
+      # @return [true or false] true if object exists, false if not.
+      # @since 3.0.5
+      def persisted?
+        raise NotImplementedError unless defined?(super)
+
+        super
       end
 
       # Returns self if the provided password is correct, false
@@ -55,22 +110,6 @@ def authenticate(password)
         raise NotImplementedError
       end
 
-      SCHEMA_ATTRIBUTES = %w[name email nickname first_name last_name location description image phone].freeze
-      # A hash of as much of the standard OmniAuth schema as is stored
-      # in this particular model. By default, this will call instance
-      # methods for each of the attributes it needs in turn, ignoring
-      # any for which `#respond_to?` is `false`.
-      #
-      # If `first_name`, `nickname`, and/or `last_name` is provided but
-      # `name` is not, it will be automatically calculated.
-      #
-      # @return [Hash] A string-keyed hash of user information.
-      def info
-        SCHEMA_ATTRIBUTES.each_with_object({}) do |attribute, hash|
-          hash[attribute] = send(attribute) if respond_to?(attribute)
-        end
-      end
-
       # An identifying string that must be globally unique to the
       # application. Defaults to stringifying the `id` method.
       #
@@ -113,6 +152,21 @@ def auth_key=(value)
           raise NotImplementedError
         end
       end
+
+      # A hash of as much of the standard OmniAuth schema as is stored
+      # in this particular model. By default, this will call instance
+      # methods for each of the attributes it needs in turn, ignoring
+      # any for which `#respond_to?` is `false`.
+      #
+      # If `first_name`, `nickname`, and/or `last_name` is provided but
+      # `name` is not, it will be automatically calculated.
+      #
+      # @return [Hash] A string-keyed hash of user information.
+      def info
+        SCHEMA_ATTRIBUTES.each_with_object({}) do |attribute, hash|
+          hash[attribute] = send(attribute) if respond_to?(attribute)
+        end
+      end
     end
   end
 end
diff --git a/lib/omniauth/identity/models/couch_potato.rb b/lib/omniauth/identity/models/couch_potato.rb
@@ -6,6 +6,7 @@ module OmniAuth
   module Identity
     module Models
       # can not be named CouchPotato since there is a class with that name
+      # NOTE: CouchPotato is based on ActiveModel.
       module CouchPotatoModule
         def self.included(base)
           base.class_eval do
diff --git a/lib/omniauth/identity/models/mongoid.rb b/lib/omniauth/identity/models/mongoid.rb
@@ -5,6 +5,7 @@
 module OmniAuth
   module Identity
     module Models
+      # NOTE: Mongoid is based on ActiveModel.
       module Mongoid
         def self.included(base)
           base.class_eval do
diff --git a/lib/omniauth/identity/models/no_brainer.rb b/lib/omniauth/identity/models/no_brainer.rb
@@ -6,6 +6,7 @@ module OmniAuth
   module Identity
     module Models
       # http://nobrainer.io/ an ORM for RethinkDB
+      # NOTE: NoBrainer is based on ActiveModel.
       module NoBrainer
         def self.included(base)
           base.class_eval do
diff --git a/lib/omniauth/identity/models/sequel.rb b/lib/omniauth/identity/models/sequel.rb
@@ -6,6 +6,9 @@ module OmniAuth
   module Identity
     module Models
       # http://sequel.jeremyevans.net/ an SQL ORM
+      # NOTE: Sequel is *not* based on ActiveModel, but supports the API we need, except for `persisted`:
+      # * create
+      # * save, but save is deprecated in favor of `save_changes`
       module Sequel
         def self.included(base)
           base.class_eval do
@@ -29,6 +32,14 @@ def self.auth_key=(key)
             def self.locate(search_hash)
               where(search_hash).first
             end
+
+            def persisted?
+              exists?
+            end
+
+            def save
+              save_changes
+            end
           end
         end
       end
diff --git a/lib/omniauth/strategies/identity.rb b/lib/omniauth/strategies/identity.rb
@@ -6,8 +6,8 @@ module Strategies
     # user authentication using the same process flow that you
     # use for external OmniAuth providers.
     class Identity
+      DEFAULT_REGISTRATION_FIELDS = %i[password password_confirmation].freeze
       include OmniAuth::Strategy
-
       option :fields, %i[name email]
 
       # Primary Feature Switches:
@@ -65,29 +65,23 @@ def registration_form(validation_message = nil)
       end
 
       def registration_phase
-        attributes = (options[:fields] + %i[password password_confirmation]).each_with_object({}) do |k, h|
+        attributes = (options[:fields] + DEFAULT_REGISTRATION_FIELDS).each_with_object({}) do |k, h|
           h[k] = request[k.to_s]
         end
         if model.respond_to?(:column_names) && model.column_names.include?('provider')
           attributes.reverse_merge!(provider: 'identity')
         end
-        @identity = model.new(attributes)
-
-        # on_validation may run a Captcha or other validation mechanism
-        # Must return true when validation passes, false otherwise
-        if options[:on_validation] && !options[:on_validation].call(env: env)
-          if options[:on_failed_registration]
-            env['omniauth.identity'] = @identity
-            options[:on_failed_registration].call(env)
+        if saving_instead_of_creating?
+          @identity = model.new(attributes)
+          env['omniauth.identity'] = @identity
+          if !validating? || valid?
+            @identity.save
+            registration_result
           else
-            validation_message = 'Validation failed'
-            registration_form(validation_message)
+            registration_failure('Validation failed')
           end
-        elsif @identity.save && @identity.persisted?
-          env['PATH_INFO'] = callback_path
-          callback_phase
         else
-          show_custom_options_or_default
+          deprecated_registration(attributes)
         end
       end
 
@@ -142,15 +136,53 @@ def build_omniauth_registration_form(validation_message)
         end
       end
 
-      def show_custom_options_or_default
+      def saving_instead_of_creating?
+        model.respond_to?(:save) && model.respond_to?(:persisted?)
+      end
+
+      # Validates the model before it is persisted
+      #
+      # @return [truthy or falsey] :on_validation option is truthy or falsey
+      def validating?
+        options[:on_validation]
+      end
+
+      # Validates the model before it is persisted
+      #
+      # @return [true or false] result of :on_validation call
+      def valid?
+        # on_validation may run a Captcha or other validation mechanism
+        # Must return true when validation passes, false otherwise
+        !!options[:on_validation].call(env: env)
+      end
+
+      def registration_failure(message)
         if options[:on_failed_registration]
-          env['omniauth.identity'] = @identity
           options[:on_failed_registration].call(env)
         else
-          validation_message = 'One or more fields were invalid'
-          registration_form(validation_message)
+          registration_form(message)
+        end
+      end
+
+      def registration_result
+        if @identity.persisted?
+          env['PATH_INFO'] = callback_path
+          callback_phase
+        else
+          registration_failure('One or more fields were invalid')
         end
       end
+
+      def deprecated_registration(attributes)
+        warn <<~CREATEDEP
+          [DEPRECATION] Please define '#{model.class}#save'.
+                        Behavior based on '#{model.class}.create' will be removed in omniauth-identity v4.0.
+                        See lib/omniauth/identity/model.rb
+        CREATEDEP
+        @identity = model.create(attributes)
+        env['omniauth.identity'] = @identity
+        registration_result
+      end
     end
   end
 end
