chore(storage): implement UpdateObject (#6091)

* chore(storage): implement UpdateObject * list acl as todo and update tests * clarify tests
googleapis · Jun 1, 2022 · ef2559a · ef2559a
1 parent a03db1f
commit ef2559a
Show file tree

Hide file tree

Showing 5 changed files with 275 additions and 15 deletions.
diff --git a/storage/client.go b/storage/client.go
@@ -58,7 +58,7 @@ type storageClient interface {
 
 	DeleteObject(ctx context.Context, bucket, object string, gen int64, conds *Conditions, opts ...storageOption) error
 	GetObject(ctx context.Context, bucket, object string, gen int64, encryptionKey []byte, conds *Conditions, opts ...storageOption) (*ObjectAttrs, error)
-	UpdateObject(ctx context.Context, bucket, object string, uattrs *ObjectAttrsToUpdate, conds *Conditions, opts ...storageOption) (*ObjectAttrs, error)
+	UpdateObject(ctx context.Context, bucket, object string, uattrs *ObjectAttrsToUpdate, gen int64, encryptionKey []byte, conds *Conditions, opts ...storageOption) (*ObjectAttrs, error)
 
 	// Default Object ACL methods.
 

diff --git a/storage/client_test.go b/storage/client_test.go
@@ -142,34 +142,34 @@ func TestUpdateBucketEmulated(t *testing.T) {
 			t.Fatal(err)
 		}
 		if diff := cmp.Diff(got.Name, want.Name); diff != "" {
-			t.Errorf("got(-),want(+):\n%s", diff)
+			t.Errorf("Name: got(-),want(+):\n%s", diff)
 		}
 		if diff := cmp.Diff(got.VersioningEnabled, want.VersioningEnabled); diff != "" {
-			t.Errorf("got(-),want(+):\n%s", diff)
+			t.Errorf("VersioningEnabled: got(-),want(+):\n%s", diff)
 		}
 		if diff := cmp.Diff(got.RequesterPays, want.RequesterPays); diff != "" {
-			t.Errorf("got(-),want(+):\n%s", diff)
+			t.Errorf("RequesterPays: got(-),want(+):\n%s", diff)
 		}
 		if diff := cmp.Diff(got.DefaultEventBasedHold, want.DefaultEventBasedHold); diff != "" {
-			t.Errorf("got(-),want(+):\n%s", diff)
+			t.Errorf("DefaultEventBasedHold: got(-),want(+):\n%s", diff)
 		}
 		if diff := cmp.Diff(got.Encryption, want.Encryption); diff != "" {
-			t.Errorf("got(-),want(+):\n%s", diff)
+			t.Errorf("Encryption: got(-),want(+):\n%s", diff)
 		}
 		if diff := cmp.Diff(got.Lifecycle, want.Lifecycle); diff != "" {
-			t.Errorf("got(-),want(+):\n%s", diff)
+			t.Errorf("Lifecycle: got(-),want(+):\n%s", diff)
 		}
 		if diff := cmp.Diff(got.Logging, want.Logging); diff != "" {
-			t.Errorf("got(-),want(+):\n%s", diff)
+			t.Errorf("Logging: got(-),want(+):\n%s", diff)
 		}
 		if diff := cmp.Diff(got.Website, want.Website); diff != "" {
-			t.Errorf("got(-),want(+):\n%s", diff)
+			t.Errorf("Website: got(-),want(+):\n%s", diff)
 		}
 		if diff := cmp.Diff(got.RPO, want.RPO); diff != "" {
-			t.Errorf("got(-),want(+):\n%s", diff)
+			t.Errorf("RPO: got(-),want(+):\n%s", diff)
 		}
 		if diff := cmp.Diff(got.StorageClass, want.StorageClass); diff != "" {
-			t.Errorf("got(-),want(+):\n%s", diff)
+			t.Errorf("StorageClass: got(-),want(+):\n%s", diff)
 		}
 	})
 }
