Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(storage): migrate top-level methods #6433

Merged
merged 8 commits into from
Jul 27, 2022
68 changes: 6 additions & 62 deletions storage/bucket.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,27 +82,11 @@ func (b *BucketHandle) Create(ctx context.Context, projectID string, attrs *Buck
ctx = trace.StartSpan(ctx, "cloud.google.com/go/storage.Bucket.Create")
defer func() { trace.EndSpan(ctx, err) }()

var bkt *raw.Bucket
if attrs != nil {
bkt = attrs.toRawBucket()
} else {
bkt = &raw.Bucket{}
}
bkt.Name = b.name
// If there is lifecycle information but no location, explicitly set
// the location. This is a GCS quirk/bug.
if bkt.Location == "" && bkt.Lifecycle != nil {
bkt.Location = "US"
}
req := b.c.raw.Buckets.Insert(projectID, bkt)
setClientHeader(req.Header())
if attrs != nil && attrs.PredefinedACL != "" {
req.PredefinedAcl(attrs.PredefinedACL)
}
if attrs != nil && attrs.PredefinedDefaultObjectACL != "" {
req.PredefinedDefaultObjectAcl(attrs.PredefinedDefaultObjectACL)
o := makeStorageOpts(true, b.retry, b.userProject)
if _, err := b.c.tc.CreateBucket(ctx, projectID, b.name, attrs, o...); err != nil {
return err
}
return run(ctx, func() error { _, err := req.Context(ctx).Do(); return err }, b.retry, true, setRetryHeaderHTTP(req))
return nil
}

// Delete deletes the Bucket.
Expand Down Expand Up @@ -2124,17 +2108,8 @@ func (it *ObjectIterator) fetch(pageSize int, pageToken string) (string, error)
//
// Note: The returned iterator is not safe for concurrent operations without explicit synchronization.
func (c *Client) Buckets(ctx context.Context, projectID string) *BucketIterator {
it := &BucketIterator{
ctx: ctx,
client: c,
projectID: projectID,
}
it.pageInfo, it.nextFunc = iterator.NewPageInfo(
it.fetch,
func() int { return len(it.buckets) },
func() interface{} { b := it.buckets; it.buckets = nil; return b })

return it
o := makeStorageOpts(true, c.retry, "")
return c.tc.ListBuckets(ctx, projectID, o...)
}

// A BucketIterator is an iterator over BucketAttrs.
Expand All @@ -2145,7 +2120,6 @@ type BucketIterator struct {
Prefix string

ctx context.Context
client *Client
projectID string
buckets []*BucketAttrs
pageInfo *iterator.PageInfo
Expand All @@ -2171,36 +2145,6 @@ func (it *BucketIterator) Next() (*BucketAttrs, error) {
// Note: This method is not safe for concurrent operations without explicit synchronization.
func (it *BucketIterator) PageInfo() *iterator.PageInfo { return it.pageInfo }

// TODO: When the transport-agnostic client interface is integrated into the Veneer,
// this method should be removed, and the iterator should be initialized by the
// transport-specific client implementations.
func (it *BucketIterator) fetch(pageSize int, pageToken string) (token string, err error) {
req := it.client.raw.Buckets.List(it.projectID)
setClientHeader(req.Header())
req.Projection("full")
req.Prefix(it.Prefix)
req.PageToken(pageToken)
if pageSize > 0 {
req.MaxResults(int64(pageSize))
}
var resp *raw.Buckets
err = run(it.ctx, func() error {
resp, err = req.Context(it.ctx).Do()
return err
}, it.client.retry, true, setRetryHeaderHTTP(req))
if err != nil {
return "", err
}
for _, item := range resp.Items {
b, err := newBucket(item)
if err != nil {
return "", err
}
it.buckets = append(it.buckets, b)
}
return resp.NextPageToken, nil
}

// RPO (Recovery Point Objective) configures the turbo replication feature. See
// https://cloud.google.com/storage/docs/managing-turbo-replication for more information.
type RPO int
Expand Down
2 changes: 1 addition & 1 deletion storage/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ type storageClient interface {
// Top-level methods.

GetServiceAccount(ctx context.Context, project string, opts ...storageOption) (string, error)
CreateBucket(ctx context.Context, project string, attrs *BucketAttrs, opts ...storageOption) (*BucketAttrs, error)
CreateBucket(ctx context.Context, project, bucket string, attrs *BucketAttrs, opts ...storageOption) (*BucketAttrs, error)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch on this

ListBuckets(ctx context.Context, project string, opts ...storageOption) *BucketIterator
Close() error

Expand Down
48 changes: 24 additions & 24 deletions storage/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func TestCreateBucketEmulated(t *testing.T) {
LogBucket: bucket,
},
}
got, err := client.CreateBucket(context.Background(), project, want)
got, err := client.CreateBucket(context.Background(), project, want.Name, want)
if err != nil {
t.Fatal(err)
}
Expand All @@ -62,7 +62,7 @@ func TestDeleteBucketEmulated(t *testing.T) {
Name: bucket,
}
// Create the bucket that will be deleted.
_, err := client.CreateBucket(context.Background(), project, b)
_, err := client.CreateBucket(context.Background(), project, b.Name, b)
if err != nil {
t.Fatalf("client.CreateBucket: %v", err)
}
Expand All @@ -80,7 +80,7 @@ func TestGetBucketEmulated(t *testing.T) {
Name: bucket,
}
// Create the bucket that will be retrieved.
_, err := client.CreateBucket(context.Background(), project, want)
_, err := client.CreateBucket(context.Background(), project, want.Name, want)
if err != nil {
t.Fatalf("client.CreateBucket: %v", err)
}
Expand All @@ -100,7 +100,7 @@ func TestUpdateBucketEmulated(t *testing.T) {
Name: bucket,
}
// Create the bucket that will be updated.
_, err := client.CreateBucket(context.Background(), project, bkt)
_, err := client.CreateBucket(context.Background(), project, bkt.Name, bkt)
if err != nil {
t.Fatalf("client.CreateBucket: %v", err)
}
Expand Down Expand Up @@ -191,7 +191,7 @@ func TestGetServiceAccountEmulated(t *testing.T) {

func TestGetSetTestIamPolicyEmulated(t *testing.T) {
transportClientTest(t, func(t *testing.T, project, bucket string, client storageClient) {
battrs, err := client.CreateBucket(context.Background(), project, &BucketAttrs{
battrs, err := client.CreateBucket(context.Background(), project, bucket, &BucketAttrs{
Name: bucket,
})
if err != nil {
Expand Down Expand Up @@ -222,7 +222,7 @@ func TestGetSetTestIamPolicyEmulated(t *testing.T) {
func TestDeleteObjectEmulated(t *testing.T) {
transportClientTest(t, func(t *testing.T, project, bucket string, client storageClient) {
// Populate test object that will be deleted.
_, err := client.CreateBucket(context.Background(), project, &BucketAttrs{
_, err := client.CreateBucket(context.Background(), project, bucket, &BucketAttrs{
Name: bucket,
})
if err != nil {
Expand All @@ -249,7 +249,7 @@ func TestDeleteObjectEmulated(t *testing.T) {
func TestGetObjectEmulated(t *testing.T) {
transportClientTest(t, func(t *testing.T, project, bucket string, client storageClient) {
// Populate test object.
_, err := client.CreateBucket(context.Background(), project, &BucketAttrs{
_, err := client.CreateBucket(context.Background(), project, bucket, &BucketAttrs{
Name: bucket,
})
if err != nil {
Expand Down Expand Up @@ -279,7 +279,7 @@ func TestGetObjectEmulated(t *testing.T) {
func TestRewriteObjectEmulated(t *testing.T) {
transportClientTest(t, func(t *testing.T, project, bucket string, client storageClient) {
// Populate test object.
_, err := client.CreateBucket(context.Background(), project, &BucketAttrs{
_, err := client.CreateBucket(context.Background(), project, bucket, &BucketAttrs{
Name: bucket,
})
if err != nil {
Expand Down Expand Up @@ -324,7 +324,7 @@ func TestRewriteObjectEmulated(t *testing.T) {
func TestUpdateObjectEmulated(t *testing.T) {
transportClientTest(t, func(t *testing.T, project, bucket string, client storageClient) {
// Populate test object.
_, err := client.CreateBucket(context.Background(), project, &BucketAttrs{
_, err := client.CreateBucket(context.Background(), project, bucket, &BucketAttrs{
Name: bucket,
})
if err != nil {
Expand Down Expand Up @@ -391,7 +391,7 @@ func TestUpdateObjectEmulated(t *testing.T) {
func TestListObjectsEmulated(t *testing.T) {
transportClientTest(t, func(t *testing.T, project, bucket string, client storageClient) {
// Populate test data.
_, err := client.CreateBucket(context.Background(), project, &BucketAttrs{
_, err := client.CreateBucket(context.Background(), project, bucket, &BucketAttrs{
Name: bucket,
})
if err != nil {
Expand Down Expand Up @@ -449,7 +449,7 @@ func TestListObjectsEmulated(t *testing.T) {
func TestListObjectsWithPrefixEmulated(t *testing.T) {
transportClientTest(t, func(t *testing.T, project, bucket string, client storageClient) {
// Populate test data.
_, err := client.CreateBucket(context.Background(), project, &BucketAttrs{
_, err := client.CreateBucket(context.Background(), project, bucket, &BucketAttrs{
Name: bucket,
})
if err != nil {
Expand Down Expand Up @@ -515,7 +515,7 @@ func TestListBucketsEmulated(t *testing.T) {
}
// Create the buckets that will be listed.
for _, b := range want {
_, err := client.CreateBucket(context.Background(), project, b)
_, err := client.CreateBucket(context.Background(), project, b.Name, b)
if err != nil {
t.Fatalf("client.CreateBucket: %v", err)
}
Expand Down Expand Up @@ -556,7 +556,7 @@ func TestListBucketACLsEmulated(t *testing.T) {
PredefinedACL: "publicRead",
}
// Create the bucket that will be retrieved.
if _, err := client.CreateBucket(ctx, project, attrs); err != nil {
if _, err := client.CreateBucket(ctx, project, attrs.Name, attrs); err != nil {
t.Fatalf("client.CreateBucket: %v", err)
}

Expand All @@ -578,7 +578,7 @@ func TestUpdateBucketACLEmulated(t *testing.T) {
PredefinedACL: "authenticatedRead",
}
// Create the bucket that will be retrieved.
if _, err := client.CreateBucket(ctx, project, attrs); err != nil {
if _, err := client.CreateBucket(ctx, project, attrs.Name, attrs); err != nil {
t.Fatalf("client.CreateBucket: %v", err)
}
var listAcls []ACLRule
Expand Down Expand Up @@ -614,7 +614,7 @@ func TestDeleteBucketACLEmulated(t *testing.T) {
PredefinedACL: "publicRead",
}
// Create the bucket that will be retrieved.
if _, err := client.CreateBucket(ctx, project, attrs); err != nil {
if _, err := client.CreateBucket(ctx, project, attrs.Name, attrs); err != nil {
t.Fatalf("client.CreateBucket: %v", err)
}
// Assert bucket has two BucketACL entities, including project owner and predefinedACL.
Expand Down Expand Up @@ -648,7 +648,7 @@ func TestDefaultObjectACLCRUDEmulated(t *testing.T) {
PredefinedDefaultObjectACL: "publicRead",
}
// Create the bucket that will be retrieved.
if _, err := client.CreateBucket(ctx, project, attrs); err != nil {
if _, err := client.CreateBucket(ctx, project, attrs.Name, attrs); err != nil {
t.Fatalf("client.CreateBucket: %v", err)
}
// Assert bucket has 2 DefaultObjectACL entities, including project owner and PredefinedDefaultObjectACL.
Expand Down Expand Up @@ -692,7 +692,7 @@ func TestObjectACLCRUDEmulated(t *testing.T) {
transportClientTest(t, func(t *testing.T, project, bucket string, client storageClient) {
// Populate test object.
ctx := context.Background()
_, err := client.CreateBucket(ctx, project, &BucketAttrs{
_, err := client.CreateBucket(ctx, project, bucket, &BucketAttrs{
Name: bucket,
})
if err != nil {
Expand Down Expand Up @@ -746,7 +746,7 @@ func TestObjectACLCRUDEmulated(t *testing.T) {
func TestOpenReaderEmulated(t *testing.T) {
transportClientTest(t, func(t *testing.T, project, bucket string, client storageClient) {
// Populate test data.
_, err := client.CreateBucket(context.Background(), project, &BucketAttrs{
_, err := client.CreateBucket(context.Background(), project, bucket, &BucketAttrs{
Name: bucket,
})
if err != nil {
Expand Down Expand Up @@ -791,7 +791,7 @@ func TestOpenReaderEmulated(t *testing.T) {
func TestOpenWriterEmulated(t *testing.T) {
transportClientTest(t, func(t *testing.T, project, bucket string, client storageClient) {
// Populate test data.
_, err := client.CreateBucket(context.Background(), project, &BucketAttrs{
_, err := client.CreateBucket(context.Background(), project, bucket, &BucketAttrs{
Name: bucket,
})
if err != nil {
Expand Down Expand Up @@ -854,7 +854,7 @@ func TestListNotificationsEmulated(t *testing.T) {
transportClientTest(t, func(t *testing.T, project, bucket string, client storageClient) {
// Populate test object.
ctx := context.Background()
_, err := client.CreateBucket(ctx, project, &BucketAttrs{
_, err := client.CreateBucket(ctx, project, bucket, &BucketAttrs{
Name: bucket,
})
if err != nil {
Expand Down Expand Up @@ -882,7 +882,7 @@ func TestCreateNotificationEmulated(t *testing.T) {
transportClientTest(t, func(t *testing.T, project, bucket string, client storageClient) {
// Populate test object.
ctx := context.Background()
_, err := client.CreateBucket(ctx, project, &BucketAttrs{
_, err := client.CreateBucket(ctx, project, bucket, &BucketAttrs{
Name: bucket,
})
if err != nil {
Expand All @@ -908,7 +908,7 @@ func TestDeleteNotificationEmulated(t *testing.T) {
transportClientTest(t, func(t *testing.T, project, bucket string, client storageClient) {
// Populate test object.
ctx := context.Background()
_, err := client.CreateBucket(ctx, project, &BucketAttrs{
_, err := client.CreateBucket(ctx, project, bucket, &BucketAttrs{
Name: bucket,
})
if err != nil {
Expand Down Expand Up @@ -982,7 +982,7 @@ func TestLockBucketRetentionPolicyEmulated(t *testing.T) {
},
}
// Create the bucket that will be locked.
_, err := client.CreateBucket(context.Background(), project, b)
_, err := client.CreateBucket(context.Background(), project, b.Name, b)
if err != nil {
t.Fatalf("client.CreateBucket: %v", err)
}
Expand All @@ -1006,7 +1006,7 @@ func TestComposeEmulated(t *testing.T) {
ctx := context.Background()

// Populate test data.
_, err := client.CreateBucket(context.Background(), project, &BucketAttrs{
_, err := client.CreateBucket(context.Background(), project, bucket, &BucketAttrs{
Name: bucket,
})
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions storage/grpc_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,10 @@ func (c *grpcStorageClient) GetServiceAccount(ctx context.Context, project strin
return resp.EmailAddress, err
}

func (c *grpcStorageClient) CreateBucket(ctx context.Context, project string, attrs *BucketAttrs, opts ...storageOption) (*BucketAttrs, error) {
func (c *grpcStorageClient) CreateBucket(ctx context.Context, project, bucket string, attrs *BucketAttrs, opts ...storageOption) (*BucketAttrs, error) {
s := callSettings(c.settings, opts...)
b := attrs.toProtoBucket()

b.Name = bucket
// If there is lifecycle information but no location, explicitly set
// the location. This is a GCS quirk/bug.
if b.GetLocation() == "" && b.GetLifecycle() != nil {
Expand Down
4 changes: 2 additions & 2 deletions storage/http_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,15 +159,15 @@ func (c *httpStorageClient) GetServiceAccount(ctx context.Context, project strin
return res.EmailAddress, nil
}

func (c *httpStorageClient) CreateBucket(ctx context.Context, project string, attrs *BucketAttrs, opts ...storageOption) (*BucketAttrs, error) {
func (c *httpStorageClient) CreateBucket(ctx context.Context, project, bucket string, attrs *BucketAttrs, opts ...storageOption) (*BucketAttrs, error) {
s := callSettings(c.settings, opts...)
var bkt *raw.Bucket
if attrs != nil {
bkt = attrs.toRawBucket()
} else {
bkt = &raw.Bucket{}
}

bkt.Name = bucket
// If there is lifecycle information but no location, explicitly set
// the location. This is a GCS quirk/bug.
if bkt.Location == "" && bkt.Lifecycle != nil {
Expand Down
14 changes: 3 additions & 11 deletions storage/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -2106,17 +2106,9 @@ func toProtoCommonObjectRequestParams(key []byte) *storagepb.CommonObjectRequest

// ServiceAccount fetches the email address of the given project's Google Cloud Storage service account.
func (c *Client) ServiceAccount(ctx context.Context, projectID string) (string, error) {
r := c.raw.Projects.ServiceAccount.Get(projectID)
var res *raw.ServiceAccount
var err error
err = run(ctx, func() error {
res, err = r.Context(ctx).Do()
return err
}, c.retry, true, setRetryHeaderHTTP(r))
if err != nil {
return "", err
}
return res.EmailAddress, nil
o := makeStorageOpts(true, c.retry, "")
return c.tc.GetServiceAccount(ctx, projectID, o...)

}

// bucketResourceName formats the given project ID and bucketResourceName ID
Expand Down
5 changes: 0 additions & 5 deletions storage/storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1354,11 +1354,6 @@ func TestRetryer(t *testing.T) {
r: c.HMACKeyHandle("pID", "accessID").retry,
want: c.retry,
},
{
name: "client.Buckets()",
r: c.Buckets(ctx, "pID").client.retry,
want: c.retry,
},
{
name: "bucket.Objects()",
r: b.Objects(ctx, nil).bucket.retry,
Expand Down