[ARVADOS] updated: 1.3.0-1001-gb3d5254ce

[Git user]

Summary of changes:
 .../api/app/controllers/application_controller.rb  |  2 +-
 .../arvados/v1/collections_controller.rb           | 12 ++++++++
 .../arvados/v1/collections_controller_test.rb      | 22 ++++++++-----
 .../api/test/integration/collections_api_test.rb   | 36 +++++++++++++++++++++-
 4 files changed, 62 insertions(+), 10 deletions(-)

       via  b3d5254ce24ca82904b13d61012b2d8d676a30d8 (commit)
       via  921537f5a37f03cdcd1f00abe25914306f83e956 (commit)
      from  0d5c86670e4ac37e4caceafabd9d6c65d2a89265 (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 b3d5254ce24ca82904b13d61012b2d8d676a30d8
Author: Lucas Di Pentima <ldipentima at veritasgenetics.com>
Date:   Tue Jun 4 20:34:21 2019 -0300

    15306: Fixes the bug.
    
    The problem was coming from 2 sides:
    
    1) The collections#show action didn't have the required parameters definitions.
    2) The load_required_parameters callback was being called after the object
       lookup callback.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <ldipentima at veritasgenetics.com>

diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 89eabdcee..40c7a8abf 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -42,9 +42,9 @@ class ApplicationController < ActionController::Base
   before_action :require_auth_scope, except: ERROR_ACTIONS
 
   before_action :catch_redirect_hint
+  before_action :load_required_parameters
   before_action(:find_object_by_uuid,
                 except: [:index, :create] + ERROR_ACTIONS)
-  before_action :load_required_parameters
   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]
diff --git a/services/api/app/controllers/arvados/v1/collections_controller.rb b/services/api/app/controllers/arvados/v1/collections_controller.rb
index 5d7a7ae26..7c7953ac9 100644
--- a/services/api/app/controllers/arvados/v1/collections_controller.rb
+++ b/services/api/app/controllers/arvados/v1/collections_controller.rb
@@ -21,6 +21,18 @@ class Arvados::V1::CollectionsController < ApplicationController
       })
   end
 
+  def self._show_requires_parameters
+    (super rescue {}).
+      merge({
+        include_trash: {
+          type: 'boolean', required: false, description: "Include collections whose is_trashed attribute is true."
+        },
+        include_old_versions: {
+          type: 'boolean', required: false, description: "Include past collection versions."
+        },
+      })
+  end
+
   def create
     if resource_attrs[:uuid] and (loc = Keep::Locator.parse(resource_attrs[:uuid]))
       resource_attrs[:portable_data_hash] = loc.to_s

commit 921537f5a37f03cdcd1f00abe25914306f83e956
Author: Lucas Di Pentima <ldipentima at veritasgenetics.com>
Date:   Tue Jun 4 20:32:13 2019 -0300

    15306: Adds more test cases confirming the issue is encoding independent.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <ldipentima at veritasgenetics.com>

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 4e8b0559a..d8017881d 100644
--- a/services/api/test/functional/arvados/v1/collections_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb
@@ -1128,14 +1128,20 @@ EOS
     end
   end
 
-  test 'get trashed collection with include_trash' do
-    uuid = 'zzzzz-4zz18-mto52zx1s7sn3ih' # expired_collection
-    authorize_with :active
-    get :show, params: {
-      id: uuid,
-      include_trash: true,
-    }
-    assert_response 200
+  [true, false].each do |include_trash|
+    test "get trashed collection with include_trash=#{include_trash}" do
+      uuid = 'zzzzz-4zz18-mto52zx1s7sn3ih' # expired_collection
+      authorize_with :active
+      get :show, params: {
+        id: uuid,
+        include_trash: include_trash,
+      }
+      if include_trash
+        assert_response 200
+      else
+        assert_response 404
+      end
+    end
   end
 
   [:admin, :active].each do |user|
diff --git a/services/api/test/integration/collections_api_test.rb b/services/api/test/integration/collections_api_test.rb
index 49c3cb425..7bbae6e35 100644
--- a/services/api/test/integration/collections_api_test.rb
+++ b/services/api/test/integration/collections_api_test.rb
@@ -281,6 +281,40 @@ class CollectionsApiTest < ActionDispatch::IntegrationTest
     ["true", true],
     ["1", true]
   ].each do |param, truthiness|
+    test "include_trash=#{param.inspect} param JSON-encoded should be interpreted as include_trash=#{truthiness}" do
+      expired_col = collections(:expired_collection)
+      assert expired_col.is_trashed
+      # Try #index first
+      get "/arvados/v1/collections",
+          params: {
+            :include_trash => param,
+            :filters => [['uuid', '=', expired_col.uuid]].to_json
+          },
+          headers: auth(:active)
+      assert_response :success
+      assert_not_nil json_response['items']
+      assert_equal truthiness, json_response['items'].collect {|c| c['uuid']}.include?(expired_col.uuid)
+      # Try #show next
+      get "/arvados/v1/collections/#{expired_col.uuid}",
+        params: {
+          :format => :json,
+          :include_trash => param,
+        },
+        headers: auth(:active)
+      if truthiness
+        assert_response :success
+      else
+        assert_response 404
+      end
+    end
+  end
+
+  [
+    ["false", false],
+    ["0", false],
+    ["true", true],
+    ["1", true]
+  ].each do |param, truthiness|
     test "include_trash=#{param.inspect} param encoding via query string should be interpreted as include_trash=#{truthiness}" do
       expired_col = collections(:expired_collection)
       assert expired_col.is_trashed
@@ -324,7 +358,7 @@ class CollectionsApiTest < ActionDispatch::IntegrationTest
       assert_not_nil json_response['items']
       assert_equal truthiness, json_response['items'].collect {|c| c['uuid']}.include?(expired_col.uuid)
       # Try #show next
-      get "/arvados/v1/collections",
+      get "/arvados/v1/collections/#{expired_col.uuid}",
         params: URI.encode_www_form([['include_trash', param]]),
         headers: {
           "Content-type" => "application/x-www-form-urlencoded"

-----------------------------------------------------------------------


hooks/post-receive
--