storage: omit bucket from copy attributes
When encoding destination attributes on a rewrite call, do not encode
the bucket. This allows us to omit the object name and content type
from the attributes.
Change-Id: I2f4d3e01e6f152543c04b7bcebf78c6ac7978d2f
Reviewed-on: https://code-review.googlesource.com/11296
Reviewed-by: Sarah Adams <shadams@google.com>
diff --git a/storage/copy.go b/storage/copy.go
index 1f28455..b2db656 100644
--- a/storage/copy.go
+++ b/storage/copy.go
@@ -20,7 +20,6 @@
import (
"errors"
"fmt"
- "reflect"
"golang.org/x/net/context"
raw "google.golang.org/api/storage/v1"
@@ -71,17 +70,11 @@
if err := c.dst.validate(); err != nil {
return nil, err
}
- var rawObject *raw.Object
- // If any attribute was set, then we make sure the name matches the destination
- // name, and we check that ContentType is non-empty so we can provide a better
- // error message than the service.
- if !reflect.DeepEqual(c.ObjectAttrs, ObjectAttrs{}) {
- c.ObjectAttrs.Name = c.dst.object
- if c.ObjectAttrs.ContentType == "" {
- return nil, errors.New("storage: Copier.ContentType must be non-empty")
- }
- rawObject = c.ObjectAttrs.toRawObject(c.dst.bucket)
- }
+ // Convert destination attributes to raw form, omitting the bucket.
+ // If the bucket is included but name or content-type aren't, the service
+ // returns a 400 with "Required" as the only message. Omitting the bucket
+ // does not cause any problems.
+ rawObject := c.ObjectAttrs.toRawObject("")
for {
res, err := c.callRewrite(ctx, c.src, rawObject)
if err != nil {
diff --git a/storage/integration_test.go b/storage/integration_test.go
index fd9e0f2..6bca383 100644
--- a/storage/integration_test.go
+++ b/storage/integration_test.go
@@ -386,21 +386,9 @@
copyObj.Bucket, copyObj.Name, bucket, copyName)
}
- // Check for error setting attributes but not ContentType.
- const (
- contentType = "text/html"
- contentEncoding = "identity"
- )
- copier := bkt.Object(copyName).CopierFrom(bkt.Object(objName))
- copier.ContentEncoding = contentEncoding
- _, err = copier.Run(ctx)
- if err == nil {
- t.Error("copy without ContentType: got nil, want error")
- }
-
// Copying with attributes.
- copier = bkt.Object(copyName).CopierFrom(bkt.Object(objName))
- copier.ContentType = contentType
+ const contentEncoding = "identity"
+ copier := bkt.Object(copyName).CopierFrom(bkt.Object(objName))
copier.ContentEncoding = contentEncoding
copyObj, err = copier.Run(ctx)
if err != nil {
@@ -410,9 +398,6 @@
t.Errorf("Copy object bucket, name: got %q.%q, want %q.%q",
copyObj.Bucket, copyObj.Name, bucket, copyName)
}
- if copyObj.ContentType != contentType {
- t.Errorf("Copy ContentType: got %q, want %q", copyObj.ContentType, contentType)
- }
if copyObj.ContentEncoding != contentEncoding {
t.Errorf("Copy ContentEncoding: got %q, want %q", copyObj.ContentEncoding, contentEncoding)
}
@@ -1004,6 +989,38 @@
}
}
+func TestIntegration_BucketInCopyAttrs(t *testing.T) {
+ // Confirm that if bucket is included in the object attributes of a rewrite
+ // call, but object name and content-type aren't, then we get an error. See
+ // the comment in Copier.Run.
+ ctx := context.Background()
+ client, bucket := testConfig(ctx, t)
+ defer client.Close()
+
+ bkt := client.Bucket(bucket)
+ obj := bkt.Object("bucketInCopyAttrs")
+ if err := writeObject(ctx, obj, "", []byte("foo")); err != nil {
+ t.Fatal(err)
+ }
+ copier := obj.CopierFrom(obj)
+ rawObject := copier.ObjectAttrs.toRawObject(bucket)
+ _, err := copier.callRewrite(ctx, obj, rawObject)
+ if err == nil {
+ t.Errorf("got nil, want error")
+ }
+}
+
+func writeObject(ctx context.Context, obj *ObjectHandle, contentType string, contents []byte) error {
+ w := obj.NewWriter(ctx)
+ w.ContentType = contentType
+ if contents != nil {
+ if _, err := w.Write([]byte(contents)); err != nil {
+ return err
+ }
+ }
+ return w.Close()
+}
+
func readObject(ctx context.Context, obj *ObjectHandle) ([]byte, error) {
r, err := obj.NewReader(ctx)
if err != nil {
