From 002aec2a7db39d269bb3c9123783022c2e32a5cc Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Wed, 20 Nov 2019 15:38:25 -0500 Subject: [PATCH] 15854: Support filtering container_requests by container fields. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- .../container_requests.html.textile.liquid | 4 +- .../methods/containers.html.textile.liquid | 2 - services/api/app/models/arvados_model.rb | 3 + services/api/lib/record_filters.rb | 89 +++++++++++-------- .../v1/container_requests_controller_test.rb | 44 +++++++++ 5 files changed, 101 insertions(+), 41 deletions(-) diff --git a/doc/api/methods/container_requests.html.textile.liquid b/doc/api/methods/container_requests.html.textile.liquid index b9a21fc0a0..cd566f5ce4 100644 --- a/doc/api/methods/container_requests.html.textile.liquid +++ b/doc/api/methods/container_requests.html.textile.liquid @@ -141,11 +141,11 @@ table(table table-bordered table-condensed). h3. list -List container_requests. +List container requests. See "common resource list method.":{{site.baseurl}}/api/methods.html#index -See the create method documentation for more information about container request-specific filters. +The @filters@ argument can also filter on attributes of the container referenced by @container_uuid@. For example, @[["container.state", "=", "Running"]]@ will match any container request whose container is running now. h3. update diff --git a/doc/api/methods/containers.html.textile.liquid b/doc/api/methods/containers.html.textile.liquid index 5ec95cee62..8a7ebc36e5 100644 --- a/doc/api/methods/containers.html.textile.liquid +++ b/doc/api/methods/containers.html.textile.liquid @@ -136,8 +136,6 @@ List containers. See "common resource list method.":{{site.baseurl}}/api/methods.html#index -See the create method documentation for more information about Container-specific filters. - h3. update Update attributes of an existing Container. diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb index f933126933..946c4262e3 100644 --- a/services/api/app/models/arvados_model.rb +++ b/services/api/app/models/arvados_model.rb @@ -457,6 +457,9 @@ class ArvadosModel < ApplicationRecord if not ft[:cond_out].any? return query end + ft[:joins].each do |t| + query = query.joins(t) + end query.where('(' + ft[:cond_out].join(') AND (') + ')', *ft[:param_out]) end diff --git a/services/api/lib/record_filters.rb b/services/api/lib/record_filters.rb index c8f024291c..994e850310 100644 --- a/services/api/lib/record_filters.rb +++ b/services/api/lib/record_filters.rb @@ -21,12 +21,14 @@ module RecordFilters # Output: # Hash with two keys: # :cond_out array of SQL fragments for each filter expression - # :param_out array of values for parameter substitution in cond_out + # :param_out array of values for parameter substitution in cond_out + # :joins array of joins: either [] or ["JOIN containers ON ..."] def record_filters filters, model_class conds_out = [] param_out = [] + joins = [] - ar_table_name = model_class.table_name + model_table_name = model_class.table_name filters.each do |filter| attrs_in, operator, operand = filter if attrs_in == 'any' && operator != '@@' @@ -71,78 +73,91 @@ module RecordFilters attrs.each do |attr| subproperty = attr.split(".", 2) - col = model_class.columns.select { |c| c.name == subproperty[0] }.first + if subproperty.length == 2 && subproperty[0] == 'container' && model_table_name == "container_requests" + # attr is "tablename.colname" -- e.g., ["container.state", "=", "Complete"] + joins = ["JOIN containers ON container_requests.container_uuid = containers.uuid"] + attr_model_class = Container + attr_table_name = "containers" + subproperty = subproperty[1].split(".", 2) + else + attr_model_class = model_class + attr_table_name = model_table_name + end + + attr = subproperty[0] + proppath = subproperty[1] + col = attr_model_class.columns.select { |c| c.name == attr }.first - if subproperty.length == 2 + if proppath if col.nil? or col.type != :jsonb - raise ArgumentError.new("Invalid attribute '#{subproperty[0]}' for subproperty filter") + raise ArgumentError.new("Invalid attribute '#{attr}' for subproperty filter") end - if subproperty[1][0] == "<" and subproperty[1][-1] == ">" - subproperty[1] = subproperty[1][1..-2] + if proppath[0] == "<" and proppath[-1] == ">" + proppath = proppath[1..-2] end # jsonb search case operator.downcase when '=', '!=' not_in = if operator.downcase == "!=" then "NOT " else "" end - cond_out << "#{not_in}(#{ar_table_name}.#{subproperty[0]} @> ?::jsonb)" - param_out << SafeJSON.dump({subproperty[1] => operand}) + cond_out << "#{not_in}(#{attr_table_name}.#{attr} @> ?::jsonb)" + param_out << SafeJSON.dump({proppath => operand}) when 'in' if operand.is_a? Array operand.each do |opr| - cond_out << "#{ar_table_name}.#{subproperty[0]} @> ?::jsonb" - param_out << SafeJSON.dump({subproperty[1] => opr}) + cond_out << "#{attr_table_name}.#{attr} @> ?::jsonb" + param_out << SafeJSON.dump({proppath => opr}) end else raise ArgumentError.new("Invalid operand type '#{operand.class}' "\ "for '#{operator}' operator in filters") end when '<', '<=', '>', '>=' - cond_out << "#{ar_table_name}.#{subproperty[0]}->? #{operator} ?::jsonb" - param_out << subproperty[1] + cond_out << "#{attr_table_name}.#{attr}->? #{operator} ?::jsonb" + param_out << proppath param_out << SafeJSON.dump(operand) when 'like', 'ilike' - cond_out << "#{ar_table_name}.#{subproperty[0]}->>? #{operator} ?" - param_out << subproperty[1] + cond_out << "#{attr_table_name}.#{attr}->>? #{operator} ?" + param_out << proppath param_out << operand when 'not in' if operand.is_a? Array - cond_out << "#{ar_table_name}.#{subproperty[0]}->>? NOT IN (?) OR #{ar_table_name}.#{subproperty[0]}->>? IS NULL" - param_out << subproperty[1] + cond_out << "#{attr_table_name}.#{attr}->>? NOT IN (?) OR #{attr_table_name}.#{attr}->>? IS NULL" + param_out << proppath param_out << operand - param_out << subproperty[1] + param_out << proppath else raise ArgumentError.new("Invalid operand type '#{operand.class}' "\ "for '#{operator}' operator in filters") end when 'exists' if operand == true - cond_out << "jsonb_exists(#{ar_table_name}.#{subproperty[0]}, ?)" + cond_out << "jsonb_exists(#{attr_table_name}.#{attr}, ?)" elsif operand == false - cond_out << "(NOT jsonb_exists(#{ar_table_name}.#{subproperty[0]}, ?)) OR #{ar_table_name}.#{subproperty[0]} is NULL" + cond_out << "(NOT jsonb_exists(#{attr_table_name}.#{attr}, ?)) OR #{attr_table_name}.#{attr} is NULL" else raise ArgumentError.new("Invalid operand '#{operand}' for '#{operator}' must be true or false") end - param_out << subproperty[1] + param_out << proppath else raise ArgumentError.new("Invalid operator for subproperty search '#{operator}'") end elsif operator.downcase == "exists" if col.type != :jsonb - raise ArgumentError.new("Invalid attribute '#{subproperty[0]}' for operator '#{operator}' in filter") + raise ArgumentError.new("Invalid attribute '#{attr}' for operator '#{operator}' in filter") end - cond_out << "jsonb_exists(#{ar_table_name}.#{subproperty[0]}, ?)" + cond_out << "jsonb_exists(#{attr_table_name}.#{attr}, ?)" param_out << operand else - if !model_class.searchable_columns(operator).index subproperty[0] - raise ArgumentError.new("Invalid attribute '#{subproperty[0]}' in filter") + if !attr_model_class.searchable_columns(operator).index attr + raise ArgumentError.new("Invalid attribute '#{attr}' in filter") end case operator.downcase when '=', '<', '<=', '>', '>=', '!=', 'like', 'ilike' - attr_type = model_class.attribute_column(attr).type + attr_type = attr_model_class.attribute_column(attr).type operator = '<>' if operator == '!=' if operand.is_a? String if attr_type == :boolean @@ -162,9 +177,9 @@ module RecordFilters end if operator == '<>' # explicitly allow NULL - cond_out << "#{ar_table_name}.#{attr} #{operator} ? OR #{ar_table_name}.#{attr} IS NULL" + cond_out << "#{attr_table_name}.#{attr} #{operator} ? OR #{attr_table_name}.#{attr} IS NULL" else - cond_out << "#{ar_table_name}.#{attr} #{operator} ?" + cond_out << "#{attr_table_name}.#{attr} #{operator} ?" end if (# any operator that operates on value rather than # representation: @@ -173,15 +188,15 @@ module RecordFilters end param_out << operand elsif operand.nil? and operator == '=' - cond_out << "#{ar_table_name}.#{attr} is null" + cond_out << "#{attr_table_name}.#{attr} is null" elsif operand.nil? and operator == '<>' - cond_out << "#{ar_table_name}.#{attr} is not null" + cond_out << "#{attr_table_name}.#{attr} is not null" elsif (attr_type == :boolean) and ['=', '<>'].include?(operator) and [true, false].include?(operand) - cond_out << "#{ar_table_name}.#{attr} #{operator} ?" + cond_out << "#{attr_table_name}.#{attr} #{operator} ?" param_out << operand elsif (attr_type == :integer) - cond_out << "#{ar_table_name}.#{attr} #{operator} ?" + cond_out << "#{attr_table_name}.#{attr} #{operator} ?" param_out << operand else raise ArgumentError.new("Invalid operand type '#{operand.class}' "\ @@ -189,11 +204,11 @@ module RecordFilters end when 'in', 'not in' if operand.is_a? Array - cond_out << "#{ar_table_name}.#{attr} #{operator} (?)" + cond_out << "#{attr_table_name}.#{attr} #{operator} (?)" param_out << operand if operator == 'not in' and not operand.include?(nil) # explicitly allow NULL - cond_out[-1] = "(#{cond_out[-1]} OR #{ar_table_name}.#{attr} IS NULL)" + cond_out[-1] = "(#{cond_out[-1]} OR #{attr_table_name}.#{attr} IS NULL)" end else raise ArgumentError.new("Invalid operand type '#{operand.class}' "\ @@ -206,14 +221,14 @@ module RecordFilters cl = ArvadosModel::kind_class op if cl if attr == 'uuid' - if model_class.uuid_prefix == cl.uuid_prefix + if attr_model_class.uuid_prefix == cl.uuid_prefix cond << "1=1" else cond << "1=0" end else # Use a substring query to support remote uuids - cond << "substring(#{ar_table_name}.#{attr}, 7, 5) = ?" + cond << "substring(#{attr_table_name}.#{attr}, 7, 5) = ?" param_out << cl.uuid_prefix end else @@ -229,7 +244,7 @@ module RecordFilters conds_out << cond_out.join(' OR ') if cond_out.any? end - {:cond_out => conds_out, :param_out => param_out} + {:cond_out => conds_out, :param_out => param_out, :joins => joins} end end diff --git a/services/api/test/functional/arvados/v1/container_requests_controller_test.rb b/services/api/test/functional/arvados/v1/container_requests_controller_test.rb index e77e2ed3c6..95c477f411 100644 --- a/services/api/test/functional/arvados/v1/container_requests_controller_test.rb +++ b/services/api/test/functional/arvados/v1/container_requests_controller_test.rb @@ -98,4 +98,48 @@ class Arvados::V1::ContainerRequestsControllerTest < ActionController::TestCase assert_equal api_client_authorizations(:spectator).token, req.runtime_token end + %w(Running Complete).each do |state| + test "filter on container.state = #{state}" do + authorize_with :active + get :index, params: { + filters: [['container.state', '=', state]], + } + assert_response :success + assert_operator json_response['items'].length, :>, 0 + json_response['items'].each do |cr| + assert_equal state, Container.find_by_uuid(cr['container_uuid']).state + end + end + end + + test "filter on container success" do + authorize_with :active + get :index, params: { + filters: [ + ['container.state', '=', 'Complete'], + ['container.exit_code', '=', '0'], + ], + } + assert_response :success + assert_operator json_response['items'].length, :>, 0 + json_response['items'].each do |cr| + assert_equal 'Complete', Container.find_by_uuid(cr['container_uuid']).state + assert_equal 0, Container.find_by_uuid(cr['container_uuid']).exit_code + end + end + + test "filter on container subproperty runtime_status[foo] = bar" do + ctr = containers(:running) + act_as_system_user do + ctr.update_attributes!(runtime_status: {foo: 'bar'}) + end + authorize_with :active + get :index, params: { + filters: [ + ['container.runtime_status.foo', '=', 'bar'], + ], + } + assert_response :success + assert_equal [ctr.uuid], json_response['items'].collect { |cr| cr['container_uuid'] }.uniq + end end -- 2.30.2