Skip to content

Commit

Permalink
Remove verbose warnings in test output (#1049)
Browse files Browse the repository at this point in the history
* Remove verbose warnings in test output

Fixes issue #976

* Ensure resource is pristine before locking

Fixes deprecation warnings in test runs when models have been
modified prior to locking in updating auth header.

Part of a fix for #976

* Suppress expected omniauth errors in tests

Fix for #976

* Refactoring resource pristine code

Code improvements for 2208ae3 noted in code climate
  • Loading branch information
asartalo authored and zachfeldman committed Dec 28, 2017
1 parent f83ec1d commit d206c40
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 42 deletions.
1 change: 1 addition & 0 deletions Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ Rake::TestTask.new(:test) do |t|
t.libs << 'test'
t.pattern = 'test/**/*_test.rb'
t.verbose = false
t.warning = false
end


Expand Down
85 changes: 49 additions & 36 deletions app/controllers/devise_token_auth/concerns/set_user_by_token.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,18 @@ def set_request_start
@is_batch_request = nil
end

def ensure_pristine_resource
if @resource.changed?
# Stash pending changes in the resource before reloading.
changes = @resource.changes
@resource.reload
end
yield
ensure
# Reapply pending changes
@resource.assign_attributes(changes) if changes
end

# user auth
def set_user_by_token(mapping=nil)
# determine target authentication class
Expand Down Expand Up @@ -100,42 +112,43 @@ def update_auth_header

else

# Lock the user record during any auth_header updates to ensure
# we don't have write contention from multiple threads
@resource.with_lock do
# should not append auth header if @resource related token was
# cleared by sign out in the meantime
return if @used_auth_by_token && @resource.tokens[@client_id].nil?

# determine batch request status after request processing, in case
# another processes has updated it during that processing
@is_batch_request = is_batch_request?(@resource, @client_id)

auth_header = {}

# extend expiration of batch buffer to account for the duration of
# this request
if @is_batch_request
auth_header = @resource.extend_batch_buffer(@token, @client_id)

# Do not return token for batch requests to avoid invalidated
# tokens returned to the client in case of race conditions.
# Use a blank string for the header to still be present and
# being passed in a XHR response in case of
# 304 Not Modified responses.
auth_header[DeviseTokenAuth.headers_names[:"access-token"]] = ' '
auth_header[DeviseTokenAuth.headers_names[:"expiry"]] = ' '

# update Authorization response header with new token
else
auth_header = @resource.create_new_auth_token(@client_id)
end

# update the response header
response.headers.merge!(auth_header)

end # end lock

ensure_pristine_resource do
# Lock the user record during any auth_header updates to ensure
# we don't have write contention from multiple threads
@resource.with_lock do
# should not append auth header if @resource related token was
# cleared by sign out in the meantime
return if @used_auth_by_token && @resource.tokens[@client_id].nil?

# determine batch request status after request processing, in case
# another processes has updated it during that processing
@is_batch_request = is_batch_request?(@resource, @client_id)

auth_header = {}

# extend expiration of batch buffer to account for the duration of
# this request
if @is_batch_request
auth_header = @resource.extend_batch_buffer(@token, @client_id)

# Do not return token for batch requests to avoid invalidated
# tokens returned to the client in case of race conditions.
# Use a blank string for the header to still be present and
# being passed in a XHR response in case of
# 304 Not Modified responses.
auth_header[DeviseTokenAuth.headers_names[:"access-token"]] = ' '
auth_header[DeviseTokenAuth.headers_names[:"expiry"]] = ' '

# update Authorization response header with new token
else
auth_header = @resource.create_new_auth_token(@client_id)
end

# update the response header
response.headers.merge!(auth_header)

end # end lock
end # end ensure_pristine_resource
end

end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -276,11 +276,13 @@ def get_success(params = {})
end

test 'renders expected data' do
get '/auth/facebook',
params: { auth_origin_url: @redirect_url,
omniauth_window_type: 'newWindow' }
silence_omniauth do
get '/auth/facebook',
params: { auth_origin_url: @redirect_url,
omniauth_window_type: 'newWindow' }

follow_all_redirects!
follow_all_redirects!
end

assert_equal 200, response.status

Expand All @@ -290,8 +292,10 @@ def get_success(params = {})
end

test 'renders something with no auth_origin_url' do
get '/auth/facebook'
follow_all_redirects!
silence_omniauth do
get '/auth/facebook'
follow_all_redirects!
end
assert_equal 200, response.status
assert_select 'body', 'invalid_credentials'
end
Expand Down
9 changes: 9 additions & 0 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,15 @@ def expire_token(user, client_id)
user.save!
end
end

# Suppress OmniAuth logger output
def silence_omniauth
previous_logger = OmniAuth.config.logger
OmniAuth.config.logger = Logger.new("/dev/null")
yield
ensure
OmniAuth.config.logger = previous_logger
end
end

class ActionController::TestCase
Expand Down

0 comments on commit d206c40

Please sign in to comment.