[ARVADOS] updated: 2.1.0-1280-g30bd9f9ef

Git user git at public.arvados.org
Wed Sep 1 18:25:02 UTC 2021


Summary of changes:
 doc/api/methods.html.textile.liquid                | 30 ++++-----
 .../install-dispatch.html.textile.liquid           |  4 +-
 lib/config/config.default.yml                      |  7 ++-
 lib/config/generated_config.go                     |  7 ++-
 lib/controller/router/router_test.go               | 43 ++++++++++---
 lib/lsf/dispatch_test.go                           | 10 ++-
 sdk/go/arvados/resource_list.go                    | 45 ++++++++------
 sdk/go/arvados/resource_list_test.go               | 72 ++++++++++------------
 sdk/go/arvadosclient/arvadosclient.go              |  2 +-
 9 files changed, 133 insertions(+), 87 deletions(-)

       via  30bd9f9ef5cda55548e0391f7925317cb65b6d9f (commit)
       via  7260f68bca6bff0a624801694e1b6b0053c0fbd2 (commit)
       via  2c2153daa103c35fe12468a853636f8125785a7e (commit)
       via  8f266e877db7d819c9fa74d680a320c6153bd207 (commit)
       via  d026d60cc8b9f1117e815d8fb76d4901ca6eaf26 (commit)
      from  2115f4609adcc86156e8e0aa59ea38ba5808378e (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 30bd9f9ef5cda55548e0391f7925317cb65b6d9f
Merge: 7260f68bc 2c2153daa
Author: Tom Clegg <tom at curii.com>
Date:   Wed Sep 1 14:24:47 2021 -0400

    17995: Merge branch 'main'
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>


commit 7260f68bca6bff0a624801694e1b6b0053c0fbd2
Author: Tom Clegg <tom at curii.com>
Date:   Wed Sep 1 14:22:59 2021 -0400

    17995: Accept boolean expressions as strings without ["...",=,true].
    
    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 87ff621b8..fd5291792 100644
--- a/doc/api/methods.html.textile.liquid
+++ b/doc/api/methods.html.textile.liquid
@@ -111,20 +111,6 @@ table(table table-bordered table-condensed).
 |@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"]@)
 (note @[..., "contains", "foo"]@ is also accepted, and is equivalent to @[..., "contains", ["foo"]]@)|
 
-h4(#filterexpression). Filtering using boolean expressions
-
-In place of an attribute, the first element of the three-element filter array can be a boolean expression. 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).
-* The second and third elements of the filter array must be @"="@ and @true@ respectively.
-
-Examples:
-* @["(replication_desired < replication_confirmed)", "=", true]@
-* @["(replication_desired = replication_confirmed)", "=", true]@
-
 h4(#substringsearchfilter). Filtering using substring search
 
 Resources can also be filtered by searching for a substring in attributes of type @string@, @array of strings@, @text@, and @hash@, which are indexed in the database specifically for search. To use substring search, the filter must:
@@ -150,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 0330ec425..639d2a28b 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<bar)%22]",
+			header:      http.Header{"Content-Type": {"application/x-www-form-urlencoded"}},
+			shouldCall:  "CollectionList",
+			withOptions: arvados.ListOptions{Limit: -1, Filters: []arvados.Filter{{"(foo<bar)", "=", true}}},
+		},
+		{
+			comment:     "form-encoded expression filter in POST body",
+			method:      "POST",
+			path:        "/arvados/v1/collections",
+			body:        "filters=[\"(foo<bar)\"]&_method=GET",
+			header:      http.Header{"Content-Type": {"application/x-www-form-urlencoded"}},
+			shouldCall:  "CollectionList",
+			withOptions: arvados.ListOptions{Limit: -1, Filters: []arvados.Filter{{"(foo<bar)", "=", true}}},
+		},
+		{
+			comment:     "json-encoded expression filter in POST body",
+			method:      "POST",
+			path:        "/arvados/v1/collections?_method=GET",
+			body:        `{"filters":["(foo<bar)",["bar","=","baz"]],"limit":2}`,
+			header:      http.Header{"Content-Type": {"application/json"}},
+			shouldCall:  "CollectionList",
+			withOptions: arvados.ListOptions{Limit: 2, Filters: []arvados.Filter{{"(foo<bar)", "=", true}, {"bar", "=", "baz"}}},
+		},
 		{
 			method:       "PATCH",
 			path:         "/arvados/v1/collections",
@@ -139,21 +166,23 @@ func (s *RouterSuite) TestOptions(c *check.C) {
 		// Reset calls captured in previous trial
 		s.stub = arvadostest.APIStub{}
 
-		c.Logf("trial: %#v", trial)
+		c.Logf("trial: %+v", trial)
+		comment := check.Commentf("trial comment: %s", trial.comment)
+
 		_, rr, _ := doRequest(c, s.rtr, token, trial.method, trial.path, trial.header, bytes.NewBufferString(trial.body))
 		if trial.shouldStatus == 0 {
-			c.Check(rr.Code, check.Equals, http.StatusOK)
+			c.Check(rr.Code, check.Equals, http.StatusOK, comment)
 		} else {
-			c.Check(rr.Code, check.Equals, trial.shouldStatus)
+			c.Check(rr.Code, check.Equals, trial.shouldStatus, comment)
 		}
 		calls := s.stub.Calls(nil)
 		if trial.shouldCall == "" {
-			c.Check(calls, check.HasLen, 0)
+			c.Check(calls, check.HasLen, 0, comment)
 		} else if len(calls) != 1 {
-			c.Check(calls, check.HasLen, 1)
+			c.Check(calls, check.HasLen, 1, comment)
 		} else {
-			c.Check(calls[0].Method, isMethodNamed, trial.shouldCall)
-			c.Check(calls[0].Options, check.DeepEquals, trial.withOptions)
+			c.Check(calls[0].Method, isMethodNamed, trial.shouldCall, comment)
+			c.Check(calls[0].Options, check.DeepEquals, trial.withOptions, comment)
 		}
 	}
 }
