From: Tom Clegg <tom@curoverse.com>
Date: Fri, 14 Mar 2014 14:17:46 +0000 (-0400)
Subject: Merge branch 'master' into 2257-inequality-conditions
X-Git-Tag: 1.1.0~2709^2~97^2^2~2
X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/35336cd73e444534cb2eda20e3730464cc4e6553?hp=113d3f94224ca507ff6341ab61ac98429cad711e

Merge branch 'master' into 2257-inequality-conditions
---

diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 7c10c8425d..2d37dc18cd 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -10,6 +10,7 @@ class ApplicationController < ActionController::Base
   before_filter :catch_redirect_hint
 
   before_filter :load_where_param, :only => :index
+  before_filter :load_filters_param, :only => :index
   before_filter :find_objects_for_index, :only => :index
   before_filter :find_object_by_uuid, :except => [:index, :create,
                                                   :render_error,
@@ -113,16 +114,63 @@ class ApplicationController < ActionController::Base
       @where = params[:where]
     elsif params[:where].is_a? String
       begin
-        @where = Oj.load(params[:where], symbol_keys: true)
+        @where = Oj.load(params[:where])
+        raise unless @where.is_a? Hash
       rescue
         raise ArgumentError.new("Could not parse \"where\" param as an object")
       end
     end
+    @where = @where.with_indifferent_access
+  end
+
+  def load_filters_param
+    if params[:filters].is_a? Array
+      @filters = params[:filters]
+    elsif params[:filters].is_a? String
+      begin
+        @filters = Oj.load params[:filters]
+        raise unless @filters.is_a? Array
+      rescue
+        raise ArgumentError.new("Could not parse \"filters\" param as an array")
+      end
+    end
   end
 
   def find_objects_for_index
     @objects ||= model_class.readable_by(current_user)
-    if !@where.empty?
+    apply_where_limit_order_params
+  end
+
+  def apply_where_limit_order_params
+    if @filters.is_a? Array and @filters.any?
+      cond_out = []
+      param_out = []
+      @filters.each do |attr, operator, operand|
+        if !model_class.searchable_columns.index attr.to_s
+          raise ArgumentError.new("Invalid attribute '#{attr}' in condition")
+        end
+        case operator.downcase
+        when '=', '<', '<=', '>', '>=', 'like'
+          if operand.is_a? String
+            cond_out << "#{table_name}.#{attr} #{operator} ?"
+            if operator.match(/[<=>]/) and
+                model_class.attribute_column(attr).type == :datetime
+              operand = Time.parse operand
+            end
+            param_out << operand
+          end
+        when 'in'
+          if operand.is_a? Array
+            cond_out << "#{table_name}.#{attr} IN (?)"
+            param_out << operand
+          end
+        end
+      end
+      if cond_out.any?
+        @objects = @objects.where(cond_out.join(' AND '), *param_out)
+      end
+    end
+    if @where.is_a? Hash and @where.any?
       conditions = ['1=1']
       @where.each do |attr,value|
         if attr == :any
@@ -386,6 +434,7 @@ class ApplicationController < ActionController::Base
 
   def self._index_requires_parameters
     {
+      filters: { type: 'array', required: false },
       where: { type: 'object', required: false },
       order: { type: 'string', required: false }
     }
diff --git a/services/api/app/controllers/arvados/v1/jobs_controller.rb b/services/api/app/controllers/arvados/v1/jobs_controller.rb
index 5c2f5db6cf..a715d0ef29 100644
--- a/services/api/app/controllers/arvados/v1/jobs_controller.rb
+++ b/services/api/app/controllers/arvados/v1/jobs_controller.rb
@@ -6,6 +6,7 @@ class Arvados::V1::JobsController < ApplicationController
   skip_before_filter :render_404_if_no_object, :only => :queue
 
   def index
+    return super unless @where.is_a? Hash
     want_ancestor = @where[:script_version_descends_from]
     if want_ancestor
       # Check for missing commit_ancestor rows, and create them if
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index c89efdf404..84bdf95763 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -40,12 +40,16 @@ class ArvadosModel < ActiveRecord::Base
 
   def self.searchable_columns
     self.columns.collect do |col|
-      if [:string, :text].index(col.type) && col.name != 'owner_uuid'
+      if [:string, :text, :datetime].index(col.type) && col.name != 'owner_uuid'
         col.name
       end
     end.compact
   end
 
+  def self.attribute_column attr
+    self.columns.select { |col| col.name == attr.to_s }.first
+  end
+
   def eager_load_associations
     self.class.columns.each do |col|
       re = col.name.match /^(.*)_kind$/
diff --git a/services/api/test/functional/arvados/v1/jobs_controller_test.rb b/services/api/test/functional/arvados/v1/jobs_controller_test.rb
index 95bbf529db..91f867a15c 100644
--- a/services/api/test/functional/arvados/v1/jobs_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/jobs_controller_test.rb
@@ -103,4 +103,78 @@ class Arvados::V1::JobsControllerTest < ActionController::TestCase
     }
     assert_response :success
   end
