From e6e66481b8ee9ea77d7a660b3e0e34388e8bd542 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jos=C3=A9=20Valim?= <jose.valim@gmail.com>
Date: Tue, 16 Feb 2010 17:00:36 +0100
Subject: [PATCH] Got all tests in test/models and failure app ones passing.
 369 tests, 805 assertions, 13 failures, 2 errors.

---
 config/locales/en.yml                         |  6 ++++++
 lib/devise/models.rb                          | 15 ++-------------
 lib/devise/models/authenticatable.rb          |  3 +--
 lib/devise/models/confirmable.rb              |  2 +-
 lib/devise/models/lockable.rb                 |  2 +-
 lib/devise/strategies/http_authenticatable.rb | 12 ++----------
 lib/devise/test_helpers.rb                    |  5 +----
 test/failure_app_test.rb                      |  7 ++++++-
 test/mapping_test.rb                          | 11 ++---------
 test/models/authenticatable_test.rb           |  6 +++---
 test/models/confirmable_test.rb               | 14 +++++++-------
 test/models/http_authenticatable_test.rb      | 19 +++++++++++++++++++
 test/models/lockable_test.rb                  | 13 +++++++------
 test/models/recoverable_test.rb               |  6 +++---
 .../app/controllers/users_controller.rb       |  4 ----
 test/rails_app/app/views/users/index.html.erb |  2 +-
 test/rails_app/config/routes.rb               |  2 +-
 test/routes_test.rb                           | 12 ++++++------
 test/test_helpers_test.rb                     | 18 +++++++++---------
 19 files changed, 78 insertions(+), 81 deletions(-)
 create mode 100644 test/models/http_authenticatable_test.rb

diff --git a/config/locales/en.yml b/config/locales/en.yml
index 3428bf067b..2887aa3a83 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -1,4 +1,10 @@
 en:
+  errors:
+    messages:
+      not_found: "not found"
+      already_confirmed: "was already confirmed"
+      not_locked: "was not locked"
+
   devise:
     sessions:
       link: 'Sign in'
diff --git a/lib/devise/models.rb b/lib/devise/models.rb
index a9e552dd5a..4b0ed5cc81 100644
--- a/lib/devise/models.rb
+++ b/lib/devise/models.rb
@@ -89,24 +89,13 @@ def find_or_initialize_with_error_by(attribute, value, error=:invalid)
         if value.present?
           record.send(:"#{attribute}=", value)
         else
-          error, skip_default = :blank, true
+          error = :blank
         end
 
-        add_error_on(record, attribute, error, !skip_default)
+        record.errors.add(attribute, error)
       end
 
       record
     end
-
-    # Wraps add error logic in a method that works for different frameworks.
-    def add_error_on(record, attribute, error, add_default=true)
-      options = add_default ? { :default => error.to_s.gsub("_", " ") } : {}
-
-      begin
-        record.errors.add(attribute, error, options)
-      rescue ArgumentError
-        record.errors.add(attribute, error.to_s.gsub("_", " "))
-      end
-    end
   end
 end
diff --git a/lib/devise/models/authenticatable.rb b/lib/devise/models/authenticatable.rb
index 6d8c09937f..95e5e9b060 100644
--- a/lib/devise/models/authenticatable.rb
+++ b/lib/devise/models/authenticatable.rb
@@ -80,8 +80,7 @@ def update_with_password(params={})
         result = if valid_password?(current_password)
           update_attributes(params)
         else
-          message = current_password.blank? ? :blank : :invalid
-          self.class.add_error_on(self, :current_password, message, false)
+          self.errors.add(:current_password, current_password.blank? ? :blank : :invalid)
           self.attributes = params
           false
         end
diff --git a/lib/devise/models/confirmable.rb b/lib/devise/models/confirmable.rb
index e3c8fc188c..5f82b3eba5 100644
--- a/lib/devise/models/confirmable.rb
+++ b/lib/devise/models/confirmable.rb
@@ -131,7 +131,7 @@ def unless_confirmed
           unless confirmed?
             yield
           else
