[ARVADOS] updated: c0aeec88f6507f441796d25ad2dfb49c78185731
git at public.curoverse.com
git at public.curoverse.com
Wed Apr 23 16:44:52 EDT 2014
Summary of changes:
apps/workbench/test/unit/user_test.rb | 6 ++-
doc/api/methods/groups.html.textile.liquid | 2 +-
doc/api/methods/users.html.textile.liquid | 2 +-
doc/api/schema/Group.html.textile.liquid | 3 +-
doc/api/schema/Link.html.textile.liquid | 6 ++--
.../api/app/controllers/application_controller.rb | 26 +++++++------
.../arvados/v1/groups_controller_test.rb | 37 +++++++++++++++++---
.../functional/arvados/v1/users_controller_test.rb | 20 +++++++++--
8 files changed, 74 insertions(+), 28 deletions(-)
via c0aeec88f6507f441796d25ad2dfb49c78185731 (commit)
via d6399c117f03f45e83648dca6e23dd7174793253 (commit)
via 8905037aede017ccdb66dd2850bfbd2284904e4e (commit)
from 7fc48a11da8740deb01b0063faa2ceb687709205 (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 c0aeec88f6507f441796d25ad2dfb49c78185731
Author: Tom Clegg <tom at curoverse.com>
Date: Wed Apr 23 16:45:21 2014 -0400
Support filters=[["attr","=",nil]]
diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 77a4071..74f6bb2 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -268,11 +268,19 @@ class ApplicationController < ActionController::Base
operand = Time.parse operand
end
param_out << operand
+ elsif operand.nil? and operator == '='
+ cond_out << "#{ar_table_name}.#{attr} is null"
+ else
+ raise ArgumentError.new("Invalid operand type '#{attr.class}' "\
+ "for '#{operator}' operator in filters")
end
when 'in'
if operand.is_a? Array
cond_out << "#{ar_table_name}.#{attr} IN (?)"
param_out << operand
+ else
+ raise ArgumentError.new("Invalid operand type '#{attr.class}' "\
+ "for '#{operator}' operator in filters")
end
when 'is_a'
operand = [operand] unless operand.is_a? Array
commit d6399c117f03f45e83648dca6e23dd7174793253
Author: Tom Clegg <tom at curoverse.com>
Date: Wed Apr 23 16:39:03 2014 -0400
Add tests, rename include_indirect to _linked, improve wording in
docs, improve offset/limit validation
diff --git a/doc/api/methods/groups.html.textile.liquid b/doc/api/methods/groups.html.textile.liquid
index bf8b0be..7f52bc7 100644
--- a/doc/api/methods/groups.html.textile.liquid
+++ b/doc/api/methods/groups.html.textile.liquid
@@ -85,7 +85,7 @@ 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)
+|include_linked|boolean|If true, results will also include items on which the given group has _can_manage_ permission, even if they are owned by different users/groups.|path|{white-space:nowrap}. @false@ (default)
@true@|
h2. show
diff --git a/doc/api/methods/users.html.textile.liquid b/doc/api/methods/users.html.textile.liquid
index 8908ab0..5fcc4c8 100644
--- a/doc/api/methods/users.html.textile.liquid
+++ b/doc/api/methods/users.html.textile.liquid
@@ -104,7 +104,7 @@ 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)
+|include_linked|boolean|If true, results will also include items on which the given user has _can_manage_ permission, even if they are owned by different users/groups.|path|{white-space:nowrap}. @false@ (default)
@true@|
h2. show
diff --git a/doc/api/schema/Group.html.textile.liquid b/doc/api/schema/Group.html.textile.liquid
index 9d29ba0..624a930 100644
--- a/doc/api/schema/Group.html.textile.liquid
+++ b/doc/api/schema/Group.html.textile.liquid
@@ -31,6 +31,7 @@ Each Group has, in addition to the usual "attributes of Arvados resources":{{sit
table(table table-bordered table-condensed).
|_. Attribute|_. Type|_. Description|_. Example|
|name|string|||
-|group_class|string|Type of group. This does not affect behavior, but determines how the group is presented in the user interface.|@folder@|
+|group_class|string|Type of group. This does not affect behavior, but determines how the group is presented in the user interface. For example, @folder@ indicates that the group should be displayed by Workbench and arv-mount as a folder for organizing and naming objects.|@"folder"@
+null|
|description|text|||
|updated_at|datetime|||
diff --git a/doc/api/schema/Link.html.textile.liquid b/doc/api/schema/Link.html.textile.liquid
index 6209a8c..5660bec 100644
--- a/doc/api/schema/Link.html.textile.liquid
+++ b/doc/api/schema/Link.html.textile.liquid
@@ -53,9 +53,9 @@ table(table table-bordered table-condensed).
|_. tail_type→head_type|_. name→head_uuid {properties}|_. Notes|
|User→Group |{white-space:nowrap}. can_manage → _group uuid_|The User can read, write, and control permissions on the Group itself, every object owned by the Group, and every object on which the Group has _can_manage_ permission.|
|User→Group |can_read → _group uuid_ |The User can retrieve the Group itself and every object that is readable by the Group.|
-|User→Job|can_write → _job uuid_ |The User can read and update the Job. (This works for all object types, not just jobs.)|
-|User→Job|can_manage → _job uuid_ |The User can read, update, and change permissions for the Job. (This works for all object types, not just jobs.)|
-|Group→Job|can_manage → _job uuid_ |Anyone with _can_manage_ permission on the Group can also read, update, and change permissions for the Job. Anyone with _can_read_ permission on the Group can read the Job. (This works for all object types, not just jobs.)|
+|User→Job|can_write → _job uuid_ |The User can read and update the Job. (This works for all objects, not just jobs.)|
+|User→Job|can_manage → _job uuid_ |The User can read, update, and change permissions for the Job. (This works for all objects, not just jobs.)|
+|Group→Job|can_manage → _job uuid_ |Anyone with _can_manage_ permission on the Group can also read, update, and change permissions for the Job. Anyone with _can_read_ permission on the Group can read the Job. (This works for all objects, not just jobs.)|
h3. resources
diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 2e3c4eb..77a4071 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_indirect: {
+ include_linked: {
type: 'boolean', required: false, default: false
},
})
@@ -72,10 +72,6 @@ class ApplicationController < ActionController::Base
all_objects = []
all_available = 0
- # We stuffed params[:uuid] into @where in find_object_by_uuid,
- # but we don't want it there any more.
- @where = {}
-
# Trick apply_where_limit_order_params into applying suitable
# per-table values. *_all are the real ones we'll apply to the
# aggregate set.
@@ -98,7 +94,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_indirect]
+ if params[:include_linked]
@objects = @objects.
joins("LEFT JOIN links mng_links"\
" ON mng_links.link_class=#{klass.sanitize 'permission'}"\
@@ -213,21 +209,19 @@ class ApplicationController < ActionController::Base
def load_limit_offset_order_params
if params[:limit]
- begin
- @limit = params[:limit].to_i
- rescue
+ unless params[:limit].to_s.match(/^\d+$/)
raise ArgumentError.new("Invalid value for limit parameter")
end
+ @limit = params[:limit].to_i
else
@limit = DEFAULT_LIMIT
end
if params[:offset]
- begin
- @offset = params[:offset].to_i
- rescue
+ unless params[:offset].to_s.match(/^\d+$/)
raise ArgumentError.new("Invalid value for offset parameter")
end
+ @offset = params[:offset].to_i
else
@offset = 0
end
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 d9cbac0..017984c 100644
--- a/services/api/test/functional/arvados/v1/groups_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/groups_controller_test.rb
@@ -23,8 +23,35 @@ class Arvados::V1::GroupsControllerTest < ActionController::TestCase
assert_equal 'folder', group['group_class']
group_uuids << group['uuid']
end
- assert_not_nil group_uuids.index groups(:afolder).uuid
- assert_not_nil group_uuids.index groups(:asubfolder).uuid
+ assert_includes group_uuids, groups(:afolder).uuid
+ assert_includes group_uuids, groups(:asubfolder).uuid
+ assert_not_includes group_uuids, groups(:system_group).uuid
+ assert_not_includes group_uuids, groups(:private).uuid
+ end
+
+ test "get list of groups that are not folders" do
+ authorize_with :active
+ get :index, filters: [['group_class', '=', nil]], format: :json
+ assert_response :success
+ group_uuids = []
+ jresponse['items'].each do |group|
+ assert_equal nil, group['group_class']
+ group_uuids << group['uuid']
+ end
+ assert_not_includes group_uuids, groups(:afolder).uuid
+ assert_not_includes group_uuids, groups(:asubfolder).uuid
+ assert_includes group_uuids, groups(:private).uuid
+ end
+
+ test "get list of groups with bogus group_class" do
+ authorize_with :active
+ get :index, {
+ filters: [['group_class', '=', 'nogrouphasthislittleclass']],
+ format: :json,
+ }
+ assert_response :success
+ assert_equal [], jresponse['items']
+ assert_equal 0, jresponse['items_available']
end
test 'get group-owned objects' do
@@ -75,7 +102,7 @@ class Arvados::V1::GroupsControllerTest < ActionController::TestCase
assert_equal 0, jresponse['items_available']
end
- test 'get group-owned objects without include_indirect' do
+ test 'get group-owned objects without include_linked' do
unexpected_uuid = specimens(:in_afolder_linked_from_asubfolder).uuid
authorize_with :active
get :owned_items, {
@@ -87,12 +114,12 @@ class Arvados::V1::GroupsControllerTest < ActionController::TestCase
assert_equal nil, uuids.index(unexpected_uuid)
end
- test 'get group-owned objects with include_indirect' do
+ test 'get group-owned objects with include_linked' do
expected_uuid = specimens(:in_afolder_linked_from_asubfolder).uuid
authorize_with :active
get :owned_items, {
id: groups(:asubfolder).uuid,
- include_indirect: true,
+ include_linked: 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 2639516..948b601 100644
--- a/services/api/test/functional/arvados/v1/users_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/users_controller_test.rb
@@ -897,7 +897,7 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
end
[false, true].each do |inc_ind|
- test "get all pages of user-owned #{'and -indirect ' if inc_ind}objects" do
+ test "get all pages of user-owned #{'and -linked ' 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_indirect: inc_ind,
+ include_linked: inc_ind,
limit: limit,
offset: offset,
format: :json,
@@ -938,7 +938,21 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
end
if inc_ind
assert_operator 0, :<, (jresponse.keys - [users(:active).uuid]).count,
- "Set include_indirect=true but did not receive any indirect items"
+ "Set include_linked=true but did not receive any non-owned items"
+ end
+ end
+ end
+
+ %w(offset limit).each do |arg|
+ ['foo', '', '1234five', '0x10', '-8'].each do |val|
+ test "Raise error on bogus #{arg} parameter #{val.inspect}" do
+ authorize_with :active
+ get :owned_items, {
+ :id => users(:active).uuid,
+ :format => :json,
+ arg => val,
+ }
+ assert_response 422
end
end
end
commit 8905037aede017ccdb66dd2850bfbd2284904e4e
Author: Tom Clegg <tom at curoverse.com>
Date: Wed Apr 23 16:36:41 2014 -0400
Add explanation comments to assertions.
diff --git a/apps/workbench/test/unit/user_test.rb b/apps/workbench/test/unit/user_test.rb
index 8c10b7e..2a47217 100644
--- a/apps/workbench/test/unit/user_test.rb
+++ b/apps/workbench/test/unit/user_test.rb
@@ -4,8 +4,10 @@ class UserTest < ActiveSupport::TestCase
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
+ assert_operator(0, :<, oi.count,
+ "Expected to find some items belonging to :active user")
+ assert_operator(0, :<, oi.items_available
+ "Expected owned_items response to have items_available > 0")
oi_uuids = oi.collect { |i| i['uuid'] }
expect = api_fixture('specimens')['owned_by_active_user']['uuid']
assert_includes(oi_uuids, expect,
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list