From 83df0b786cd8766a500b3fd3213c55598492196b Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Thu, 14 Jan 2021 00:20:09 +0300 Subject: [PATCH 1/7] feat(storage): add projection parameter for BucketHandle.Objects() --- storage/bucket.go | 2 +- storage/bucket_test.go | 28 ++++++++++++++++++++++++++++ storage/storage.go | 22 ++++++++++++++++++++++ 3 files changed, 51 insertions(+), 1 deletion(-) diff --git a/storage/bucket.go b/storage/bucket.go index 0d3be39eebc..f8f4a35499a 100644 --- a/storage/bucket.go +++ b/storage/bucket.go @@ -1204,7 +1204,7 @@ func (it *ObjectIterator) Next() (*ObjectAttrs, error) { func (it *ObjectIterator) fetch(pageSize int, pageToken string) (string, error) { req := it.bucket.c.raw.Objects.List(it.bucket.name) setClientHeader(req.Header()) - req.Projection("full") + req.Projection(it.query.GetProjection()) req.Delimiter(it.query.Delimiter) req.Prefix(it.query.Prefix) req.StartOffset(it.query.StartOffset) diff --git a/storage/bucket_test.go b/storage/bucket_test.go index 123e319a84e..d0f002325ba 100644 --- a/storage/bucket_test.go +++ b/storage/bucket_test.go @@ -663,3 +663,31 @@ func TestNewBucket(t *testing.T) { t.Errorf("got=-, want=+:\n%s", diff) } } + +func TestObjectsWithProjection(t *testing.T) { + samples := []struct { + req, expect string + } { + {"", ProjectionFull}, + {ProjectionFull, ProjectionFull}, + {ProjectionNoAcl, ProjectionNoAcl}, + } + res := &http.Response{StatusCode: http.StatusInternalServerError} + transport := &mockTransport{} + client := mockClient(t, transport) + bucket := client.Bucket("mock-bucket") + ctx := context.Background() + + for _, sample := range samples { + transport.addResult(res, nil) + query := &Query{Projection: sample.req} + bucket.Objects(ctx, query).Next() + + projection := transport.gotReq.URL.Query()["projection"] + if len(projection) == 0 { + t.Errorf("requesting %q projection: no projection parameter generated", sample.req) + } else if projection[0] != sample.expect { + t.Errorf("requesting %q projection: expecting %q, got %q", sample.req, sample.expect, projection[0]) + } + } +} \ No newline at end of file diff --git a/storage/storage.go b/storage/storage.go index d4c25b83e0d..f4a8a49e4fe 100644 --- a/storage/storage.go +++ b/storage/storage.go @@ -1306,6 +1306,14 @@ func encodeUint32(u uint32) string { return base64.StdEncoding.EncodeToString(b) } +const ( + // ProjectionFull returns all fields of object(s). + ProjectionFull = "full" + + // ProjectionNoAcl returns all fields of object(s) except for owner and acl. + ProjectionNoAcl = "noAcl" +) + // Query represents a query to filter objects from a bucket. type Query struct { // Delimiter returns results in a directory-like fashion. @@ -1341,6 +1349,10 @@ type Query struct { // lexicographically before endOffset. If startOffset is also set, the objects // listed will have names between startOffset (inclusive) and endOffset (exclusive). EndOffset string + + // Projection defines set of properties to return. This may be either + // ProjectionFull or ProjectionNoAcl, zero value means ProjectionFull. + Projection string } // attrToFieldMap maps the field names of ObjectAttrs to the underlying field @@ -1411,6 +1423,16 @@ func (q *Query) SetAttrSelection(attrs []string) error { return nil } +// GetProjection returns either value of Projection field or +// default projection when this field is empty. +func (q *Query) GetProjection() string { + if q.Projection == "" { + return ProjectionFull + } else { + return q.Projection + } +} + // Conditions constrain methods to act on specific generations of // objects. // From f0527a40d04ca167f8fab76e584e878a8a5d7a94 Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Fri, 15 Jan 2021 19:52:26 +0300 Subject: [PATCH 2/7] chore: remove unnecessary getter, fix names and comments --- storage/bucket.go | 6 +++++- storage/storage.go | 19 +++++-------------- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/storage/bucket.go b/storage/bucket.go index f8f4a35499a..9add7559af7 100644 --- a/storage/bucket.go +++ b/storage/bucket.go @@ -1204,7 +1204,11 @@ func (it *ObjectIterator) Next() (*ObjectAttrs, error) { func (it *ObjectIterator) fetch(pageSize int, pageToken string) (string, error) { req := it.bucket.c.raw.Objects.List(it.bucket.name) setClientHeader(req.Header()) - req.Projection(it.query.GetProjection()) + projection := it.query.Projection + if projection == "" { + projection = ProjectionFull + } + req.Projection(projection) req.Delimiter(it.query.Delimiter) req.Prefix(it.query.Prefix) req.StartOffset(it.query.StartOffset) diff --git a/storage/storage.go b/storage/storage.go index f4a8a49e4fe..a86a8e3e379 100644 --- a/storage/storage.go +++ b/storage/storage.go @@ -1310,8 +1310,8 @@ const ( // ProjectionFull returns all fields of object(s). ProjectionFull = "full" - // ProjectionNoAcl returns all fields of object(s) except for owner and acl. - ProjectionNoAcl = "noAcl" + // ProjectionNoACL returns all fields of object(s) except for Owner and ACL. + ProjectionNoACL = "noAcl" ) // Query represents a query to filter objects from a bucket. @@ -1350,8 +1350,9 @@ type Query struct { // listed will have names between startOffset (inclusive) and endOffset (exclusive). EndOffset string - // Projection defines set of properties to return. This may be either - // ProjectionFull or ProjectionNoAcl, zero value means ProjectionFull. + // Projection defines the set of properties to return. It will default to ProjectionFull, + // which returns all properties. Passing ProjectionNoACL will omit Owner and ACL, + // which may improve performance when listing many objects. Projection string } @@ -1423,16 +1424,6 @@ func (q *Query) SetAttrSelection(attrs []string) error { return nil } -// GetProjection returns either value of Projection field or -// default projection when this field is empty. -func (q *Query) GetProjection() string { - if q.Projection == "" { - return ProjectionFull - } else { - return q.Projection - } -} - // Conditions constrain methods to act on specific generations of // objects. // From ef828cd777a955376cebad1f10adc84942739dc9 Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Fri, 15 Jan 2021 19:55:22 +0300 Subject: [PATCH 3/7] test: move objects projection test to integration test --- storage/bucket_test.go | 28 ---------------------------- storage/integration_test.go | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 28 deletions(-) diff --git a/storage/bucket_test.go b/storage/bucket_test.go index d0f002325ba..123e319a84e 100644 --- a/storage/bucket_test.go +++ b/storage/bucket_test.go @@ -663,31 +663,3 @@ func TestNewBucket(t *testing.T) { t.Errorf("got=-, want=+:\n%s", diff) } } - -func TestObjectsWithProjection(t *testing.T) { - samples := []struct { - req, expect string - } { - {"", ProjectionFull}, - {ProjectionFull, ProjectionFull}, - {ProjectionNoAcl, ProjectionNoAcl}, - } - res := &http.Response{StatusCode: http.StatusInternalServerError} - transport := &mockTransport{} - client := mockClient(t, transport) - bucket := client.Bucket("mock-bucket") - ctx := context.Background() - - for _, sample := range samples { - transport.addResult(res, nil) - query := &Query{Projection: sample.req} - bucket.Objects(ctx, query).Next() - - projection := transport.gotReq.URL.Query()["projection"] - if len(projection) == 0 { - t.Errorf("requesting %q projection: no projection parameter generated", sample.req) - } else if projection[0] != sample.expect { - t.Errorf("requesting %q projection: expecting %q, got %q", sample.req, sample.expect, projection[0]) - } - } -} \ No newline at end of file diff --git a/storage/integration_test.go b/storage/integration_test.go index c21ff8430a0..cb8ebbad0d0 100644 --- a/storage/integration_test.go +++ b/storage/integration_test.go @@ -698,6 +698,7 @@ func TestIntegration_Objects(t *testing.T) { testObjectsIterateSelectedAttrs(t, bkt, objects) testObjectsIterateAllSelectedAttrs(t, bkt, objects) testObjectIteratorWithOffset(t, bkt, objects) + testObjectsIterateWithProjection(t, bkt) t.Run("testObjectsIterateSelectedAttrsDelimiter", func(t *testing.T) { query := &Query{Prefix: "", Delimiter: "/"} if err := query.SetAttrSelection([]string{"Name"}); err != nil { @@ -1236,6 +1237,42 @@ func testObjectsIterateAllSelectedAttrs(t *testing.T, bkt *BucketHandle, objects } } +func testObjectsIterateWithProjection(t *testing.T, bkt *BucketHandle) { + projections := map[string]bool { + "": true, + ProjectionFull: true, + ProjectionNoACL: false, + } + + for projection, expectACL := range projections { + query := &Query{Projection: projection} + it := bkt.Objects(context.Background(), query) + attrs, err := it.Next() + if err == iterator.Done { + t.Fatalf("no objects") + } + if err != nil { + t.Fatalf("iterator.Next: %v", err) + } + + if expectACL { + if attrs.Owner == "" { + t.Errorf("projection: %q, empty Owner", projection) + } + if len(attrs.ACL) == 0 { + t.Errorf("projection: %q, empty ACL", projection) + } + } else { + if attrs.Owner != "" { + t.Errorf("projection: %q, expected empty Owner, got %q", projection, attrs.Owner) + } + if len(attrs.ACL) != 0 { + t.Errorf("projection: %q, expected empty ACL, got %d entries", projection, len(attrs.ACL)) + } + } + } +} + func TestIntegration_SignedURL(t *testing.T) { if testing.Short() { // do not test during replay t.Skip("Integration tests skipped in short mode") From d211bc1243be84b90449b82605b4c7605067dc7c Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Fri, 15 Jan 2021 21:17:42 +0300 Subject: [PATCH 4/7] style: fix formatting in integration test --- storage/integration_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/storage/integration_test.go b/storage/integration_test.go index cb8ebbad0d0..db8593d2366 100644 --- a/storage/integration_test.go +++ b/storage/integration_test.go @@ -1238,9 +1238,9 @@ func testObjectsIterateAllSelectedAttrs(t *testing.T, bkt *BucketHandle, objects } func testObjectsIterateWithProjection(t *testing.T, bkt *BucketHandle) { - projections := map[string]bool { - "": true, - ProjectionFull: true, + projections := map[string]bool{ + "": true, + ProjectionFull: true, ProjectionNoACL: false, } From f3e6c56aeb69bbbf20821bde556dec0ff604fea9 Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Wed, 20 Jan 2021 01:07:02 +0300 Subject: [PATCH 5/7] refactor: transform Query.Projection into enum --- storage/bucket.go | 4 ++-- storage/integration_test.go | 8 ++++---- storage/storage.go | 21 ++++++++++++++++++--- 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/storage/bucket.go b/storage/bucket.go index 9add7559af7..7b1757b83de 100644 --- a/storage/bucket.go +++ b/storage/bucket.go @@ -1205,10 +1205,10 @@ func (it *ObjectIterator) fetch(pageSize int, pageToken string) (string, error) req := it.bucket.c.raw.Objects.List(it.bucket.name) setClientHeader(req.Header()) projection := it.query.Projection - if projection == "" { + if projection == ProjectionDefault { projection = ProjectionFull } - req.Projection(projection) + req.Projection(projection.String()) req.Delimiter(it.query.Delimiter) req.Prefix(it.query.Prefix) req.StartOffset(it.query.StartOffset) diff --git a/storage/integration_test.go b/storage/integration_test.go index db8593d2366..85d6f2d807a 100644 --- a/storage/integration_test.go +++ b/storage/integration_test.go @@ -1238,10 +1238,10 @@ func testObjectsIterateAllSelectedAttrs(t *testing.T, bkt *BucketHandle, objects } func testObjectsIterateWithProjection(t *testing.T, bkt *BucketHandle) { - projections := map[string]bool{ - "": true, - ProjectionFull: true, - ProjectionNoACL: false, + projections := map[Projection]bool{ + ProjectionDefault: true, + ProjectionFull: true, + ProjectionNoACL: false, } for projection, expectACL := range projections { diff --git a/storage/storage.go b/storage/storage.go index a86a8e3e379..9759ee38eb3 100644 --- a/storage/storage.go +++ b/storage/storage.go @@ -1306,14 +1306,29 @@ func encodeUint32(u uint32) string { return base64.StdEncoding.EncodeToString(b) } +type Projection int + const ( + ProjectionDefault Projection = iota + // ProjectionFull returns all fields of object(s). - ProjectionFull = "full" + ProjectionFull // ProjectionNoACL returns all fields of object(s) except for Owner and ACL. - ProjectionNoACL = "noAcl" + ProjectionNoACL ) +func (p Projection) String() string { + switch p { + case ProjectionFull: + return "full" + case ProjectionNoACL: + return "noAcl" + default: + return "" + } +} + // Query represents a query to filter objects from a bucket. type Query struct { // Delimiter returns results in a directory-like fashion. @@ -1353,7 +1368,7 @@ type Query struct { // Projection defines the set of properties to return. It will default to ProjectionFull, // which returns all properties. Passing ProjectionNoACL will omit Owner and ACL, // which may improve performance when listing many objects. - Projection string + Projection Projection } // attrToFieldMap maps the field names of ObjectAttrs to the underlying field From bd28c6f6043fad933467dadcbb7833048ab66aee Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Wed, 20 Jan 2021 01:57:14 +0300 Subject: [PATCH 6/7] chore: add missing comments for exported type and const --- storage/storage.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/storage/storage.go b/storage/storage.go index 9759ee38eb3..8852b9c6fc5 100644 --- a/storage/storage.go +++ b/storage/storage.go @@ -1306,15 +1306,17 @@ func encodeUint32(u uint32) string { return base64.StdEncoding.EncodeToString(b) } +// Projection is enumerated type for Query.Projection. type Projection int const ( + // ProjectionDefault returns all fields of objects. ProjectionDefault Projection = iota - // ProjectionFull returns all fields of object(s). + // ProjectionFull returns all fields of objects. ProjectionFull - // ProjectionNoACL returns all fields of object(s) except for Owner and ACL. + // ProjectionNoACL returns all fields of objects except for Owner and ACL. ProjectionNoACL ) From 1e3c4df8c46f3b70d8117cbe49b9bc7c18e23f8d Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Fri, 22 Jan 2021 18:58:37 +0300 Subject: [PATCH 7/7] style: more helpful test error messages --- storage/integration_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/storage/integration_test.go b/storage/integration_test.go index 85d6f2d807a..788aa41e44e 100644 --- a/storage/integration_test.go +++ b/storage/integration_test.go @@ -1249,7 +1249,7 @@ func testObjectsIterateWithProjection(t *testing.T, bkt *BucketHandle) { it := bkt.Objects(context.Background(), query) attrs, err := it.Next() if err == iterator.Done { - t.Fatalf("no objects") + t.Fatalf("iterator: no objects") } if err != nil { t.Fatalf("iterator.Next: %v", err) @@ -1257,17 +1257,17 @@ func testObjectsIterateWithProjection(t *testing.T, bkt *BucketHandle) { if expectACL { if attrs.Owner == "" { - t.Errorf("projection: %q, empty Owner", projection) + t.Errorf("projection %q: Owner is empty, want nonempty Owner", projection) } if len(attrs.ACL) == 0 { - t.Errorf("projection: %q, empty ACL", projection) + t.Errorf("projection %q: ACL is empty, want at least one ACL rule", projection) } } else { if attrs.Owner != "" { - t.Errorf("projection: %q, expected empty Owner, got %q", projection, attrs.Owner) + t.Errorf("projection %q: got Owner = %q, want empty Owner", projection, attrs.Owner) } if len(attrs.ACL) != 0 { - t.Errorf("projection: %q, expected empty ACL, got %d entries", projection, len(attrs.ACL)) + t.Errorf("projection %q: got %d ACL rules, want empty ACL", projection, len(attrs.ACL)) } } }