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.

Sign up for GitHub

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

Jump to bottom

chore(storage): add Bucket Get & Delete implementations #5775

Merged
merged 3 commits into from
Mar 23, 2022
Merged

chore(storage): add Bucket Get & Delete implementations #5775

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b4299ee
chore(storage): add Bucket Get & Delete implementations
noahdietz Mar 21, 2022
19f2fc4
Merge branch 'main' into bucket-get-delete
noahdietz Mar 22, 2022
e5578d4
use errors instead of xerrors
noahdietz Mar 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions storage/bucket.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1284,6 +1284,32 @@ func applyBucketConds(method string, conds *BucketConditions, call interface{})
return nil
}

// applyBucketConds modifies the provided request message using the conditions
// in conds. msg is a protobuf Message that has fields if_metageneration_match
// and if_metageneration_not_match.
func applyBucketCondsProto(method string, conds *BucketConditions, msg proto.Message) error {
rmsg := msg.ProtoReflect()

if conds == nil {
return nil
}
if err := conds.validate(method); err != nil {
return err
}

switch {
case conds.MetagenerationMatch != 0:
if !setConditionProtoField(rmsg, "if_metageneration_match", conds.MetagenerationMatch) {
return fmt.Errorf("storage: %s: ifMetagenerationMatch not supported", method)
}
case conds.MetagenerationNotMatch != 0:
if !setConditionProtoField(rmsg, "if_metageneration_not_match", conds.MetagenerationNotMatch) {
return fmt.Errorf("storage: %s: ifMetagenerationNotMatch not supported", method)
}
}
return nil
}