diff --git a/sdk/go/arvados/resource_list.go b/sdk/go/arvados/resource_list.go
index a5cc7d3b9..7f319b412 100644
--- a/sdk/go/arvados/resource_list.go
+++ b/sdk/go/arvados/resource_list.go
@@ -37,28 +37,37 @@ func (f *Filter) MarshalJSON() ([]byte, error) {
 
 // UnmarshalJSON decodes a JSON array to a Filter.
 func (f *Filter) UnmarshalJSON(data []byte) error {
-	var elements []interface{}
-	err := json.Unmarshal(data, &elements)
+	var decoded interface{}
+	err := json.Unmarshal(data, &decoded)
 	if err != nil {
 		return err
 	}
-	if len(elements) != 3 {
-		return fmt.Errorf("invalid filter %q: must have 3 elements", data)
-	}
-	attr, ok := elements[0].(string)
-	if !ok {
-		return fmt.Errorf("invalid filter attr %q", elements[0])
-	}
-	op, ok := elements[1].(string)
-	if !ok {
-		return fmt.Errorf("invalid filter operator %q", elements[1])
-	}
-	operand := elements[2]
-	switch operand.(type) {
-	case string, float64, []interface{}, nil, bool:
+	switch decoded := decoded.(type) {
+	case string:
+		// Accept "(foo < bar)" as a more obvious way to spell
+		// ["(foo < bar)","=",true]
+		*f = Filter{decoded, "=", true}
+	case []interface{}:
+		if len(decoded) != 3 {
+			return fmt.Errorf("invalid filter %q: must have 3 decoded", data)
+		}
+		attr, ok := decoded[0].(string)
+		if !ok {
+			return fmt.Errorf("invalid filter attr %q", decoded[0])
+		}
+		op, ok := decoded[1].(string)
+		if !ok {
+			return fmt.Errorf("invalid filter operator %q", decoded[1])
+		}
+		operand := decoded[2]
+		switch operand.(type) {
+		case string, float64, []interface{}, nil, bool:
+		default:
+			return fmt.Errorf("invalid filter operand %q", decoded[2])
+		}
+		*f = Filter{attr, op, operand}
 	default:
-		return fmt.Errorf("invalid filter operand %q", elements[2])
+		return fmt.Errorf("invalid filter: json decoded as %T instead of array or string", decoded)
 	}
-	*f = Filter{attr, op, operand}
 	return nil
 }
diff --git a/sdk/go/arvados/resource_list_test.go b/sdk/go/arvados/resource_list_test.go
index b36e82c91..e4d89d62a 100644
--- a/sdk/go/arvados/resource_list_test.go
+++ b/sdk/go/arvados/resource_list_test.go
@@ -5,69 +5,59 @@
 package arvados
 
 import (
-	"bytes"
 	"encoding/json"
-	"testing"
 	"time"
+
+	check "gopkg.in/check.v1"
 )
 
-func TestMarshalFiltersWithNanoseconds(t *testing.T) {
+var _ = check.Suite(&filterEncodingSuite{})
+
+type filterEncodingSuite struct{}
+
+func (s *filterEncodingSuite) TestMarshalNanoseconds(c *check.C) {
 	t0 := time.Now()
 	t0str := t0.Format(time.RFC3339Nano)
 	buf, err := json.Marshal([]Filter{
 		{Attr: "modified_at", Operator: "=", Operand: t0}})
-	if err != nil {
-		t.Fatal(err)
-	}
-	if expect := []byte(`[["modified_at","=","` + t0str + `"]]`); 0 != bytes.Compare(buf, expect) {
-		t.Errorf("Encoded as %q, expected %q", buf, expect)
-	}
+	c.Assert(err, check.IsNil)
+	c.Check(string(buf), check.Equals, `[["modified_at","=","`+t0str+`"]]`)
 }
 
-func TestMarshalFiltersWithNil(t *testing.T) {
+func (s *filterEncodingSuite) TestMarshalNil(c *check.C) {
 	buf, err := json.Marshal([]Filter{
 		{Attr: "modified_at", Operator: "=", Operand: nil}})
-	if err != nil {
-		t.Fatal(err)
-	}
-	if expect := []byte(`[["modified_at","=",null]]`); 0 != bytes.Compare(buf, expect) {
-		t.Errorf("Encoded as %q, expected %q", buf, expect)
-	}
+	c.Assert(err, check.IsNil)
+	c.Check(string(buf), check.Equals, `[["modified_at","=",null]]`)
 }
 
-func TestUnmarshalFiltersWithNil(t *testing.T) {
+func (s *filterEncodingSuite) TestUnmarshalNil(c *check.C) {
 	buf := []byte(`["modified_at","=",null]`)
-	f := &Filter{}
+	var f Filter
 	err := f.UnmarshalJSON(buf)
-	if err != nil {
-		t.Fatal(err)
-	}
-	expect := Filter{Attr: "modified_at", Operator: "=", Operand: nil}
-	if f.Attr != expect.Attr || f.Operator != expect.Operator || f.Operand != expect.Operand {
-		t.Errorf("Decoded as %q, expected %q", f, expect)
-	}
+	c.Assert(err, check.IsNil)
+	c.Check(f, check.DeepEquals, Filter{Attr: "modified_at", Operator: "=", Operand: nil})
 }
 
-func TestMarshalFiltersWithBoolean(t *testing.T) {
+func (s *filterEncodingSuite) TestMarshalBoolean(c *check.C) {
 	buf, err := json.Marshal([]Filter{
 		{Attr: "is_active", Operator: "=", Operand: true}})
-	if err != nil {
-		t.Fatal(err)
-	}
-	if expect := []byte(`[["is_active","=",true]]`); 0 != bytes.Compare(buf, expect) {
-		t.Errorf("Encoded as %q, expected %q", buf, expect)
-	}
+	c.Assert(err, check.IsNil)
+	c.Check(string(buf), check.Equals, `[["is_active","=",true]]`)
 }
 
-func TestUnmarshalFiltersWithBoolean(t *testing.T) {
+func (s *filterEncodingSuite) TestUnmarshalBoolean(c *check.C) {
 	buf := []byte(`["is_active","=",true]`)
-	f := &Filter{}
+	var f Filter
+	err := f.UnmarshalJSON(buf)
+	c.Assert(err, check.IsNil)
+	c.Check(f, check.DeepEquals, Filter{Attr: "is_active", Operator: "=", Operand: true})
+}
+
+func (s *filterEncodingSuite) TestUnmarshalBooleanExpression(c *check.C) {
+	buf := []byte(`"(foo < bar)"`)
+	var f Filter
 	err := f.UnmarshalJSON(buf)
-	if err != nil {
-		t.Fatal(err)
-	}
-	expect := Filter{Attr: "is_active", Operator: "=", Operand: true}
-	if f.Attr != expect.Attr || f.Operator != expect.Operator || f.Operand != expect.Operand {
-		t.Errorf("Decoded as %q, expected %q", f, expect)
-	}
+	c.Assert(err, check.IsNil)
+	c.Check(f, check.DeepEquals, Filter{Attr: "(foo < bar)", Operator: "=", Operand: true})
 }

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list