From f7caef2d262e4bb718153e5388dd58fcc342dc8b Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Sun, 3 May 2015 23:16:10 -0700 Subject: [PATCH] Change CreateObject() to take size argument from content-length --- pkg/api/api_object_handlers.go | 9 ++++--- pkg/api/api_test.go | 34 ++++++++++++------------- pkg/storage/donut/donut_bucket.go | 8 +++++- pkg/storage/donut/donut_test.go | 17 ++++++++----- pkg/storage/drivers/api_testsuite.go | 37 +++++++++++++++------------- pkg/storage/drivers/driver.go | 2 +- pkg/storage/drivers/errors.go | 7 +++--- 7 files changed, 65 insertions(+), 49 deletions(-) diff --git a/pkg/api/api_object_handlers.go b/pkg/api/api_object_handlers.go index 790c7de98..cf7bc1db3 100644 --- a/pkg/api/api_object_handlers.go +++ b/pkg/api/api_object_handlers.go @@ -18,6 +18,7 @@ package api import ( "net/http" + "strconv" "github.com/gorilla/mux" "github.com/minio-io/minio/pkg/iodine" @@ -155,10 +156,10 @@ func (server *minioAPI) putObjectHandler(w http.ResponseWriter, req *http.Reques writeErrorResponse(w, req, InvalidDigest, acceptsContentType, req.URL.Path) return } - /// if Content-Length missing, incomplete request throw IncompleteBody + /// if Content-Length missing, throw away size := req.Header.Get("Content-Length") if size == "" { - writeErrorResponse(w, req, IncompleteBody, acceptsContentType, req.URL.Path) + writeErrorResponse(w, req, MissingContentLength, acceptsContentType, req.URL.Path) return } /// maximum Upload size for objects in a single operation @@ -171,7 +172,9 @@ func (server *minioAPI) putObjectHandler(w http.ResponseWriter, req *http.Reques writeErrorResponse(w, req, EntityTooSmall, acceptsContentType, req.URL.Path) return } - calculatedMD5, err := server.driver.CreateObject(bucket, object, "", md5, req.Body) + // ignoring error here, TODO find a way to reply back if we can + sizeInt, _ := strconv.ParseInt(size, 10, 64) + calculatedMD5, err := server.driver.CreateObject(bucket, object, "", md5, sizeInt, req.Body) switch err := iodine.ToError(err).(type) { case nil: { diff --git a/pkg/api/api_test.go b/pkg/api/api_test.go index cafc19a4d..c5cb5ef01 100644 --- a/pkg/api/api_test.go +++ b/pkg/api/api_test.go @@ -168,7 +168,7 @@ func (s *MySuite) TestEmptyObject(c *C) { Size: 0, } typedDriver.On("CreateBucket", "bucket", "private").Return(nil).Once() - typedDriver.On("CreateObject", "bucket", "object", "", "", mock.Anything).Return(metadata.Md5, nil).Once() + typedDriver.On("CreateObject", "bucket", "object", "", "", 0, mock.Anything).Return(metadata.Md5, nil).Once() typedDriver.On("GetBucketMetadata", "bucket").Return(drivers.BucketMetadata{}, nil).Twice() typedDriver.On("GetObjectMetadata", "bucket", "object", "").Return(metadata, nil).Once() typedDriver.On("GetObject", mock.Anything, "bucket", "object").Return(int64(0), nil).Once() @@ -179,7 +179,7 @@ func (s *MySuite) TestEmptyObject(c *C) { buffer := bytes.NewBufferString("") driver.CreateBucket("bucket", "private") - driver.CreateObject("bucket", "object", "", "", buffer) + driver.CreateObject("bucket", "object", "", "", 0, buffer) request, err := http.NewRequest("GET", testServer.URL+"/bucket/object", nil) c.Assert(err, IsNil) @@ -250,7 +250,7 @@ func (s *MySuite) TestObject(c *C) { Size: 11, } typedDriver.On("CreateBucket", "bucket", "private").Return(nil).Once() - typedDriver.On("CreateObject", "bucket", "object", "", "", mock.Anything).Return(metadata.Md5, nil).Once() + typedDriver.On("CreateObject", "bucket", "object", "", "", mock.Anything, mock.Anything).Return(metadata.Md5, nil).Once() typedDriver.On("GetBucketMetadata", "bucket").Return(drivers.BucketMetadata{}, nil).Twice() typedDriver.On("GetObjectMetadata", "bucket", "object", "").Return(metadata, nil).Twice() typedDriver.SetGetObjectWriter("bucket", "object", []byte("hello world")) @@ -262,7 +262,7 @@ func (s *MySuite) TestObject(c *C) { buffer := bytes.NewBufferString("hello world") driver.CreateBucket("bucket", "private") - driver.CreateObject("bucket", "object", "", "", buffer) + driver.CreateObject("bucket", "object", "", "", int64(buffer.Len()), buffer) request, err := http.NewRequest("GET", testServer.URL+"/bucket/object", nil) c.Assert(err, IsNil) @@ -325,12 +325,12 @@ func (s *MySuite) TestMultipleObjects(c *C) { typedDriver.On("CreateBucket", "bucket", "private").Return(nil).Once() driver.CreateBucket("bucket", "private") - typedDriver.On("CreateObject", "bucket", "object1", "", "", mock.Anything).Return(metadata1.Md5, nil).Once() - driver.CreateObject("bucket", "object1", "", "", buffer1) - typedDriver.On("CreateObject", "bucket", "object2", "", "", mock.Anything).Return(metadata2.Md5, nil).Once() - driver.CreateObject("bucket", "object2", "", "", buffer2) - typedDriver.On("CreateObject", "bucket", "object3", "", "", mock.Anything).Return(metadata3.Md5, nil).Once() - driver.CreateObject("bucket", "object3", "", "", buffer3) + typedDriver.On("CreateObject", "bucket", "object1", "", "", mock.Anything, mock.Anything).Return(metadata1.Md5, nil).Once() + driver.CreateObject("bucket", "object1", "", "", int64(buffer1.Len()), buffer1) + typedDriver.On("CreateObject", "bucket", "object2", "", "", mock.Anything, mock.Anything).Return(metadata2.Md5, nil).Once() + driver.CreateObject("bucket", "object2", "", "", int64(buffer2.Len()), buffer2) + typedDriver.On("CreateObject", "bucket", "object3", "", "", mock.Anything, mock.Anything).Return(metadata3.Md5, nil).Once() + driver.CreateObject("bucket", "object3", "", "", int64(buffer3.Len()), buffer3) // test non-existant object typedDriver.On("GetBucketMetadata", "bucket").Return(drivers.BucketMetadata{}, nil).Once() @@ -506,8 +506,8 @@ func (s *MySuite) TestHeader(c *C) { buffer := bytes.NewBufferString("hello world") typedDriver.On("GetBucketMetadata", "foo").Return(bucketMetadata, nil).Once() - typedDriver.On("CreateObject", "bucket", "object", "", "", mock.Anything).Return(objectMetadata.Md5, nil).Once() - driver.CreateObject("bucket", "object", "", "", buffer) + typedDriver.On("CreateObject", "bucket", "object", "", "", mock.Anything, mock.Anything).Return(objectMetadata.Md5, nil).Once() + driver.CreateObject("bucket", "object", "", "", int64(buffer.Len()), buffer) typedDriver.On("GetBucketMetadata", "bucket").Return(bucketMetadata, nil).Once() typedDriver.On("GetObjectMetadata", "bucket", "object", "").Return(objectMetadata, nil).Once() @@ -618,7 +618,7 @@ func (s *MySuite) TestPutObject(c *C) { Size: 11, } - typedDriver.On("CreateObject", "bucket", "two", "", "", mock.Anything).Return(twoMetadata.Md5, nil).Once() + typedDriver.On("CreateObject", "bucket", "two", "", "", mock.Anything, mock.Anything).Return(twoMetadata.Md5, nil).Once() request, err = http.NewRequest("PUT", testServer.URL+"/bucket/two", bytes.NewBufferString("hello world")) c.Assert(err, IsNil) setAuthHeader(request) @@ -905,7 +905,7 @@ func (s *MySuite) TestContentTypePersists(c *C) { } typedDriver.On("GetBucketMetadata", "bucket").Return(metadata, nil).Once() - typedDriver.On("CreateObject", "bucket", "one", "", "", mock.Anything).Return(oneMetadata.Md5, nil).Once() + typedDriver.On("CreateObject", "bucket", "one", "", "", mock.Anything, mock.Anything).Return(oneMetadata.Md5, nil).Once() request, err := http.NewRequest("PUT", testServer.URL+"/bucket/one", bytes.NewBufferString("hello world")) delete(request.Header, "Content-Type") c.Assert(err, IsNil) @@ -952,7 +952,7 @@ func (s *MySuite) TestContentTypePersists(c *C) { } typedDriver.On("GetBucketMetadata", "bucket").Return(metadata, nil).Once() - typedDriver.On("CreateObject", "bucket", "two", "", "", mock.Anything).Return(twoMetadata.Md5, nil).Once() + typedDriver.On("CreateObject", "bucket", "two", "", "", mock.Anything, mock.Anything).Return(twoMetadata.Md5, nil).Once() request, err = http.NewRequest("PUT", testServer.URL+"/bucket/two", bytes.NewBufferString("hello world")) delete(request.Header, "Content-Type") request.Header.Add("Content-Type", "application/json") @@ -1010,11 +1010,11 @@ func (s *MySuite) TestPartialContent(c *C) { } typedDriver.On("CreateBucket", "foo", "private").Return(nil).Once() - typedDriver.On("CreateObject", "foo", "bar", "", "", mock.Anything).Return(metadata.Md5, nil).Once() + typedDriver.On("CreateObject", "foo", "bar", "", "", mock.Anything, mock.Anything).Return(metadata.Md5, nil).Once() err := driver.CreateBucket("foo", "private") c.Assert(err, IsNil) - driver.CreateObject("foo", "bar", "", "", bytes.NewBufferString("hello world")) + driver.CreateObject("foo", "bar", "", "", int64(len("hello world")), bytes.NewBufferString("hello world")) // prepare for GET on range request typedDriver.SetGetObjectWriter("foo", "bar", []byte("hello world")) diff --git a/pkg/storage/donut/donut_bucket.go b/pkg/storage/donut/donut_bucket.go index 6732490cf..475ffa13c 100644 --- a/pkg/storage/donut/donut_bucket.go +++ b/pkg/storage/donut/donut_bucket.go @@ -151,11 +151,17 @@ func (b bucket) PutObject(objectName string, objectData io.Reader, expectedMD5Su donutObjectMetadata := make(map[string]string) objectMetadata["version"] = "1.0" donutObjectMetadata["version"] = "1.0" + size := metadata["contentLength"] + sizeInt, err := strconv.ParseInt(size, 10, 64) + if err != nil { + return "", iodine.New(err, nil) + } + // if total writers are only '1' do not compute erasure switch len(writers) == 1 { case true: mw := io.MultiWriter(writers[0], summer) - totalLength, err := io.Copy(mw, objectData) + totalLength, err := io.CopyN(mw, objectData, sizeInt) if err != nil { return "", iodine.New(err, nil) } diff --git a/pkg/storage/donut/donut_test.go b/pkg/storage/donut/donut_test.go index 720942fe0..58eaad620 100644 --- a/pkg/storage/donut/donut_test.go +++ b/pkg/storage/donut/donut_test.go @@ -184,6 +184,7 @@ func (s *MySuite) TestNewObjectMetadata(c *C) { hasher.Write([]byte(data)) expectedMd5Sum := hex.EncodeToString(hasher.Sum(nil)) reader := ioutil.NopCloser(bytes.NewReader([]byte(data))) + metadata["contentLength"] = strconv.Itoa(len(data)) err = donut.MakeBucket("foo", "private") c.Assert(err, IsNil) @@ -210,9 +211,6 @@ func (s *MySuite) TestNewObjectFailsWithEmptyName(c *C) { _, err = donut.PutObject("foo", "", "", nil, nil) c.Assert(err, Not(IsNil)) - - _, err = donut.PutObject("foo", " ", "", nil, nil) - c.Assert(err, Not(IsNil)) } // test create object @@ -234,6 +232,7 @@ func (s *MySuite) TestNewObjectCanBeWritten(c *C) { hasher.Write([]byte(data)) expectedMd5Sum := hex.EncodeToString(hasher.Sum(nil)) reader := ioutil.NopCloser(bytes.NewReader([]byte(data))) + metadata["contentLength"] = strconv.Itoa(len(data)) calculatedMd5Sum, err := donut.PutObject("foo", "obj", expectedMd5Sum, reader, metadata) c.Assert(err, IsNil) @@ -268,11 +267,16 @@ func (s *MySuite) TestMultipleNewObjects(c *C) { c.Assert(donut.MakeBucket("foo", "private"), IsNil) one := ioutil.NopCloser(bytes.NewReader([]byte("one"))) - _, err = donut.PutObject("foo", "obj1", "", one, nil) + metadata := make(map[string]string) + metadata["contentLength"] = strconv.Itoa(len("one")) + + _, err = donut.PutObject("foo", "obj1", "", one, metadata) c.Assert(err, IsNil) two := ioutil.NopCloser(bytes.NewReader([]byte("two"))) - _, err = donut.PutObject("foo", "obj2", "", two, nil) + + metadata["contentLength"] = strconv.Itoa(len("two")) + _, err = donut.PutObject("foo", "obj2", "", two, metadata) c.Assert(err, IsNil) obj1, size, err := donut.GetObject("foo", "obj1") @@ -315,7 +319,8 @@ func (s *MySuite) TestMultipleNewObjects(c *C) { c.Assert(listObjects, DeepEquals, []string{"obj1", "obj2"}) three := ioutil.NopCloser(bytes.NewReader([]byte("three"))) - _, err = donut.PutObject("foo", "obj3", "", three, nil) + metadata["contentLength"] = strconv.Itoa(len("three")) + _, err = donut.PutObject("foo", "obj3", "", three, metadata) c.Assert(err, IsNil) obj3, size, err := donut.GetObject("foo", "obj3") diff --git a/pkg/storage/drivers/api_testsuite.go b/pkg/storage/drivers/api_testsuite.go index 21d76466d..ab4cf742a 100644 --- a/pkg/storage/drivers/api_testsuite.go +++ b/pkg/storage/drivers/api_testsuite.go @@ -73,7 +73,8 @@ func testMultipleObjectCreation(c *check.C, create func() Driver) { key := "obj" + strconv.Itoa(i) objects[key] = []byte(randomString) - calculatedmd5sum, err := drivers.CreateObject("bucket", key, "", expectedmd5Sum, bytes.NewBufferString(randomString)) + calculatedmd5sum, err := drivers.CreateObject("bucket", key, "", expectedmd5Sum, int64(len(randomString)), + bytes.NewBufferString(randomString)) c.Assert(err, check.IsNil) c.Assert(calculatedmd5sum, check.Equals, expectedmd5Sumhex) } @@ -107,7 +108,7 @@ func testPaging(c *check.C, create func() Driver) { // check before paging occurs for i := 0; i < 5; i++ { key := "obj" + strconv.Itoa(i) - drivers.CreateObject("bucket", key, "", "", bytes.NewBufferString(key)) + drivers.CreateObject("bucket", key, "", "", int64(len(key)), bytes.NewBufferString(key)) resources.Maxkeys = 5 resources.Prefix = "" objects, resources, err = drivers.ListObjects("bucket", resources) @@ -118,7 +119,7 @@ func testPaging(c *check.C, create func() Driver) { // check after paging occurs pages work for i := 6; i <= 10; i++ { key := "obj" + strconv.Itoa(i) - drivers.CreateObject("bucket", key, "", "", bytes.NewBufferString(key)) + drivers.CreateObject("bucket", key, "", "", int64(len(key)), bytes.NewBufferString(key)) resources.Maxkeys = 5 resources.Prefix = "" objects, resources, err = drivers.ListObjects("bucket", resources) @@ -128,8 +129,8 @@ func testPaging(c *check.C, create func() Driver) { } // check paging with prefix at end returns less objects { - drivers.CreateObject("bucket", "newPrefix", "", "", bytes.NewBufferString("prefix1")) - drivers.CreateObject("bucket", "newPrefix2", "", "", bytes.NewBufferString("prefix2")) + drivers.CreateObject("bucket", "newPrefix", "", "", int64(len("prefix1")), bytes.NewBufferString("prefix1")) + drivers.CreateObject("bucket", "newPrefix2", "", "", int64(len("prefix2")), bytes.NewBufferString("prefix2")) resources.Prefix = "new" resources.Maxkeys = 5 objects, resources, err = drivers.ListObjects("bucket", resources) @@ -150,8 +151,8 @@ func testPaging(c *check.C, create func() Driver) { // check delimited results with delimiter and prefix { - drivers.CreateObject("bucket", "this/is/delimited", "", "", bytes.NewBufferString("prefix1")) - drivers.CreateObject("bucket", "this/is/also/a/delimited/file", "", "", bytes.NewBufferString("prefix2")) + drivers.CreateObject("bucket", "this/is/delimited", "", "", int64(len("prefix1")), bytes.NewBufferString("prefix1")) + drivers.CreateObject("bucket", "this/is/also/a/delimited/file", "", "", int64(len("prefix2")), bytes.NewBufferString("prefix2")) var prefixes []string resources.CommonPrefixes = prefixes // allocate new everytime resources.Delimiter = "/" @@ -210,14 +211,14 @@ func testObjectOverwriteFails(c *check.C, create func() Driver) { hasher1.Write([]byte("one")) md5Sum1 := base64.StdEncoding.EncodeToString(hasher1.Sum(nil)) md5Sum1hex := hex.EncodeToString(hasher1.Sum(nil)) - md5Sum11, err := drivers.CreateObject("bucket", "object", "", md5Sum1, bytes.NewBufferString("one")) + md5Sum11, err := drivers.CreateObject("bucket", "object", "", md5Sum1, int64(len("one")), bytes.NewBufferString("one")) c.Assert(err, check.IsNil) c.Assert(md5Sum1hex, check.Equals, md5Sum11) hasher2 := md5.New() hasher2.Write([]byte("three")) md5Sum2 := base64.StdEncoding.EncodeToString(hasher2.Sum(nil)) - _, err = drivers.CreateObject("bucket", "object", "", md5Sum2, bytes.NewBufferString("three")) + _, err = drivers.CreateObject("bucket", "object", "", md5Sum2, int64(len("three")), bytes.NewBufferString("three")) c.Assert(err, check.Not(check.IsNil)) var bytesBuffer bytes.Buffer @@ -229,7 +230,7 @@ func testObjectOverwriteFails(c *check.C, create func() Driver) { func testNonExistantBucketOperations(c *check.C, create func() Driver) { drivers := create() - _, err := drivers.CreateObject("bucket", "object", "", "", bytes.NewBufferString("one")) + _, err := drivers.CreateObject("bucket", "object", "", "", int64(len("one")), bytes.NewBufferString("one")) c.Assert(err, check.Not(check.IsNil)) } @@ -260,7 +261,8 @@ func testPutObjectInSubdir(c *check.C, create func() Driver) { hasher.Write([]byte("hello world")) md5Sum1 := base64.StdEncoding.EncodeToString(hasher.Sum(nil)) md5Sum1hex := hex.EncodeToString(hasher.Sum(nil)) - md5Sum11, err := drivers.CreateObject("bucket", "dir1/dir2/object", "", md5Sum1, bytes.NewBufferString("hello world")) + md5Sum11, err := drivers.CreateObject("bucket", "dir1/dir2/object", "", md5Sum1, int64(len("hello world")), + bytes.NewBufferString("hello world")) c.Assert(err, check.IsNil) c.Assert(md5Sum11, check.Equals, md5Sum1hex) @@ -356,7 +358,8 @@ func testGetDirectoryReturnsObjectNotFound(c *check.C, create func() Driver) { err := drivers.CreateBucket("bucket", "") c.Assert(err, check.IsNil) - _, err = drivers.CreateObject("bucket", "dir1/dir2/object", "", "", bytes.NewBufferString("hello world")) + _, err = drivers.CreateObject("bucket", "dir1/dir2/object", "", "", int64(len("hello world")), + bytes.NewBufferString("hello world")) c.Assert(err, check.IsNil) var byteBuffer bytes.Buffer @@ -400,19 +403,19 @@ func testDefaultContentType(c *check.C, create func() Driver) { c.Assert(err, check.IsNil) // test empty - _, err = drivers.CreateObject("bucket", "one", "", "", bytes.NewBufferString("one")) + _, err = drivers.CreateObject("bucket", "one", "", "", int64(len("one")), bytes.NewBufferString("one")) metadata, err := drivers.GetObjectMetadata("bucket", "one", "") c.Assert(err, check.IsNil) c.Assert(metadata.ContentType, check.Equals, "application/octet-stream") // test custom - drivers.CreateObject("bucket", "two", "application/text", "", bytes.NewBufferString("two")) + drivers.CreateObject("bucket", "two", "application/text", "", int64(len("two")), bytes.NewBufferString("two")) metadata, err = drivers.GetObjectMetadata("bucket", "two", "") c.Assert(err, check.IsNil) c.Assert(metadata.ContentType, check.Equals, "application/text") // test trim space - drivers.CreateObject("bucket", "three", "\tapplication/json ", "", bytes.NewBufferString("three")) + drivers.CreateObject("bucket", "three", "\tapplication/json ", "", int64(len("three")), bytes.NewBufferString("three")) metadata, err = drivers.GetObjectMetadata("bucket", "three", "") c.Assert(err, check.IsNil) c.Assert(metadata.ContentType, check.Equals, "application/json") @@ -425,12 +428,12 @@ func testContentMd5Set(c *check.C, create func() Driver) { // test md5 invalid badmd5Sum := "NWJiZjVhNTIzMjhlNzQzOWFlNmU3MTlkZmU3MTIyMDA" - calculatedmd5sum, err := drivers.CreateObject("bucket", "one", "", badmd5Sum, bytes.NewBufferString("one")) + calculatedmd5sum, err := drivers.CreateObject("bucket", "one", "", badmd5Sum, int64(len("one")), bytes.NewBufferString("one")) c.Assert(err, check.Not(check.IsNil)) c.Assert(calculatedmd5sum, check.Not(check.Equals), badmd5Sum) goodmd5sum := "NWJiZjVhNTIzMjhlNzQzOWFlNmU3MTlkZmU3MTIyMDA=" - calculatedmd5sum, err = drivers.CreateObject("bucket", "two", "", goodmd5sum, bytes.NewBufferString("one")) + calculatedmd5sum, err = drivers.CreateObject("bucket", "two", "", goodmd5sum, int64(len("one")), bytes.NewBufferString("one")) c.Assert(err, check.IsNil) c.Assert(calculatedmd5sum, check.Equals, goodmd5sum) } diff --git a/pkg/storage/drivers/driver.go b/pkg/storage/drivers/driver.go index 8f50acba7..f4402674f 100644 --- a/pkg/storage/drivers/driver.go +++ b/pkg/storage/drivers/driver.go @@ -37,7 +37,7 @@ type Driver interface { GetPartialObject(w io.Writer, bucket, object string, start, length int64) (int64, error) GetObjectMetadata(bucket string, object string, prefix string) (ObjectMetadata, error) ListObjects(bucket string, resources BucketResourcesMetadata) ([]ObjectMetadata, BucketResourcesMetadata, error) - CreateObject(bucket string, key string, contentType string, md5sum string, data io.Reader) (string, error) + CreateObject(bucket string, key string, contentType string, md5sum string, size int64, data io.Reader) (string, error) } // BucketACL - bucket level access control diff --git a/pkg/storage/drivers/errors.go b/pkg/storage/drivers/errors.go index 4a25ad610..b3dfa9925 100644 --- a/pkg/storage/drivers/errors.go +++ b/pkg/storage/drivers/errors.go @@ -92,9 +92,8 @@ type ObjectExists GenericObjectError // EntityTooLarge - object size exceeds maximum limit type EntityTooLarge struct { GenericObjectError - Size string - TotalSize string - MaxSize string + Size string + MaxSize string } // ObjectNameInvalid - object name provided is invalid @@ -170,7 +169,7 @@ func (e ObjectNameInvalid) Error() string { // Return string an error formatted as the given text func (e EntityTooLarge) Error() string { - return e.Bucket + "#" + e.Object + "with " + e.Size + "reached maximum allowed size limit " + e.TotalSize + return e.Bucket + "#" + e.Object + "with " + e.Size + "reached maximum allowed size limit " + e.MaxSize } // Return string an error formatted as the given text