18122: Update "distinct" docs (default is false) and tidy up code.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>

diff --git a/doc/api/methods.html.textile.liquid b/doc/api/methods.html.textile.liquid
index 508a288557089bd0dc5540826c859963160c1ead..dea80fea860bc2ab24726625910463f2264990cf 100644 (file)
--- 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
-@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@.|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 9db8b447118187ebd5b465dfa8d8e8ad16cb121b..f986e88cdebc19def2ac84c66190eddb6dfd90dd 100644 (file)
--- 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 203c9c078952a9c1403cc1015717ef2737d6655a..9a360c538b6e432e36ad72cfe00d98493af925b6 100644 (file)
--- 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 7fbab3b3b00e484a5653435452f7d620178af251..2ee3b3cf94cab5c73f3b4e809db8b6afac7aec81 100644 (file)
--- 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