Refactoring a bit models and lockable. Also remove devise :all deprecation.

lib/devise/models.rb

@@ -51,26 +51,18 @@ module Devise
     #
     #   devise :authenticatable, :confirmable, :recoverable
     #
-    # You can also give the following configuration values in a hash: :pepper,
-    # :stretches, :confirm_within and :remember_for. Please check your Devise
-    # initialiazer for a complete description on those values.
+    # You can also give any of the devise configuration values in form of a hash,
+    # with specific values for this model. Please check your Devise initializer
+    # for a complete description on those values.
     #
     def devise(*modules)
       raise "You need to give at least one Devise module" if modules.empty?
       options  = modules.extract_options!
 
-      # TODO Remove me
-      if modules.delete(:all)
-        ActiveSupport::Deprecation.warn "devise :all is deprecated. List your modules instead", caller
-        modules += Devise.all
-      end
+      @devise_modules = modules.map(&:to_sym).uniq
 
-      modules -= Array(options.delete(:except))
-      modules  = Devise::ALL & modules.uniq
-
-      Devise.orm_class.included_modules_hook(self, modules) do
-        modules.each do |m|
-          devise_modules << m.to_sym
+      Devise.orm_class.included_modules_hook(self) do
+        devise_modules.each do |m|
           include Devise::Models.const_get(m.to_s.classify)
         end
 

lib/devise/models/lockable.rb

@@ -5,7 +5,6 @@ module Devise
 
     module Lockable
       include Devise::Models::Activatable
-      include Devise::Models::Authenticatable
 
       def self.included(base)
         base.class_eval do
@@ -16,15 +15,15 @@ module Devise
       # Lock an user setting it's locked_at to actual time.
       def lock
         self.locked_at = Time.now
-        if [:both, :email].include?(self.class.unlock_strategy)
+        if unlock_strategy_enabled?(:email)
           generate_unlock_token
-          self.send_unlock_instructions
+          send_unlock_instructions
         end
       end
 
       # calls lock and save the model
       def lock!
-        self.lock
+        lock
         save(false)
       end
 
@@ -40,7 +39,7 @@ module Devise
 
       # Verifies whether a user is locked or not
       def locked?
-        self.locked_at && !lock_expired?
+        locked_at && !lock_expired?
       end
 
       # Send unlock instructions by email
@@ -51,7 +50,7 @@ module Devise
       # Resend the unlock instructions if the user is locked
       def resend_unlock!
         if_locked do
-          generate_unlock_token unless self.unlock_token.present?
+          generate_unlock_token unless unlock_token.present?
           save(false)
           send_unlock_instructions
         end
@@ -63,20 +62,6 @@ module Devise
         super && !locked?
       end
 
-      # Overwrites valid_for_authentication? from Devise::Models::Authenticatable
-      # for verifying whether an user is allowed to sign in or not. If the user
-      # is locked, it should never be allowed.
-      def valid_for_authentication?(attributes)
-        if result = super
-          self.failed_attempts = 0
-        else
-          self.failed_attempts += 1
-          self.lock if self.failed_attempts > self.class.maximum_attempts
-        end
-        save(false) if changed?
-        result
-      end
-
       # Overwrites invalid_message from Devise::Models::Authenticatable to define
       # the correct reason for blocking the sign in.
       def inactive_message
@@ -87,6 +72,20 @@ module Devise
         end
       end
 
+      # Overwrites valid_for_authentication? from Devise::Models::Authenticatable
+      # for verifying whether an user is allowed to sign in or not. If the user
+      # is locked, it should never be allowed.
+      def valid_for_authentication?(attributes)
+        if result = super
+          self.failed_attempts = 0
+        else
+          self.failed_attempts += 1
+          lock if failed_attempts > self.class.maximum_attempts
+        end
+        save(false) if changed?
+        result
+      end
+
       protected
 
         # Generates unlock token
@@ -96,8 +95,8 @@ module Devise
 
         # Tells if the lock is expired if :time unlock strategy is active
         def lock_expired?