-            self.class.add_error_on(self, :email, :already_confirmed)
+            self.errors.add(:email, :already_confirmed)
             false
           end
         end
diff --git a/lib/devise/models/lockable.rb b/lib/devise/models/lockable.rb
index d0d424a0ec..68fa04b752 100644
--- a/lib/devise/models/lockable.rb
+++ b/lib/devise/models/lockable.rb
@@ -122,7 +122,7 @@ def if_locked
           if locked?
             yield
           else
-            self.class.add_error_on(self, :email, :not_locked)
+            self.errors.add(:email, :not_locked)
             false
           end
         end
diff --git a/lib/devise/strategies/http_authenticatable.rb b/lib/devise/strategies/http_authenticatable.rb
index 31b55c3aa6..38319b9611 100644
--- a/lib/devise/strategies/http_authenticatable.rb
+++ b/lib/devise/strategies/http_authenticatable.rb
@@ -5,7 +5,7 @@ module Strategies
     # Sign in an user using HTTP authentication.
     class HttpAuthenticatable < Base
       def valid?
-        http_authentication? && mapping.to.respond_to?(:authenticate_with_http)
+        request.authorization && mapping.to.respond_to?(:authenticate_with_http)
       end
 
       def authenticate!
@@ -24,16 +24,8 @@ def username_and_password
         decode_credentials(request).split(/:/, 2)
       end
 
-      def http_authentication
-        request.env['HTTP_AUTHORIZATION']   ||
-        request.env['X-HTTP_AUTHORIZATION'] ||
-        request.env['X_HTTP_AUTHORIZATION'] ||
-        request.env['REDIRECT_X_HTTP_AUTHORIZATION']
-      end
-      alias :http_authentication? :http_authentication
-
       def decode_credentials(request)
-        ActiveSupport::Base64.decode64(http_authentication.split(' ', 2).last || '')
+        ActiveSupport::Base64.decode64(request.authorization.split(' ', 2).last || '')
       end
 
       def custom_headers
diff --git a/lib/devise/test_helpers.rb b/lib/devise/test_helpers.rb
index a774afafe3..194c4714e6 100644
--- a/lib/devise/test_helpers.rb
+++ b/lib/devise/test_helpers.rb
@@ -45,10 +45,7 @@ def catch_with_redirect(&block)
 
     # We need to setup the environment variables and the response in the controller.
     def setup_controller_for_warden #:nodoc:
-      @request.env['action_controller.rescue.request']  = @request
-      @request.env['action_controller.rescue.response'] = @response
-      @request.env['rack.session'] = session
-      @controller.response = @response
+      @request.env['action_controller.instance'] = @controller
     end
 
     # Quick access to Warden::Proxy.
diff --git a/test/failure_app_test.rb b/test/failure_app_test.rb
index 7cbe259b63..cd9461d3b5 100644
--- a/test/failure_app_test.rb
+++ b/test/failure_app_test.rb
@@ -4,7 +4,12 @@
 class FailureTest < ActiveSupport::TestCase
 
   def call_failure(env_params={})
-    env = {'warden.options' => { :scope => :user }}.merge!(env_params)
+    env = {
+      'warden.options' => { :scope => :user },
+      'REQUEST_URI' => 'http://test.host/',
+      'REQUEST_METHOD' => 'GET',
+      'rack.session' => {}
+    }.merge!(env_params)
     Devise::FailureApp.call(env)
   end
 
diff --git a/test/mapping_test.rb b/test/mapping_test.rb
index 36ec606469..1624942fdb 100644
--- a/test/mapping_test.rb
+++ b/test/mapping_test.rb
@@ -67,7 +67,7 @@ class MappingTest < ActiveSupport::TestCase
     assert_equal 'sign_out',     mapping.path_names[:sign_out]
     assert_equal 'password',     mapping.path_names[:password]
     assert_equal 'confirmation', mapping.path_names[:confirmation]
