From 1d0116a6929d322d41c660c43167e8eb7466461e Mon Sep 17 00:00:00 2001 From: Noah Dietz Date: Tue, 11 Oct 2022 11:42:14 -0700 Subject: [PATCH] chore(storage): implement gRPC ListObjects read_mask support (#6808) This adds `Query`-to-`FieldMask` conversion for `Object` attributes and incorporates that into `ListObjects` for gRPC. This also carves out the `ListObjects`-specific tests from `TestIntegration_Objects` and enables those tests for multi-transport testing. --- storage/grpc_client.go | 24 +- storage/http_client.go | 4 +- storage/integration_test.go | 827 +++++++++++++++++++----------------- storage/storage.go | 118 ++++- 4 files changed, 559 insertions(+), 414 deletions(-) diff --git a/storage/grpc_client.go b/storage/grpc_client.go index 86a13efe5cf..d215bf1cf27 100644 --- a/storage/grpc_client.go +++ b/storage/grpc_client.go @@ -380,14 +380,14 @@ func (c *grpcStorageClient) ListObjects(ctx context.Context, bucket string, q *Q it.query = *q } req := &storagepb.ListObjectsRequest{ - Parent: bucketResourceName(globalProjectAlias, bucket), - Prefix: it.query.Prefix, - Delimiter: it.query.Delimiter, - Versions: it.query.Versions, - LexicographicStart: it.query.StartOffset, - LexicographicEnd: it.query.EndOffset, - // TODO(noahietz): Convert a projection to a FieldMask. - // ReadMask: q.Projection, + Parent: bucketResourceName(globalProjectAlias, bucket), + Prefix: it.query.Prefix, + Delimiter: it.query.Delimiter, + Versions: it.query.Versions, + LexicographicStart: it.query.StartOffset, + LexicographicEnd: it.query.EndOffset, + IncludeTrailingDelimiter: it.query.IncludeTrailingDelimiter, + ReadMask: q.toFieldMask(), // a nil Query still results in a "*" FieldMask } if s.userProject != "" { ctx = setUserProjectMetadata(ctx, s.userProject) @@ -411,6 +411,12 @@ func (c *grpcStorageClient) ListObjects(ctx context.Context, bucket string, q *Q it.items = append(it.items, b) } + // Response is always non-nil after a successful request. + res := gitr.Response.(*storagepb.ListObjectsResponse) + for _, prefix := range res.GetPrefixes() { + it.items = append(it.items, &ObjectAttrs{Prefix: prefix}) + } + return token, nil } it.pageInfo, it.nextFunc = iterator.NewPageInfo( @@ -449,6 +455,8 @@ func (c *grpcStorageClient) GetObject(ctx context.Context, bucket, object string req := &storagepb.GetObjectRequest{ Bucket: bucketResourceName(globalProjectAlias, bucket), Object: object, + // ProjectionFull by default. + ReadMask: &fieldmaskpb.FieldMask{Paths: []string{"*"}}, } if err := applyCondsProto("grpcStorageClient.GetObject", gen, conds, req); err != nil { return nil, err diff --git a/storage/http_client.go b/storage/http_client.go index 321b6f67c14..84edf20f636 100644 --- a/storage/http_client.go +++ b/storage/http_client.go @@ -344,8 +344,8 @@ func (c *httpStorageClient) ListObjects(ctx context.Context, bucket string, q *Q req.EndOffset(it.query.EndOffset) req.Versions(it.query.Versions) req.IncludeTrailingDelimiter(it.query.IncludeTrailingDelimiter) - if len(it.query.fieldSelection) > 0 { - req.Fields("nextPageToken", googleapi.Field(it.query.fieldSelection)) + if selection := it.query.toFieldSelection(); selection != "" { + req.Fields("nextPageToken", googleapi.Field(selection)) } req.PageToken(pageToken) if s.userProject != "" { diff --git a/storage/integration_test.go b/storage/integration_test.go index cce5f577786..f00f17d02b4 100644 --- a/storage/integration_test.go +++ b/storage/integration_test.go @@ -1472,462 +1472,497 @@ func TestIntegration_ConditionalDownload(t *testing.T) { }) } -func TestIntegration_Objects(t *testing.T) { - // TODO(jba): Use subtests (Go 1.7). - ctx := context.Background() - client := testConfig(ctx, t) - defer client.Close() - // Reset testTime, 'cause object last modification time should be within 5 min - // from test (test iteration if -count passed) start time. - testTime = time.Now().UTC() - newBucketName := uidSpace.New() - h := testHelper{t} - bkt := client.Bucket(newBucketName).Retryer(WithPolicy(RetryAlways)) +func TestIntegration_ObjectIteration(t *testing.T) { - h.mustCreate(bkt, testutil.ProjID(), nil) - defer func() { - if err := killBucket(ctx, client, newBucketName); err != nil { - log.Printf("deleting %q: %v", newBucketName, err) - } - }() - const defaultType = "text/plain" + multiTransportTest(context.Background(), t, func(t *testing.T, ctx context.Context, _ string, prefix string, client *Client) { + // Reset testTime, 'cause object last modification time should be within 5 min + // from test (test iteration if -count passed) start time. + testTime = time.Now().UTC() + newBucketName := prefix + uidSpace.New() + h := testHelper{t} + bkt := client.Bucket(newBucketName).Retryer(WithPolicy(RetryAlways)) - // Populate object names and make a map for their contents. - objects := []string{ - "obj1", - "obj2", - "obj/with/slashes", - "obj/", - } - contents := make(map[string][]byte) + h.mustCreate(bkt, testutil.ProjID(), nil) + defer func() { + if err := killBucket(ctx, client, newBucketName); err != nil { + log.Printf("deleting %q: %v", newBucketName, err) + } + }() + const defaultType = "text/plain" + + // Populate object names and make a map for their contents. + objects := []string{ + "obj1", + "obj2", + "obj/with/slashes", + "obj/", + } + contents := make(map[string][]byte) + + // Test Writer. + for _, obj := range objects { + c := randomContents() + if err := writeObject(ctx, bkt.Object(obj), defaultType, c); err != nil { + t.Errorf("Write for %v failed with %v", obj, err) + } + contents[obj] = c + } + + testObjectIterator(t, bkt, objects) + 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 { + t.Fatalf("selecting query attrs: %v", err) + } - // Test Writer. - for _, obj := range objects { - c := randomContents() - if err := writeObject(ctx, bkt.Object(obj), defaultType, c); err != nil { - t.Errorf("Write for %v failed with %v", obj, err) - } - contents[obj] = c - } + var gotNames []string + var gotPrefixes []string + it := bkt.Objects(context.Background(), query) + for { + attrs, err := it.Next() + if err == iterator.Done { + break + } + if err != nil { + t.Fatalf("iterator.Next: %v", err) + } + if attrs.Name != "" { + gotNames = append(gotNames, attrs.Name) + } else if attrs.Prefix != "" { + gotPrefixes = append(gotPrefixes, attrs.Prefix) + } - testObjectIterator(t, bkt, objects) - 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 { - t.Fatalf("selecting query attrs: %v", err) - } + if attrs.Bucket != "" { + t.Errorf("Bucket field not selected, want empty, got = %v", attrs.Bucket) + } + } - var gotNames []string - var gotPrefixes []string - it := bkt.Objects(context.Background(), query) - for { - attrs, err := it.Next() - if err == iterator.Done { - break + sortedNames := []string{"obj1", "obj2"} + if !cmp.Equal(sortedNames, gotNames) { + t.Errorf("names = %v, want %v", gotNames, sortedNames) } - if err != nil { - t.Fatalf("iterator.Next: %v", err) + sortedPrefixes := []string{"obj/"} + if !cmp.Equal(sortedPrefixes, gotPrefixes) { + t.Errorf("prefixes = %v, want %v", gotPrefixes, sortedPrefixes) } - if attrs.Name != "" { - gotNames = append(gotNames, attrs.Name) - } else if attrs.Prefix != "" { - gotPrefixes = append(gotPrefixes, attrs.Prefix) + }) + t.Run("testObjectsIterateSelectedAttrsDelimiterIncludeTrailingDelimiter", func(t *testing.T) { + query := &Query{Prefix: "", Delimiter: "/", IncludeTrailingDelimiter: true} + if err := query.SetAttrSelection([]string{"Name"}); err != nil { + t.Fatalf("selecting query attrs: %v", err) } - if attrs.Bucket != "" { - t.Errorf("Bucket field not selected, want empty, got = %v", attrs.Bucket) + var gotNames []string + var gotPrefixes []string + it := bkt.Objects(context.Background(), query) + for { + attrs, err := it.Next() + if err == iterator.Done { + break + } + if err != nil { + t.Fatalf("iterator.Next: %v", err) + } + if attrs.Name != "" { + gotNames = append(gotNames, attrs.Name) + } else if attrs.Prefix != "" { + gotPrefixes = append(gotPrefixes, attrs.Prefix) + } + + if attrs.Bucket != "" { + t.Errorf("Bucket field not selected, want empty, got = %v", attrs.Bucket) + } } - } - sortedNames := []string{"obj1", "obj2"} - if !cmp.Equal(sortedNames, gotNames) { - t.Errorf("names = %v, want %v", gotNames, sortedNames) - } - sortedPrefixes := []string{"obj/"} - if !cmp.Equal(sortedPrefixes, gotPrefixes) { - t.Errorf("prefixes = %v, want %v", gotPrefixes, sortedPrefixes) - } + sortedNames := []string{"obj/", "obj1", "obj2"} + if !cmp.Equal(sortedNames, gotNames) { + t.Errorf("names = %v, want %v", gotNames, sortedNames) + } + sortedPrefixes := []string{"obj/"} + if !cmp.Equal(sortedPrefixes, gotPrefixes) { + t.Errorf("prefixes = %v, want %v", gotPrefixes, sortedPrefixes) + } + }) }) - t.Run("testObjectsIterateSelectedAttrsDelimiterIncludeTrailingDelimiter", func(t *testing.T) { - query := &Query{Prefix: "", Delimiter: "/", IncludeTrailingDelimiter: true} - if err := query.SetAttrSelection([]string{"Name"}); err != nil { - t.Fatalf("selecting query attrs: %v", err) - } +} - var gotNames []string - var gotPrefixes []string - it := bkt.Objects(context.Background(), query) - for { - attrs, err := it.Next() - if err == iterator.Done { - break +func TestIntegration_Objects(t *testing.T) { + multiTransportTest(skipGRPC("temporary skip - needs deliberate refactoring"), t, func(t *testing.T, ctx context.Context, _ string, prefix string, client *Client) { + // Reset testTime, 'cause object last modification time should be within 5 min + // from test (test iteration if -count passed) start time. + testTime = time.Now().UTC() + newBucketName := prefix + uidSpace.New() + h := testHelper{t} + bkt := client.Bucket(newBucketName).Retryer(WithPolicy(RetryAlways)) + + h.mustCreate(bkt, testutil.ProjID(), nil) + defer func() { + if err := killBucket(ctx, client, newBucketName); err != nil { + log.Printf("deleting %q: %v", newBucketName, err) } + }() + const defaultType = "text/plain" + + // Populate object names and make a map for their contents. + objects := []string{ + "obj1", + "obj2", + "obj/with/slashes", + "obj/", + } + contents := make(map[string][]byte) + + // Test Writer. + for _, obj := range objects { + c := randomContents() + if err := writeObject(ctx, bkt.Object(obj), defaultType, c); err != nil { + t.Errorf("Write for %v failed with %v", obj, err) + } + contents[obj] = c + } + // Test Reader. + for _, obj := range objects { + rc, err := bkt.Object(obj).NewReader(ctx) if err != nil { - t.Fatalf("iterator.Next: %v", err) + t.Errorf("Can't create a reader for %v, errored with %v", obj, err) + continue + } + if !rc.checkCRC { + t.Errorf("%v: not checking CRC", obj) + } + slurp, err := ioutil.ReadAll(rc) + if err != nil { + t.Errorf("Can't ReadAll object %v, errored with %v", obj, err) + } + if got, want := slurp, contents[obj]; !bytes.Equal(got, want) { + t.Errorf("Contents (%q) = %q; want %q", obj, got, want) + } + if got, want := rc.Size(), len(contents[obj]); got != int64(want) { + t.Errorf("Size (%q) = %d; want %d", obj, got, want) + } + if got, want := rc.ContentType(), "text/plain"; got != want { + t.Errorf("ContentType (%q) = %q; want %q", obj, got, want) } - if attrs.Name != "" { - gotNames = append(gotNames, attrs.Name) - } else if attrs.Prefix != "" { - gotPrefixes = append(gotPrefixes, attrs.Prefix) + if got, want := rc.CacheControl(), "public, max-age=60"; got != want { + t.Errorf("CacheControl (%q) = %q; want %q", obj, got, want) } + // We just wrote these objects, so they should have a recent last-modified time. + lm, err := rc.LastModified() + // Accept a time within +/- of the test time, to account for natural + // variation and the fact that testTime is set at the start of the test run. + expectedVariance := 5 * time.Minute + if err != nil { + t.Errorf("LastModified (%q): got error %v", obj, err) + } else if lm.Before(testTime.Add(-expectedVariance)) || lm.After(testTime.Add(expectedVariance)) { + t.Errorf("LastModified (%q): got %s, which not the %v from now (%v)", obj, lm, expectedVariance, testTime) + } + rc.Close() - if attrs.Bucket != "" { - t.Errorf("Bucket field not selected, want empty, got = %v", attrs.Bucket) + // Check early close. + buf := make([]byte, 1) + rc, err = bkt.Object(obj).NewReader(ctx) + if err != nil { + t.Fatalf("%v: %v", obj, err) + } + _, err = rc.Read(buf) + if err != nil { + t.Fatalf("%v: %v", obj, err) + } + if got, want := buf, contents[obj][:1]; !bytes.Equal(got, want) { + t.Errorf("Contents[0] (%q) = %q; want %q", obj, got, want) + } + if err := rc.Close(); err != nil { + t.Errorf("%v Close: %v", obj, err) } } - sortedNames := []string{"obj/", "obj1", "obj2"} - if !cmp.Equal(sortedNames, gotNames) { - t.Errorf("names = %v, want %v", gotNames, sortedNames) - } - sortedPrefixes := []string{"obj/"} - if !cmp.Equal(sortedPrefixes, gotPrefixes) { - t.Errorf("prefixes = %v, want %v", gotPrefixes, sortedPrefixes) - } - }) + obj := objects[0] + objlen := int64(len(contents[obj])) + // Test Range Reader. + for i, r := range []struct { + offset, length, want int64 + }{ + {0, objlen, objlen}, + {0, objlen / 2, objlen / 2}, + {objlen / 2, objlen, objlen / 2}, + {0, 0, 0}, + {objlen / 2, 0, 0}, + {objlen / 2, -1, objlen / 2}, + {0, objlen * 2, objlen}, + {-2, -1, 2}, + {-objlen, -1, objlen}, + {-(objlen / 2), -1, objlen / 2}, + } { + rc, err := bkt.Object(obj).NewRangeReader(ctx, r.offset, r.length) + if err != nil { + t.Errorf("%+v: Can't create a range reader for %v, errored with %v", i, obj, err) + continue + } + if rc.Size() != objlen { + t.Errorf("%+v: Reader has a content-size of %d, want %d", i, rc.Size(), objlen) + } + if rc.Remain() != r.want { + t.Errorf("%+v: Reader's available bytes reported as %d, want %d", i, rc.Remain(), r.want) + } + slurp, err := ioutil.ReadAll(rc) + if err != nil { + t.Errorf("%+v: can't ReadAll object %v, errored with %v", r, obj, err) + continue + } + if len(slurp) != int(r.want) { + t.Errorf("%+v: RangeReader (%d, %d): Read %d bytes, wanted %d bytes", i, r.offset, r.length, len(slurp), r.want) + continue + } - // Test Reader. - for _, obj := range objects { - rc, err := bkt.Object(obj).NewReader(ctx) - if err != nil { - t.Errorf("Can't create a reader for %v, errored with %v", obj, err) - continue - } - if !rc.checkCRC { - t.Errorf("%v: not checking CRC", obj) - } - slurp, err := ioutil.ReadAll(rc) - if err != nil { - t.Errorf("Can't ReadAll object %v, errored with %v", obj, err) + switch { + case r.offset < 0: // The case of reading the last N bytes. + start := objlen + r.offset + if got, want := slurp, contents[obj][start:]; !bytes.Equal(got, want) { + t.Errorf("RangeReader (%d, %d) = %q; want %q", r.offset, r.length, got, want) + } + + default: + if got, want := slurp, contents[obj][r.offset:r.offset+r.want]; !bytes.Equal(got, want) { + t.Errorf("RangeReader (%d, %d) = %q; want %q", r.offset, r.length, got, want) + } + } + rc.Close() } - if got, want := slurp, contents[obj]; !bytes.Equal(got, want) { - t.Errorf("Contents (%q) = %q; want %q", obj, got, want) + + objName := objects[0] + + // Test NewReader googleapi.Error. + // Since a 429 or 5xx is hard to cause, we trigger a 416. + realLen := len(contents[objName]) + _, err := bkt.Object(objName).NewRangeReader(ctx, int64(realLen*2), 10) + var e *googleapi.Error + if ok := errors.As(err, &e); !ok { + t.Error("NewRangeReader did not return a googleapi.Error") + } else { + if e.Code != 416 { + t.Errorf("Code = %d; want %d", e.Code, 416) + } + if len(e.Header) == 0 { + t.Error("Missing googleapi.Error.Header") + } + if len(e.Body) == 0 { + t.Error("Missing googleapi.Error.Body") + } } - if got, want := rc.Size(), len(contents[obj]); got != int64(want) { - t.Errorf("Size (%q) = %d; want %d", obj, got, want) + + // Test StatObject. + o := h.mustObjectAttrs(bkt.Object(objName)) + if got, want := o.Name, objName; got != want { + t.Errorf("Name (%v) = %q; want %q", objName, got, want) } - if got, want := rc.ContentType(), "text/plain"; got != want { - t.Errorf("ContentType (%q) = %q; want %q", obj, got, want) + if got, want := o.ContentType, defaultType; got != want { + t.Errorf("ContentType (%v) = %q; want %q", objName, got, want) } - if got, want := rc.CacheControl(), "public, max-age=60"; got != want { - t.Errorf("CacheControl (%q) = %q; want %q", obj, got, want) + created := o.Created + // Check that the object is newer than its containing bucket. + bAttrs := h.mustBucketAttrs(bkt) + if o.Created.Before(bAttrs.Created) { + t.Errorf("Object %v is older than its containing bucket, %v", o, bAttrs) } - // We just wrote these objects, so they should have a recent last-modified time. - lm, err := rc.LastModified() - // Accept a time within +/- of the test time, to account for natural - // variation and the fact that testTime is set at the start of the test run. - expectedVariance := 5 * time.Minute + + // Test object copy. + copyName := "copy-" + objName + copyObj, err := bkt.Object(copyName).CopierFrom(bkt.Object(objName)).Run(ctx) if err != nil { - t.Errorf("LastModified (%q): got error %v", obj, err) - } else if lm.Before(testTime.Add(-expectedVariance)) || lm.After(testTime.Add(expectedVariance)) { - t.Errorf("LastModified (%q): got %s, which not the %v from now (%v)", obj, lm, expectedVariance, testTime) + t.Errorf("Copier.Run failed with %v", err) + } else if !namesEqual(copyObj, newBucketName, copyName) { + t.Errorf("Copy object bucket, name: got %q.%q, want %q.%q", + copyObj.Bucket, copyObj.Name, newBucketName, copyName) } - rc.Close() - // Check early close. - buf := make([]byte, 1) - rc, err = bkt.Object(obj).NewReader(ctx) + // Copying with attributes. + const contentEncoding = "identity" + copier := bkt.Object(copyName).CopierFrom(bkt.Object(objName)) + copier.ContentEncoding = contentEncoding + copyObj, err = copier.Run(ctx) if err != nil { - t.Fatalf("%v: %v", obj, err) + t.Errorf("Copier.Run failed with %v", err) + } else { + if !namesEqual(copyObj, newBucketName, copyName) { + t.Errorf("Copy object bucket, name: got %q.%q, want %q.%q", + copyObj.Bucket, copyObj.Name, newBucketName, copyName) + } + if copyObj.ContentEncoding != contentEncoding { + t.Errorf("Copy ContentEncoding: got %q, want %q", copyObj.ContentEncoding, contentEncoding) + } } - _, err = rc.Read(buf) - if err != nil { - t.Fatalf("%v: %v", obj, err) + + objectHandle := bkt.Object(objName) + + // Test UpdateAttrs. + metadata := map[string]string{"key": "value"} + updated := h.mustUpdateObject(objectHandle, ObjectAttrsToUpdate{ + ContentType: "text/html", + ContentLanguage: "en", + Metadata: metadata, + ACL: []ACLRule{{Entity: "domain-google.com", Role: RoleReader}}, + }, h.mustObjectAttrs(objectHandle).Metageneration) + if got, want := updated.ContentType, "text/html"; got != want { + t.Errorf("updated.ContentType == %q; want %q", got, want) } - if got, want := buf, contents[obj][:1]; !bytes.Equal(got, want) { - t.Errorf("Contents[0] (%q) = %q; want %q", obj, got, want) + if got, want := updated.ContentLanguage, "en"; got != want { + t.Errorf("updated.ContentLanguage == %q; want %q", updated.ContentLanguage, want) } - if err := rc.Close(); err != nil { - t.Errorf("%v Close: %v", obj, err) + if got, want := updated.Metadata, metadata; !testutil.Equal(got, want) { + t.Errorf("updated.Metadata == %+v; want %+v", updated.Metadata, want) + } + if got, want := updated.Created, created; got != want { + t.Errorf("updated.Created == %q; want %q", got, want) + } + if !updated.Created.Before(updated.Updated) { + t.Errorf("updated.Updated should be newer than update.Created") } - } - obj := objects[0] - objlen := int64(len(contents[obj])) - // Test Range Reader. - for i, r := range []struct { - offset, length, want int64 - }{ - {0, objlen, objlen}, - {0, objlen / 2, objlen / 2}, - {objlen / 2, objlen, objlen / 2}, - {0, 0, 0}, - {objlen / 2, 0, 0}, - {objlen / 2, -1, objlen / 2}, - {0, objlen * 2, objlen}, - {-2, -1, 2}, - {-objlen, -1, objlen}, - {-(objlen / 2), -1, objlen / 2}, - } { - rc, err := bkt.Object(obj).NewRangeReader(ctx, r.offset, r.length) - if err != nil { - t.Errorf("%+v: Can't create a range reader for %v, errored with %v", i, obj, err) - continue + // Delete ContentType and ContentLanguage. + updated = h.mustUpdateObject(objectHandle, ObjectAttrsToUpdate{ + ContentType: "", + ContentLanguage: "", + Metadata: map[string]string{}, + }, h.mustObjectAttrs(objectHandle).Metageneration) + if got, want := updated.ContentType, ""; got != want { + t.Errorf("updated.ContentType == %q; want %q", got, want) } - if rc.Size() != objlen { - t.Errorf("%+v: Reader has a content-size of %d, want %d", i, rc.Size(), objlen) + if got, want := updated.ContentLanguage, ""; got != want { + t.Errorf("updated.ContentLanguage == %q; want %q", updated.ContentLanguage, want) } - if rc.Remain() != r.want { - t.Errorf("%+v: Reader's available bytes reported as %d, want %d", i, rc.Remain(), r.want) + if updated.Metadata != nil { + t.Errorf("updated.Metadata == %+v; want nil", updated.Metadata) } - slurp, err := ioutil.ReadAll(rc) - if err != nil { - t.Errorf("%+v: can't ReadAll object %v, errored with %v", r, obj, err) - continue + if got, want := updated.Created, created; got != want { + t.Errorf("updated.Created == %q; want %q", got, want) } - if len(slurp) != int(r.want) { - t.Errorf("%+v: RangeReader (%d, %d): Read %d bytes, wanted %d bytes", i, r.offset, r.length, len(slurp), r.want) - continue + if !updated.Created.Before(updated.Updated) { + t.Errorf("updated.Updated should be newer than update.Created") } - switch { - case r.offset < 0: // The case of reading the last N bytes. - start := objlen + r.offset - if got, want := slurp, contents[obj][start:]; !bytes.Equal(got, want) { - t.Errorf("RangeReader (%d, %d) = %q; want %q", r.offset, r.length, got, want) + // Test checksums. + checksumCases := []struct { + name string + contents [][]byte + size int64 + md5 string + crc32c uint32 + }{ + { + name: "checksum-object", + contents: [][]byte{[]byte("hello"), []byte("world")}, + size: 10, + md5: "fc5e038d38a57032085441e7fe7010b0", + crc32c: 1456190592, + }, + { + name: "zero-object", + contents: [][]byte{}, + size: 0, + md5: "d41d8cd98f00b204e9800998ecf8427e", + crc32c: 0, + }, + } + for _, c := range checksumCases { + wc := bkt.Object(c.name).NewWriter(ctx) + for _, data := range c.contents { + if _, err := wc.Write(data); err != nil { + t.Errorf("Write(%q) failed with %q", data, err) + } } - - default: - if got, want := slurp, contents[obj][r.offset:r.offset+r.want]; !bytes.Equal(got, want) { - t.Errorf("RangeReader (%d, %d) = %q; want %q", r.offset, r.length, got, want) + if err = wc.Close(); err != nil { + t.Errorf("%q: close failed with %q", c.name, err) + } + obj := wc.Attrs() + if got, want := obj.Size, c.size; got != want { + t.Errorf("Object (%q) Size = %v; want %v", c.name, got, want) + } + if got, want := fmt.Sprintf("%x", obj.MD5), c.md5; got != want { + t.Errorf("Object (%q) MD5 = %q; want %q", c.name, got, want) + } + if got, want := obj.CRC32C, c.crc32c; got != want { + t.Errorf("Object (%q) CRC32C = %v; want %v", c.name, got, want) } } - rc.Close() - } - objName := objects[0] - - // Test NewReader googleapi.Error. - // Since a 429 or 5xx is hard to cause, we trigger a 416. - realLen := len(contents[objName]) - _, err := bkt.Object(objName).NewRangeReader(ctx, int64(realLen*2), 10) - var e *googleapi.Error - if ok := errors.As(err, &e); !ok { - t.Error("NewRangeReader did not return a googleapi.Error") - } else { - if e.Code != 416 { - t.Errorf("Code = %d; want %d", e.Code, 416) - } - if len(e.Header) == 0 { - t.Error("Missing googleapi.Error.Header") + // Test public ACL. + publicObj := objects[0] + if err = bkt.Object(publicObj).ACL().Set(ctx, AllUsers, RoleReader); err != nil { + t.Errorf("PutACLEntry failed with %v", err) } - if len(e.Body) == 0 { - t.Error("Missing googleapi.Error.Body") + publicClient, err := newTestClient(ctx, option.WithoutAuthentication()) + if err != nil { + t.Fatal(err) } - } - // Test StatObject. - o := h.mustObjectAttrs(bkt.Object(objName)) - if got, want := o.Name, objName; got != want { - t.Errorf("Name (%v) = %q; want %q", objName, got, want) - } - if got, want := o.ContentType, defaultType; got != want { - t.Errorf("ContentType (%v) = %q; want %q", objName, got, want) - } - created := o.Created - // Check that the object is newer than its containing bucket. - bAttrs := h.mustBucketAttrs(bkt) - if o.Created.Before(bAttrs.Created) { - t.Errorf("Object %v is older than its containing bucket, %v", o, bAttrs) - } - - // Test object copy. - copyName := "copy-" + objName - copyObj, err := bkt.Object(copyName).CopierFrom(bkt.Object(objName)).Run(ctx) - if err != nil { - t.Errorf("Copier.Run failed with %v", err) - } else if !namesEqual(copyObj, newBucketName, copyName) { - t.Errorf("Copy object bucket, name: got %q.%q, want %q.%q", - copyObj.Bucket, copyObj.Name, newBucketName, copyName) - } - - // Copying with attributes. - const contentEncoding = "identity" - copier := bkt.Object(copyName).CopierFrom(bkt.Object(objName)) - copier.ContentEncoding = contentEncoding - copyObj, err = copier.Run(ctx) - if err != nil { - t.Errorf("Copier.Run failed with %v", err) - } else { - if !namesEqual(copyObj, newBucketName, copyName) { - t.Errorf("Copy object bucket, name: got %q.%q, want %q.%q", - copyObj.Bucket, copyObj.Name, newBucketName, copyName) + slurp := h.mustRead(publicClient.Bucket(newBucketName).Object(publicObj)) + if !bytes.Equal(slurp, contents[publicObj]) { + t.Errorf("Public object's content: got %q, want %q", slurp, contents[publicObj]) } - if copyObj.ContentEncoding != contentEncoding { - t.Errorf("Copy ContentEncoding: got %q, want %q", copyObj.ContentEncoding, contentEncoding) - } - } - - objectHandle := bkt.Object(objName) - // Test UpdateAttrs. - metadata := map[string]string{"key": "value"} - updated := h.mustUpdateObject(objectHandle, ObjectAttrsToUpdate{ - ContentType: "text/html", - ContentLanguage: "en", - Metadata: metadata, - ACL: []ACLRule{{Entity: "domain-google.com", Role: RoleReader}}, - }, h.mustObjectAttrs(objectHandle).Metageneration) - if got, want := updated.ContentType, "text/html"; got != want { - t.Errorf("updated.ContentType == %q; want %q", got, want) - } - if got, want := updated.ContentLanguage, "en"; got != want { - t.Errorf("updated.ContentLanguage == %q; want %q", updated.ContentLanguage, want) - } - if got, want := updated.Metadata, metadata; !testutil.Equal(got, want) { - t.Errorf("updated.Metadata == %+v; want %+v", updated.Metadata, want) - } - if got, want := updated.Created, created; got != want { - t.Errorf("updated.Created == %q; want %q", got, want) - } - if !updated.Created.Before(updated.Updated) { - t.Errorf("updated.Updated should be newer than update.Created") - } - - // Delete ContentType and ContentLanguage. - updated = h.mustUpdateObject(objectHandle, ObjectAttrsToUpdate{ - ContentType: "", - ContentLanguage: "", - Metadata: map[string]string{}, - }, h.mustObjectAttrs(objectHandle).Metageneration) - if got, want := updated.ContentType, ""; got != want { - t.Errorf("updated.ContentType == %q; want %q", got, want) - } - if got, want := updated.ContentLanguage, ""; got != want { - t.Errorf("updated.ContentLanguage == %q; want %q", updated.ContentLanguage, want) - } - if updated.Metadata != nil { - t.Errorf("updated.Metadata == %+v; want nil", updated.Metadata) - } - if got, want := updated.Created, created; got != want { - t.Errorf("updated.Created == %q; want %q", got, want) - } - if !updated.Created.Before(updated.Updated) { - t.Errorf("updated.Updated should be newer than update.Created") - } - - // Test checksums. - checksumCases := []struct { - name string - contents [][]byte - size int64 - md5 string - crc32c uint32 - }{ - { - name: "checksum-object", - contents: [][]byte{[]byte("hello"), []byte("world")}, - size: 10, - md5: "fc5e038d38a57032085441e7fe7010b0", - crc32c: 1456190592, - }, - { - name: "zero-object", - contents: [][]byte{}, - size: 0, - md5: "d41d8cd98f00b204e9800998ecf8427e", - crc32c: 0, - }, - } - for _, c := range checksumCases { - wc := bkt.Object(c.name).NewWriter(ctx) - for _, data := range c.contents { - if _, err := wc.Write(data); err != nil { - t.Errorf("Write(%q) failed with %q", data, err) - } - } - if err = wc.Close(); err != nil { - t.Errorf("%q: close failed with %q", c.name, err) + // Test writer error handling. + wc := publicClient.Bucket(newBucketName).Object(publicObj).NewWriter(ctx) + if _, err := wc.Write([]byte("hello")); err != nil { + t.Errorf("Write unexpectedly failed with %v", err) } - obj := wc.Attrs() - if got, want := obj.Size, c.size; got != want { - t.Errorf("Object (%q) Size = %v; want %v", c.name, got, want) + if err = wc.Close(); err == nil { + t.Error("Close expected an error, found none") } - if got, want := fmt.Sprintf("%x", obj.MD5), c.md5; got != want { - t.Errorf("Object (%q) MD5 = %q; want %q", c.name, got, want) + + // Test deleting the copy object. + h.mustDeleteObject(bkt.Object(copyName)) + // Deleting it a second time should return ErrObjectNotExist. + if err := bkt.Object(copyName).Delete(ctx); err != ErrObjectNotExist { + t.Errorf("second deletion of %v = %v; want ErrObjectNotExist", copyName, err) } - if got, want := obj.CRC32C, c.crc32c; got != want { - t.Errorf("Object (%q) CRC32C = %v; want %v", c.name, got, want) + _, err = bkt.Object(copyName).Attrs(ctx) + if err != ErrObjectNotExist { + t.Errorf("Copy is expected to be deleted, stat errored with %v", err) } - } - // Test public ACL. - publicObj := objects[0] - if err = bkt.Object(publicObj).ACL().Set(ctx, AllUsers, RoleReader); err != nil { - t.Errorf("PutACLEntry failed with %v", err) - } - publicClient, err := newTestClient(ctx, option.WithoutAuthentication()) - if err != nil { - t.Fatal(err) - } - - slurp := h.mustRead(publicClient.Bucket(newBucketName).Object(publicObj)) - if !bytes.Equal(slurp, contents[publicObj]) { - t.Errorf("Public object's content: got %q, want %q", slurp, contents[publicObj]) - } - - // Test writer error handling. - wc := publicClient.Bucket(newBucketName).Object(publicObj).NewWriter(ctx) - if _, err := wc.Write([]byte("hello")); err != nil { - t.Errorf("Write unexpectedly failed with %v", err) - } - if err = wc.Close(); err == nil { - t.Error("Close expected an error, found none") - } - - // Test deleting the copy object. - h.mustDeleteObject(bkt.Object(copyName)) - // Deleting it a second time should return ErrObjectNotExist. - if err := bkt.Object(copyName).Delete(ctx); err != ErrObjectNotExist { - t.Errorf("second deletion of %v = %v; want ErrObjectNotExist", copyName, err) - } - _, err = bkt.Object(copyName).Attrs(ctx) - if err != ErrObjectNotExist { - t.Errorf("Copy is expected to be deleted, stat errored with %v", err) - } - - // Test object composition. - var compSrcs []*ObjectHandle - var wantContents []byte - for _, obj := range objects { - compSrcs = append(compSrcs, bkt.Object(obj)) - wantContents = append(wantContents, contents[obj]...) - } - checkCompose := func(obj *ObjectHandle, wantContentType string) { - rc := h.mustNewReader(obj) - slurp, err = ioutil.ReadAll(rc) - if err != nil { - t.Fatalf("ioutil.ReadAll: %v", err) - } - defer rc.Close() - if !bytes.Equal(slurp, wantContents) { - t.Errorf("Composed object contents\ngot: %q\nwant: %q", slurp, wantContents) + // Test object composition. + var compSrcs []*ObjectHandle + var wantContents []byte + for _, obj := range objects { + compSrcs = append(compSrcs, bkt.Object(obj)) + wantContents = append(wantContents, contents[obj]...) } - if got := rc.ContentType(); got != wantContentType { - t.Errorf("Composed object content-type = %q, want %q", got, wantContentType) + checkCompose := func(obj *ObjectHandle, wantContentType string) { + rc := h.mustNewReader(obj) + slurp, err = ioutil.ReadAll(rc) + if err != nil { + t.Fatalf("ioutil.ReadAll: %v", err) + } + defer rc.Close() + if !bytes.Equal(slurp, wantContents) { + t.Errorf("Composed object contents\ngot: %q\nwant: %q", slurp, wantContents) + } + if got := rc.ContentType(); got != wantContentType { + t.Errorf("Composed object content-type = %q, want %q", got, wantContentType) + } } - } - // Compose should work even if the user sets no destination attributes. - compDst := bkt.Object("composed1") - c := compDst.ComposerFrom(compSrcs...) - if _, err := c.Run(ctx); err != nil { - t.Fatalf("ComposeFrom error: %v", err) - } - checkCompose(compDst, "application/octet-stream") + // Compose should work even if the user sets no destination attributes. + compDst := bkt.Object("composed1") + c := compDst.ComposerFrom(compSrcs...) + if _, err := c.Run(ctx); err != nil { + t.Fatalf("ComposeFrom error: %v", err) + } + checkCompose(compDst, "application/octet-stream") - // It should also work if we do. - compDst = bkt.Object("composed2") - c = compDst.ComposerFrom(compSrcs...) - c.ContentType = "text/json" - if _, err := c.Run(ctx); err != nil { - t.Fatalf("ComposeFrom error: %v", err) - } - checkCompose(compDst, "text/json") + // It should also work if we do. + compDst = bkt.Object("composed2") + c = compDst.ComposerFrom(compSrcs...) + c.ContentType = "text/json" + if _, err := c.Run(ctx); err != nil { + t.Fatalf("ComposeFrom error: %v", err) + } + checkCompose(compDst, "text/json") + }) } func TestIntegration_Encoding(t *testing.T) { diff --git a/storage/storage.go b/storage/storage.go index 3ed70571ef0..855792f474f 100644 --- a/storage/storage.go +++ b/storage/storage.go @@ -52,6 +52,7 @@ import ( htransport "google.golang.org/api/transport/http" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoreflect" + "google.golang.org/protobuf/types/known/fieldmaskpb" "google.golang.org/protobuf/types/known/timestamppb" ) @@ -1483,10 +1484,11 @@ type Query struct { // object will be included in the results. Versions bool - // fieldSelection is used to select only specific fields to be returned by - // the query. It's used internally and is populated for the user by - // calling Query.SetAttrSelection - fieldSelection string + // attrSelection is used to select only specific fields to be returned by + // the query. It is set by the user calling calling SetAttrSelection. These + // are used by toFieldMask and toFieldSelection for gRPC and HTTP/JSON + // clients repsectively. + attrSelection []string // StartOffset is used to filter results to objects whose names are // lexicographically equal to or after startOffset. If endOffset is also set, @@ -1546,6 +1548,39 @@ var attrToFieldMap = map[string]string{ "CustomTime": "customTime", } +// attrToProtoFieldMap maps the field names of ObjectAttrs to the underlying field +// names in the protobuf Object message. +var attrToProtoFieldMap = map[string]string{ + "Name": "name", + "Bucket": "bucket", + "Etag": "etag", + "Generation": "generation", + "Metageneration": "metageneration", + "StorageClass": "storage_class", + "Size": "size", + "ContentEncoding": "content_encoding", + "ContentDisposition": "content_disposition", + "CacheControl": "cache_control", + "ACL": "acl", + "ContentLanguage": "content_language", + "Deleted": "delete_time", + "ContentType": "content_type", + "Created": "create_time", + "CRC32C": "checksums.crc32c", + "MD5": "checksums.md5_hash", + "Updated": "update_time", + "KMSKeyName": "kms_key", + "TemporaryHold": "temporary_hold", + "RetentionExpirationTime": "retention_expire_time", + "Metadata": "metadata", + "EventBasedHold": "event_based_hold", + "Owner": "owner", + "CustomerKeySHA256": "customer_encryption", + "CustomTime": "custom_time", + // MediaLink was explicitly excluded from the proto as it is an HTTP-ism. + // "MediaLink": "mediaLink", +} + // SetAttrSelection makes the query populate only specific attributes of // objects. When iterating over objects, if you only need each object's name // and size, pass []string{"Name", "Size"} to this method. Only these fields @@ -1554,16 +1589,42 @@ var attrToFieldMap = map[string]string{ // optimization; for more information, see // https://cloud.google.com/storage/docs/json_api/v1/how-tos/performance func (q *Query) SetAttrSelection(attrs []string) error { + // Validate selections. + for _, attr := range attrs { + // If the attr is acceptable for one of the two sets, then it is OK. + // If it is not acceptable for either, then return an error. + // The respective masking implementations ignore unknown attrs which + // makes switching between transports a little easier. + _, okJSON := attrToFieldMap[attr] + _, okGRPC := attrToProtoFieldMap[attr] + + if !okJSON && !okGRPC { + return fmt.Errorf("storage: attr %v is not valid", attr) + } + } + + q.attrSelection = attrs + + return nil +} + +func (q *Query) toFieldSelection() string { + if q == nil || len(q.attrSelection) == 0 { + return "" + } fieldSet := make(map[string]bool) - for _, attr := range attrs { + for _, attr := range q.attrSelection { field, ok := attrToFieldMap[attr] if !ok { - return fmt.Errorf("storage: attr %v is not valid", attr) + // Future proofing, skip unknown fields, let SetAttrSelection handle + // error modes. + continue } fieldSet[field] = true } + var s string if len(fieldSet) > 0 { var b bytes.Buffer b.WriteString("prefixes,items(") @@ -1576,9 +1637,50 @@ func (q *Query) SetAttrSelection(attrs []string) error { b.WriteString(field) } b.WriteString(")") - q.fieldSelection = b.String() + s = b.String() } - return nil + return s +} + +func (q *Query) toFieldMask() *fieldmaskpb.FieldMask { + // The default behavior with no Query is ProjectionDefault (i.e. ProjectionFull). + if q == nil { + return &fieldmaskpb.FieldMask{Paths: []string{"*"}} + } + + // User selected attributes via q.SetAttrSeleciton. This takes precedence + // over the Projection. + if numSelected := len(q.attrSelection); numSelected > 0 { + protoFieldPaths := make([]string, 0, numSelected) + + for _, attr := range q.attrSelection { + pf, ok := attrToProtoFieldMap[attr] + if !ok { + // Future proofing, skip unknown fields, let SetAttrSelection + // handle error modes. + continue + } + protoFieldPaths = append(protoFieldPaths, pf) + } + + return &fieldmaskpb.FieldMask{Paths: protoFieldPaths} + } + + // ProjectDefault == ProjectionFull which means all fields. + fm := &fieldmaskpb.FieldMask{Paths: []string{"*"}} + if q.Projection == ProjectionNoACL { + paths := make([]string, 0, len(attrToProtoFieldMap)-2) // omitting two fields + for _, f := range attrToProtoFieldMap { + // Skip the acl and owner fields for "NoACL". + if f == "acl" || f == "owner" { + continue + } + paths = append(paths, f) + } + fm.Paths = paths + } + + return fm } // Conditions constrain methods to act on specific generations of