Merge branch '19297-inexistent-field-filter-fix'. Closes #19297

Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima@curii.com>

diff --git a/services/api/app/controllers/arvados/v1/groups_controller.rb b/services/api/app/controllers/arvados/v1/groups_controller.rb
index 3473c7e4e0c361e3c594569f83178e2ea18ebed2..e9bc006a36664bb1a929bf72dccc9730fa9b049c 100644 (file)
--- a/services/api/app/controllers/arvados/v1/groups_controller.rb
+++ b/services/api/app/controllers/arvados/v1/groups_controller.rb
@@ -263,6 +263,9 @@ class Arvados::V1::GroupsController < ApplicationController
     included_by_uuid = {}
 
     seen_last_class = false
+    error_by_class = {}
+    any_success = false
+
     klasses.each do |klass|
       # check if current klass is same as params['last_object_class']
       seen_last_class = true if((params['count'].andand.==('none')) and
@@ -318,7 +321,19 @@ class Arvados::V1::GroupsController < ApplicationController
       # Adjust the limit based on number of objects fetched so far
       klass_limit = limit_all - all_objects.count
       @limit = klass_limit
-      apply_where_limit_order_params klass
+
+      begin
+        apply_where_limit_order_params klass
+      rescue ArgumentError => e
+        if e.inspect =~ /Invalid attribute '.+' for operator '.+' in filter/ or
+          e.inspect =~ /Invalid attribute '.+' for subproperty filter/
+          error_by_class[klass.name] = e
+          next
+        end
+        raise
+      else
+        any_success = true
+      end
 
       # This actually fetches the objects
       klass_object_list = object_list(model_class: klass)
@@ -349,6 +364,14 @@ class Arvados::V1::GroupsController < ApplicationController
       end
     end
 
+    # Only error out when every searchable object type errored out
+    if !any_success && error_by_class.size > 0
+      error_msg = error_by_class.collect do |klass, err|
+        "#{err} on object type #{klass}"
+      end.join("\n")
+      raise ArgumentError.new(error_msg)
+    end
+
     if params["include"]
       @extra_included = included_by_uuid.values
     end

diff --git a/services/api/lib/record_filters.rb b/services/api/lib/record_filters.rb
index 2f5b67074a9bdf5b24d3689333d17ee6e98e0745..65c25810acf2e3ef98422a1de3a5c8503e75edfe 100644 (file)
--- a/services/api/lib/record_filters.rb
+++ b/services/api/lib/record_filters.rb
@@ -136,7 +136,7 @@ module RecordFilters
             raise ArgumentError.new("Invalid operator for subproperty search '#{operator}'")
           end
         elsif operator == "exists"
-          if col.type != :jsonb
+          if col.nil? or col.type != :jsonb
             raise ArgumentError.new("Invalid attribute '#{attr}' for operator '#{operator}' in filter")
           end
 

diff --git a/services/api/test/functional/arvados/v1/filters_test.rb b/services/api/test/functional/arvados/v1/filters_test.rb
index dd8eeaa7bead1e260d46e5da4142707792edd42a..3916d63c5ed1cce10cca11182b23682db512d8d1 100644 (file)
--- a/services/api/test/functional/arvados/v1/filters_test.rb
+++ b/services/api/test/functional/arvados/v1/filters_test.rb
@@ -236,6 +236,58 @@ class Arvados::V1::FiltersTest < ActionController::TestCase
                  json_response['errors'].join(' '))
   end
 
+  test "groups contents with properties filter succeeds on objects with properties field" do
+    @controller = Arvados::V1::GroupsController.new
+    authorize_with :admin
+    get :contents, params: {
+      filters: [
+        ['properties', 'exists', 'foo'],
+        ['uuid', 'is_a', ["arvados#group","arvados#collection","arvados#containerRequest"]],
+      ]
+    }
+    assert_response 200
+    assert json_response['items'].length == 0
+  end
+
+  # Tests bug #19297
+  test "groups contents with properties filter succeeds on some objects with properties field" do
+    @controller = Arvados::V1::GroupsController.new
+    authorize_with :admin
+    get :contents, params: {
+      filters: [
+        ['properties', 'exists', 'foo'],
+        ['uuid', 'is_a', ["arvados#group","arvados#workflow"]],
+      ]
+    }
+    assert_response 200
+    assert json_response['items'].length == 0
+  end
+
+  # Tests bug #19297
+  test "groups contents with properties filter fails on objects without properties field" do
+    @controller = Arvados::V1::GroupsController.new
+    authorize_with :admin
+    get :contents, params: {
+      filters: [
+        ['properties', 'exists', 'foo'],
+        ['uuid', 'is_a', ["arvados#workflow"]],
+      ]
+    }
+    assert_response 422
+    assert_match(/Invalid attribute 'properties' for operator 'exists'.*on object type Workflow/, json_response['errors'].join(' '))
+  end
+
+  test "groups contents without filters and limit=0, count=none" do
+    @controller = Arvados::V1::GroupsController.new
+    authorize_with :admin
+    get :contents, params: {
+      limit: 0,
+      count: 'none',
+    }
+    assert_response 200
+    assert json_response['items'].length == 0
+  end
+
   test "replication_desired = 2" do
     @controller = Arvados::V1::CollectionsController.new
     authorize_with :admin