+
+  test "search jobs by uuid with >= query" do
+    authorize_with :active
+    get :index, {
+      filters: [['uuid', '>=', 'zzzzz-8i9sb-pshmckwoma9plh7']]
+    }
+    assert_response :success
+    found = assigns(:objects).collect(&:uuid)
+    assert_equal true, !!found.index('zzzzz-8i9sb-pshmckwoma9plh7')
+    assert_equal false, !!found.index('zzzzz-8i9sb-4cf0nhn6xte809j')
+  end
+
+  test "search jobs by uuid with <= query" do
+    authorize_with :active
+    get :index, {
+      filters: [['uuid', '<=', 'zzzzz-8i9sb-pshmckwoma9plh7']]
+    }
+    assert_response :success
+    found = assigns(:objects).collect(&:uuid)
+    assert_equal true, !!found.index('zzzzz-8i9sb-pshmckwoma9plh7')
+    assert_equal true, !!found.index('zzzzz-8i9sb-4cf0nhn6xte809j')
+  end
+
+  test "search jobs by uuid with >= and <= query" do
+    authorize_with :active
+    get :index, {
+      filters: [['uuid', '>=', 'zzzzz-8i9sb-pshmckwoma9plh7'],
+              ['uuid', '<=', 'zzzzz-8i9sb-pshmckwoma9plh7']]
+    }
+    assert_response :success
+    found = assigns(:objects).collect(&:uuid)
+    assert_equal found, ['zzzzz-8i9sb-pshmckwoma9plh7']
+  end
+
+  test "search jobs by uuid with < query" do
+    authorize_with :active
+    get :index, {
+      filters: [['uuid', '<', 'zzzzz-8i9sb-pshmckwoma9plh7']]
+    }
+    assert_response :success
+    found = assigns(:objects).collect(&:uuid)
+    assert_equal false, !!found.index('zzzzz-8i9sb-pshmckwoma9plh7')
+    assert_equal true, !!found.index('zzzzz-8i9sb-4cf0nhn6xte809j')
+  end
+
+  test "search jobs by uuid with like query" do
+    authorize_with :active
+    get :index, {
+      filters: [['uuid', 'like', '%hmckwoma9pl%']]
+    }
+    assert_response :success
+    found = assigns(:objects).collect(&:uuid)
+    assert_equal found, ['zzzzz-8i9sb-pshmckwoma9plh7']
+  end
+
+  test "search jobs by uuid with 'in' query" do
+    authorize_with :active
+    get :index, {
+      filters: [['uuid', 'in', ['zzzzz-8i9sb-4cf0nhn6xte809j',
+                                'zzzzz-8i9sb-pshmckwoma9plh7']]]
+    }
+    assert_response :success
+    found = assigns(:objects).collect(&:uuid)
+    assert_equal found.sort, ['zzzzz-8i9sb-4cf0nhn6xte809j',
+                              'zzzzz-8i9sb-pshmckwoma9plh7']
+  end
+
+  test "search jobs by nonexistent column with < query" do
+    authorize_with :active
+    get :index, {
+      filters: [['is_borked', '<', 'fizzbuzz']]
+    }
+    assert_response 422
+  end
 end