[arvados] created: 2.6.0-5-g86833a7af
git repository hosting
git at public.arvados.org
Thu Apr 13 19:42:40 UTC 2023
at 86833a7af56d6ec96fcd8c6f3c2f7f2fea795ea3 (commit)
commit 86833a7af56d6ec96fcd8c6f3c2f7f2fea795ea3
Author: Tom Clegg <tom at curii.com>
Date: Thu Apr 13 15:42:22 2023 -0400
20336: Allow filtering on set of integers.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/doc/api/methods.html.textile.liquid b/doc/api/methods.html.textile.liquid
index 7f05142db..7b28533e3 100644
--- a/doc/api/methods.html.textile.liquid
+++ b/doc/api/methods.html.textile.liquid
@@ -110,7 +110,7 @@ table(table table-bordered table-condensed).
@["storage_classes_desired","=","[\"default\"]"]@|
|@<@, @<=@, @>=@, @>@|string, number, or timestamp|Ordering comparison|@["script_version",">","123"]@|
|@like@, @ilike@|string|SQL pattern match. Single character match is @_@ and wildcard is @%@. The @ilike@ operator is case-insensitive|@["script_version","like","d00220fb%"]@|
-|@in@, @not in@|array of strings|Set membership|@["script_version","in",["main","d00220fb38d4b85ca8fc28a8151702a2b9d1dec5"]]@|
+|@in@, @not in@|array of strings or integers|Set membership|@["script_version","in",["main","d00220fb38d4b85ca8fc28a8151702a2b9d1dec5"]]@|
|@is_a@|string|Arvados object type|@["head_uuid","is_a","arvados#collection"]@|
|@exists@|string|Presence of subproperty|@["properties","exists","my_subproperty"]@|
|@contains@|string, array of strings|Presence of one or more keys or array elements|@["storage_classes_desired", "contains", ["foo", "bar"]]@ (matches both @["foo", "bar"]@ and @["foo", "bar", "baz"]@)
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 33f950de3..d910320ec 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -156,7 +156,7 @@ class ArvadosModel < ApplicationRecord
end
def self.searchable_columns operator
- textonly_operator = !operator.match(/[<=>]/)
+ textonly_operator = !operator.match(/[<=>]/) && !operator.in?(['in', 'not in'])
self.columns.select do |col|
case col.type
when :string, :text
diff --git a/services/api/lib/record_filters.rb b/services/api/lib/record_filters.rb
index 6e6dc2c5f..b15207b14 100644
--- a/services/api/lib/record_filters.rb
+++ b/services/api/lib/record_filters.rb
@@ -164,10 +164,10 @@ module RecordFilters
!(col.andand.type == :jsonb && ['contains', '=', '<>', '!='].index(operator))
raise ArgumentError.new("Invalid attribute '#{attr}' in filter")
end
+ attr_type = attr_model_class.attribute_column(attr).type
case operator
when '=', '<', '<=', '>', '>=', '!=', 'like', 'ilike'
- attr_type = attr_model_class.attribute_column(attr).type
operator = '<>' if operator == '!='
if operand.is_a? String
if attr_type == :boolean
@@ -217,17 +217,24 @@ module RecordFilters
"for '#{operator}' operator in filters")
end
when 'in', 'not in'
- if operand.is_a? Array
- 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 #{attr_table_name}.#{attr} IS NULL)"
- end
- else
+ if !operand.is_a? Array
raise ArgumentError.new("Invalid operand type '#{operand.class}' "\
"for '#{operator}' operator in filters")
end
+ if attr_type == :integer
+ operand.each do |el|
+ if !el.is_a?(Integer) || el.bit_length > 64
+ raise ArgumentError.new("Invalid element '#{el}' in array "\
+ "for integer attribute '#{attr}'")
+ end
+ end
+ end
+ 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 #{attr_table_name}.#{attr} IS NULL)"
+ end
when 'is_a'
operand = [operand] unless operand.is_a? Array
cond = []
diff --git a/services/api/test/functional/arvados/v1/filters_test.rb b/services/api/test/functional/arvados/v1/filters_test.rb
index 6331e4ef0..5d343314c 100644
--- a/services/api/test/functional/arvados/v1/filters_test.rb
+++ b/services/api/test/functional/arvados/v1/filters_test.rb
@@ -52,6 +52,18 @@ class Arvados::V1::FiltersTest < ActionController::TestCase
assert_match(/Invalid operand .* integer attribute/, json_response['errors'].join(' '))
end
+ ['in', 'not in'].each do |operator|
+ test "error message for int64 overflow ('#{operator}' filter)" do
+ @controller = Arvados::V1::ContainerRequestsController.new
+ authorize_with :active
+ get :index, params: {
+ filters: [['priority', operator, [9, 123412341234123412341234]]],
+ }
+ assert_response 422
+ assert_match(/Invalid element .* integer attribute/, json_response['errors'].join(' '))
+ end
+ end
+
test 'error message for invalid boolean operand' do
@controller = Arvados::V1::GroupsController.new
authorize_with :active
commit 7f284bc93becb372f88b918d4dd1414903d1ad3f
Author: Tom Clegg <tom at curii.com>
Date: Thu Apr 13 13:27:34 2023 -0400
20336: Reject >64 bit operands when filtering on integer columns.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/sdk/go/arvados/log.go b/sdk/go/arvados/log.go
index 06d7987e3..b5860d059 100644
--- a/sdk/go/arvados/log.go
+++ b/sdk/go/arvados/log.go
@@ -10,7 +10,7 @@ import (
// Log is an arvados#log record
type Log struct {
- ID uint64 `json:"id"`
+ ID int64 `json:"id"`
UUID string `json:"uuid"`
OwnerUUID string `json:"owner_uuid"`
ObjectUUID string `json:"object_uuid"`
diff --git a/services/api/lib/record_filters.rb b/services/api/lib/record_filters.rb
index 65c25810a..6e6dc2c5f 100644
--- a/services/api/lib/record_filters.rb
+++ b/services/api/lib/record_filters.rb
@@ -181,8 +181,8 @@ module RecordFilters
when '0', 'f', 'false', 'n', 'no'
operand = false
else
- raise ArgumentError("Invalid operand '#{operand}' for " \
- "boolean attribute '#{attr}'")
+ raise ArgumentError.new("Invalid operand '#{operand}' for " \
+ "boolean attribute '#{attr}'")
end
end
if operator == '<>'
@@ -206,6 +206,10 @@ module RecordFilters
cond_out << "#{attr_table_name}.#{attr} #{operator} ?"
param_out << operand
elsif (attr_type == :integer)
+ if !operand.is_a?(Integer) || operand.bit_length > 64
+ raise ArgumentError.new("Invalid operand '#{operand}' "\
+ "for integer attribute '#{attr}'")
+ end
cond_out << "#{attr_table_name}.#{attr} #{operator} ?"
param_out << operand
else
diff --git a/services/api/test/functional/arvados/v1/filters_test.rb b/services/api/test/functional/arvados/v1/filters_test.rb
index 3916d63c5..6331e4ef0 100644
--- a/services/api/test/functional/arvados/v1/filters_test.rb
+++ b/services/api/test/functional/arvados/v1/filters_test.rb
@@ -39,6 +39,29 @@ class Arvados::V1::FiltersTest < ActionController::TestCase
assert_match(/no longer supported/, json_response['errors'].join(' '))
end
+ test 'error message for int64 overflow' do
+ # some versions of ActiveRecord cast >64-bit ints to postgres
+ # numeric type, but this is never useful because database content
+ # is 64 bit.
+ @controller = Arvados::V1::LogsController.new
+ authorize_with :active
+ get :index, params: {
+ filters: [['id', '=', 123412341234123412341234]],
+ }
+ assert_response 422
+ assert_match(/Invalid operand .* integer attribute/, json_response['errors'].join(' '))
+ end
+
+ test 'error message for invalid boolean operand' do
+ @controller = Arvados::V1::GroupsController.new
+ authorize_with :active
+ get :index, params: {
+ filters: [['is_trashed', '=', 'fourty']],
+ }
+ assert_response 422
+ assert_match(/Invalid operand .* boolean attribute/, json_response['errors'].join(' '))
+ end
+
test 'api responses provide timestamps with nanoseconds' do
@controller = Arvados::V1::CollectionsController.new
authorize_with :active
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list