func (rp *RetentionPolicy) toRawRetentionPolicy() *raw.BucketRetentionPolicy {
if rp == nil {
return nil
Expand Down
53 changes: 52 additions & 1 deletion storage/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func TestCreateBucketEmulated(t *testing.T) {
}
got, err := client.CreateBucket(context.Background(), project, want)
if err != nil {
t.Fatal(err)
t.Fatalf("%s, %v", transport, err)
BrennaEpp marked this conversation as resolved.
Show resolved Hide resolved
}
want.Location = "US"
if diff := cmp.Diff(got.Name, want.Name); diff != "" {
Expand All @@ -49,6 +49,57 @@ func TestCreateBucketEmulated(t *testing.T) {
}
}

func TestDeleteBucketEmulated(t *testing.T) {
checkEmulatorEnvironment(t)

for transport, client := range emulatorClients {
project := fmt.Sprintf("%s-project", transport)
b := &BucketAttrs{
Name: fmt.Sprintf("%s-bucket-%d", transport, time.Now().Nanosecond()),
}
// Create the bucket that will be deleted.
_, err := client.CreateBucket(context.Background(), project, b)
if err != nil {
// Flag the error and continue so the Delete is skipped and the next
// transport can run its test.
t.Errorf("%s: %v", transport, err)
continue
}
// Delete the bucket that was just created.
err = client.DeleteBucket(context.Background(), b.Name, nil)
if err != nil {
t.Errorf("%s: %v", transport, err)
}
}
}

func TestGetBucketEmulated(t *testing.T) {
checkEmulatorEnvironment(t)

for transport, client := range emulatorClients {
project := fmt.Sprintf("%s-project", transport)
want := &BucketAttrs{
Name: fmt.Sprintf("%s-bucket-%d", transport, time.Now().Nanosecond()),
}
// Create the bucket that will be retrieved.
_, err := client.CreateBucket(context.Background(), project, want)
if err != nil {
// Flag the error and continue so the Get is skipped and the next
// transport can run its test.
t.Errorf("%s: %v", transport, err)
continue
}
got, err := client.GetBucket(context.Background(), want.Name, &BucketConditions{MetagenerationMatch: 1})
if err != nil {
t.Errorf("%s: %v", transport, err)
continue
}
if diff := cmp.Diff(got.Name, want.Name); diff != "" {
t.Errorf("%s: got(-),want(+):\n%s", transport, diff)
}
}
}

func initEmulatorClients() func() error {
noopCloser := func() error { return nil }
if !isEmulatorEnvironmentSet() {
Expand Down
48 changes: 46 additions & 2 deletions storage/grpc_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import (
iampb "google.golang.org/genproto/googleapis/iam/v1"
storagepb "google.golang.org/genproto/googleapis/storage/v2"
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
)

const (
Expand Down Expand Up @@ -144,10 +146,52 @@ func (c *grpcStorageClient) ListBuckets(ctx context.Context, project string, opt
// Bucket methods.

func (c *grpcStorageClient) DeleteBucket(ctx context.Context, bucket string, conds *BucketConditions, opts ...storageOption) error {
return errMethodNotSupported
s := callSettings(c.settings, opts...)
req := &storagepb.DeleteBucketRequest{
Name: bucketResourceName( /* project */ "_", bucket),
}
if err := applyBucketCondsProto("grpcStorageClient.DeleteBucket", conds, req); err != nil {
return err
}
if s.userProject != "" {
req.CommonRequestParams = &storagepb.CommonRequestParams{
UserProject: toProjectResource(s.userProject),
}
}

return run(ctx, func() error {
return c.raw.DeleteBucket(ctx, req, s.gax...)
}, s.retry, s.idempotent)
}

func (c *grpcStorageClient) GetBucket(ctx context.Context, bucket string, conds *BucketConditions, opts ...storageOption) (*BucketAttrs, error) {
return nil, errMethodNotSupported
s := callSettings(c.settings, opts...)
req := &storagepb.GetBucketRequest{
Name: bucketResourceName( /* project */ "_", bucket),
}
if err := applyBucketCondsProto("grpcStorageClient.GetBucket", conds, req); err != nil {
return nil, err
}
if s.userProject != "" {
req.CommonRequestParams = &storagepb.CommonRequestParams{
UserProject: toProjectResource(s.userProject),
}
}

var battrs *BucketAttrs
err := run(ctx, func() error {
res, err := c.raw.GetBucket(ctx, req, s.gax...)

battrs = newBucketFromProto(res)

return err
}, s.retry, s.idempotent)

if s, ok := status.FromError(err); ok && s.Code() == codes.NotFound {
return nil, ErrBucketNotExist
}

return battrs, err
}
func (c *grpcStorageClient) UpdateBucket(ctx context.Context, uattrs *BucketAttrsToUpdate, conds *BucketConditions, opts ...storageOption) (*BucketAttrs, error) {
return nil, errMethodNotSupported
Expand Down
41 changes: 39 additions & 2 deletions storage/http_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,15 @@ package storage

import (
"context"
"errors"
"fmt"
"net/http"
"net/url"
"os"
"strings"

"golang.org/x/oauth2/google"
"google.golang.org/api/googleapi"
"google.golang.org/api/option"
"google.golang.org/api/option/internaloption"
raw "google.golang.org/api/storage/v1"
Expand Down Expand Up @@ -178,10 +180,45 @@ func (c *httpStorageClient) ListBuckets(ctx context.Context, project string, opt
// Bucket methods.

func (c *httpStorageClient) DeleteBucket(ctx context.Context, bucket string, conds *BucketConditions, opts ...storageOption) error {
return errMethodNotSupported
s := callSettings(c.settings, opts...)
req := c.raw.Buckets.Delete(bucket)
setClientHeader(req.Header())
if err := applyBucketConds("httpStorageClient.DeleteBucket", conds, req); err != nil {
return err
}
if s.userProject != "" {
req.UserProject(s.userProject)
}

return run(ctx, func() error { return req.Context(ctx).Do() }, s.retry, s.idempotent)
Copy link
Contributor

Choose a reason for hiding this comment

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

How do the settings know the default idempotency value for this delete operation?

Copy link
Contributor Author

@noahdietz noahdietz Mar 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Communicating the idempotency will the responsibility of the transport-agnostic client layer (i.e. the layer above these transport-specific implementations that will call them). It will communicate to this transport-specific layer via the storageOption.idempotent(bool). This is basically the same as it is today, where the transport-agnostic client layer supplies true or false to the run call. Swap out the run call for the transport-client interface call with the option idempotent(true) and we are good to do.

}

func (c *httpStorageClient) GetBucket(ctx context.Context, bucket string, conds *BucketConditions, opts ...storageOption) (*BucketAttrs, error) {
return nil, errMethodNotSupported
s := callSettings(c.settings, opts...)
req := c.raw.Buckets.Get(bucket).Projection("full")
setClientHeader(req.Header())
err := applyBucketConds("httpStorageClient.GetBucket", conds, req)
if err != nil {
return nil, err
}
if s.userProject != "" {
req.UserProject(s.userProject)
}

var resp *raw.Bucket
err = run(ctx, func() error {
resp, err = req.Context(ctx).Do()
return err
}, s.retry, s.idempotent)

var e *googleapi.Error
if ok := errors.As(err, &e); ok && e.Code == http.StatusNotFound {
return nil, ErrBucketNotExist
}
if err != nil {
return nil, err
}
return newBucket(resp)
}
func (c *httpStorageClient) UpdateBucket(ctx context.Context, uattrs *BucketAttrsToUpdate, conds *BucketConditions, opts ...storageOption) (*BucketAttrs, error) {
return nil, errMethodNotSupported
Expand Down