From a793472a3e28e8b0dec137531e3de64d91ff81ec Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Wed, 10 Feb 2021 17:17:29 -0300 Subject: [PATCH] Replace XML with JSON serialization across the test suite This allows us to remove the dependency on the XML serializer provided by the external `activemodel-serializers-xml` gem, and eliminates the following deprecation warning: DEPRECATION WARNING: ActiveModel::Errors#to_xml is deprecated and will be removed in Rails 6.2. Please note: this does not mean Devise doesn't support XML, it simply means our test suite will use JSON to test non-navigatable formats instead of XML, for simplicity. Devise's job is not to test object serialization, so as long as your objects properly serialize to XML/JSON/any other format, it should work out of the box. --- Gemfile | 2 - Gemfile.lock | 10 ---- gemfiles/Gemfile-rails-5-0 | 2 - gemfiles/Gemfile-rails-5-1 | 2 - gemfiles/Gemfile-rails-5-2 | 2 - gemfiles/Gemfile-rails-6-0 | 2 - test/failure_app_test.rb | 10 ++-- test/integration/authenticatable_test.rb | 33 ++++-------- test/integration/confirmable_test.rb | 54 ++++++++----------- test/integration/http_authenticatable_test.rb | 23 ++++---- test/integration/lockable_test.rb | 31 +++++------ test/integration/recoverable_test.rb | 46 +++++++--------- test/integration/registerable_test.rb | 37 ++++++------- test/models/serializable_test.rb | 15 ------ test/rails_app/app/active_record/user.rb | 1 - .../app/controllers/users_controller.rb | 3 +- test/routes_test.rb | 12 ++--- test/test/controller_helpers_test.rb | 6 +-- 18 files changed, 104 insertions(+), 187 deletions(-) diff --git a/Gemfile b/Gemfile index 22ca4afe..2bca3406 100644 --- a/Gemfile +++ b/Gemfile @@ -9,8 +9,6 @@ gem "omniauth" gem "omniauth-oauth2" gem "rdoc" -gem "activemodel-serializers-xml", github: "rails/activemodel-serializers-xml" - gem "rails-controller-testing", github: "rails/rails-controller-testing" gem "responders", "~> 3.0" diff --git a/Gemfile.lock b/Gemfile.lock index e08446d7..e19528ac 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,12 +1,3 @@ -GIT - remote: https://github.com/rails/activemodel-serializers-xml.git - revision: 694f4071c6b16e4c8597cc323c241b5f787b3ea8 - specs: - activemodel-serializers-xml (1.0.2) - activemodel (>= 5.0.0.a) - activesupport (>= 5.0.0.a) - builder (~> 3.1) - GIT remote: https://github.com/rails/rails-controller-testing.git revision: 4b15c86e82ee380f2a7cc009e470368f7520560a @@ -213,7 +204,6 @@ PLATFORMS ruby DEPENDENCIES - activemodel-serializers-xml! devise! mocha (~> 1.1) omniauth diff --git a/gemfiles/Gemfile-rails-5-0 b/gemfiles/Gemfile-rails-5-0 index dcd1ac14..2f60c3a2 100644 --- a/gemfiles/Gemfile-rails-5-0 +++ b/gemfiles/Gemfile-rails-5-0 @@ -9,8 +9,6 @@ gem "omniauth" gem "omniauth-oauth2" gem "rdoc" -gem "activemodel-serializers-xml", github: "rails/activemodel-serializers-xml" - gem "rails-controller-testing" gem "responders", "~> 2.1" diff --git a/gemfiles/Gemfile-rails-5-1 b/gemfiles/Gemfile-rails-5-1 index c2b8f523..c566e9c8 100644 --- a/gemfiles/Gemfile-rails-5-1 +++ b/gemfiles/Gemfile-rails-5-1 @@ -7,8 +7,6 @@ gem "omniauth" gem "omniauth-oauth2" gem "rdoc" -gem "activemodel-serializers-xml", github: "rails/activemodel-serializers-xml" - gem "rails-controller-testing" gem "responders", "~> 2.1" diff --git a/gemfiles/Gemfile-rails-5-2 b/gemfiles/Gemfile-rails-5-2 index dbfbd5f6..5dc267de 100644 --- a/gemfiles/Gemfile-rails-5-2 +++ b/gemfiles/Gemfile-rails-5-2 @@ -7,8 +7,6 @@ gem "omniauth" gem "omniauth-oauth2" gem "rdoc" -gem "activemodel-serializers-xml", github: "rails/activemodel-serializers-xml" - gem "rails-controller-testing" gem "responders", "~> 2.1" diff --git a/gemfiles/Gemfile-rails-6-0 b/gemfiles/Gemfile-rails-6-0 index d01a464f..f840fc8d 100644 --- a/gemfiles/Gemfile-rails-6-0 +++ b/gemfiles/Gemfile-rails-6-0 @@ -7,8 +7,6 @@ gem "omniauth" gem "omniauth-oauth2" gem "rdoc" -gem "activemodel-serializers-xml", github: "rails/activemodel-serializers-xml" - gem "rails-controller-testing", github: "rails/rails-controller-testing" gem "responders", "~> 3.0" diff --git a/test/failure_app_test.rb b/test/failure_app_test.rb index 1b0aeb04..809f668d 100644 --- a/test/failure_app_test.rb +++ b/test/failure_app_test.rb @@ -220,8 +220,8 @@ class FailureTest < ActiveSupport::TestCase end test 'works for any navigational format' do - swap Devise, navigational_formats: [:xml] do - call_failure('formats' => Mime[:xml]) + swap Devise, navigational_formats: [:json] do + call_failure('formats' => Mime[:json]) assert_equal 302, @response.first end end @@ -236,7 +236,7 @@ class FailureTest < ActiveSupport::TestCase context 'For HTTP request' do test 'return 401 status' do - call_failure('formats' => Mime[:xml]) + call_failure('formats' => Mime[:json]) assert_equal 401, @response.first end @@ -258,13 +258,13 @@ class FailureTest < ActiveSupport::TestCase end test 'return WWW-authenticate headers if model allows' do - call_failure('formats' => Mime[:xml]) + call_failure('formats' => Mime[:json]) assert_equal 'Basic realm="Application"', @response.second["WWW-Authenticate"] end test 'does not return WWW-authenticate headers if model does not allow' do swap Devise, http_authenticatable: false do - call_failure('formats' => Mime[:xml]) + call_failure('formats' => Mime[:json]) assert_nil @response.second["WWW-Authenticate"] end end diff --git a/test/integration/authenticatable_test.rb b/test/integration/authenticatable_test.rb index fcc1d734..fbe1da6c 100644 --- a/test/integration/authenticatable_test.rb +++ b/test/integration/authenticatable_test.rb @@ -462,14 +462,6 @@ class AuthenticationOthersTest < Devise::IntegrationTest end end - test 'sign in stub in xml format' do - get new_user_session_path(format: 'xml') - assert_match '', response.body - assert_match %r{.*}m, response.body - assert_match '', response.body - assert_match '\n) + assert_includes response.body, '{"user":{' end - test 'sign in with xml format is idempotent' do - get new_user_session_path(format: 'xml') + test 'sign in with json format is idempotent' do + get new_user_session_path(format: 'json') assert_response :success create_user - post user_session_path(format: 'xml'), params: { user: {email: "user@test.com", password: '12345678'} } + post user_session_path(format: 'json'), params: { user: {email: "user@test.com", password: '12345678'} } assert_response :success - get new_user_session_path(format: 'xml') + get new_user_session_path(format: 'json') assert_response :success - post user_session_path(format: 'xml'), params: { user: {email: "user@test.com", password: '12345678'} } + post user_session_path(format: 'json'), params: { user: {email: "user@test.com", password: '12345678'} } assert_response :success - assert_includes response.body, %(\n) + assert_includes response.body, '{"user":{' end test 'sign out with html redirects' do @@ -527,13 +519,6 @@ class AuthenticationOthersTest < Devise::IntegrationTest assert_current_url '/' end - test 'sign out with xml format returns no content' do - sign_in_as_user - delete destroy_user_session_path(format: 'xml') - assert_response :no_content - refute warden.authenticated?(:user) - end - test 'sign out with json format returns no content' do sign_in_as_user delete destroy_user_session_path(format: 'json') diff --git a/test/integration/confirmable_test.rb b/test/integration/confirmable_test.rb index 16595461..278f9488 100644 --- a/test/integration/confirmable_test.rb +++ b/test/integration/confirmable_test.rb @@ -214,42 +214,34 @@ class ConfirmationTest < Devise::IntegrationTest end end - test 'resent confirmation token with valid E-Mail in XML format should return valid response' do + test 'resent confirmation token with valid e-mail in JSON format should return empty and valid response' do user = create_user(confirm: false) - post user_confirmation_path(format: 'xml'), params: { user: { email: user.email } } - assert_response :success - assert_equal({}.to_xml, response.body) - end - - test 'resent confirmation token with invalid E-Mail in XML format should return invalid response' do - create_user(confirm: false) - post user_confirmation_path(format: 'xml'), params: { user: { email: 'invalid.test@test.com' } } - assert_response :unprocessable_entity - assert_includes response.body, %(\n) - end - - test 'confirm account with valid confirmation token in XML format should return valid response' do - user = create_user(confirm: false) - get user_confirmation_path(confirmation_token: user.raw_confirmation_token, format: 'xml') - assert_response :success - assert_includes response.body, %(\n) - end - - test 'confirm account with invalid confirmation token in XML format should return invalid response' do - create_user(confirm: false) - get user_confirmation_path(confirmation_token: 'invalid_confirmation', format: 'xml') - assert_response :unprocessable_entity - assert_includes response.body, %(\n) - end - - test 'request an account confirmation account with JSON, should return an empty JSON' do - user = create_user(confirm: false) - - post user_confirmation_path, params: { user: { email: user.email }, format: :json } + post user_confirmation_path(format: 'json'), params: { user: { email: user.email } } assert_response :success assert_equal({}.to_json, response.body) end + test 'resent confirmation token with invalid e-mail in JSON format should return invalid response' do + create_user(confirm: false) + post user_confirmation_path(format: 'json'), params: { user: { email: 'invalid.test@test.com' } } + assert_response :unprocessable_entity + assert_includes response.body, '{"errors":{' + end + + test 'confirm account with valid confirmation token in JSON format should return valid response' do + user = create_user(confirm: false) + get user_confirmation_path(confirmation_token: user.raw_confirmation_token, format: 'json') + assert_response :success + assert_includes response.body, '{"user":{' + end + + test 'confirm account with invalid confirmation token in JSON format should return invalid response' do + create_user(confirm: false) + get user_confirmation_path(confirmation_token: 'invalid_confirmation', format: 'json') + assert_response :unprocessable_entity + assert_includes response.body, '{"confirmation_token":[' + end + test "when in paranoid mode and with a valid e-mail, should not say that the e-mail is valid" do swap Devise, paranoid: true do user = create_user(confirm: false) diff --git a/test/integration/http_authenticatable_test.rb b/test/integration/http_authenticatable_test.rb index 619a3cd8..68321595 100644 --- a/test/integration/http_authenticatable_test.rb +++ b/test/integration/http_authenticatable_test.rb @@ -22,10 +22,10 @@ class HttpAuthenticationTest < Devise::IntegrationTest swap Devise, skip_session_storage: [] do sign_in_as_new_user_with_http assert_response 200 - assert_match 'user@test.com', response.body + assert_match '"email":"user@test.com"', response.body assert warden.authenticated?(:user) - get users_path(format: :xml) + get users_path(format: :json) assert_response 200 end end @@ -34,10 +34,10 @@ class HttpAuthenticationTest < Devise::IntegrationTest swap Devise, skip_session_storage: [:http_auth] do sign_in_as_new_user_with_http assert_response 200 - assert_match 'user@test.com', response.body + assert_match '"email":"user@test.com"', response.body assert warden.authenticated?(:user) - get users_path(format: :xml) + get users_path(format: :json) assert_response 401 end end @@ -51,8 +51,8 @@ class HttpAuthenticationTest < Devise::IntegrationTest test 'uses the request format as response content type' do sign_in_as_new_user_with_http("unknown") assert_equal 401, status - assert_equal "application/xml; charset=utf-8", headers["Content-Type"] - assert_match "Invalid Email or password.", response.body + assert_equal "application/json; charset=utf-8", headers["Content-Type"] + assert_match '"error":"Invalid Email or password."', response.body end test 'returns a custom response with www-authenticate and chosen realm' do @@ -67,7 +67,7 @@ class HttpAuthenticationTest < Devise::IntegrationTest swap Devise, authentication_keys: [:username] do sign_in_as_new_user_with_http("usertest") assert_response :success - assert_match 'user@test.com', response.body + assert_match '"email":"user@test.com"', response.body assert warden.authenticated?(:user) end end @@ -76,7 +76,7 @@ class HttpAuthenticationTest < Devise::IntegrationTest swap Devise, authentication_keys: { username: false, email: false } do sign_in_as_new_user_with_http("usertest") assert_response :success - assert_match 'user@test.com', response.body + assert_match '"email":"user@test.com"', response.body assert warden.authenticated?(:user) end end @@ -85,7 +85,7 @@ class HttpAuthenticationTest < Devise::IntegrationTest swap Devise, authentication_keys: { email: false, username: false }, http_authentication_key: :username do sign_in_as_new_user_with_http("usertest") assert_response :success - assert_match 'user@test.com', response.body + assert_match '"email":"user@test.com"', response.body assert warden.authenticated?(:user) end end @@ -101,14 +101,13 @@ class HttpAuthenticationTest < Devise::IntegrationTest private def sign_in_as_new_user_with_http(username = "user@test.com", password = "12345678") user = create_user - get users_path(format: :xml), headers: { "HTTP_AUTHORIZATION" => "Basic #{Base64.encode64("#{username}:#{password}")}" } + get users_path(format: :json), headers: { "HTTP_AUTHORIZATION" => "Basic #{Base64.encode64("#{username}:#{password}")}" } user end # Sign in with oauth2 token. This is just to test that it isn't misinterpreted as basic authentication def add_oauth2_header user = create_user - get users_path(format: :xml), headers: { "HTTP_AUTHORIZATION" => "OAuth #{Base64.encode64("#{user.email}:12345678")}" } + get users_path(format: :json), headers: { "HTTP_AUTHORIZATION" => "OAuth #{Base64.encode64("#{user.email}:12345678")}" } end - end diff --git a/test/integration/lockable_test.rb b/test/integration/lockable_test.rb index b0eaf02f..437d8eec 100644 --- a/test/integration/lockable_test.rb +++ b/test/integration/lockable_test.rb @@ -130,46 +130,39 @@ class LockTest < Devise::IntegrationTest end end - test 'user should be able to request a new unlock token via XML request' do + test 'user should be able to request a new unlock token via JSON request and should return empty and valid response' do user = create_user(locked: true) ActionMailer::Base.deliveries.clear - post user_unlock_path(format: 'xml'), params: { user: {email: user.email} } + post user_unlock_path(format: 'json'), params: { user: {email: user.email} } assert_response :success - assert_equal({}.to_xml, response.body) + assert_equal({}.to_json, response.body) assert_equal 1, ActionMailer::Base.deliveries.size end - test 'unlocked user should not be able to request a unlock token via XML request' do + test 'unlocked user should not be able to request a unlock token via JSON request' do user = create_user(locked: false) ActionMailer::Base.deliveries.clear - post user_unlock_path(format: 'xml'), params: { user: {email: user.email} } + post user_unlock_path(format: 'json'), params: { user: {email: user.email} } assert_response :unprocessable_entity - assert_includes response.body, %(\n) + assert_includes response.body, '{"errors":{' assert_equal 0, ActionMailer::Base.deliveries.size end - test 'user with valid unlock token should be able to unlock account via XML request' do + test 'user with valid unlock token should be able to unlock account via JSON request' do user = create_user() raw = user.lock_access! assert user.access_locked? - get user_unlock_path(format: 'xml', unlock_token: raw) + get user_unlock_path(format: 'json', unlock_token: raw) assert_response :success - assert_includes response.body, %(\n) + assert_includes response.body, '{"user":{' end - test 'user with invalid unlock token should not be able to unlock the account via XML request' do - get user_unlock_path(format: 'xml', unlock_token: 'invalid_token') + test 'user with invalid unlock token should not be able to unlock the account via JSON request' do + get user_unlock_path(format: 'json', unlock_token: 'invalid_token') assert_response :unprocessable_entity - assert_includes response.body, %(\n) - end - - test "when using json to ask a unlock request, should not return the user" do - user = create_user(locked: true) - post user_unlock_path(format: "json", user: {email: user.email}) - assert_response :success - assert_equal({}.to_json, response.body) + assert_includes response.body, '{"unlock_token":[' end test "in paranoid mode, when trying to unlock a user that exists it should not say that it exists if it is locked" do diff --git a/test/integration/recoverable_test.rb b/test/integration/recoverable_test.rb index 2f1ca6e9..76266078 100644 --- a/test/integration/recoverable_test.rb +++ b/test/integration/recoverable_test.rb @@ -261,63 +261,53 @@ class PasswordTest < Devise::IntegrationTest end end - test 'reset password request with valid E-Mail in XML format should return valid response' do + test 'reset password request with valid e-mail in JSON format should return empty and valid response' do create_user - post user_password_path(format: 'xml'), params: { user: {email: "user@test.com"} } + post user_password_path(format: 'json'), params: { user: {email: "user@test.com"} } assert_response :success - assert_equal({}.to_xml, response.body) + assert_equal({}.to_json, response.body) end - test 'reset password request with invalid E-Mail in XML format should return valid response' do + test 'reset password request with invalid e-mail in JSON format should return valid response' do create_user - post user_password_path(format: 'xml'), params: { user: {email: "invalid.test@test.com"} } + post user_password_path(format: 'json'), params: { user: {email: "invalid.test@test.com"} } assert_response :unprocessable_entity - assert_includes response.body, %(\n) + assert_includes response.body, '{"errors":{' end - test 'reset password request with invalid E-Mail in XML format should return empty and valid response' do + test 'reset password request with invalid e-mail in JSON format should return empty and valid response in paranoid mode' do swap Devise, paranoid: true do create_user - post user_password_path(format: 'xml'), params: { user: {email: "invalid@test.com"} } + post user_password_path(format: 'json'), params: { user: {email: "invalid@test.com"} } assert_response :success - assert_equal({}.to_xml, response.body) + assert_equal({}.to_json, response.body) end end - test 'change password with valid parameters in XML format should return valid response' do + test 'change password with valid parameters in JSON format should return valid response' do create_user request_forgot_password - put user_password_path(format: 'xml'), params: { user: { + put user_password_path(format: 'json'), params: { user: { reset_password_token: 'abcdef', password: '987654321', password_confirmation: '987654321' - } - } + } } assert_response :success assert warden.authenticated?(:user) end - test 'change password with invalid token in XML format should return invalid response' do + test 'change password with invalid token in JSON format should return invalid response' do create_user request_forgot_password - put user_password_path(format: 'xml'), params: { user: {reset_password_token: 'invalid.token', password: '987654321', password_confirmation: '987654321'} } + put user_password_path(format: 'json'), params: { user: {reset_password_token: 'invalid.token', password: '987654321', password_confirmation: '987654321'} } assert_response :unprocessable_entity - assert_includes response.body, %(\n) + assert_includes response.body, '{"errors":{' end - test 'change password with invalid new password in XML format should return invalid response' do + test 'change password with invalid new password in JSON format should return invalid response' do user = create_user request_forgot_password - put user_password_path(format: 'xml'), params: { user: {reset_password_token: user.reload.reset_password_token, password: '', password_confirmation: '987654321'} } + put user_password_path(format: 'json'), params: { user: {reset_password_token: user.reload.reset_password_token, password: '', password_confirmation: '987654321'} } assert_response :unprocessable_entity - assert_includes response.body, %(\n) - end - - test "when using json requests to ask a confirmable request, should not return the object" do - user = create_user(confirm: false) - - post user_password_path(format: :json), params: { user: { email: user.email } } - - assert_response :success - assert_equal "{}", response.body + assert_includes response.body, '{"errors":{' end test "when in paranoid mode and with an invalid e-mail, asking to reset a password should display a message that does not indicates that the e-mail does not exists in the database" do diff --git a/test/integration/registerable_test.rb b/test/integration/registerable_test.rb index fa2610ed..b4072233 100644 --- a/test/integration/registerable_test.rb +++ b/test/integration/registerable_test.rb @@ -283,13 +283,6 @@ class RegistrationTest < Devise::IntegrationTest assert_redirected_to new_user_registration_path end - test 'a user with XML sign up stub' do - get new_user_registration_path(format: 'xml') - assert_response :success - assert_match %(\n), response.body - assert_no_match(/\n) + assert_includes response.body, '{"admin":{' admin = Admin.to_adapter.find_first(order: [:id, :desc]) assert_equal 'new_user@test.com', admin.email end - test 'a user sign up with valid information in XML format should return valid response' do - post user_registration_path(format: 'xml'), params: { user: { email: 'new_user@test.com', password: 'new_user123', password_confirmation: 'new_user123' } } + test 'a user sign up with valid information in JSON format should return valid response' do + post user_registration_path(format: 'json'), params: { user: { email: 'new_user@test.com', password: 'new_user123', password_confirmation: 'new_user123' } } assert_response :success - assert_includes response.body, %(\n) + assert_includes response.body, '{"user":{' user = User.to_adapter.find_first(order: [:id, :desc]) assert_equal 'new_user@test.com', user.email end - test 'a user sign up with invalid information in XML format should return invalid response' do - post user_registration_path(format: 'xml'), params: { user: { email: 'new_user@test.com', password: 'new_user123', password_confirmation: 'invalid' } } + test 'a user sign up with invalid information in JSON format should return invalid response' do + post user_registration_path(format: 'json'), params: { user: { email: 'new_user@test.com', password: 'new_user123', password_confirmation: 'invalid' } } assert_response :unprocessable_entity - assert_includes response.body, %(\n) + assert_includes response.body, '{"errors":{' end - test 'a user update information with valid data in XML format should return valid response' do + test 'a user update information with valid data in JSON format should return valid response' do user = sign_in_as_user - put user_registration_path(format: 'xml'), params: { user: { current_password: '12345678', email: 'user.new@test.com' } } + put user_registration_path(format: 'json'), params: { user: { current_password: '12345678', email: 'user.new@test.com' } } assert_response :success assert_equal 'user.new@test.com', user.reload.email end - test 'a user update information with invalid data in XML format should return invalid response' do + test 'a user update information with invalid data in JSON format should return invalid response' do user = sign_in_as_user - put user_registration_path(format: 'xml'), params: { user: { current_password: 'invalid', email: 'user.new@test.com' } } + put user_registration_path(format: 'json'), params: { user: { current_password: 'invalid', email: 'user.new@test.com' } } assert_response :unprocessable_entity assert_equal 'user@test.com', user.reload.email end - test 'a user cancel their account in XML format should return valid response' do + test 'a user cancel their account in JSON format should return valid response' do sign_in_as_user - delete user_registration_path(format: 'xml') + delete user_registration_path(format: 'json') assert_response :success assert_equal 0, User.to_adapter.find_all.size end diff --git a/test/models/serializable_test.rb b/test/models/serializable_test.rb index 602cbe37..53f0f59f 100644 --- a/test/models/serializable_test.rb +++ b/test/models/serializable_test.rb @@ -7,21 +7,6 @@ class SerializableTest < ActiveSupport::TestCase @user = create_user end - test 'should not include unsafe keys on XML' do - assert_match(/email/, @user.to_xml) - assert_no_match(/confirmation-token/, @user.to_xml) - end - - test 'should not include unsafe keys on XML even if a new except is provided' do - assert_no_match(/email/, @user.to_xml(except: :email)) - assert_no_match(/confirmation-token/, @user.to_xml(except: :email)) - end - - test 'should include unsafe keys on XML if a force_except is provided' do - assert_no_match(/