[ARVADOS] updated: 7fc48a11da8740deb01b0063faa2ceb687709205
git at public.curoverse.com
git at public.curoverse.com
Tue Apr 22 00:53:03 EDT 2014
Summary of changes:
apps/workbench/app/models/arvados_api_client.rb | 2 +-
apps/workbench/app/models/group.rb | 4 +
apps/workbench/app/models/user.rb | 5 ++
apps/workbench/test/test_helper.rb | 16 ++++-
apps/workbench/test/unit/user_test.rb | 13 +++-
doc/api/methods/groups.html.textile.liquid | 12 ++++
doc/api/methods/users.html.textile.liquid | 12 ++++
.../api/app/controllers/application_controller.rb | 62 +++++++++++++++-----
services/api/test/fixtures/links.yml | 2 +-
.../arvados/v1/groups_controller_test.rb | 25 ++++++++
.../functional/arvados/v1/users_controller_test.rb | 47 +++++++++++++++
11 files changed, 175 insertions(+), 25 deletions(-)
via 7fc48a11da8740deb01b0063faa2ceb687709205 (commit)
via 694e4fdd124150f1b0a237ce6a698f3d00d92eb9 (commit)
via 7efcc87e3cba4f03429751f9f3d109cc88e6926c (commit)
via 75fa6afdd3f0f85d48e7a95372dd8cf094811221 (commit)
via d2d9ff48a6c111293340ac351f94428e5204366f (commit)
from d325f035a861d1421f6e2fe0d2f01e9f3e93d749 (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 7fc48a11da8740deb01b0063faa2ceb687709205
Author: Tom Clegg <tom at curoverse.com>
Date: Tue Apr 22 00:52:42 2014 -0400
Build response objects based on item type rather than list type.
diff --git a/apps/workbench/app/models/arvados_api_client.rb b/apps/workbench/app/models/arvados_api_client.rb
index 39036bc..cf14106 100644
--- a/apps/workbench/app/models/arvados_api_client.rb
+++ b/apps/workbench/app/models/arvados_api_client.rb
@@ -108,7 +108,7 @@ class ArvadosApiClient
def unpack_api_response(j, kind=nil)
if j.is_a? Hash and j[:items].is_a? Array and j[:kind].match(/(_list|List)$/)
- ary = j[:items].collect { |x| unpack_api_response x, j[:kind] }
+ ary = j[:items].collect { |x| unpack_api_response x, x[:kind] }
self.class.patch_paging_vars(ary, j[:items_available], j[:offset], j[:limit])
elsif j.is_a? Hash and (kind || j[:kind])
oclass = self.kind_class(kind || j[:kind])
commit 694e4fdd124150f1b0a237ce6a698f3d00d92eb9
Author: Tom Clegg <tom at curoverse.com>
Date: Tue Apr 22 00:51:56 2014 -0400
Add unit test for User#owned_items.
diff --git a/apps/workbench/app/models/group.rb b/apps/workbench/app/models/group.rb
index dd04c00..f53a6f4 100644
--- a/apps/workbench/app/models/group.rb
+++ b/apps/workbench/app/models/group.rb
@@ -1,2 +1,6 @@
class Group < ArvadosBase
+ def self.owned_items
+ res = $arvados_api_client.api self, "/#{self.uuid}/owned_items", {}
+ $arvados_api_client.unpack_api_response(res)
+ end
end
diff --git a/apps/workbench/app/models/user.rb b/apps/workbench/app/models/user.rb
index 44d615b..c03e317 100644
--- a/apps/workbench/app/models/user.rb
+++ b/apps/workbench/app/models/user.rb
@@ -17,6 +17,11 @@ class User < ArvadosBase
end
end
+ def owned_items
+ res = $arvados_api_client.api self.class, "/#{self.uuid}/owned_items"
+ $arvados_api_client.unpack_api_response(res)
+ end
+
def full_name
(self.first_name || "") + " " + (self.last_name || "")
end
diff --git a/apps/workbench/test/test_helper.rb b/apps/workbench/test/test_helper.rb
index 145914f..cbbf562 100644
--- a/apps/workbench/test/test_helper.rb
+++ b/apps/workbench/test/test_helper.rb
@@ -6,13 +6,21 @@ $ARV_API_SERVER_DIR = File.expand_path('../../../../services/api', __FILE__)
SERVER_PID_PATH = 'tmp/pids/server.pid'
class ActiveSupport::TestCase
- # Setup all fixtures in test/fixtures/*.(yml|csv) for all tests in alphabetical order.
+ # Setup all fixtures in test/fixtures/*.(yml|csv) for all tests in
+ # alphabetical order.
#
- # Note: You'll currently still have to declare fixtures explicitly in integration tests
- # -- they do not yet inherit this setting
+ # Note: You'll currently still have to declare fixtures explicitly
+ # in integration tests -- they do not yet inherit this setting
fixtures :all
+ def use_token token_name
+ auth = api_fixture('api_client_authorizations')[token_name.to_s]
+ Thread.current[:arvados_api_token] = auth['api_token']
+ end
- # Add more helper methods to be used by all tests here...
+ def teardown
+ Thread.current[:arvados_api_token] = nil
+ super
+ end
end
module ApiFixtureLoader
diff --git a/apps/workbench/test/unit/user_test.rb b/apps/workbench/test/unit/user_test.rb
index 82f61e0..8c10b7e 100644
--- a/apps/workbench/test/unit/user_test.rb
+++ b/apps/workbench/test/unit/user_test.rb
@@ -1,7 +1,14 @@
require 'test_helper'
class UserTest < ActiveSupport::TestCase
- # test "the truth" do
- # assert true
- # end
+ test "get owned_items" do
+ use_token :active
+ oi = User.find(api_fixture('users')['active']['uuid']).owned_items
+ assert_operator 0, :<, oi.count
+ assert_operator 0, :<, oi.items_available
+ oi_uuids = oi.collect { |i| i['uuid'] }
+ expect = api_fixture('specimens')['owned_by_active_user']['uuid']
+ assert_includes(oi_uuids, expect,
+ "Expected active user's owned_items to include #{expect}")
+ end
end
commit 7efcc87e3cba4f03429751f9f3d109cc88e6926c
Author: Tom Clegg <tom at curoverse.com>
Date: Mon Apr 21 23:55:06 2014 -0400
Add docs for owned_items.
diff --git a/doc/api/methods/groups.html.textile.liquid b/doc/api/methods/groups.html.textile.liquid
index e09b817..bf8b0be 100644
--- a/doc/api/methods/groups.html.textile.liquid
+++ b/doc/api/methods/groups.html.textile.liquid
@@ -76,6 +76,18 @@ table(table table-bordered table-condensed).
|q|string|Query string for searching groups.|query||
|where|object|Conditions for filtering groups.|query||
+h2. owned_items
+
+Retrieve a list of items which are owned by the given group.
+
+Arguments:
+
+table(table table-bordered table-condensed).
+|_. Argument |_. Type |_. Description |_. Location |_. Example |
+{background:#ccffcc}.|uuid|string|The UUID of the group in question.|path||
+|include_indirect|boolean|If true, results will include items on which the given group has _can_manage_ permission, although they are owned by different users/groups.|path|{white-space:nowrap}. @false@ (default)
+ at true@|
+
h2. show
show groups
diff --git a/doc/api/methods/users.html.textile.liquid b/doc/api/methods/users.html.textile.liquid
index 649b368..8908ab0 100644
--- a/doc/api/methods/users.html.textile.liquid
+++ b/doc/api/methods/users.html.textile.liquid
@@ -95,6 +95,18 @@ table(table table-bordered table-condensed).
|q|string|Query string for searching users.|query||
|where|object|Conditions for filtering users.|query||
+h2. owned_items
+
+Retrieve a list of items which are owned by the given user.
+
+Arguments:
+
+table(table table-bordered table-condensed).
+|_. Argument |_. Type |_. Description |_. Location |_. Example |
+{background:#ccffcc}.|uuid|string|The UUID of the user in question.|path||
+|include_indirect|boolean|If true, results will include items on which the given user has _can_manage_ permission, although they are owned by different users/groups.|path|{white-space:nowrap}. @false@ (default)
+ at true@|
+
h2. show
show users
commit 75fa6afdd3f0f85d48e7a95372dd8cf094811221
Author: Tom Clegg <tom at curoverse.com>
Date: Mon Apr 21 23:50:22 2014 -0400
Rename include_managed to include_indirect.
diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 9826ee9..2e3c4eb 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -62,7 +62,7 @@ class ApplicationController < ActionController::Base
def self._owned_items_requires_parameters
_index_requires_parameters.
merge({
- include_managed: {
+ include_indirect: {
type: 'boolean', required: false, default: false
},
})
@@ -98,7 +98,7 @@ class ApplicationController < ActionController::Base
@objects = klass.readable_by(current_user)
cond_sql = "#{klass.table_name}.owner_uuid = ?"
cond_params = [@object.uuid]
- if params[:include_managed]
+ if params[:include_indirect]
@objects = @objects.
joins("LEFT JOIN links mng_links"\
" ON mng_links.link_class=#{klass.sanitize 'permission'}"\
diff --git a/services/api/test/functional/arvados/v1/groups_controller_test.rb b/services/api/test/functional/arvados/v1/groups_controller_test.rb
index f2ec649..d9cbac0 100644
--- a/services/api/test/functional/arvados/v1/groups_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/groups_controller_test.rb
@@ -75,7 +75,7 @@ class Arvados::V1::GroupsControllerTest < ActionController::TestCase
assert_equal 0, jresponse['items_available']
end
- test 'get group-owned objects without include_managed' do
+ test 'get group-owned objects without include_indirect' do
unexpected_uuid = specimens(:in_afolder_linked_from_asubfolder).uuid
authorize_with :active
get :owned_items, {
@@ -87,12 +87,12 @@ class Arvados::V1::GroupsControllerTest < ActionController::TestCase
assert_equal nil, uuids.index(unexpected_uuid)
end
- test 'get group-owned objects with include_managed' do
+ test 'get group-owned objects with include_indirect' do
expected_uuid = specimens(:in_afolder_linked_from_asubfolder).uuid
authorize_with :active
get :owned_items, {
id: groups(:asubfolder).uuid,
- include_managed: true,
+ include_indirect: true,
format: :json,
}
assert_response :success
diff --git a/services/api/test/functional/arvados/v1/users_controller_test.rb b/services/api/test/functional/arvados/v1/users_controller_test.rb
index e805b24..2639516 100644
--- a/services/api/test/functional/arvados/v1/users_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/users_controller_test.rb
@@ -896,8 +896,8 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
assert_equal expect_kinds, (expect_kinds & kinds)
end
- [false, true].each do |inc_mgd|
- test "get all pages of user-owned #{'and -managed ' if inc_mgd}objects" do
+ [false, true].each do |inc_ind|
+ test "get all pages of user-owned #{'and -indirect ' if inc_ind}objects" do
authorize_with :active
limit = 5
offset = 0
@@ -910,7 +910,7 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
@jresponse = nil
get :owned_items, {
id: users(:active).uuid,
- include_managed: inc_mgd,
+ include_indirect: inc_ind,
limit: limit,
offset: offset,
format: :json,
@@ -930,15 +930,15 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
uuid_received[uuid] = true
owner_received[item['owner_uuid']] = true
offset += 1
- if not inc_mgd
+ if not inc_ind
assert_equal users(:active).uuid, item['owner_uuid']
end
end
break if offset >= items_available
end
- if inc_mgd
+ if inc_ind
assert_operator 0, :<, (jresponse.keys - [users(:active).uuid]).count,
- "Set include_managed=true but did not receive any managed items"
+ "Set include_indirect=true but did not receive any indirect items"
end
end
end
commit d2d9ff48a6c111293340ac351f94428e5204366f
Author: Tom Clegg <tom at curoverse.com>
Date: Mon Apr 21 23:39:20 2014 -0400
Support limit and offset params in owned_items.
diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 4836a73..9826ee9 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -59,6 +59,15 @@ class ApplicationController < ActionController::Base
show
end
+ def self._owned_items_requires_parameters
+ _index_requires_parameters.
+ merge({
+ include_managed: {
+ type: 'boolean', required: false, default: false
+ },
+ })
+ end
+
def owned_items
all_objects = []
all_available = 0
@@ -66,33 +75,51 @@ class ApplicationController < ActionController::Base
# We stuffed params[:uuid] into @where in find_object_by_uuid,
# but we don't want it there any more.
@where = {}
- # Order, limit, offset don't work here.
+
+ # Trick apply_where_limit_order_params into applying suitable
+ # per-table values. *_all are the real ones we'll apply to the
+ # aggregate set.
limit_all = @limit
- @limit = DEFAULT_LIMIT
offset_all = @offset
- @offset = 0
- orders_all = @orders
@orders = []
- ArvadosModel.descendants.reject(&:abstract_class?).sort_by(&:to_s).each do |klass|
+ ArvadosModel.descendants.
+ reject(&:abstract_class?).
+ sort_by(&:to_s).
+ each do |klass|
case klass.to_s
- when *%w(ApiClientAuthorization Link ApiClient)
+ # We might expect klass==Link etc. here, but we would be
+ # disappointed: when Rails reloads model classes, we get two
+ # distinct classes called Link which do not equal each
+ # other. But we can still rely on klass.to_s to be "Link".
+ when 'ApiClientAuthorization'
# Do not want.
else
- @objects = klass.
- readable_by(current_user).
- where(owner_uuid: @object.uuid)
+ @objects = klass.readable_by(current_user)
+ cond_sql = "#{klass.table_name}.owner_uuid = ?"
+ cond_params = [@object.uuid]
+ if params[:include_managed]
+ @objects = @objects.
+ joins("LEFT JOIN links mng_links"\
+ " ON mng_links.link_class=#{klass.sanitize 'permission'}"\
+ " AND mng_links.name=#{klass.sanitize 'can_manage'}"\
+ " AND mng_links.tail_uuid=#{klass.sanitize @object.uuid}"\
+ " AND mng_links.head_uuid=#{klass.table_name}.uuid")
+ cond_sql += " OR mng_links.uuid IS NOT NULL"
+ end
+ @objects = @objects.where(cond_sql, *cond_params)
+ @limit = limit_all - all_objects.count
apply_where_limit_order_params
- # TODO: follow links, too
- all_available += @objects.
+ items_available = @objects.
except(:limit).except(:offset).
count(:id, distinct: true)
- if all_objects.length < limit_all + offset_all
- all_objects += @objects.to_a
- end
+ all_available += items_available
+ @offset = [@offset - items_available, 0].max
+
+ all_objects += @objects.to_a
end
end
- @objects = all_objects[offset_all..(offset_all+limit_all-1)] || []
+ @objects = all_objects || []
@object_list = {
:kind => "arvados#objectList",
:etag => "",
@@ -459,6 +486,7 @@ class ApplicationController < ActionController::Base
@limit = 1
@orders = []
@filters = []
+ @objects = nil
find_objects_for_index
@object = @objects.first
end
@@ -525,7 +553,9 @@ class ApplicationController < ActionController::Base
{
filters: { type: 'array', required: false },
where: { type: 'object', required: false },
- order: { type: 'string', required: false }
+ order: { type: 'string', required: false },
+ limit: { type: 'integer', required: false, default: DEFAULT_LIMIT },
+ offset: { type: 'integer', required: false, default: 0 },
}
end
diff --git a/services/api/test/fixtures/links.yml b/services/api/test/fixtures/links.yml
index 46f4afe..ac1494b 100644
--- a/services/api/test/fixtures/links.yml
+++ b/services/api/test/fixtures/links.yml
@@ -298,7 +298,7 @@ specimen_is_in_two_folders:
modified_by_user_uuid: zzzzz-tpzed-xurymjxw79nv3jz
modified_at: 2014-04-21 15:37:48 -0400
updated_at: 2014-04-21 15:37:48 -0400
- tail_uuid: zzzzz-j7d0g-v955i6s2oi1cbso
+ tail_uuid: zzzzz-j7d0g-axqo7eu9pwvna1x
head_uuid: zzzzz-2x53u-5gid26432uujf79
link_class: permission
name: can_manage
diff --git a/services/api/test/functional/arvados/v1/groups_controller_test.rb b/services/api/test/functional/arvados/v1/groups_controller_test.rb
index 5b0fd59..f2ec649 100644
--- a/services/api/test/functional/arvados/v1/groups_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/groups_controller_test.rb
@@ -74,4 +74,29 @@ class Arvados::V1::GroupsControllerTest < ActionController::TestCase
assert_equal [], jresponse['items']
assert_equal 0, jresponse['items_available']
end
+
+ test 'get group-owned objects without include_managed' do
+ unexpected_uuid = specimens(:in_afolder_linked_from_asubfolder).uuid
+ authorize_with :active
+ get :owned_items, {
+ id: groups(:asubfolder).uuid,
+ format: :json,
+ }
+ assert_response :success
+ uuids = jresponse['items'].collect { |i| i['uuid'] }
+ assert_equal nil, uuids.index(unexpected_uuid)
+ end
+
+ test 'get group-owned objects with include_managed' do
+ expected_uuid = specimens(:in_afolder_linked_from_asubfolder).uuid
+ authorize_with :active
+ get :owned_items, {
+ id: groups(:asubfolder).uuid,
+ include_managed: true,
+ format: :json,
+ }
+ assert_response :success
+ uuids = jresponse['items'].collect { |i| i['uuid'] }
+ assert_includes uuids, expected_uuid, "Did not get #{expected_uuid}"
+ end
end
diff --git a/services/api/test/functional/arvados/v1/users_controller_test.rb b/services/api/test/functional/arvados/v1/users_controller_test.rb
index c466e90..e805b24 100644
--- a/services/api/test/functional/arvados/v1/users_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/users_controller_test.rb
@@ -885,6 +885,7 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
authorize_with :active
get :owned_items, {
id: users(:active).uuid,
+ limit: 500,
format: :json,
}
assert_response :success
@@ -895,4 +896,50 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
assert_equal expect_kinds, (expect_kinds & kinds)
end
+ [false, true].each do |inc_mgd|
+ test "get all pages of user-owned #{'and -managed ' if inc_mgd}objects" do
+ authorize_with :active
+ limit = 5
+ offset = 0
+ items_available = nil
+ uuid_received = {}
+ owner_received = {}
+ while true
+ # Behaving badly here, using the same controller multiple
+ # times within a test.
+ @jresponse = nil
+ get :owned_items, {
+ id: users(:active).uuid,
+ include_managed: inc_mgd,
+ limit: limit,
+ offset: offset,
+ format: :json,
+ }
+ assert_response :success
+ assert_operator(0, :<, jresponse['items'].count,
+ "items_available=#{items_available} but received 0 "\
+ "items with offset=#{offset}")
+ items_available ||= jresponse['items_available']
+ assert_equal(items_available, jresponse['items_available'],
+ "items_available changed between page #{offset/limit} "\
+ "and page #{1+offset/limit}")
+ jresponse['items'].each do |item|
+ uuid = item['uuid']
+ assert_equal(nil, uuid_received[uuid],
+ "Received '#{uuid}' again on page #{1+offset/limit}")
+ uuid_received[uuid] = true
+ owner_received[item['owner_uuid']] = true
+ offset += 1
+ if not inc_mgd
+ assert_equal users(:active).uuid, item['owner_uuid']
+ end
+ end
+ break if offset >= items_available
+ end
+ if inc_mgd
+ assert_operator 0, :<, (jresponse.keys - [users(:active).uuid]).count,
+ "Set include_managed=true but did not receive any managed items"
+ end
+ end
+ end
end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list