From d346250f1c2ea59d73119c52c41c76e5503a7e41 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Tue, 28 Jul 2015 19:33:56 -0700 Subject: [PATCH] Collapse GetPartialObject() into GetObject() --- pkg/donut/donut-v1_test.go | 8 +-- pkg/donut/donut-v2.go | 98 ++++++++++++------------------- pkg/donut/donut-v2_test.go | 8 +-- pkg/donut/interfaces.go | 3 +- pkg/donut/signature-v4.go | 3 +- pkg/server/api/headers.go | 29 +++++---- pkg/server/api/object-handlers.go | 24 +++----- pkg/server/api/range.go | 34 ++++++----- 8 files changed, 90 insertions(+), 117 deletions(-) diff --git a/pkg/donut/donut-v1_test.go b/pkg/donut/donut-v1_test.go index d288f927f..2227dec6e 100644 --- a/pkg/donut/donut-v1_test.go +++ b/pkg/donut/donut-v1_test.go @@ -200,7 +200,7 @@ func (s *MyDonutSuite) TestNewObjectCanBeWritten(c *C) { c.Assert(actualMetadata.MD5Sum, Equals, hex.EncodeToString(hasher.Sum(nil))) var buffer bytes.Buffer - size, err := dd.GetObject(&buffer, "foo", "obj") + size, err := dd.GetObject(&buffer, "foo", "obj", 0, 0) c.Assert(err, IsNil) c.Assert(size, Equals, int64(len(data))) c.Assert(buffer.Bytes(), DeepEquals, []byte(data)) @@ -225,13 +225,13 @@ func (s *MyDonutSuite) TestMultipleNewObjects(c *C) { c.Assert(err, IsNil) var buffer1 bytes.Buffer - size, err := dd.GetObject(&buffer1, "foo5", "obj1") + size, err := dd.GetObject(&buffer1, "foo5", "obj1", 0, 0) c.Assert(err, IsNil) c.Assert(size, Equals, int64(len([]byte("one")))) c.Assert(buffer1.Bytes(), DeepEquals, []byte("one")) var buffer2 bytes.Buffer - size, err = dd.GetObject(&buffer2, "foo5", "obj2") + size, err = dd.GetObject(&buffer2, "foo5", "obj2", 0, 0) c.Assert(err, IsNil) c.Assert(size, Equals, int64(len([]byte("two")))) @@ -274,7 +274,7 @@ func (s *MyDonutSuite) TestMultipleNewObjects(c *C) { c.Assert(err, IsNil) var buffer bytes.Buffer - size, err = dd.GetObject(&buffer, "foo5", "obj3") + size, err = dd.GetObject(&buffer, "foo5", "obj3", 0, 0) c.Assert(err, IsNil) c.Assert(size, Equals, int64(len([]byte("three")))) c.Assert(buffer.Bytes(), DeepEquals, []byte("three")) diff --git a/pkg/donut/donut-v2.go b/pkg/donut/donut-v2.go index 51f06cf87..c49684900 100644 --- a/pkg/donut/donut-v2.go +++ b/pkg/donut/donut-v2.go @@ -21,7 +21,6 @@ import ( "crypto/md5" "encoding/base64" "encoding/hex" - "errors" "io" "io/ioutil" "log" @@ -127,53 +126,7 @@ func New() (Interface, error) { /// V2 API functions // GetObject - GET object from cache buffer -func (donut API) GetObject(w io.Writer, bucket string, object string) (int64, error) { - donut.lock.Lock() - defer donut.lock.Unlock() - - if !IsValidBucket(bucket) { - return 0, iodine.New(BucketNameInvalid{Bucket: bucket}, nil) - } - if !IsValidObjectName(object) { - return 0, iodine.New(ObjectNameInvalid{Object: object}, nil) - } - if !donut.storedBuckets.Exists(bucket) { - return 0, iodine.New(BucketNotFound{Bucket: bucket}, nil) - } - objectKey := bucket + "/" + object - data, ok := donut.objects.Get(objectKey) - if !ok { - if len(donut.config.NodeDiskMap) > 0 { - reader, size, err := donut.getObject(bucket, object) - if err != nil { - return 0, iodine.New(err, nil) - } - // new proxy writer to capture data read from disk - pw := NewProxyWriter(w) - written, err := io.CopyN(pw, reader, size) - if err != nil { - return 0, iodine.New(err, nil) - } - /// cache object read from disk - ok := donut.objects.Append(objectKey, pw.writtenBytes) - pw.writtenBytes = nil - go debug.FreeOSMemory() - if !ok { - return 0, iodine.New(InternalError{}, nil) - } - return written, nil - } - return 0, iodine.New(ObjectNotFound{Object: object}, nil) - } - written, err := io.CopyN(w, bytes.NewBuffer(data), int64(donut.objects.Len(objectKey))) - if err != nil { - return 0, iodine.New(err, nil) - } - return written, nil -} - -// GetPartialObject - GET object from cache buffer range -func (donut API) GetPartialObject(w io.Writer, bucket, object string, start, length int64) (int64, error) { +func (donut API) GetObject(w io.Writer, bucket string, object string, start, length int64) (int64, error) { donut.lock.Lock() defer donut.lock.Unlock() @@ -196,35 +149,58 @@ func (donut API) GetPartialObject(w io.Writer, bucket, object string, start, len Length: length, }, errParams) } + if !donut.storedBuckets.Exists(bucket) { + return 0, iodine.New(BucketNotFound{Bucket: bucket}, errParams) + } objectKey := bucket + "/" + object data, ok := donut.objects.Get(objectKey) + var written int64 + var err error if !ok { if len(donut.config.NodeDiskMap) > 0 { - reader, _, err := donut.getObject(bucket, object) + reader, size, err := donut.getObject(bucket, object) if err != nil { return 0, iodine.New(err, nil) } - if _, err := io.CopyN(ioutil.Discard, reader, start); err != nil { - return 0, iodine.New(err, nil) + if start > 0 { + if _, err := io.CopyN(ioutil.Discard, reader, start); err != nil { + return 0, iodine.New(err, errParams) + } } + // new proxy writer to capture data read from disk pw := NewProxyWriter(w) - written, err := io.CopyN(w, reader, length) - if err != nil { - return 0, iodine.New(err, nil) + if length > 0 { + written, err = io.CopyN(pw, reader, length) + if err != nil { + return 0, iodine.New(err, errParams) + } + } else { + written, err = io.CopyN(pw, reader, size) + if err != nil { + return 0, iodine.New(err, errParams) + } } + /// cache object read from disk ok := donut.objects.Append(objectKey, pw.writtenBytes) pw.writtenBytes = nil go debug.FreeOSMemory() if !ok { - return 0, iodine.New(InternalError{}, nil) + return 0, iodine.New(InternalError{}, errParams) } return written, nil } - return 0, iodine.New(ObjectNotFound{Object: object}, nil) + return 0, iodine.New(ObjectNotFound{Object: object}, errParams) } - written, err := io.CopyN(w, bytes.NewBuffer(data[start:]), length) - if err != nil { - return 0, iodine.New(err, nil) + if start == 0 && length == 0 { + written, err = io.CopyN(w, bytes.NewBuffer(data), int64(donut.objects.Len(objectKey))) + if err != nil { + return 0, iodine.New(err, nil) + } + } else { + written, err = io.CopyN(w, bytes.NewBuffer(data[start:]), length) + if err != nil { + return 0, iodine.New(err, nil) + } } return written, nil } @@ -306,11 +282,11 @@ func isMD5SumEqual(expectedMD5Sum, actualMD5Sum string) error { return iodine.New(err, nil) } if !bytes.Equal(expectedMD5SumBytes, actualMD5SumBytes) { - return iodine.New(errors.New("bad digest, md5sum mismatch"), nil) + return iodine.New(BadDigest{}, nil) } return nil } - return iodine.New(errors.New("invalid argument"), nil) + return iodine.New(InvalidArgument{}, nil) } // CreateObject - create an object diff --git a/pkg/donut/donut-v2_test.go b/pkg/donut/donut-v2_test.go index 3599984e2..7f00f2df2 100644 --- a/pkg/donut/donut-v2_test.go +++ b/pkg/donut/donut-v2_test.go @@ -175,7 +175,7 @@ func (s *MyCacheSuite) TestNewObjectCanBeWritten(c *C) { c.Assert(actualMetadata.MD5Sum, Equals, hex.EncodeToString(hasher.Sum(nil))) var buffer bytes.Buffer - size, err := dc.GetObject(&buffer, "foo", "obj") + size, err := dc.GetObject(&buffer, "foo", "obj", 0, 0) c.Assert(err, IsNil) c.Assert(size, Equals, int64(len(data))) c.Assert(buffer.Bytes(), DeepEquals, []byte(data)) @@ -200,13 +200,13 @@ func (s *MyCacheSuite) TestMultipleNewObjects(c *C) { c.Assert(err, IsNil) var buffer1 bytes.Buffer - size, err := dc.GetObject(&buffer1, "foo5", "obj1") + size, err := dc.GetObject(&buffer1, "foo5", "obj1", 0, 0) c.Assert(err, IsNil) c.Assert(size, Equals, int64(len([]byte("one")))) c.Assert(buffer1.Bytes(), DeepEquals, []byte("one")) var buffer2 bytes.Buffer - size, err = dc.GetObject(&buffer2, "foo5", "obj2") + size, err = dc.GetObject(&buffer2, "foo5", "obj2", 0, 0) c.Assert(err, IsNil) c.Assert(size, Equals, int64(len([]byte("two")))) @@ -249,7 +249,7 @@ func (s *MyCacheSuite) TestMultipleNewObjects(c *C) { c.Assert(err, IsNil) var buffer bytes.Buffer - size, err = dc.GetObject(&buffer, "foo5", "obj3") + size, err = dc.GetObject(&buffer, "foo5", "obj3", 0, 0) c.Assert(err, IsNil) c.Assert(size, Equals, int64(len([]byte("three")))) c.Assert(buffer.Bytes(), DeepEquals, []byte("three")) diff --git a/pkg/donut/interfaces.go b/pkg/donut/interfaces.go index 5f0ae65e6..0f3536453 100644 --- a/pkg/donut/interfaces.go +++ b/pkg/donut/interfaces.go @@ -38,8 +38,7 @@ type CloudStorage interface { ListObjects(string, BucketResourcesMetadata, *Signature) ([]ObjectMetadata, BucketResourcesMetadata, error) // Object operations - GetObject(w io.Writer, bucket, object string) (int64, error) - GetPartialObject(w io.Writer, bucket, object string, start, length int64) (int64, error) + GetObject(w io.Writer, bucket, object string, start, length int64) (int64, error) GetObjectMetadata(bucket, object string, signature *Signature) (ObjectMetadata, error) // bucket, object, expectedMD5Sum, size, reader, metadata, signature CreateObject(string, string, string, int64, io.Reader, map[string]string, *Signature) (ObjectMetadata, error) diff --git a/pkg/donut/signature-v4.go b/pkg/donut/signature-v4.go index 34a855fc0..b344ff798 100644 --- a/pkg/donut/signature-v4.go +++ b/pkg/donut/signature-v4.go @@ -20,7 +20,6 @@ import ( "bytes" "crypto/hmac" "encoding/hex" - "errors" "net/http" "regexp" "sort" @@ -79,7 +78,7 @@ func urlEncodeName(name string) (string, error) { default: len := utf8.RuneLen(s) if len < 0 { - return "", errors.New("invalid utf-8") + return "", iodine.New(InvalidArgument{}, nil) } u := make([]byte, len) utf8.EncodeRune(u, s) diff --git a/pkg/server/api/headers.go b/pkg/server/api/headers.go index e8f4e6bd0..30c7b4e91 100644 --- a/pkg/server/api/headers.go +++ b/pkg/server/api/headers.go @@ -78,23 +78,30 @@ func encodeErrorResponse(response interface{}, acceptsType contentType) []byte { } // Write object header -func setObjectHeaders(w http.ResponseWriter, metadata donut.ObjectMetadata) { +func setObjectHeaders(w http.ResponseWriter, metadata donut.ObjectMetadata, contentRange *httpRange) { + // set common headers + if contentRange != nil { + if contentRange.length > 0 { + setCommonHeaders(w, metadata.Metadata["contentType"], int(contentRange.length)) + } else { + setCommonHeaders(w, metadata.Metadata["contentType"], int(metadata.Size)) + } + } else { + setCommonHeaders(w, metadata.Metadata["contentType"], int(metadata.Size)) + } + // set object headers lastModified := metadata.Created.Format(http.TimeFormat) - // common headers - setCommonHeaders(w, metadata.Metadata["contentType"], int(metadata.Size)) // object related headers w.Header().Set("ETag", "\""+metadata.MD5Sum+"\"") w.Header().Set("Last-Modified", lastModified) -} -// Write range object header -func setRangeObjectHeaders(w http.ResponseWriter, metadata donut.ObjectMetadata, contentRange *httpRange) { - // set common headers - setCommonHeaders(w, metadata.Metadata["contentType"], int(metadata.Size)) - // set object headers - setObjectHeaders(w, metadata) // set content range - w.Header().Set("Content-Range", contentRange.getContentRange()) + if contentRange != nil { + if contentRange.start > 0 || contentRange.length > 0 { + w.Header().Set("Content-Range", contentRange.String()) + w.WriteHeader(http.StatusPartialContent) + } + } } func encodeSuccessResponse(response interface{}, acceptsType contentType) []byte { diff --git a/pkg/server/api/object-handlers.go b/pkg/server/api/object-handlers.go index daab928de..1f35cca17 100644 --- a/pkg/server/api/object-handlers.go +++ b/pkg/server/api/object-handlers.go @@ -69,26 +69,16 @@ func (api Minio) GetObjectHandler(w http.ResponseWriter, req *http.Request) { switch iodine.ToError(err).(type) { case nil: // success { - httpRange, err := getRequestedRange(req, metadata.Size) + httpRange, err := getRequestedRange(req.Header.Get("Range"), metadata.Size) if err != nil { writeErrorResponse(w, req, InvalidRange, acceptsContentType, req.URL.Path) return } - switch httpRange.start == 0 && httpRange.length == 0 { - case true: - setObjectHeaders(w, metadata) - if _, err := api.Donut.GetObject(w, bucket, object); err != nil { - // unable to write headers, we've already printed data. Just close the connection. - log.Error.Println(iodine.New(err, nil)) - } - case false: - metadata.Size = httpRange.length - setRangeObjectHeaders(w, metadata, httpRange) - w.WriteHeader(http.StatusPartialContent) - if _, err := api.Donut.GetPartialObject(w, bucket, object, httpRange.start, httpRange.length); err != nil { - // unable to write headers, we've already printed data. Just close the connection. - log.Error.Println(iodine.New(err, nil)) - } + setObjectHeaders(w, metadata, httpRange) + if _, err := api.Donut.GetObject(w, bucket, object, httpRange.start, httpRange.length); err != nil { + // unable to write headers, we've already printed data. Just close the connection. + log.Error.Println(iodine.New(err, nil)) + return } } case donut.SignatureDoesNotMatch: @@ -144,7 +134,7 @@ func (api Minio) HeadObjectHandler(w http.ResponseWriter, req *http.Request) { metadata, err := api.Donut.GetObjectMetadata(bucket, object, signature) switch iodine.ToError(err).(type) { case nil: - setObjectHeaders(w, metadata) + setObjectHeaders(w, metadata, nil) w.WriteHeader(http.StatusOK) case donut.SignatureDoesNotMatch: writeErrorResponse(w, req, SignatureDoesNotMatch, acceptsContentType, req.URL.Path) diff --git a/pkg/server/api/range.go b/pkg/server/api/range.go index 40fddc517..051dd7524 100644 --- a/pkg/server/api/range.go +++ b/pkg/server/api/range.go @@ -19,9 +19,11 @@ package api import ( "errors" "fmt" - "net/http" "strconv" "strings" + + "github.com/minio/minio/pkg/donut" + "github.com/minio/minio/pkg/iodine" ) const ( @@ -33,23 +35,23 @@ type httpRange struct { start, length, size int64 } -// GetContentRange populate range header -func (r *httpRange) getContentRange() string { +// String populate range stringer interface +func (r *httpRange) String() string { return fmt.Sprintf("bytes %d-%d/%d", r.start, r.start+r.length-1, r.size) } // Grab new range from request header -func getRequestedRange(req *http.Request, size int64) (*httpRange, error) { +func getRequestedRange(hrange string, size int64) (*httpRange, error) { r := &httpRange{ start: 0, length: 0, size: 0, } r.size = size - if s := req.Header.Get("Range"); s != "" { - err := r.parseRange(s) + if hrange != "" { + err := r.parseRange(hrange) if err != nil { - return nil, err + return nil, iodine.New(err, nil) } } return r, nil @@ -58,7 +60,7 @@ func getRequestedRange(req *http.Request, size int64) (*httpRange, error) { func (r *httpRange) parse(ra string) error { i := strings.Index(ra, "-") if i < 0 { - return errors.New("invalid range") + return iodine.New(donut.InvalidRange{}, nil) } start, end := strings.TrimSpace(ra[:i]), strings.TrimSpace(ra[i+1:]) if start == "" { @@ -66,7 +68,7 @@ func (r *httpRange) parse(ra string) error { // range start relative to the end of the file. i, err := strconv.ParseInt(end, 10, 64) if err != nil { - return errors.New("invalid range") + return iodine.New(donut.InvalidRange{}, nil) } if i > r.size { i = r.size @@ -76,7 +78,7 @@ func (r *httpRange) parse(ra string) error { } else { i, err := strconv.ParseInt(start, 10, 64) if err != nil || i > r.size || i < 0 { - return errors.New("invalid range") + return iodine.New(donut.InvalidRange{}, nil) } r.start = i if end == "" { @@ -85,7 +87,7 @@ func (r *httpRange) parse(ra string) error { } else { i, err := strconv.ParseInt(end, 10, 64) if err != nil || r.start > i { - return errors.New("invalid range") + return iodine.New(donut.InvalidRange{}, nil) } if i >= r.size { i = r.size - 1 @@ -99,24 +101,24 @@ func (r *httpRange) parse(ra string) error { // parseRange parses a Range header string as per RFC 2616. func (r *httpRange) parseRange(s string) error { if s == "" { - return errors.New("header not present") + return iodine.New(errors.New("header not present"), nil) } if !strings.HasPrefix(s, b) { - return errors.New("invalid range") + return iodine.New(donut.InvalidRange{}, nil) } ras := strings.Split(s[len(b):], ",") if len(ras) == 0 { - return errors.New("invalid request") + return iodine.New(errors.New("invalid request"), nil) } // Just pick the first one and ignore the rest, we only support one range per object if len(ras) > 1 { - return errors.New("multiple ranges specified") + return iodine.New(errors.New("multiple ranges specified"), nil) } ra := strings.TrimSpace(ras[0]) if ra == "" { - return errors.New("invalid range") + return iodine.New(donut.InvalidRange{}, nil) } return r.parse(ra) }