s3manager: ensure parity between UploadInput and PutObjectInput

[fsouza]

Fix #88.

[jasdel]

Thanks for posting this PR @fsouza, looks like there are actually a couple fields that need to be updated. the s3manager's tests are supposed to catch this, but it looks like these test ignore the field's type, and only compare name.

would you mind also updating the upload_test.go tests to correctly check against the type. This will show that the StorageClass, ACL, and RequestPayer fields are also need to be updated.

 var emptyList = []string{}

 func val(i interface{}, s string) interface{} {
-       fmt.Println("INNER", i)
        v, err := awsutil.ValuesAtPath(i, s)
        if err != nil || len(v) == 0 {
                return nil
@@ -772,7 +771,12 @@ func TestUploadInputS3PutObjectInputPairity(t *testing.T) {
        bOnly := []string{}

        for k, c := range matchings {
-               if c == 1 && k != "ContentLength" {
+               if strings.HasPrefix(k, "Body-") || strings.HasPrefix(k, "ContentLength-") {
+                       // Ignored fields
+                       continue
+               }
+
+               if c == 1 {
                        aOnly = append(aOnly, k)
                } else if c == 2 {
                        bOnly = append(bOnly, k)
@@ -850,18 +854,24 @@ func compareStructType(a, b reflect.Type) map[string]int {

        for i := 0; i < len(aFields) || i < len(bFields); i++ {
                if i < len(aFields) {
-                       c := matchings[aFields[i].Name]
-                       matchings[aFields[i].Name] = c + 1
+                       name := matchName(aFields[i])
+                       c := matchings[name]
+                       matchings[name] = c + 1
                }
                if i < len(bFields) {
-                       c := matchings[bFields[i].Name]
-                       matchings[bFields[i].Name] = c + 2
+                       name := matchName(bFields[i])
+                       c := matchings[name]
+                       matchings[name] = c + 2
                }
        }

        return matchings
 }

+func matchName(field reflect.StructField) string {
+       return field.Name + "-" + field.Type.String()
+}
+
 func enumFields(v reflect.Type) []reflect.StructField {
        fields := []reflect.StructField{}

without your fix this updated tests should show the following errors:

--- FAIL: TestUploadInputS3PutObjectInputPairity (0.00s)
	upload_test.go:787: Expected empty array, but received [ACL-s3.ObjectCannedACL StorageClass-s3.StorageClass RequestPayer-s3.RequestPayer Metadata-map[string]string]
	upload_test.go:791: Expected empty array, but received [Metadata-map[string]*string StorageClass-*string ACL-*string RequestPayer-*string]

[fsouza]

Hey @jasdel, thanks for the feedback. I implemented those fixes and TestUploadInputS3PutObjectInputPairity is passing locally, but when I try to run all package tests I get a panic and the test suite aborts:

% go test
--- FAIL: TestUploadOrderMultiDifferentPartSize (0.04s)
panic: interface conversion: io.SectionReader is not io.Reader: missing method Read [recovered]
	panic: interface conversion: io.SectionReader is not io.Reader: missing method Read

goroutine 227 [running]:
testing.tRunner.func1(0xc42101c780)
	/Users/fsouza/.gimme/versions/go/src/testing/testing.go:742 +0x291
panic(0x141a600, 0xc42117bec0)
	/Users/fsouza/.gimme/versions/go/src/runtime/panic.go:502 +0x229
github.com/aws/aws-sdk-go-v2/service/s3/s3manager_test.buflen(0x1459c00, 0xc420183740, 0x14b789a)
	/Users/fsouza/src/github.com/aws/aws-sdk-go-v2/service/s3/s3manager/upload_test.go:96 +0x41
github.com/aws/aws-sdk-go-v2/service/s3/s3manager_test.TestUploadOrderMultiDifferentPartSize(0xc42101c780)
	/Users/fsouza/src/github.com/aws/aws-sdk-go-v2/service/s3/s3manager/upload_test.go:209 +0x437
testing.tRunner(0xc42101c780, 0x14d3018)
	/Users/fsouza/.gimme/versions/go/src/testing/testing.go:777 +0xc4
created by testing.(*T).Run
	/Users/fsouza/.gimme/versions/go/src/testing/testing.go:824 +0x2ca
exit status 2
FAIL	github.com/aws/aws-sdk-go-v2/service/s3/s3manager	1.782s

Do you see that in your environment too? It looks like the code is trying to use an io.SectionReader (instead of *io.SectionReader) as an io.Reader.

[jasdel]

@fsouza hmm, I'm not running into that issue. Can you verify that you're pr is based of tip of the SDK?

[fsouza]

@jasdel yeah, with latest master :(

I created this Dockerfile to show how to reproduce the issue: https://gist.github.com/fsouza/d0b6374b972dd3041b9d99de9db1fb9f

The test also fails without applying the patch.

[xibz]

@fsouza - Looks like one of our dependencies broke. I'll go ahead and take a look into this.

[jasdel]

@fsouza one workaround for this issue to update your Go workspace so that github.com/jmespath/go-jmespath is on 0b12d6b5.

With git directly you can do:

cd $(go env GOPATH)/src/github.com/jmespath/go-jmespath
git checkout 0b12d6b5

Alternatively, if your application uses dep it should automatically pull in the correct version as the SDK uses 0b12d6b5 in its Gopkg.toml file.

[fsouza]

@jasdel thanks for the info, I'm using dep in the application. I checked out that version of go-jmespath locally and the tests are now passing:

% go test
PASS
ok  	github.com/aws/aws-sdk-go-v2/service/s3/s3manager	2.320s

I think this PR is good for review now! :D

[jasdel]

Thanks for taking the time to put this PR together, and adding the test. The change looks good.