Bug #19297
closed
RailsAPI error when filtering by an inexistent column
100%
Description
Workbench2 since v2.4.1 added arvados#workflow to the list of object types to include in its properties advanced search. The following is an example of filter passed to the controller:
[["groups.properties.IDTAGANIMALS","contains","IDVALANIMALS2"],["collections.properties.IDTAGANIMALS","contains","IDVALANIMALS2"],["container_requests.properties.IDTAGANIMALS","contains","IDVALANIMALS2"],["properties","exists","IDTAGANIMALS"],["uuid","is_a",["arvados#group","arvados#collection","arvados#containerRequest"]],["uuid","is_a",["arvados#group","arvados#containerRequest","arvados#collection","arvados#workflow"]]]
This generate an error message like: 422 Unprocessable Entity: #<NoMethodError: undefined method `type' for nil:NilClass>
An example of backtrace from ce8i5:
"/var/www/arvados-api/current/lib/record_filters.rb:139:in `block (2 levels) in record_filters' /var/www/arvados-api/current/lib/record_filters.rb:61:in `each' /var/www/arvados-api/current/lib/record_filters.rb:61:in `block in record_filters' /var/www/arvados-api/current/lib/record_filters.rb:32:in `each' /var/www/arvados-api/current/lib/record_filters.rb:32:in `record_filters' /var/www/arvados-api/current/app/models/arvados_model.rb:573:in `apply_filters' /var/www/arvados-api/current/app/controllers/application_controller.rb:227:in `apply_filters' /var/www/arvados-api/current/app/controllers/application_controller.rb:232:in `apply_where_limit_order_params' /var/www/arvados-api/current/app/controllers/arvados/v1/groups_controller.rb:321:in `block in load_searchable_objects' /var/www/arvados-api/current/app/controllers/arvados/v1/groups_controller.rb:266:in `each' ...
The workflows table doesn't currently have a properties column so it would seem that the code at record_filters.rb isn't handling this correctly.
Updated by Lucas Di Pentima over 2 years ago
- Status changed from New to In Progress
Updated by Lucas Di Pentima over 2 years ago
Was able to produce the error by just making the groups#contents request with filters: [['properties', 'exists', 'foo']]
Updated by Lucas Di Pentima over 2 years ago
Updates at 6cd62b6 - branch 19297-inexistent-field-filter-fix
Test run: developer-run-tests: #3249
- Adds check for inexistent column on
existsoperator code path. - Record all the errors for every object type and re-raise them when every object type errored out.
- Adds tests.
This update will make wb2 to not require any change because is making a search looking for properties on types that do have the properties field, in addition to workflows.
Updated by Lucas Di Pentima over 2 years ago
WB1 failed tests re-run: developer-run-tests-apps-workbench-integration: #3494
Updated by Tom Clegg over 2 years ago
# Only error out when every searchable object type errored out
if !any_success
error_msg = error_by_class.collect do |klass, err|
"#{err} on object type #{klass}"
This fails with valid/no filters and limit=0 count=none, because we break out of the loop without setting the per-table params and therefore without setting any_success=true. Maybe worth changing to
if !any_success && error_by_class.size > 0
This has the opposite problem that it would return OK with an invalid filter and limit=0 count=none. Since the results would always be empty, I'm not sure how much we should care about either case, but returning OK seems easier for client-side devs...?
Rest LGTM, thanks.
Updated by Lucas Di Pentima over 2 years ago
Updates at 3e95703e7
Test run: developer-run-tests: #3250
- Adds check so that it doesn't error out when
limit=0, count=noneis used, as suggested. - Adds test for this particular case.
Updated by Lucas Di Pentima over 2 years ago
- Status changed from In Progress to Resolved
Applied in changeset arvados-private:commit:arvados|1923ceda3c8845526f1ddcdd6275513760e0cd84.