Add missing ORM specs and fix bugs discovered

Author: pboling

.github/workflows/test.yml

@@ -48,7 +48,7 @@ jobs:
         if: matrix.ruby == '2.7' && github.event_name != 'pull_request'
         continue-on-error: ${{ matrix.allow_failures != 'false' }}
       - name: Run tests
-        run: bundle exec rake test
+        run: bundle exec rake
       - name: CodeClimate Post-build Notification
         run: cc-test-reporter after-build
         if: matrix.ruby == '2.7' && github.event_name != 'pull_request' && always()

.gitignore

@@ -11,6 +11,7 @@
 /pkg/
 /spec/reports/
 /tmp/
+/rethinkdb_data
 
 *.gem
 

.simplecov

@@ -1,6 +1,10 @@
 # frozen_string_literal: true
 
 SimpleCov.start do
+  add_filter '/.github/'
+  add_filter '/coverage/'
+  add_filter '/rethinkdb_data/'
   add_filter '/spec/'
+  add_filter '/spec_orms/'
   add_filter '/vendor/bundle/'
 end

CHANGELOG.md

@@ -8,6 +8,33 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [3.0.7] - 2021-03-23
+
+### Fixed
+
+- \[ActiveRecord\] Fixed [#110](https://github.com/omniauth/omniauth-identity/issues/110) which prevented `OmniAuth::Identity::Models::ActiveRecord`-based records from saving.
+- \[CouchPotato\] Fixed `OmniAuth::Identity::Models::CouchPotato`'s `#save`.
+- \[Sequel\] Fixed `OmniAuth::Identity::Models::Sequel`'s `#save`.
+- \[Model\] Only define `::create`, `#save`, and `#persisted?` when not already defined.
+- \[Model\] Restore original `info` functionality which set `name` based on `first_name`, `last_name`, or `nickname`
+
+### Changed
+
+- Upgraded to a newer `OmniAuth::Identity::SecurePassword` ripped from [Rails 6-1-stable](https://github.com/rails/rails/blob/6-1-stable/activemodel/lib/active_model/secure_password.rb)
+  - Aeons ago the original was ripped from Rails 3.1, and frozen in time.
+    While writing specs, it was discovered to be incompatible with this gems' Sequel adapter.
+  - Specs validate that the new version does work.
+    In any case, the ripped version is only used when the `has_secure_password` macro is not yet defined in the class.
+
+### Added
+
+- New specs to cover real use cases and implementations of each ORM model adapter that ships with the gem:
+  - ActiveRecord (Polyglot - Many Relational Databases)
+  - Sequel (Polyglot - Many Relational Databases)
+  - CouchPotato (CouchDB)
+  - Mongoid (MongoDB)
+  - NoBrainer (RethinkDB)
+
 ## [3.0.6] - 2021-03-20
 
 ### Fixed

Gemfile

@@ -17,11 +17,18 @@ group :documentation do
 end
 
 group :development, :test do
+  # ORMs
   if ruby_version < Gem::Version.new('2.5.0')
-    gem 'activerecord', '~> 5' # rails 5 works with Ruby 2.4
+    gem 'activerecord', '~> 5', require: false # rails 5 works with Ruby 2.4
   else
-    gem 'activerecord', '~> 6' # rails 6 requires Ruby 2.5 or later
+    gem 'activerecord', '~> 6', require: false # rails 6 requires Ruby 2.5 or later
   end
+  gem 'anonymous_active_record', '~> 1', require: false
+  gem 'mongoid', '~> 7', require: false
+  gem 'nobrainer', '~> 0', require: false
+  gem 'sequel', '~> 5', require: false
+  gem 'couch_potato', github: 'langalex/couch_potato', require: false
+  gem 'mongoid-rspec', github: 'mongoid/mongoid-rspec', require: false
 
   if ruby_version >= Gem::Version.new('2.4')
     # No need to run byebug / pry on earlier versions
@@ -44,12 +51,10 @@ group :development, :test do
     gem 'simplecov', '~> 0.21', platform: :mri
   end
 
-  gem 'couch_potato', github: 'langalex/couch_potato'
   gem 'growl'
   gem 'guard'
   gem 'guard-bundler'
   gem 'guard-rspec'
-  gem 'mongoid-rspec', github: 'mongoid/mongoid-rspec'
   gem 'rb-fsevent'
 end
 

README.md

@@ -93,8 +93,8 @@ Just include `OmniAuth::Identity::Models::Sequel` mixin, and specify
 whatever else you will need.
 
 ```ruby
-class SequelTestIdentity < Sequel::Model
-  include OmniAuth::Identity::Models::Sequel
+class SequelTestIdentity < Sequel::Model(:identities)
+  include ::OmniAuth::Identity::Models::Sequel
   auth_key :email
   # whatever else you want!
 end
@@ -108,8 +108,8 @@ fields that you will need.
 
 ```ruby
 class Identity
-  include Mongoid::Document
-  include OmniAuth::Identity::Models::Mongoid
+  include ::Mongoid::Document
+  include ::OmniAuth::Identity::Models::Mongoid
 
   field :email, type: String
   field :name, type: String
@@ -124,8 +124,9 @@ fields that you will need.
 
 ```ruby
 class Identity
-  include CouchPotato::Persistence
-  include OmniAuth::Identity::Models::CouchPotatoModule
+  # NOTE: CouchPotato::Persistence must be included before OmniAuth::Identity::Models::CouchPotatoModule
+  include ::CouchPotato::Persistence
+  include ::OmniAuth::Identity::Models::CouchPotatoModule
 
   property :email
   property :password_digest
@@ -147,8 +148,8 @@ fields that you will need.
 
 ```ruby
 class Identity
-  include NoBrainer::Document
-  include OmniAuth::Identity::Models::NoBrainer
+  include ::NoBrainer::Document
+  include ::OmniAuth::Identity::Models::NoBrainer
 
   auth_key :email
 end
@@ -265,6 +266,35 @@ 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. 😭
 
+## Contributing
+
+1. Fork it
+2. Create your feature branch (`git checkout -b my-new-feature`)
+3. Commit your changes (`git commit -am ‘Added some feature’`)
+4. Push to the branch (`git push origin my-new-feature`)
+5. Make sure to add tests for it. This is important so I don’t break it in a future version unintentionally.
+   - NOTE: In order to run *all* the tests you will need to have the following databases installed and configured.
+     1. [RethinkDB](https://rethinkdb.com), an open source, real-time, web database, [installed](https://rethinkdb.com/docs/install/) and [running](https://rethinkdb.com/docs/start-a-server/), e.g.
+      ```bash
+      brew install rethinkdb
+      rethinkdb
+      ```
+      2. [MongoDB](https://docs.mongodb.com/manual/administration/install-community/)
+      ```bash
+      brew tap mongodb/brew
+      brew install [email protected]
+     
+      ```
+   To skip the RethinkDB-based, NoBrainer (ORM), tests run:
+   ```bash
+   bundle exec rspec spec --tag '~nobrainer'
+   ```
+   Or if you have RethinkDB setup, then run all:
+   ```bash
+   bundle exec rspec spec
+   ```
+6. Create new Pull Request
+
 ## License
 
 MIT License. See LICENSE for details.

Rakefile

@@ -4,7 +4,17 @@ require 'bundler/gem_tasks'
 
 begin
   require 'rspec/core/rake_task'
-  RSpec::Core::RakeTask.new(:spec)
+  primary = RSpec::Core::RakeTask.new(:spec)
+  active_record = RSpec::Core::RakeTask.new(:spec_orm_active_record)
+  active_record.pattern = "spec_orms/active_record_spec.rb"
+  couch_potato = RSpec::Core::RakeTask.new(:spec_orm_couch_potato)
+  couch_potato.pattern = "spec_orms/couch_potato_spec.rb"
+  mongoid = RSpec::Core::RakeTask.new(:spec_orm_mongoid)
+  mongoid.pattern = "spec_orms/mongoid_spec.rb"
+  no_brainer = RSpec::Core::RakeTask.new(:spec_orm_no_brainer)
+  no_brainer.pattern = "spec_orms/no_brainer_spec.rb"
+  sequel = RSpec::Core::RakeTask.new(:spec_orm_sequel)
+  sequel.pattern = "spec_orms/sequel_spec.rb"
 rescue LoadError
   task :spec do
     warn 'RSpec is disabled'
@@ -21,4 +31,11 @@ rescue LoadError
   end
 end
 
-task default: [:test]
+task default: [
+  :test,
+  :spec_orm_active_record,
+  :spec_orm_couch_potato,
+  :spec_orm_mongoid,
+  :spec_orm_no_brainer,
+  :spec_orm_sequel
+]

lib/omniauth/identity/model.rb

@@ -25,11 +25,13 @@ module Model
 
       def self.included(base)
         base.extend ClassMethods
+        base.extend ClassCreateApi unless base.respond_to?(:create)
+        im = base.instance_methods
+        base.include InstanceSaveApi unless im.include?(:save)
+        base.include InstancePersistedApi unless im.include?(:persisted?)
       end
 
       module ClassMethods
-        extend Gem::Deprecate
-
         # Authenticate a user with the given key and password.
         #
         # @param [String] key The unique login key provided for a given identity.
@@ -52,52 +54,56 @@ def auth_key(method = false)
           @auth_key || 'email'
         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
+
+      module ClassCreateApi
         # Persists a new Identity object to the ORM.
-        # Defaults to calling super.  Override as needed per ORM.
+        # Only included if the class doesn't define create, as a reminder to define create.
+        # 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
+        def create(*_args)
+          raise NotImplementedError
         end
+      end
 
-        # Locate an identity given its unique login key.
+      module InstanceSaveApi
+        # Persists a new Identity object to the ORM.
+        # Default raises an error.  Override as needed per ORM.
+        # This base version's arguments are modeled after ActiveModel
+        #   since it is a pattern many ORMs follow
         #
         # @abstract
-        # @param [String] key The unique login key.
         # @return [Model] An instance of the identity model class.
-        def locate(key)
+        # @since 3.0.5
+        def save(**_options, &_block)
           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
+      module InstancePersistedApi
+        # Checks if the Identity object is persisted in the ORM.
+        # Default raises an error.  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
+        end
       end
 
       # Returns self if the provided password is correct, false
@@ -106,7 +112,7 @@ def persisted?
       # @abstract
       # @param [String] password The password to check.
       # @return [self or false] Self if authenticated, false if not.
-      def authenticate(password)
+      def authenticate(_password)
         raise NotImplementedError
       end
 
@@ -163,9 +169,13 @@ def auth_key=(value)
       #
       # @return [Hash] A string-keyed hash of user information.
       def info
-        SCHEMA_ATTRIBUTES.each_with_object({}) do |attribute, hash|
+        info = {}
+        SCHEMA_ATTRIBUTES.each_with_object(info) do |attribute, hash|
           hash[attribute] = send(attribute) if respond_to?(attribute)
         end
+        info['name'] ||= [info['first_name'], info['last_name']].join(' ').strip if info['first_name'] || info['last_name']
+        info['name'] ||= info['nickname']
+        info
       end
     end
   end

lib/omniauth/identity/models/active_record.rb

@@ -6,8 +6,8 @@ module OmniAuth
   module Identity
     module Models
       class ActiveRecord < ::ActiveRecord::Base
-        include OmniAuth::Identity::Model
-        include OmniAuth::Identity::SecurePassword
+        include ::OmniAuth::Identity::Model
+        include ::OmniAuth::Identity::SecurePassword
 
         self.abstract_class = true
         has_secure_password

lib/omniauth/identity/models/couch_potato.rb

@@ -7,6 +7,7 @@ module Identity
     module Models
       # can not be named CouchPotato since there is a class with that name
       # NOTE: CouchPotato is based on ActiveModel.
+      # NOTE: CouchPotato::Persistence must be included before OmniAuth::Identity::Models::CouchPotatoModule
       module CouchPotatoModule
         def self.included(base)
           base.class_eval do
@@ -23,6 +24,10 @@ def self.auth_key=(key)
             def self.locate(search_hash)
               where(search_hash).first
             end
+
+            def save
+              CouchPotato.database.save(self)
+            end
           end
         end
       end