-    assert_equal 'sign_up', mapping.path_names[:sign_up]
+    assert_equal 'sign_up',      mapping.path_names[:sign_up]
     assert_equal 'unlock',       mapping.path_names[:unlock]
   end
 
@@ -129,20 +129,13 @@ class MappingTest < ActiveSupport::TestCase
     end
   end
 
-  test 'should have default route options' do
-    assert_equal({}, Devise.mappings[:user].route_options)
-  end
-
-  test 'should allow passing route options to devise routes' do
-    assert_equal({ :requirements => { :extra => 'value' } }, Devise.mappings[:manager].route_options)
-  end
-
   test 'magic predicates' do
     mapping = Devise.mappings[:user]
     assert mapping.authenticatable?
     assert mapping.confirmable?
     assert mapping.recoverable?
     assert mapping.rememberable?
+    assert mapping.registerable?
 
     mapping = Devise.mappings[:admin]
     assert mapping.authenticatable?
diff --git a/test/models/authenticatable_test.rb b/test/models/authenticatable_test.rb
index 8ec097283f..96711f47ce 100644
--- a/test/models/authenticatable_test.rb
+++ b/test/models/authenticatable_test.rb
@@ -100,7 +100,7 @@ def encrypt_password(user, pepper=User.pepper, stretches=User.stretches, encrypt
 
   test 'should authenticate a valid user with email and password and return it' do
     user = create_user
-    User.any_instance.stubs(:confirmed?).returns(true)
+    user.confirm!
     authenticated_user = User.authenticate(:email => user.email, :password => user.password)
     assert_equal authenticated_user, user
   end
@@ -146,7 +146,7 @@ def encrypt_password(user, pepper=User.pepper, stretches=User.stretches, encrypt
     assert_not user.update_with_password(:current_password => 'other',
       :password => 'pass321', :password_confirmation => 'pass321')
     assert user.reload.valid_password?('123456')
-    assert_match /invalid/, user.errors[:current_password]
+    assert_match "is invalid", user.errors[:current_password].join
   end
 
   test 'should add an error to current password when it is blank' do
@@ -154,7 +154,7 @@ def encrypt_password(user, pepper=User.pepper, stretches=User.stretches, encrypt
     assert_not user.update_with_password(:password => 'pass321',
       :password_confirmation => 'pass321')
     assert user.reload.valid_password?('123456')
-    assert_match /blank/, user.errors[:current_password]
+    assert_match "can't be blank", user.errors[:current_password].join
   end
 
   test 'should ignore password and its confirmation if they are blank' do
diff --git a/test/models/confirmable_test.rb b/test/models/confirmable_test.rb
index 8696e0ab2a..58efdc2d40 100644
--- a/test/models/confirmable_test.rb
+++ b/test/models/confirmable_test.rb
@@ -54,10 +54,10 @@ def setup
   test 'should not confirm a user already confirmed' do
     user = create_user
     assert user.confirm!
-    assert_nil user.errors[:email]
+    assert_blank user.errors[:email]
 
     assert_not user.confirm!
-    assert_match /already confirmed/, user.errors[:email]
+    assert_equal "was already confirmed", user.errors[:email].join
   end
 
   test 'should find and confirm an user automatically' do
@@ -70,13 +70,13 @@ def setup
   test 'should return a new record with errors when a invalid token is given' do
     confirmed_user = User.confirm!(:confirmation_token => 'invalid_confirmation_token')
     assert confirmed_user.new_record?
-    assert_match /invalid/, confirmed_user.errors[:confirmation_token]
+    assert_equal "is invalid", confirmed_user.errors[:confirmation_token].join
   end
 
   test 'should return a new record with errors when a blank token is given' do
     confirmed_user = User.confirm!(:confirmation_token => '')
     assert confirmed_user.new_record?
-    assert_match /blank/, confirmed_user.errors[:confirmation_token]
+    assert_equal "can't be blank", confirmed_user.errors[:confirmation_token].join
   end
 
   test 'should generate errors for a user email if user is already confirmed' do
@@ -85,7 +85,7 @@ def setup
     user.save
     confirmed_user = User.confirm!(:confirmation_token => user.confirmation_token)
     assert confirmed_user.confirmed?
-    assert confirmed_user.errors[:email]
+    assert_equal "was already confirmed", confirmed_user.errors[:email].join
   end
 
   test 'should authenticate a confirmed user' do
@@ -134,7 +134,7 @@ def setup
   test 'should add error to new user email if no email was found' do
     confirmation_user = User.send_confirmation_instructions(:email => "invalid@email.com")
     assert confirmation_user.errors[:email]
-    assert_equal 'not found', confirmation_user.errors[:email]
+    assert_equal "not found", confirmation_user.errors[:email].join
   end
 
   test 'should generate a confirmation token before send the confirmation instructions email' do
@@ -175,7 +175,7 @@ def setup
     user.confirm!
     assert_not user.resend_confirmation!
     assert user.confirmed?
-    assert_equal 'already confirmed', user.errors[:email]
+    assert_equal 'was already confirmed', user.errors[:email].join
   end
 
   test 'confirm time should fallback to devise confirm in default configuration' do
diff --git a/test/models/http_authenticatable_test.rb b/test/models/http_authenticatable_test.rb
new file mode 100644
index 0000000000..948fcafb20
--- /dev/null
+++ b/test/models/http_authenticatable_test.rb
@@ -0,0 +1,19 @@
+require 'test/test_helper'
+
+class HttpAuthenticatableTest < ActiveSupport::TestCase
+  test 'should authenticate a valid user with email and password and return it' do
+    user = create_user
+    user.confirm!
+
+    authenticated_user = User.authenticate_with_http(user.email, user.password)
+    assert_equal authenticated_user, user
+  end
+
+  test 'should return nil when authenticating an invalid user by email' do
+    user = create_user
+    user.confirm!
+
+    authenticated_user = User.authenticate_with_http('another.email@email.com', user.password)
+    assert_nil authenticated_user
+  end
+end
diff --git a/test/models/lockable_test.rb b/test/models/lockable_test.rb
index ebc3f40407..a761b422ae 100644
--- a/test/models/lockable_test.rb
+++ b/test/models/lockable_test.rb
@@ -9,6 +9,7 @@ def setup
   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
@@ -56,6 +57,7 @@ def setup
     user.lock!
     assert_not_nil user.reload.locked_at
     assert_not_nil user.reload.unlock_token
+
     user.unlock!
     assert_nil user.reload.locked_at
     assert_nil user.reload.unlock_token
@@ -65,7 +67,7 @@ def setup
   test 'should not unlock an unlocked user' do
     user = create_user
     assert_not user.unlock!
-    assert_match /not locked/, user.errors[:email]
+    assert_match "was not locked", user.errors[:email].join
   end
 
   test "new user should not be locked and should have zero failed_attempts" do
@@ -157,13 +159,13 @@ def setup
   test 'should return a new record with errors when a invalid token is given' do
     locked_user = User.unlock!(:unlock_token => 'invalid_token')
     assert locked_user.new_record?
-    assert_match /invalid/, locked_user.errors[:unlock_token]
+    assert_equal "is invalid", locked_user.errors[:unlock_token].join
   end
 
   test 'should return a new record with errors when a blank token is given' do
     locked_user = User.unlock!(:unlock_token => '')
     assert locked_user.new_record?
-    assert_match /blank/, locked_user.errors[:unlock_token]
+    assert_equal "can't be blank", locked_user.errors[:unlock_token].join
   end
 
   test 'should authenticate a unlocked user' do
@@ -188,15 +190,14 @@ def setup
 
   test 'should add error to new user email if no email was found' do
     unlock_user = User.send_unlock_instructions(:email => "invalid@email.com")
-    assert unlock_user.errors[:email]
-    assert_equal 'not found', unlock_user.errors[:email]
+    assert_equal 'not found', unlock_user.errors[:email].join
   end
 
   test 'should not be able to send instructions if the user is not locked' do
     user = create_user
     assert_not user.resend_unlock!
     assert_not user.locked?
-    assert_equal 'not locked', user.errors[:email]
+    assert_equal 'was not locked', user.errors[:email].join
   end
 
 end
diff --git a/test/models/recoverable_test.rb b/test/models/recoverable_test.rb
index 55c38b1473..82a56f50fa 100644
--- a/test/models/recoverable_test.rb
+++ b/test/models/recoverable_test.rb
@@ -83,7 +83,7 @@ def setup
   test 'should return a new record with errors if user was not found by e-mail' do
     reset_password_user = User.send_reset_password_instructions(:email => "invalid@email.com")
     assert reset_password_user.new_record?
-    assert_match /not found/, reset_password_user.errors[:email]
+    assert_equal "not found", reset_password_user.errors[:email].join
   end
 
   test 'should reset reset_password_token before send the reset instructions email' do
@@ -111,13 +111,13 @@ def setup
   test 'should a new record with errors if no reset_password_token is found' do
     reset_password_user = User.reset_password!(:reset_password_token => 'invalid_token')
     assert reset_password_user.new_record?
-    assert_match /invalid/, reset_password_user.errors[:reset_password_token]
+    assert_equal "is invalid", reset_password_user.errors[:reset_password_token].join
   end
 
   test 'should a new record with errors if reset_password_token is blank' do
     reset_password_user = User.reset_password!(:reset_password_token => '')
     assert reset_password_user.new_record?
-    assert_match /blank/, reset_password_user.errors[:reset_password_token]
+    assert_match "can't be blank", reset_password_user.errors[:reset_password_token].join
   end
 
   test 'should reset successfully user password given the new password and confirmation' do
diff --git a/test/rails_app/app/controllers/users_controller.rb b/test/rails_app/app/controllers/users_controller.rb
index ef3884639b..7f61902217 100644
--- a/test/rails_app/app/controllers/users_controller.rb
+++ b/test/rails_app/app/controllers/users_controller.rb
@@ -9,8 +9,4 @@ def expire
     user_session['last_request_at'] = 31.minutes.ago.utc
     render :text => 'User will be expired on next request'
   end
-
-  def show
-    render :text => current_user.id.to_s
-  end
 end
diff --git a/test/rails_app/app/views/users/index.html.erb b/test/rails_app/app/views/users/index.html.erb
index 93b6afc32b..df405f2d8a 100644
--- a/test/rails_app/app/views/users/index.html.erb
+++ b/test/rails_app/app/views/users/index.html.erb
@@ -1 +1 @@
-Welcome User!
+Welcome User #<%= current_user.id %>!
diff --git a/test/rails_app/config/routes.rb b/test/rails_app/config/routes.rb
index facd7b81c0..da67adbab6 100644
--- a/test/rails_app/config/routes.rb
+++ b/test/rails_app/config/routes.rb
@@ -6,7 +6,7 @@
   devise_for :users
   devise_for :admin, :as => 'admin_area'
   devise_for :accounts, :scope => 'manager', :path_prefix => ':locale',
-    :class_name => "User", :requirements => { :extra => 'value' }, :path_names => {
+    :class_name => "User", :path_names => {
       :sign_in => 'login', :sign_out => 'logout',
       :password => 'secret', :confirmation => 'verification',
       :unlock => 'unblock', :sign_up => 'register'
diff --git a/test/routes_test.rb b/test/routes_test.rb
index a7b12c9c6f..851bff8f04 100644
--- a/test/routes_test.rb
+++ b/test/routes_test.rb
@@ -85,26 +85,26 @@ class MapRoutingTest < ActionController::TestCase
   end
 
   test 'map account with custom path name for session sign in' do
-    assert_recognizes({:controller => 'sessions', :action => 'new', :locale => 'en', :extra => 'value'}, '/en/accounts/login')
+    assert_recognizes({:controller => 'sessions', :action => 'new', :locale => 'en'}, '/en/accounts/login')
   end
 
   test 'map account with custom path name for session sign out' do
-    assert_recognizes({:controller => 'sessions', :action => 'destroy', :locale => 'en', :extra => 'value'}, '/en/accounts/logout')
+    assert_recognizes({:controller => 'sessions', :action => 'destroy', :locale => 'en'}, '/en/accounts/logout')
   end
 
   test 'map account with custom path name for password' do
-    assert_recognizes({:controller => 'passwords', :action => 'new', :locale => 'en', :extra => 'value'}, '/en/accounts/secret/new')
+    assert_recognizes({:controller => 'passwords', :action => 'new', :locale => 'en'}, '/en/accounts/secret/new')
   end
 
   test 'map account with custom path name for confirmation' do
-    assert_recognizes({:controller => 'confirmations', :action => 'new', :locale => 'en', :extra => 'value'}, '/en/accounts/verification/new')
+    assert_recognizes({:controller => 'confirmations', :action => 'new', :locale => 'en'}, '/en/accounts/verification/new')
   end
 
   test 'map account with custom path name for unlock' do
-    assert_recognizes({:controller => 'unlocks', :action => 'new', :locale => 'en', :extra => 'value'}, '/en/accounts/unblock/new')
+    assert_recognizes({:controller => 'unlocks', :action => 'new', :locale => 'en'}, '/en/accounts/unblock/new')
   end
 
   test 'map account with custom path name for registration' do
-    assert_recognizes({:controller => 'registrations', :action => 'new', :locale => 'en', :extra => 'value'}, '/en/accounts/register')
+    assert_recognizes({:controller => 'registrations', :action => 'new', :locale => 'en'}, '/en/accounts/register')
   end
 end
diff --git a/test/test_helpers_test.rb b/test/test_helpers_test.rb
index 8cd82da288..533c1f3d55 100644
--- a/test/test_helpers_test.rb
+++ b/test/test_helpers_test.rb
@@ -5,14 +5,14 @@ class TestHelpersTest < ActionController::TestCase
   include Devise::TestHelpers
 
   test "redirects if attempting to access a page unauthenticated" do
-    get :show
+    get :index
     assert_redirected_to "/users/sign_in?unauthenticated=true"
   end
 
   test "redirects if attempting to access a page with a unconfirmed account" do
     swap Devise, :confirm_within => 0 do
       sign_in create_user
-      get :show
+      get :index
       assert_redirected_to "/users/sign_in?unconfirmed=true"
     end
   end
@@ -22,7 +22,7 @@ class TestHelpersTest < ActionController::TestCase
     user.confirm!
 
     sign_in user
-    get :show
+    get :index
     assert_response :success
   end
 
@@ -31,10 +31,10 @@ class TestHelpersTest < ActionController::TestCase
     user.confirm!
 
     sign_in user
-    get :show
+    get :index
 
     sign_out user
-    get :show
+    get :index
     assert_redirected_to "/users/sign_in?unauthenticated=true"
   end
 
@@ -43,15 +43,15 @@ class TestHelpersTest < ActionController::TestCase
     first_user.confirm!
 
     sign_in first_user
-    get :show
-    assert_equal first_user.id.to_s, @response.body
+    get :index
+    assert_match /User ##{first_user.id}/, @response.body
     sign_out first_user
 
     second_user = create_user
     second_user.confirm!
 
     sign_in second_user
-    get :show
-    assert_equal second_user.id.to_s, @response.body
+    get :index
+    assert_match /User ##{second_user.id}/, @response.body
   end
 end