[ARVADOS] updated: 91b6e4f4ae93850fb3cc38cf8566115a66f44b27
Git user
git at public.curoverse.com
Thu Mar 24 13:59:39 EDT 2016
Summary of changes:
.../v1/api_client_authorizations_controller.rb | 33 +++++++++++++-----
.../api_client_authorizations_controller_test.rb | 40 +++++++++++++++++-----
2 files changed, 56 insertions(+), 17 deletions(-)
via 91b6e4f4ae93850fb3cc38cf8566115a66f44b27 (commit)
via 48fce933fec998722e669d7d33a62f3c0a10b05e (commit)
from f49a4d7ff243bb3e8b15f4c5adf77d6355fb6bcd (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit 91b6e4f4ae93850fb3cc38cf8566115a66f44b27
Merge: f49a4d7 48fce93
Author: Tom Clegg <tom at curoverse.com>
Date: Thu Mar 24 13:58:21 2016 -0400
Merge branch '8767-items-available'
refs #8767
commit 48fce933fec998722e669d7d33a62f3c0a10b05e
Author: Tom Clegg <tom at curoverse.com>
Date: Tue Mar 22 16:31:56 2016 -0400
8767: Make offset work properly in ApiClientAuthorizationsController#index.
Before this, #index was ignoring the "offset" request param and was
not providing an "items_available" attribute in the response. This
made Workbench's "get all pages" routine an infinite loop.
diff --git a/services/api/app/controllers/arvados/v1/api_client_authorizations_controller.rb b/services/api/app/controllers/arvados/v1/api_client_authorizations_controller.rb
index 56d0d85..83968be 100644
--- a/services/api/app/controllers/arvados/v1/api_client_authorizations_controller.rb
+++ b/services/api/app/controllers/arvados/v1/api_client_authorizations_controller.rb
@@ -69,14 +69,27 @@ class Arvados::V1::ApiClientAuthorizationsController < ApplicationController
val.is_a?(String) && (attr == 'uuid' || attr == 'api_token')
}
end
- @objects = model_class.
- includes(:user, :api_client).
- where('user_id=?', current_user.id)
- super
- wanted_scopes.compact.each do |scope_list|
- sorted_scopes = scope_list.sort
- @objects = @objects.select { |auth| auth.scopes.sort == sorted_scopes }
+ @objects = model_class.where('user_id=?', current_user.id)
+ if wanted_scopes.compact.any?
+ # We can't filter on scopes effectively using AR/postgres.
+ # Instead we get the entire result set, do our own filtering on
+ # scopes to get a list of UUIDs, then start a new query
+ # (restricted to the selected UUIDs) so super can apply the
+ # offset/limit/order params in the usual way.
+ @request_limit = @limit
+ @request_offset = @offset
+ @limit = @objects.count
+ @offset = 0
+ super
+ wanted_scopes.compact.each do |scope_list|
+ sorted_scopes = scope_list.sort
+ @objects = @objects.select { |auth| auth.scopes.sort == sorted_scopes }
+ end
+ @limit = @request_limit
+ @offset = @request_offset
+ @objects = model_class.where('uuid in (?)', @objects.collect(&:uuid))
end
+ super
end
def find_object_by_uuid
@@ -110,8 +123,10 @@ class Arvados::V1::ApiClientAuthorizationsController < ApplicationController
# The @filters test here also prevents a non-trusted token from
# filtering on its own scopes, and discovering whether any _other_
# equally scoped tokens exist (403=yes, 200=no).
- if (@objects.andand.count == 1 and
- @objects.first.uuid == current_api_client_authorization.andand.uuid and
+ return forbidden if !@objects
+ full_set = @objects.except(:limit).except(:offset) if @objects
+ if (full_set.count == 1 and
+ full_set.first.uuid == current_api_client_authorization.andand.uuid and
(@filters.map(&:first) & %w(uuid api_token)).any?)
return true
end
diff --git a/services/api/test/functional/arvados/v1/api_client_authorizations_controller_test.rb b/services/api/test/functional/arvados/v1/api_client_authorizations_controller_test.rb
index 192e6b9..9f0f555 100644
--- a/services/api/test/functional/arvados/v1/api_client_authorizations_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/api_client_authorizations_controller_test.rb
@@ -38,9 +38,11 @@ class Arvados::V1::ApiClientAuthorizationsControllerTest < ActionController::Tes
assert_response 403
end
- def assert_found_tokens(auth, search_params, *expected_tokens)
+ def assert_found_tokens(auth, search_params, expected)
authorize_with auth
- expected_tokens.map! { |name| api_client_authorizations(name).api_token }
+ expected_tokens = expected.map do |name|
+ api_client_authorizations(name).api_token
+ end
get :index, search_params
assert_response :success
got_tokens = JSON.parse(@response.body)['items']
@@ -52,19 +54,26 @@ class Arvados::V1::ApiClientAuthorizationsControllerTest < ActionController::Tes
# Three-tuples with auth to use, scopes to find, and expected tokens.
# Make two tests for each tuple, one searching with where and the other
# with filter.
- [[:admin_trustedclient, [], :admin_noscope],
- [:active_trustedclient, ["GET /arvados/v1/users"], :active_userlist],
+ [[:admin_trustedclient, [], [:admin_noscope]],
+ [:active_trustedclient, ["GET /arvados/v1/users"], [:active_userlist]],
[:active_trustedclient,
["POST /arvados/v1/api_client_authorizations",
"GET /arvados/v1/api_client_authorizations"],
- :active_apitokens],
- ].each do |auth, scopes, *expected|
+ [:active_apitokens]],
+ ].each do |auth, scopes, expected|
test "#{auth.to_s} can find auths where scopes=#{scopes.inspect}" do
- assert_found_tokens(auth, {where: {scopes: scopes}}, *expected)
+ assert_found_tokens(auth, {where: {scopes: scopes}}, expected)
end
test "#{auth.to_s} can find auths filtered with scopes=#{scopes.inspect}" do
- assert_found_tokens(auth, {filters: [['scopes', '=', scopes]]}, *expected)
+ assert_found_tokens(auth, {filters: [['scopes', '=', scopes]]}, expected)
+ end
+
+ test "#{auth.to_s} offset works with filter scopes=#{scopes.inspect}" do
+ assert_found_tokens(auth, {
+ offset: expected.length,
+ filters: [['scopes', '=', scopes]]
+ }, [])
end
end
@@ -112,6 +121,20 @@ class Arvados::V1::ApiClientAuthorizationsControllerTest < ActionController::Tes
assert_response expect_list_response
if expect_list_items
assert_equal assigns(:objects).length, expect_list_items
+ assert_equal json_response['items_available'], expect_list_items
+ end
+ end
+
+ if expect_list_items
+ test "using '#{user}', list '#{token}' by uuid with offset" do
+ authorize_with user
+ get :index, {
+ filters: [['uuid','=',api_client_authorizations(token).uuid]],
+ offset: expect_list_items,
+ }
+ assert_response expect_list_response
+ assert_equal json_response['items_available'], expect_list_items
+ assert_equal json_response['items'].length, 0
end
end
@@ -123,6 +146,7 @@ class Arvados::V1::ApiClientAuthorizationsControllerTest < ActionController::Tes
assert_response expect_list_response
if expect_list_items
assert_equal assigns(:objects).length, expect_list_items
+ assert_equal json_response['items_available'], expect_list_items
end
end
end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list