[ARVADOS] created: 2.1.0-1348-g89d309ae0
Git user
git at public.arvados.org
Wed Sep 15 19:47:21 UTC 2021
at 89d309ae02a0b47be4b201b78449afd4267effef (commit)
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