[ARVADOS] created: 1.3.0-1892-g002aec2a7

Git user git at public.curoverse.com
Wed Nov 20 20:38:44 UTC 2019


        at  002aec2a7db39d269bb3c9123783022c2e32a5cc (commit)


commit 002aec2a7db39d269bb3c9123783022c2e32a5cc
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Wed Nov 20 15:38:25 2019 -0500

    15854: Support filtering container_requests by container fields.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/doc/api/methods/container_requests.html.textile.liquid b/doc/api/methods/container_requests.html.textile.liquid
index b9a21fc0a..cd566f5ce 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 at . 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 5ec95cee6..8a7ebc36e 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 f93312693..946c4262e 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 c8f024291..994e85031 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 e77e2ed3c..95c477f41 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

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list