[ARVADOS] created: 2.1.0-1350-g5d7bef412
Git user
git at public.arvados.org
Thu Sep 16 14:18:10 UTC 2021
at 5d7bef4122c59bec9145f2853b76170b4ddca67f (commit)
commit 5d7bef4122c59bec9145f2853b76170b4ddca67f
Author: Tom Clegg <tom at curii.com>
Date: Thu Sep 16 09:54:38 2021 -0400
18122: Update "distinct" docs (default is false) and tidy up code.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/doc/api/methods.html.textile.liquid b/doc/api/methods.html.textile.liquid
index 508a28855..dea80fea8 100644
--- a/doc/api/methods.html.textile.liquid
+++ b/doc/api/methods.html.textile.liquid
@@ -84,8 +84,8 @@ Example: @["head_uuid asc","modified_at desc"]@
Default: @["modified_at desc", "uuid asc"]@|query|
|select |array |Attributes of each object to return in the response (by default, all available attributes are returned, except collections, which do not return @manifest_text@ unless explicitly selected).
Example: @["uuid","name","modified_at"]@|query|
-|distinct|boolean|@true@: (default) do not return duplicate objects
- at false@: permitted to return duplicates|query|
+|distinct|boolean|When returning multiple records whose whose selected attributes (see @select@) are equal, return them as a single response entry.
+Default is @false at .|query|
|count|string|@"exact"@ (default): Include an @items_available@ response field giving the number of distinct matching items that can be retrieved (irrespective of @limit@ and @offset@ arguments).
@"none"@: Omit the @items_available@ response field. This option will produce a faster response.|query|
diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 9db8b4471..f986e88cd 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -305,7 +305,7 @@ class ApplicationController < ActionController::Base
@objects = @objects.order(@orders.join ", ") if @orders.any?
@objects = @objects.limit(@limit)
@objects = @objects.offset(@offset)
- @objects = @objects.distinct(@distinct) if not @distinct.nil?
+ @objects = @objects.distinct() if @distinct
end
# limit_database_read ensures @objects (which must be an
@@ -654,7 +654,7 @@ class ApplicationController < ActionController::Base
where: { type: 'object', required: false },
order: { type: 'array', required: false },
select: { type: 'array', required: false },
- distinct: { type: 'boolean', required: false },
+ distinct: { type: 'boolean', required: false, default: false },
limit: { type: 'integer', required: false, default: DEFAULT_LIMIT },
offset: { type: 'integer', required: false, default: 0 },
count: { type: 'string', required: false, default: 'exact' },
diff --git a/services/api/lib/load_param.rb b/services/api/lib/load_param.rb
index 203c9c078..9a360c538 100644
--- a/services/api/lib/load_param.rb
+++ b/services/api/lib/load_param.rb
@@ -145,8 +145,7 @@ module LoadParam
end
end
- @distinct = true if (params[:distinct] == true || params[:distinct] == "true")
- @distinct = false if (params[:distinct] == false || params[:distinct] == "false")
+ @distinct = params[:distinct] && true
end
def load_select_param
diff --git a/services/api/test/integration/select_test.rb b/services/api/test/integration/select_test.rb
index 7fbab3b3b..2ee3b3cf9 100644
--- a/services/api/test/integration/select_test.rb
+++ b/services/api/test/integration/select_test.rb
@@ -16,11 +16,17 @@ class SelectTest < ActionDispatch::IntegrationTest
end
test "fewer distinct than total count" do
+ get "/arvados/v1/links",
+ params: {:format => :json, :select => ['link_class']},
+ headers: auth(:active)
+ assert_response :success
+ distinct_unspecified = json_response['items']
+
get "/arvados/v1/links",
params: {:format => :json, :select => ['link_class'], :distinct => false},
headers: auth(:active)
assert_response :success
- links = json_response['items']
+ distinct_false = json_response['items']
get "/arvados/v1/links",
params: {:format => :json, :select => ['link_class'], :distinct => true},
@@ -28,9 +34,11 @@ class SelectTest < ActionDispatch::IntegrationTest
assert_response :success
distinct = json_response['items']
- assert_operator(distinct.count, :<, links.count,
- "distinct count should be less than link count")
- assert_equal links.uniq.count, distinct.count
+ assert_operator(distinct.count, :<, distinct_false.count,
+ "distinct=true count should be less than distinct=false count")
+ assert_equal(distinct_unspecified.count, distinct_false.count,
+ "distinct=false should be the default")
+ assert_equal distinct_false.uniq.count, distinct.count
end
test "select with order" do
commit 7db1d0f490158ffa5672eda245dacb355e196d3d
Author: Tom Clegg <tom at curii.com>
Date: Thu Sep 16 09:39:18 2021 -0400
18122: Update API docs.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/doc/api/methods.html.textile.liquid b/doc/api/methods.html.textile.liquid
index fd5291792..508a28855 100644
--- a/doc/api/methods.html.textile.liquid
+++ b/doc/api/methods.html.textile.liquid
@@ -37,6 +37,8 @@ table(table table-bordered table-condensed).
|_. Argument |_. Type |_. Description |_. Location |
|{resource_type}|object|Name is the singular form of the resource type, e.g., for the "collections" resource, this argument is "collection"|body|
|{cluster_id}|string|Optional, the cluster on which to create the object if not the current cluster.|query|
+|select |array |Attributes of the new object to return in the response (by default, all available attributes are returned).
+Example: @["uuid","name","modified_at"]@|query|
h2. delete
@@ -49,6 +51,8 @@ Arguments:
table(table table-bordered table-condensed).
|_. Argument |_. Type |_. Description |_. Location |
{background:#ccffcc}.|uuid|string|The UUID of the object in question.|path|
+|select |array |Attributes of the deleted object to return in the response (by default, all available attributes are returned).
+Example: @["uuid","name","modified_at"]@|query|
h2. get
@@ -61,10 +65,12 @@ Arguments:
table(table table-bordered table-condensed).
|_. Argument |_. Type |_. Description |_. Location |
{background:#ccffcc}.|uuid|string|The UUID of the object in question.|path|
+|select |array |Attributes of the object to return in the response (by default, all available attributes are returned).
+Example: @["uuid","name","modified_at"]@|query|
h2(#index). list
-The @list@ method requests an list of resources of that type. It corresponds to the HTTP request @GET /arvados/v1/resource_type at . All resources support "list" method unless otherwise noted.
+The @list@ method requests an list of resources of that type. It corresponds to the HTTP request @GET /arvados/v1/resource_type at . All resources support the @list@ method unless otherwise noted.
Arguments:
@@ -76,9 +82,8 @@ table(table table-bordered table-condensed).
|order |array |Attributes to use as sort keys to determine the order resources are returned, each optionally followed by @asc@ or @desc@ to indicate ascending or descending order. (If not specified, it will be ascending).
Example: @["head_uuid asc","modified_at desc"]@
Default: @["modified_at desc", "uuid asc"]@|query|
-|select |array |Set of attributes to include in the response.
-Example: @["head_uuid","tail_uuid"]@
-Default: all available attributes. As a special case, collections do not return "manifest_text" unless explicitly selected.|query|
+|select |array |Attributes of each object to return in the response (by default, all available attributes are returned, except collections, which do not return @manifest_text@ unless explicitly selected).
+Example: @["uuid","name","modified_at"]@|query|
|distinct|boolean|@true@: (default) do not return duplicate objects
@false@: permitted to return duplicates|query|
|count|string|@"exact"@ (default): Include an @items_available@ response field giving the number of distinct matching items that can be retrieved (irrespective of @limit@ and @offset@ arguments).
@@ -187,3 +192,5 @@ table(table table-bordered table-condensed).
|_. Argument |_. Type |_. Description |_. Location |
{background:#ccffcc}.|uuid|string|The UUID of the resource in question.|path||
|{resource_type}|object||query||
+|select |array |Attributes of the updated object to return in the response (by default, all available attributes are returned).
+Example: @["uuid","name","modified_at"]@|query|
commit 89d309ae02a0b47be4b201b78449afd4267effef
Author: Tom Clegg <tom at curii.com>
Date: Wed Sep 15 15:44:21 2021 -0400
18122: Accept "select" param in get/update/delete calls.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/lib/controller/router/router_test.go b/lib/controller/router/router_test.go
index 639d2a28b..722895645 100644
--- a/lib/controller/router/router_test.go
+++ b/lib/controller/router/router_test.go
@@ -125,7 +125,6 @@ func (s *RouterSuite) TestOptions(c *check.C) {
comment: "form-encoded expression filter in query string",
method: "GET",
path: "/arvados/v1/collections?filters=[%22(foo<bar)%22]",
- header: http.Header{"Content-Type": {"application/x-www-form-urlencoded"}},
shouldCall: "CollectionList",
withOptions: arvados.ListOptions{Limit: -1, Filters: []arvados.Filter{{"(foo<bar)", "=", true}}},
},
@@ -147,6 +146,13 @@ func (s *RouterSuite) TestOptions(c *check.C) {
shouldCall: "CollectionList",
withOptions: arvados.ListOptions{Limit: 2, Filters: []arvados.Filter{{"(foo<bar)", "=", true}, {"bar", "=", "baz"}}},
},
+ {
+ comment: "json-encoded select param in query string",
+ method: "GET",
+ path: "/arvados/v1/collections/" + arvadostest.FooCollection + "?select=[%22portable_data_hash%22]",
+ shouldCall: "CollectionGet",
+ withOptions: arvados.GetOptions{UUID: arvadostest.FooCollection, Select: []string{"portable_data_hash"}},
+ },
{
method: "PATCH",
path: "/arvados/v1/collections",
@@ -376,7 +382,6 @@ func (s *RouterIntegrationSuite) TestSelectParam(c *check.C) {
for _, sel := range [][]string{
{"uuid", "command"},
{"uuid", "command", "uuid"},
- {"", "command", "uuid"},
} {
j, err := json.Marshal(sel)
c.Assert(err, check.IsNil)
@@ -384,8 +389,6 @@ func (s *RouterIntegrationSuite) TestSelectParam(c *check.C) {
c.Check(rr.Code, check.Equals, http.StatusOK)
c.Check(resp["kind"], check.Equals, "arvados#container")
- c.Check(resp["etag"], check.FitsTypeOf, "")
- c.Check(resp["etag"], check.Not(check.Equals), "")
c.Check(resp["uuid"], check.HasLen, 27)
c.Check(resp["command"], check.HasLen, 2)
c.Check(resp["mounts"], check.IsNil)
diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index c39bdde4b..9db8b4471 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -43,13 +43,14 @@ class ApplicationController < ActionController::Base
before_action :catch_redirect_hint
before_action :load_required_parameters
+ before_action :load_limit_offset_order_params, only: [:index, :contents]
+ before_action :load_select_param
before_action(:find_object_by_uuid,
except: [:index, :create] + ERROR_ACTIONS)
- before_action(:set_nullable_attrs_to_null, only: [:update, :create])
- before_action :load_limit_offset_order_params, only: [:index, :contents]
before_action :load_where_param, only: [:index, :contents]
before_action :load_filters_param, only: [:index, :contents]
before_action :find_objects_for_index, :only => :index
+ before_action(:set_nullable_attrs_to_null, only: [:update, :create])
before_action :reload_object_before_update, :only => :update
before_action(:render_404_if_no_object,
except: [:index, :create] + ERROR_ACTIONS)
@@ -618,6 +619,7 @@ class ApplicationController < ActionController::Base
def self._create_requires_parameters
{
+ select: { type: 'array', required: false },
ensure_unique_name: {
type: "boolean",
description: "Adjust name to ensure uniqueness instead of returning an error on (owner_uuid, name) collision.",
@@ -635,7 +637,15 @@ class ApplicationController < ActionController::Base
end
def self._update_requires_parameters
- {}
+ {
+ select: { type: 'array', required: false },
+ }
+ end
+
+ def self._show_requires_parameters
+ {
+ select: { type: 'array', required: false },
+ }
end
def self._index_requires_parameters
diff --git a/services/api/app/controllers/arvados/v1/collections_controller.rb b/services/api/app/controllers/arvados/v1/collections_controller.rb
index 13b0261d9..1b0850a58 100644
--- a/services/api/app/controllers/arvados/v1/collections_controller.rb
+++ b/services/api/app/controllers/arvados/v1/collections_controller.rb
@@ -80,7 +80,13 @@ class Arvados::V1::CollectionsController < ApplicationController
# it will select the Collection object with the longest
# available lifetime.
- if c = Collection.readable_by(*@read_users, opts).where({ portable_data_hash: loc.to_s }).order("trash_at desc").limit(1).first
+ if c = Collection.
+ readable_by(*@read_users, opts).
+ where({ portable_data_hash: loc.to_s }).
+ order("trash_at desc").
+ select(((@select || ["manifest_text"]) | ["portable_data_hash", "trash_at"]).join(", ")).
+ limit(1).
+ first
@object = {
uuid: c.portable_data_hash,
portable_data_hash: c.portable_data_hash,
@@ -321,7 +327,7 @@ class Arvados::V1::CollectionsController < ApplicationController
protected
- def load_limit_offset_order_params *args
+ def load_select_param *args
super
if action_name == 'index'
# Omit manifest_text and unsigned_manifest_text from index results unless expressly selected.
diff --git a/services/api/app/controllers/arvados/v1/healthcheck_controller.rb b/services/api/app/controllers/arvados/v1/healthcheck_controller.rb
index 6c3822437..c56208207 100644
--- a/services/api/app/controllers/arvados/v1/healthcheck_controller.rb
+++ b/services/api/app/controllers/arvados/v1/healthcheck_controller.rb
@@ -8,6 +8,7 @@ class Arvados::V1::HealthcheckController < ApplicationController
skip_before_action :find_object_by_uuid
skip_before_action :load_filters_param
skip_before_action :load_limit_offset_order_params
+ skip_before_action :load_select_param
skip_before_action :load_read_auths
skip_before_action :load_where_param
skip_before_action :render_404_if_no_object
diff --git a/services/api/app/controllers/arvados/v1/schema_controller.rb b/services/api/app/controllers/arvados/v1/schema_controller.rb
index 8b71f7237..43d9d3b03 100644
--- a/services/api/app/controllers/arvados/v1/schema_controller.rb
+++ b/services/api/app/controllers/arvados/v1/schema_controller.rb
@@ -8,6 +8,7 @@ class Arvados::V1::SchemaController < ApplicationController
skip_before_action :find_object_by_uuid
skip_before_action :load_filters_param
skip_before_action :load_limit_offset_order_params
+ skip_before_action :load_select_param
skip_before_action :load_read_auths
skip_before_action :load_where_param
skip_before_action :render_404_if_no_object
diff --git a/services/api/lib/load_param.rb b/services/api/lib/load_param.rb
index 7119eb234..203c9c078 100644
--- a/services/api/lib/load_param.rb
+++ b/services/api/lib/load_param.rb
@@ -145,6 +145,11 @@ module LoadParam
end
end
+ @distinct = true if (params[:distinct] == true || params[:distinct] == "true")
+ @distinct = false if (params[:distinct] == false || params[:distinct] == "false")
+ end
+
+ def load_select_param
case params[:select]
when Array
@select = params[:select]
@@ -157,7 +162,7 @@ module LoadParam
end
end
- if @select
+ if @select && @orders
# Any ordering columns must be selected when doing select,
# otherwise it is an SQL error, so filter out invaliding orderings.
@orders.select! { |o|
@@ -166,9 +171,6 @@ module LoadParam
@select.select { |s| col == "#{table_name}.#{s}" }.any?
}
end
-
- @distinct = true if (params[:distinct] == true || params[:distinct] == "true")
- @distinct = false if (params[:distinct] == false || params[:distinct] == "false")
end
end
diff --git a/services/api/test/functional/arvados/v1/collections_controller_test.rb b/services/api/test/functional/arvados/v1/collections_controller_test.rb
index eac393104..3b217f06d 100644
--- a/services/api/test/functional/arvados/v1/collections_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb
@@ -1461,4 +1461,34 @@ EOS
end
end
end
+
+ test "select param is respected in 'show' response" do
+ authorize_with :active
+ get :show, params: {
+ id: collections(:collection_owned_by_active).uuid,
+ select: ["name"],
+ }
+ assert_response :success
+ assert_raises ActiveModel::MissingAttributeError do
+ assigns(:object).manifest_text
+ end
+ assert_nil json_response["manifest_text"]
+ assert_nil json_response["properties"]
+ assert_equal collections(:collection_owned_by_active).name, json_response["name"]
+ end
+
+ test "select param is respected in 'update' response" do
+ authorize_with :active
+ post :update, params: {
+ id: collections(:collection_owned_by_active).uuid,
+ collection: {
+ manifest_text: ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:foobar.txt\n",
+ },
+ select: ["name"],
+ }
+ assert_response :success
+ assert_nil json_response["manifest_text"]
+ assert_nil json_response["properties"]
+ assert_equal collections(:collection_owned_by_active).name, json_response["name"]
+ end
end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list