[ARVADOS] created: 5d4e8f44974b54bb28141154dcd4a6f987453f4e

[git at public.curoverse.com]

        at  5d4e8f44974b54bb28141154dcd4a6f987453f4e (commit)


commit 5d4e8f44974b54bb28141154dcd4a6f987453f4e
Author: Tom Clegg <tom at curoverse.com>
Date:   Sun Apr 13 16:03:54 2014 -0400

    Improve error checking when applying filters. Add tests.

diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 06e1838..4b24452 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -146,8 +146,10 @@ class ApplicationController < ActionController::Base
       cond_out = []
       param_out = []
       @filters.each do |attr, operator, operand|
-        if !model_class.searchable_columns(operator).index attr.to_s
-          raise ArgumentError.new("Invalid attribute '#{attr}' in condition")
+        if !operator.is_a? String
+          raise ArgumentError.new("Invalid operator '#{operator}' (#{operator.class}) in filter")
+        elsif !model_class.searchable_columns(operator).index attr.to_s
+          raise ArgumentError.new("Invalid attribute '#{attr}' in filter")
         end
         case operator.downcase
         when '=', '<', '<=', '>', '>=', 'like'
@@ -160,12 +162,18 @@ class ApplicationController < ActionController::Base
               operand = Time.parse operand
             end
             param_out << operand
+          else
+            raise ArgumentError.new("Invalid operand type '#{operand.class}' for '#{operator}' operator")
           end
         when 'in'
           if operand.is_a? Array
             cond_out << "#{table_name}.#{attr} IN (?)"
             param_out << operand
+          else
+            raise ArgumentError.new("Invalid argument '#{operand}' for 'in' operator")
           end
+        else
+          raise ArgumentError.new("Invalid operator '#{attr}' in filter")
         end
       end
       if cond_out.any?
diff --git a/services/api/test/integration/collections_api_test.rb b/services/api/test/integration/collections_api_test.rb
index 5fa77c3..b8de580 100644
--- a/services/api/test/integration/collections_api_test.rb
+++ b/services/api/test/integration/collections_api_test.rb
@@ -15,6 +15,30 @@ class CollectionsApiTest < ActionDispatch::IntegrationTest
     assert_equal "arvados#collectionList", jresponse['kind']
   end
 
+  test "get index with invalid filters (array of strings) responds 422" do
+    get "/arvados/v1/collections", {
+      :format => :json,
+      :filters => ['uuid', '=', 'ad02e37b6a7f45bbe2ead3c29a109b8a+54']
+    }, auth(:active)
+    assert_response 422
+  end
+
+  test "get index with invalid filters (unsearchable column) responds 422" do
+    get "/arvados/v1/collections", {
+      :format => :json,
+      :filters => [['this_column_does_not_exist', '=', 'bogus']]
+    }, auth(:active)
+    assert_response 422
+  end
+
+  test "get index with invalid filters (invalid operator) responds 422" do
+    get "/arvados/v1/collections", {
+      :format => :json,
+      :filters => [['uuid', ':-(', 'displeased']]
+    }, auth(:active)
+    assert_response 422
+  end
+
   test "get index with where= (empty string)" do
     get "/arvados/v1/collections", {:format => :json, :where => ''}, auth(:active)
     assert_response :success
diff --git a/services/api/test/integration/links_api_test.rb b/services/api/test/integration/links_api_test.rb
new file mode 100644
index 0000000..363538b
--- /dev/null
+++ b/services/api/test/integration/links_api_test.rb
@@ -0,0 +1,57 @@
+require 'test_helper'
+
+class LinksApiTest < ActionDispatch::IntegrationTest
+  fixtures :all
+
+  test "should get index" do
+    get "/arvados/v1/links", {
+      :where => '{"tail_kind":"arvados#user"}',
+      :format => :json
+    }, auth(:active)
+    assert_response :success
+    assert_equal "arvados#linkList", jresponse['kind']
+  end
+
+  test "get index with tail_kind filter" do
+    get "/arvados/v1/links", {
+      :filters => '[["tail_kind","=","arvados#user"]]',
+      :format => :json
+    }, auth(:active)
+    assert_response :success
+    assert_equal "arvados#linkList", jresponse['kind']
+    jresponse['items'].each do |i|
+      assert_equal 'arvados#user', i['tail_kind']
+    end
+  end
+
+  test "get index with name and tail_uuid filter" do
+    name_list = %w(can_manage can_read)
+    get "/arvados/v1/links", {
+      :filters => [
+                   ["tail_uuid", "=", users(:active).uuid],
+                   ["name", "in", name_list],
+                  ].to_json,
+      :format => :json
+    }, auth(:active)
+    assert_response :success
+    assert_equal "arvados#linkList", jresponse['kind']
+    jresponse['items'].each do |i|
+      assert_equal 'arvados#user', i['tail_kind']
+      assert_equal users(:active).uuid, i['tail_uuid']
+      assert_not_nil name_list.index(i['name'])
+    end
+  end
+
+  test "get index with tail_kind in filters[] and in where{}" do
+    get "/arvados/v1/links", {
+      :where => '{"tail_kind":"arvados#user"}',
+      :filters => '[["tail_kind","=","arvados#user"]]',
+      :format => :json
+    }, auth(:active)
+    assert_response :success
+    assert_equal "arvados#linkList", jresponse['kind']
+    jresponse['items'].each do |i|
+      assert_equal 'arvados#user', i['tail_kind']
+    end
+  end
+end

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


hooks/post-receive
--