From: Tom Clegg Date: Fri, 10 Sep 2021 14:25:58 +0000 (-0400) Subject: Merge branch '17995-filter-by-comparing-attrs' X-Git-Tag: 2.3.0~75 X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/7aaf9f22aa646077b4b7fd961d6b731185b88137?hp=b8b6fb68a54f0f81d8ad6a75cad2650179108d66 Merge branch '17995-filter-by-comparing-attrs' closes #17995 Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- diff --git a/doc/api/methods.html.textile.liquid b/doc/api/methods.html.textile.liquid index e051ab66fa..fd52917928 100644 --- a/doc/api/methods.html.textile.liquid +++ b/doc/api/methods.html.textile.liquid @@ -136,6 +136,22 @@ table(table table-bordered table-condensed). Note that exclusion filters @!=@ and @not in@ will return records for which the property is not defined at all. To restrict filtering to records on which the subproperty is defined, combine with an @exists@ filter. +h4(#filterexpression). Filtering using boolean expressions + +In addition to the three-element array form described above, a string containing a boolean expression is also accepted. The following restrictions apply: +* The expression must contain exactly one operator. +* The operator must be @=@, @<@, @<=@, @>@, or @>=@. +* There must be exactly one pair of parentheses, surrounding the entire expression. +* Each operand must be the name of a numeric attribute like @replication_desired@ (literal values like @3@ and non-numeric attributes like @uuid@ are not accepted). +* The expression must not contain whitespace other than an ASCII space (newline and tab characters are not accepted). + +Examples: +* @(replication_desired > replication_confirmed)@ +* @(replication_desired = replication_confirmed)@ + +Both types of filter (boolean expressions and @[attribute, operator, operand]@ filters) can be combined in the same API call. Example: +* @{"filters": ["(replication_desired > replication_confirmed)", ["replication_desired", "<", 2]]}@ + h4. Federated listing Federated listing forwards a request to multiple clusters and combines the results. Currently only a very restricted form of the "list" method is supported. diff --git a/lib/controller/router/router_test.go b/lib/controller/router/router_test.go index 0330ec4252..639d2a28b4 100644 --- a/lib/controller/router/router_test.go +++ b/lib/controller/router/router_test.go @@ -47,6 +47,7 @@ func (s *RouterSuite) SetUpTest(c *check.C) { func (s *RouterSuite) TestOptions(c *check.C) { token := arvadostest.ActiveToken for _, trial := range []struct { + comment string // unparsed -- only used to help match test failures to trials method string path string header http.Header @@ -120,6 +121,32 @@ func (s *RouterSuite) TestOptions(c *check.C) { shouldCall: "CollectionList", withOptions: arvados.ListOptions{Limit: 123, Offset: 456, IncludeTrash: true, IncludeOldVersions: true}, }, + { + comment: "form-encoded expression filter in query string", + method: "GET", + path: "/arvados/v1/collections?filters=[%22(foo=?|=) *(\w+) *\) *$/.match(attr) + if operator != '=' || ![true,"true"].index(operand) + raise ArgumentError.new("Invalid expression filter '#{attr}': subsequent elements must be [\"=\", true]") + end + operator = expr[2] + attr1, attr2 = expr[1], expr[3] + allowed = attr_model_class.searchable_columns(operator) + [attr1, attr2].each do |tok| + if !allowed.index(tok) + raise ArgumentError.new("Invalid attribute in expression: '#{tok}'") + end + col = attr_model_class.columns.select { |c| c.name == tok }.first + if col.type != :integer + raise ArgumentError.new("Non-numeric attribute in expression: '#{tok}'") + end + end + cond_out << "#{attr1} #{operator} #{attr2}" else if !attr_model_class.searchable_columns(operator).index(attr) && !(col.andand.type == :jsonb && ['contains', '=', '<>', '!='].index(operator)) diff --git a/services/api/test/functional/arvados/v1/collections_controller_test.rb b/services/api/test/functional/arvados/v1/collections_controller_test.rb index 6c923ff38d..eac393104c 100644 --- a/services/api/test/functional/arvados/v1/collections_controller_test.rb +++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb @@ -1404,6 +1404,50 @@ EOS assert_equal col.version, json_response['version'], 'Trashing a collection should not create a new version' end + [['<', :<], + ['<=', :<=], + ['>', :>], + ['>=', :>=], + ['=', :==]].each do |op, rubyop| + test "filter collections by replication_desired #{op} replication_confirmed" do + authorize_with(:active) + get :index, params: { + filters: [["(replication_desired #{op} replication_confirmed)", "=", true]], + } + assert_response :success + json_response["items"].each do |c| + assert_operator(c["replication_desired"], rubyop, c["replication_confirmed"]) + end + end + end + + ["(replication_desired < bogus)", + "replication_desired < replication_confirmed", + "(replication_desired < replication_confirmed", + "(replication_desired ! replication_confirmed)", + "(replication_desired <)", + "(replication_desired < manifest_text)", + "(manifest_text < manifest_text)", # currently only numeric attrs are supported + "(replication_desired < 2)", # currently only attrs are supported, not literals + "(1 < 2)", + ].each do |expr| + test "invalid filter expression #{expr}" do + authorize_with(:active) + get :index, params: { + filters: [[expr, "=", true]], + } + assert_response 422 + end + end + + test "invalid op/arg with filter expression" do + authorize_with(:active) + get :index, params: { + filters: [["replication_desired < replication_confirmed", "!=", false]], + } + assert_response 422 + end + ["storage_classes_desired", "storage_classes_confirmed"].each do |attr| test "filter collections by #{attr}" do authorize_with(:active)