-          if [:both, :time].include?(self.class.unlock_strategy)
-            self.locked_at && self.locked_at < self.class.unlock_in.ago
+          if unlock_strategy_enabled?(:time)
+            locked_at && locked_at < self.class.unlock_in.ago
           else
             false
           end
@@ -114,6 +113,11 @@ module Devise
           end
         end
 
+        # Is the unlock enabled for the given unlock option?
+        def unlock_strategy_enabled?(strategy)
+          [:both, strategy].include?(self.class.unlock_strategy)
+        end
+
       module ClassMethods
         # Attempt to find a user by it's email. If a record is found, send new
         # unlock instructions to it. If not user is found, returns a new user
@@ -139,4 +143,4 @@ module Devise
       end
     end
   end
-end
+end

lib/devise/orm/active_record.rb

@@ -20,7 +20,7 @@ module Devise
     #
     module ActiveRecord
       # Required ORM hook. Just yield the given block in ActiveRecord.
-      def self.included_modules_hook(klass, modules)
+      def self.included_modules_hook(klass)
         yield
       end
 

lib/devise/orm/data_mapper.rb

@@ -11,13 +11,13 @@ module Devise
         end
       end
 
-      def self.included_modules_hook(klass, modules)
+      def self.included_modules_hook(klass)
         klass.send :extend,  self
         klass.send :include, InstanceMethods
 
         yield
 
-        modules.each do |mod|
+        klass.devise_modules.each do |mod|
           klass.send(mod) if klass.respond_to?(mod)
         end
       end
@@ -80,4 +80,4 @@ module Devise
   end
 end
 
-DataMapper::Model.send(:include, Devise::Models)
+DataMapper::Model.send(:include, Devise::Models)

lib/devise/orm/mongo_mapper.rb

@@ -11,12 +11,12 @@ module Devise
         end
       end
 
-      def self.included_modules_hook(klass, modules)
+      def self.included_modules_hook(klass)
         klass.send :extend,  self
         klass.send :include, InstanceMethods
         yield
 
-        modules.each do |mod|
+        klass.devise_modules.each do |mod|
           klass.send(mod) if klass.respond_to?(mod)
         end
       end
@@ -47,4 +47,4 @@ module Devise
 end
 
 MongoMapper::Document::ClassMethods.send(:include, Devise::Models)
-MongoMapper::EmbeddedDocument::ClassMethods.send(:include, Devise::Models)
+MongoMapper::EmbeddedDocument::ClassMethods.send(:include, Devise::Models)

test/models/lockable_test.rb

@@ -1,25 +1,25 @@
 require 'test/test_helper'
 
 class LockableTest < ActiveSupport::TestCase
-  
+
   def setup
     setup_mailer
   end
-  
+
   test "should increment failed attempts on unsuccessful authentication" do
     user = create_user
     assert_equal 0, user.failed_attempts
     authenticated_user = User.authenticate(:email => user.email, :password => "anotherpassword")
     assert_equal 1, user.reload.failed_attempts
   end
-  
+
   test "should lock account base on maximum_attempts" do
     user = create_user
     attempts = Devise.maximum_attempts + 1
     attempts.times { authenticated_user = User.authenticate(:email => user.email, :password => "anotherpassword") }
     assert user.reload.locked?
   end
-  
+
   test "should respect maximum attempts configuration" do
     user = create_user
     swap Devise, :maximum_attempts => 2 do
@@ -27,7 +27,7 @@ class LockableTest < ActiveSupport::TestCase
       assert user.reload.locked?
     end
   end
-  
+
   test "should clear failed_attempts on successfull sign in" do
     user = create_user
     User.authenticate(:email => user.email, :password => "anotherpassword")
@@ -61,8 +61,8 @@ class LockableTest < ActiveSupport::TestCase
     assert_nil user.reload.unlock_token
     assert 0, user.reload.failed_attempts
   end
-  
-  test 'should not unlcok an unlocked user' do
+
+  test 'should not unlock an unlocked user' do
     user = create_user
     assert_not user.unlock!
     assert_match /not locked/, user.errors[:email]
@@ -199,4 +199,4 @@ class LockableTest < ActiveSupport::TestCase
     assert_equal 'not locked', user.errors[:email]
   end
 
-end
+end