From 058604aa21f0b376fc1685c80823f1bd2cf4050b Mon Sep 17 00:00:00 2001 From: "Frederick F. Kautz IV" Date: Sat, 9 May 2015 17:42:14 -0700 Subject: [PATCH] Adding tests and fixes for multipart uploads uncovered from tests --- pkg/api/api_object_handlers.go | 9 ++- pkg/api/api_router.go | 3 +- pkg/api/api_test.go | 101 ++++++++++++++++++++++++++++ pkg/storage/drivers/mocks/Driver.go | 5 +- 4 files changed, 113 insertions(+), 5 deletions(-) diff --git a/pkg/api/api_object_handlers.go b/pkg/api/api_object_handlers.go index afcad2dcc..494bb6767 100644 --- a/pkg/api/api_object_handlers.go +++ b/pkg/api/api_object_handlers.go @@ -27,6 +27,7 @@ import ( "github.com/minio-io/minio/pkg/iodine" "github.com/minio-io/minio/pkg/storage/drivers" "github.com/minio-io/minio/pkg/utils/log" + "strings" ) // GET Object @@ -299,6 +300,10 @@ func (server *minioAPI) putObjectPartHandler(w http.ResponseWriter, req *http.Re bucket = vars["bucket"] object = vars["object"] uploadID := vars["uploadId"] + // workaround for mux not splitting on & properly + if len(uploadID) > 1 { + uploadID = strings.Split(uploadID, "&")[0] + } partIDString := vars["partNumber"] partID, err := strconv.Atoi(partIDString) if err != nil { @@ -411,7 +416,7 @@ func (server *minioAPI) completeMultipartUploadHandler(w http.ResponseWriter, re parts := &CompleteMultipartUpload{} err := decoder.Decode(parts) if err != nil { - log.Error.Println(err) + log.Error.Println(iodine.New(err, nil)) writeErrorResponse(w, req, InternalError, acceptsContentType, req.URL.Path) return } @@ -444,7 +449,7 @@ func (server *minioAPI) completeMultipartUploadHandler(w http.ResponseWriter, re case drivers.InvalidUploadID: writeErrorResponse(w, req, NoSuchUpload, acceptsContentType, req.URL.Path) default: - log.Println(err) + log.Println(iodine.New(err, nil)) writeErrorResponse(w, req, InternalError, acceptsContentType, req.URL.Path) } } diff --git a/pkg/api/api_router.go b/pkg/api/api_router.go index bc37d182e..dceabee3d 100644 --- a/pkg/api/api_router.go +++ b/pkg/api/api_router.go @@ -48,8 +48,7 @@ func HTTPHandler(driver drivers.Driver) http.Handler { mux.HandleFunc("/{bucket}/{object:.*}", api.headObjectHandler).Methods("HEAD") if featureflags.Get(featureflags.MultipartPutObject) { log.Println("Enabling feature", featureflags.MultipartPutObject) - mux.HandleFunc("/{bucket}/{object:.*}", api.putObjectPartHandler).Queries("partNumber", - "{partNumber:[0-9]+}", "uploadId", "{uploadId:.*}").Methods("PUT") + mux.HandleFunc("/{bucket}/{object:.*}", api.putObjectPartHandler).Queries("partNumber", "{partNumber:[0-9]+}", "uploadId", "{uploadId:.*}").Methods("PUT") mux.HandleFunc("/{bucket}/{object:.*}", api.listObjectPartsHandler).Queries("uploadId", "{uploadId:.*}").Methods("GET") mux.HandleFunc("/{bucket}/{object:.*}", api.completeMultipartUploadHandler).Queries("uploadId", "{uploadId:.*}").Methods("POST") mux.HandleFunc("/{bucket}/{object:.*}", api.newMultipartUploadHandler).Methods("POST") diff --git a/pkg/api/api_test.go b/pkg/api/api_test.go index 466057dfe..1b2ac3f61 100644 --- a/pkg/api/api_test.go +++ b/pkg/api/api_test.go @@ -36,6 +36,7 @@ import ( "net/http" "net/http/httptest" + "github.com/minio-io/minio/pkg/featureflags" "github.com/minio-io/minio/pkg/storage/drivers" "github.com/minio-io/minio/pkg/storage/drivers/donut" "github.com/minio-io/minio/pkg/storage/drivers/memory" @@ -1318,6 +1319,106 @@ func (s *MySuite) TestGetObjectRangeErrors(c *C) { verifyError(c, response, "InvalidRange", "The requested range cannot be satisfied.", http.StatusRequestedRangeNotSatisfiable) } +func (s *MySuite) TestPutMultipart(c *C) { + switch driver := s.Driver.(type) { + case *mocks.Driver: + { + driver.AssertExpectations(c) + } + default: + { + return + } + } + driver := s.Driver + typedDriver := s.MockDriver + featureflags.Enable(featureflags.MultipartPutObject) + + httpHandler := HTTPHandler(driver) + testServer := httptest.NewServer(httpHandler) + defer testServer.Close() + client := http.Client{} + + // create bucket + typedDriver.On("CreateBucket", "foo", "private").Return(nil).Once() + request, err := http.NewRequest("PUT", testServer.URL+"/foo", bytes.NewBufferString("")) + c.Assert(err, IsNil) + response, err := client.Do(request) + c.Assert(err, IsNil) + c.Assert(response.StatusCode, Equals, 200) + + // Initiate multipart upload + typedDriver.On("GetBucketMetadata", "foo").Return(drivers.BucketMetadata{}, nil).Once() + typedDriver.On("NewMultipartUpload", "foo", "object", "").Return("uploadid", nil).Once() + request, err = http.NewRequest("POST", testServer.URL+"/foo/object?uploads", bytes.NewBufferString("")) + c.Assert(err, IsNil) + response, err = client.Do(request) + c.Assert(response.StatusCode, Equals, http.StatusOK) + + decoder := xml.NewDecoder(response.Body) + newResponse := &InitiateMultipartUploadResult{} + + err = decoder.Decode(newResponse) + c.Assert(err, IsNil) + c.Assert(newResponse.UploadID, Equals, "uploadid") + + // put part one + typedDriver.On("GetBucketMetadata", "foo").Return(drivers.BucketMetadata{}, nil).Once() + typedDriver.On("CreateObjectPart", "foo", "object", "uploadid", 1, "", "", 11, mock.Anything).Return("5eb63bbbe01eeed093cb22bb8f5acdc3", nil).Once() + request, err = http.NewRequest("PUT", testServer.URL+"/foo/object?uploadId=uploadid&partNumber=1", bytes.NewBufferString("hello world")) + c.Assert(err, IsNil) + response, err = client.Do(request) + c.Assert(err, IsNil) + c.Assert(response.StatusCode, Equals, http.StatusOK) + + // // put part two + typedDriver.On("GetBucketMetadata", "foo").Return(drivers.BucketMetadata{}, nil).Once() + typedDriver.On("CreateObjectPart", "foo", "object", "uploadid", 2, "", "", 11, mock.Anything).Return("5eb63bbbe01eeed093cb22bb8f5acdc3", nil).Once() + request, err = http.NewRequest("PUT", testServer.URL+"/foo/object?uploadId=uploadid&partNumber=2", bytes.NewBufferString("hello world")) + c.Assert(err, IsNil) + response, err = client.Do(request) + c.Assert(err, IsNil) + c.Assert(response.StatusCode, Equals, http.StatusOK) + // + // complete multipart upload + + completeUploads := &CompleteMultipartUpload{ + Part: []Part{ + { + PartNumber: 1, + }, + { + PartNumber: 2, + }, + }, + } + + var completeBuffer bytes.Buffer + encoder := xml.NewEncoder(&completeBuffer) + encoder.Encode(completeUploads) + + typedDriver.On("CompleteMultipartUpload", "foo", "object", "uploadid", mock.Anything).Return("etag", nil).Once() + request, err = http.NewRequest("POST", testServer.URL+"/foo/object?uploadId=uploadid", &completeBuffer) + c.Assert(err, IsNil) + response, err = client.Do(request) + c.Assert(err, IsNil) + c.Assert(response.StatusCode, Equals, http.StatusOK) + + // get data + typedDriver.On("GetBucketMetadata", "foo").Return(drivers.BucketMetadata{}, nil).Once() + typedDriver.On("GetObjectMetadata", "foo", "object", "").Return(drivers.ObjectMetadata{Size: 22}, nil).Once() + typedDriver.On("GetObject", mock.Anything, "foo", "object").Return(int64(22), nil).Once() + typedDriver.SetGetObjectWriter("foo", "object", []byte("hello worldhello world")) + request, err = http.NewRequest("GET", testServer.URL+"/foo/object", nil) + c.Assert(err, IsNil) + response, err = client.Do(request) + c.Assert(err, IsNil) + c.Assert(response.StatusCode, Equals, http.StatusOK) + object, err := ioutil.ReadAll(response.Body) + c.Assert(err, IsNil) + c.Assert(string(object), Equals, ("hello worldhello world")) +} + func verifyError(c *C, response *http.Response, code, description string, statusCode int) { data, err := ioutil.ReadAll(response.Body) c.Assert(err, IsNil) diff --git a/pkg/storage/drivers/mocks/Driver.go b/pkg/storage/drivers/mocks/Driver.go index 7b278fa8a..fd515c760 100644 --- a/pkg/storage/drivers/mocks/Driver.go +++ b/pkg/storage/drivers/mocks/Driver.go @@ -65,7 +65,10 @@ func (m *Driver) GetObject(w io.Writer, bucket, object string) (int64, error) { r1 := ret.Error(1) if r1 == nil { if obj, ok := m.ObjectWriterData[bucket+":"+object]; ok { - n, _ := io.Copy(w, bytes.NewBuffer(obj)) + n, err := io.Copy(w, bytes.NewBuffer(obj)) + if err != nil { + panic(err) + } r0 = n } }