@@ -270,6 +270,73 @@ func TestGetObjectEmulated(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{
+			Name: bucket,
+		})
+		if err != nil {
+			t.Fatalf("client.CreateBucket: %v", err)
+		}
+		ct := time.Date(2022, 5, 25, 12, 12, 12, 0, time.UTC)
+		o := ObjectAttrs{
+			Bucket:     bucket,
+			Name:       fmt.Sprintf("testObject-%d", time.Now().Nanosecond()),
+			CustomTime: ct,
+		}
+		w := veneerClient.Bucket(bucket).Object(o.Name).NewWriter(context.Background())
+		if _, err := w.Write(randomBytesToWrite); err != nil {
+			t.Fatalf("failed to populate test object: %v", err)
+		}
+		if err := w.Close(); err != nil {
+			t.Fatalf("closing object: %v", err)
+		}
+		want := &ObjectAttrsToUpdate{
+			EventBasedHold:     false,
+			TemporaryHold:      false,
+			ContentType:        "text/html",
+			ContentLanguage:    "en",
+			ContentEncoding:    "gzip",
+			ContentDisposition: "",
+			CacheControl:       "",
+			CustomTime:         ct.Add(10 * time.Hour),
+		}
+
+		got, err := client.UpdateObject(context.Background(), bucket, o.Name, want, defaultGen, nil, &Conditions{MetagenerationMatch: 1})
+		if err != nil {
+			t.Fatalf("client.UpdateObject: %v", err)
+		}
+		if diff := cmp.Diff(got.Name, o.Name); diff != "" {
+			t.Errorf("Name: got(-),want(+):\n%s", diff)
+		}
+		if diff := cmp.Diff(got.EventBasedHold, want.EventBasedHold); diff != "" {
+			t.Errorf("EventBasedHold: got(-),want(+):\n%s", diff)
+		}
+		if diff := cmp.Diff(got.TemporaryHold, want.TemporaryHold); diff != "" {
+			t.Errorf("TemporaryHold: got(-),want(+):\n%s", diff)
+		}
+		if diff := cmp.Diff(got.ContentType, want.ContentType); diff != "" {
+			t.Errorf("ContentType: got(-),want(+):\n%s", diff)
+		}
+		if diff := cmp.Diff(got.ContentLanguage, want.ContentLanguage); diff != "" {
+			t.Errorf("ContentLanguage: got(-),want(+):\n%s", diff)
+		}
+		if diff := cmp.Diff(got.ContentEncoding, want.ContentEncoding); diff != "" {
+			t.Errorf("ContentEncoding: got(-),want(+):\n%s", diff)
+		}
+		if diff := cmp.Diff(got.ContentDisposition, want.ContentDisposition); diff != "" {
+			t.Errorf("ContentDisposition: got(-),want(+):\n%s", diff)
+		}
+		if diff := cmp.Diff(got.CacheControl, want.CacheControl); diff != "" {
+			t.Errorf("CacheControl: got(-),want(+):\n%s", diff)
+		}
+		if diff := cmp.Diff(got.CustomTime, want.CustomTime); diff != "" {
+			t.Errorf("CustomTime: got(-),want(+):\n%s", diff)
+		}
+	})
+}
+
 func TestListObjectsEmulated(t *testing.T) {
 	transportClientTest(t, func(t *testing.T, project, bucket string, client storageClient) {
 		// Populate test data.

diff --git a/storage/grpc_client.go b/storage/grpc_client.go
@@ -459,8 +459,67 @@ func (c *grpcStorageClient) GetObject(ctx context.Context, bucket, object string
 	return attrs, err
 }
 
-func (c *grpcStorageClient) UpdateObject(ctx context.Context, bucket, object string, uattrs *ObjectAttrsToUpdate, conds *Conditions, opts ...storageOption) (*ObjectAttrs, error) {
-	return nil, errMethodNotSupported
+func (c *grpcStorageClient) UpdateObject(ctx context.Context, bucket, object string, uattrs *ObjectAttrsToUpdate, gen int64, encryptionKey []byte, conds *Conditions, opts ...storageOption) (*ObjectAttrs, error) {
+	s := callSettings(c.settings, opts...)
+	o := uattrs.toProtoObject(bucketResourceName(globalProjectAlias, bucket), object)
+	req := &storagepb.UpdateObjectRequest{
+		Object: o,
+	}
+	if err := applyCondsProto("grpcStorageClient.UpdateObject", gen, conds, req); err != nil {
+		return nil, err
+	}
+	if s.userProject != "" {
+		ctx = setUserProjectMetadata(ctx, s.userProject)
+	}
+	if encryptionKey != nil {
+		req.CommonObjectRequestParams = toProtoCommonObjectRequestParams(encryptionKey)
+	}
+
+	var paths []string
+	fieldMask := &fieldmaskpb.FieldMask{
+		Paths: paths,
+	}
+	if uattrs.EventBasedHold != nil {
+		fieldMask.Paths = append(fieldMask.Paths, "event_based_hold")
+	}
+	if uattrs.TemporaryHold != nil {
+		fieldMask.Paths = append(fieldMask.Paths, "temporary_hold")
+	}
+	if uattrs.ContentType != nil {
+		fieldMask.Paths = append(fieldMask.Paths, "content_type")
+	}
+	if uattrs.ContentLanguage != nil {
+		fieldMask.Paths = append(fieldMask.Paths, "content_language")
+	}
+	if uattrs.ContentEncoding != nil {
+		fieldMask.Paths = append(fieldMask.Paths, "content_encoding")
+	}
+	if uattrs.ContentDisposition != nil {
+		fieldMask.Paths = append(fieldMask.Paths, "content_disposition")
+	}
+	if uattrs.CacheControl != nil {
+		fieldMask.Paths = append(fieldMask.Paths, "cache_control")
+	}
+	if !uattrs.CustomTime.IsZero() {
+		fieldMask.Paths = append(fieldMask.Paths, "custom_time")
+	}
+
+	// TODO(cathyo): Handle ACL and PredefinedACL. Pending b/233617896.
+	// TODO(cathyo): Handle metadata. Pending b/230510191.
+
+	req.UpdateMask = fieldMask
+
+	var attrs *ObjectAttrs
+	err := run(ctx, func() error {
+		res, err := c.raw.UpdateObject(ctx, req, s.gax...)
+		attrs = newObjectFromProto(res)
+		return err
+	}, s.retry, s.idempotent, setRetryHeaderGRPC(ctx))
+	if e, ok := status.FromError(err); ok && e.Code() == codes.NotFound {
+		return nil, ErrObjectNotExist
+	}
+
+	return attrs, err
 }
 
 // Default Object ACL methods.

diff --git a/storage/http_client.go b/storage/http_client.go
@@ -28,6 +28,7 @@ import (
 	"strings"
 	"time"
 
+	"cloud.google.com/go/internal/optional"
 	"cloud.google.com/go/internal/trace"
 	"golang.org/x/oauth2/google"
 	"google.golang.org/api/googleapi"
@@ -428,8 +429,98 @@ func (c *httpStorageClient) GetObject(ctx context.Context, bucket, object string
 	return newObject(obj), nil
 }
 
-func (c *httpStorageClient) UpdateObject(ctx context.Context, bucket, object string, uattrs *ObjectAttrsToUpdate, conds *Conditions, opts ...storageOption) (*ObjectAttrs, error) {
-	return nil, errMethodNotSupported
+func (c *httpStorageClient) UpdateObject(ctx context.Context, bucket, object string, uattrs *ObjectAttrsToUpdate, gen int64, encryptionKey []byte, conds *Conditions, opts ...storageOption) (*ObjectAttrs, error) {
+	s := callSettings(c.settings, opts...)
+
+	var attrs ObjectAttrs
+	// Lists of fields to send, and set to null, in the JSON.
+	var forceSendFields, nullFields []string
+	if uattrs.ContentType != nil {
+		attrs.ContentType = optional.ToString(uattrs.ContentType)
+		// For ContentType, sending the empty string is a no-op.
+		// Instead we send a null.
+		if attrs.ContentType == "" {
+			nullFields = append(nullFields, "ContentType")
+		} else {
+			forceSendFields = append(forceSendFields, "ContentType")
+		}
+	}
+	if uattrs.ContentLanguage != nil {
+		attrs.ContentLanguage = optional.ToString(uattrs.ContentLanguage)
+		// For ContentLanguage it's an error to send the empty string.
+		// Instead we send a null.
+		if attrs.ContentLanguage == "" {
+			nullFields = append(nullFields, "ContentLanguage")
+		} else {
+			forceSendFields = append(forceSendFields, "ContentLanguage")
+		}
+	}
+	if uattrs.ContentEncoding != nil {
+		attrs.ContentEncoding = optional.ToString(uattrs.ContentEncoding)
+		forceSendFields = append(forceSendFields, "ContentEncoding")
+	}
+	if uattrs.ContentDisposition != nil {
+		attrs.ContentDisposition = optional.ToString(uattrs.ContentDisposition)
+		forceSendFields = append(forceSendFields, "ContentDisposition")
+	}
+	if uattrs.CacheControl != nil {
+		attrs.CacheControl = optional.ToString(uattrs.CacheControl)
+		forceSendFields = append(forceSendFields, "CacheControl")
+	}
+	if uattrs.EventBasedHold != nil {
+		attrs.EventBasedHold = optional.ToBool(uattrs.EventBasedHold)
+		forceSendFields = append(forceSendFields, "EventBasedHold")
+	}
+	if uattrs.TemporaryHold != nil {
+		attrs.TemporaryHold = optional.ToBool(uattrs.TemporaryHold)
+		forceSendFields = append(forceSendFields, "TemporaryHold")
+	}
+	if !uattrs.CustomTime.IsZero() {
+		attrs.CustomTime = uattrs.CustomTime
+		forceSendFields = append(forceSendFields, "CustomTime")
+	}
+	if uattrs.Metadata != nil {
+		attrs.Metadata = uattrs.Metadata
+		if len(attrs.Metadata) == 0 {
+			// Sending the empty map is a no-op. We send null instead.
+			nullFields = append(nullFields, "Metadata")
+		} else {
+			forceSendFields = append(forceSendFields, "Metadata")
+		}
+	}
+	if uattrs.ACL != nil {
+		attrs.ACL = uattrs.ACL
+		// It's an error to attempt to delete the ACL, so
+		// we don't append to nullFields here.
+		forceSendFields = append(forceSendFields, "Acl")
+	}
+	rawObj := attrs.toRawObject(bucket)
+	rawObj.ForceSendFields = forceSendFields
+	rawObj.NullFields = nullFields
+	call := c.raw.Objects.Patch(bucket, object, rawObj).Projection("full").Context(ctx)
+	if err := applyConds("Update", gen, conds, call); err != nil {
+		return nil, err
+	}
+	if s.userProject != "" {
+		call.UserProject(s.userProject)
+	}
+	if uattrs.PredefinedACL != "" {
+		call.PredefinedAcl(uattrs.PredefinedACL)
+	}
+	if err := setEncryptionHeaders(call.Header(), encryptionKey, false); err != nil {
+		return nil, err
+	}
+	var obj *raw.Object
+	var err error
+	err = run(ctx, func() error { obj, err = call.Do(); return err }, s.retry, s.idempotent, setRetryHeaderHTTP(call))
+	var e *googleapi.Error
+	if errors.As(err, &e) && e.Code == http.StatusNotFound {
+		return nil, ErrObjectNotExist
+	}
+	if err != nil {
+		return nil, err
+	}
+	return newObject(obj), nil
 }
 
 // Default Object ACL methods.

diff --git a/storage/storage.go b/storage/storage.go
@@ -1245,6 +1245,49 @@ func (o *ObjectAttrs) toProtoObject(b string) *storagepb.Object {
 	}
 }
 
+// toProtoObject copies the attributes to update from uattrs to the proto library's Object type.
+func (uattrs *ObjectAttrsToUpdate) toProtoObject(bucket, object string) *storagepb.Object {
+	o := &storagepb.Object{
+		Name:   object,
+		Bucket: bucket,
+	}
+	if uattrs == nil {
+		return o
+	}
+
+	if uattrs.EventBasedHold != nil {
+		o.EventBasedHold = proto.Bool(optional.ToBool(uattrs.EventBasedHold))
+	}
+	if uattrs.TemporaryHold != nil {
+		o.TemporaryHold = optional.ToBool(uattrs.TemporaryHold)
+	}
+	if uattrs.ContentType != nil {
+		o.ContentType = optional.ToString(uattrs.ContentType)
+	}
+	if uattrs.ContentLanguage != nil {
+		o.ContentLanguage = optional.ToString(uattrs.ContentLanguage)
+	}
+	if uattrs.ContentEncoding != nil {
+		o.ContentEncoding = optional.ToString(uattrs.ContentEncoding)
+	}
+	if uattrs.ContentDisposition != nil {
+		o.ContentDisposition = optional.ToString(uattrs.ContentDisposition)
+	}
+	if uattrs.CacheControl != nil {
+		o.CacheControl = optional.ToString(uattrs.CacheControl)
+	}
+	if !uattrs.CustomTime.IsZero() {
+		o.CustomTime = toProtoTimestamp(uattrs.CustomTime)
+	}
+	if uattrs.ACL != nil {
+		o.Acl = toProtoObjectACL(uattrs.ACL)
+	}
+
+	// TODO(cathyo): Handle metadata. Pending b/230510191.
+
+	return o
+}
+
 // ObjectAttrs represents the metadata for a Google Cloud Storage (GCS) object.
 type ObjectAttrs struct {
 	// Bucket is the name of the bucket containing this GCS object.