Skip to content

Commit

Permalink
storage: always send destination in compose request
Browse files Browse the repository at this point in the history
The Compose call returns an error if the destination field
of the JSON body is empty. So we set it unconditionally.

The previous code set both the bucket and name fields, but
that isn't necessary. Setting just the bucket is sufficient.

Change-Id: Ib4f83da0c71df73516be39401053b6ea739c6d31
Reviewed-on: https://code-review.googlesource.com/8992
Reviewed-by: Jonathan Amsterdam <jba@google.com>
  • Loading branch information
jba committed Oct 31, 2016
1 parent 67f57c5 commit f9c9ec4
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 22 deletions.
8 changes: 3 additions & 5 deletions storage/copy.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,11 +148,9 @@ func (c *Composer) Run(ctx context.Context) (*ObjectAttrs, error) {
}

req := &raw.ComposeRequest{}
if !reflect.DeepEqual(c.ObjectAttrs, ObjectAttrs{}) {
req.Destination = c.ObjectAttrs.toRawObject(c.dst.bucket)
req.Destination.Name = c.dst.object
}

// Compose requires a non-empty Destination, so we always set it,
// even if the caller-provided ObjectAttrs is the zero value.
req.Destination = c.ObjectAttrs.toRawObject(c.dst.bucket)
for _, src := range c.srcs {
if err := src.validate(); err != nil {
return nil, err
Expand Down
45 changes: 29 additions & 16 deletions storage/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -550,33 +550,46 @@ func TestObjects(t *testing.T) {
}

// Test object composition.
compDst := bkt.Object("composed")
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, err := obj.NewReader(ctx)
if err != nil {
t.Fatalf("NewReader: %v", err)
}
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...)
c.ContentType = "text/json"
if _, err := c.Run(ctx); err != nil {
t.Fatalf("ComposeFrom error: %v", err)
}
rc, err := compDst.NewReader(ctx)
if err != nil {
t.Fatalf("compDst.NewReader: %v", err)
}
slurp, err = ioutil.ReadAll(rc)
if err != nil {
t.Fatalf("compDst 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, want := rc.ContentType(), "text/json"; got != want {
t.Errorf("Composed object content-type = %q, want %q", got, want)
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")
}

func namesEqual(obj *ObjectAttrs, bucketName, objectName string) bool {
Expand Down
4 changes: 3 additions & 1 deletion storage/storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,7 @@ func TestObjectCompose(t *testing.T) {
},
wantURL: "/storage/v1/b/foo/o/bar/compose?alt=json",
wantReq: raw.ComposeRequest{
Destination: &raw.Object{Bucket: "foo"},
SourceObjects: []*raw.ComposeRequestSourceObjects{
{Name: "baz"},
{Name: "quux"},
Expand All @@ -488,7 +489,7 @@ func TestObjectCompose(t *testing.T) {
wantReq: raw.ComposeRequest{
Destination: &raw.Object{
Bucket: "foo",
Name: "bar",
Name: "not-bar",
ContentType: "application/json",
},
SourceObjects: []*raw.ComposeRequestSourceObjects{
Expand All @@ -509,6 +510,7 @@ func TestObjectCompose(t *testing.T) {
},
wantURL: "/storage/v1/b/foo/o/bar/compose?alt=json&ifGenerationMatch=12&ifMetagenerationMatch=34",
wantReq: raw.ComposeRequest{
Destination: &raw.Object{Bucket: "foo"},
SourceObjects: []*raw.ComposeRequestSourceObjects{
{
Name: "baz",
Expand Down

0 comments on commit f9c9ec4

Please sign in to comment.