[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