Merge pull request #585 from fkautz/pr_out_adding_tests_and_fixes_for_multipart_uploads_uncovered_from_tests

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)
 	}
 }

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",
+		mux.HandleFunc("/{bucket}/{object:.*}", api.putObjectPartHandler).Queries("partNumber", "{partNumber:[0-9]+}", "uploadId", "{uploadId:.*}").Methods("PUT")
-			"{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")

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)

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
 		}
 	}