[arvados] created: 2.6.0-5-g5f0b4881b
git repository hosting
git at public.arvados.org
Thu Apr 13 20:30:21 UTC 2023
at 5f0b4881b7522a952fda5be96c7041a6519d485d (commit)
commit 5f0b4881b7522a952fda5be96c7041a6519d485d
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 c8492303cf40c55f121720dfd0407822aa7d074e
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
diff --git a/services/ws/event.go b/services/ws/event.go
index c989c0ca5..8b6a2e81b 100644
--- a/services/ws/event.go
+++ b/services/ws/event.go
@@ -26,7 +26,7 @@ type eventSource interface {
}
type event struct {
- LogID uint64
+ LogID int64
Received time.Time
Ready time.Time
Serial uint64
diff --git a/services/ws/event_source.go b/services/ws/event_source.go
index 3593c3aeb..923a341b7 100644
--- a/services/ws/event_source.go
+++ b/services/ws/event_source.go
@@ -281,7 +281,7 @@ func (ps *pgEventSource) Run() {
ps.Logger.WithField("pqEvent", pqEvent).Error("unexpected notify from wrong channel")
continue
}
- logID, err := strconv.ParseUint(pqEvent.Extra, 10, 64)
+ logID, err := strconv.ParseInt(pqEvent.Extra, 10, 64)
if err != nil {
ps.Logger.WithField("pqEvent", pqEvent).Error("bad notify payload")
continue
diff --git a/services/ws/event_source_test.go b/services/ws/event_source_test.go
index b7b8ac300..d02b19993 100644
--- a/services/ws/event_source_test.go
+++ b/services/ws/event_source_test.go
@@ -80,14 +80,14 @@ func (*eventSourceSuite) TestEventSource(c *check.C) {
for i := 0; i <= si; i++ {
ev := <-sinks[si].Channel()
c.Logf("sink %d received event %d", si, i)
- c.Check(ev.LogID, check.Equals, uint64(i))
+ c.Check(ev.LogID, check.Equals, int64(i))
row := ev.Detail()
if i == 0 {
// no matching row, null event
c.Check(row, check.IsNil)
} else {
c.Check(row, check.NotNil)
- c.Check(row.ID, check.Equals, uint64(i))
+ c.Check(row.ID, check.Equals, int64(i))
c.Check(row.UUID, check.Not(check.Equals), "")
}
}
diff --git a/services/ws/session_v0.go b/services/ws/session_v0.go
index 309352b39..66bc6ac7f 100644
--- a/services/ws/session_v0.go
+++ b/services/ws/session_v0.go
@@ -201,9 +201,9 @@ func (sub *v0subscribe) sendOldEvents(sess *v0session) {
return
}
- var ids []uint64
+ var ids []int64
for rows.Next() {
- var id uint64
+ var id int64
err := rows.Scan(&id)
if err != nil {
sess.log.WithError(err).Error("sendOldEvents row Scan failed")
diff --git a/services/ws/session_v0_test.go b/services/ws/session_v0_test.go
index 7986cc7b0..d7c9edb24 100644
--- a/services/ws/session_v0_test.go
+++ b/services/ws/session_v0_test.go
@@ -35,7 +35,7 @@ type v0Suite struct {
token string
toDelete []string
wg sync.WaitGroup
- ignoreLogID uint64
+ ignoreLogID int64
}
func (s *v0Suite) SetUpTest(c *check.C) {
@@ -363,8 +363,8 @@ func (s *v0Suite) testClient() (*websocket.Conn, *json.Decoder, *json.Encoder) {
return conn, r, w
}
-func (s *v0Suite) lastLogID(c *check.C) uint64 {
- var lastID uint64
+func (s *v0Suite) lastLogID(c *check.C) int64 {
+ var lastID int64
c.Assert(testDB().QueryRow(`SELECT MAX(id) FROM logs`).Scan(&lastID), check.IsNil)
return lastID
}
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list