From f3c25bcfc4af9a990799e39651049ca6c4e8853e Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Sat, 27 Jun 2015 13:12:33 -0700 Subject: [PATCH] Keeping the lexical order same add optimizations, provide a comprehensive response from ListObjects() --- pkg/storage/donut/bucket.go | 11 +++++++--- pkg/storage/donut/definitions.go | 7 +++++++ pkg/storage/donut/donut.go | 12 +++++------ pkg/storage/donut/donut_test.go | 33 +++++++++++++++--------------- pkg/storage/donut/interfaces.go | 2 +- pkg/storage/drivers/donut/donut.go | 26 +++++++++-------------- 6 files changed, 49 insertions(+), 42 deletions(-) diff --git a/pkg/storage/donut/bucket.go b/pkg/storage/donut/bucket.go index 8116c1c89..8e0e1e5ca 100644 --- a/pkg/storage/donut/bucket.go +++ b/pkg/storage/donut/bucket.go @@ -145,7 +145,7 @@ func (b bucket) getBucketMetadata() (*AllBuckets, error) { } // ListObjects - list all objects -func (b bucket) ListObjects(prefix, marker, delimiter string, maxkeys int) ([]string, []string, bool, error) { +func (b bucket) ListObjects(prefix, marker, delimiter string, maxkeys int) (ListObjects, error) { b.lock.RLock() defer b.lock.RUnlock() if maxkeys <= 0 { @@ -155,7 +155,7 @@ func (b bucket) ListObjects(prefix, marker, delimiter string, maxkeys int) ([]st var objects []string bucketMetadata, err := b.getBucketMetadata() if err != nil { - return nil, nil, false, iodine.New(err, nil) + return ListObjects{}, iodine.New(err, nil) } for objectName := range bucketMetadata.Buckets[b.getBucketName()].BucketObjects { if strings.HasPrefix(objectName, strings.TrimSpace(prefix)) { @@ -193,7 +193,12 @@ func (b bucket) ListObjects(prefix, marker, delimiter string, maxkeys int) ([]st } sort.Strings(results) sort.Strings(commonPrefixes) - return results, commonPrefixes, isTruncated, nil + + listObjects := ListObjects{} + listObjects.Objects = results + listObjects.CommonPrefixes = commonPrefixes + listObjects.IsTruncated = isTruncated + return listObjects, nil } // ReadObject - open an object to read diff --git a/pkg/storage/donut/definitions.go b/pkg/storage/donut/definitions.go index 0888c7077..d1212c943 100644 --- a/pkg/storage/donut/definitions.go +++ b/pkg/storage/donut/definitions.go @@ -64,3 +64,10 @@ type BucketMetadata struct { Metadata map[string]string `json:"metadata"` BucketObjects map[string]interface{} `json:"objects"` } + +// ListObjects container for list objects response +type ListObjects struct { + Objects []string `json:"objects"` + CommonPrefixes []string `json:"commonPrefixes"` + IsTruncated bool `json:"isTruncated"` +} diff --git a/pkg/storage/donut/donut.go b/pkg/storage/donut/donut.go index f2ed0d7e9..7925fb874 100644 --- a/pkg/storage/donut/donut.go +++ b/pkg/storage/donut/donut.go @@ -151,7 +151,7 @@ func (dt donut) ListBuckets() (map[string]BucketMetadata, error) { } // ListObjects - return list of objects -func (dt donut) ListObjects(bucket, prefix, marker, delimiter string, maxkeys int) ([]string, []string, bool, error) { +func (dt donut) ListObjects(bucket, prefix, marker, delimiter string, maxkeys int) (ListObjects, error) { dt.lock.RLock() defer dt.lock.RUnlock() errParams := map[string]string{ @@ -162,16 +162,16 @@ func (dt donut) ListObjects(bucket, prefix, marker, delimiter string, maxkeys in "maxkeys": strconv.Itoa(maxkeys), } if err := dt.listDonutBuckets(); err != nil { - return nil, nil, false, iodine.New(err, errParams) + return ListObjects{}, iodine.New(err, errParams) } if _, ok := dt.buckets[bucket]; !ok { - return nil, nil, false, iodine.New(BucketNotFound{Bucket: bucket}, errParams) + return ListObjects{}, iodine.New(BucketNotFound{Bucket: bucket}, errParams) } - objects, commonPrefixes, isTruncated, err := dt.buckets[bucket].ListObjects(prefix, marker, delimiter, maxkeys) + listObjects, err := dt.buckets[bucket].ListObjects(prefix, marker, delimiter, maxkeys) if err != nil { - return nil, nil, false, iodine.New(err, errParams) + return ListObjects{}, iodine.New(err, errParams) } - return objects, commonPrefixes, isTruncated, nil + return listObjects, nil } // PutObject - put object diff --git a/pkg/storage/donut/donut_test.go b/pkg/storage/donut/donut_test.go index 476ef6b76..2993f8de6 100644 --- a/pkg/storage/donut/donut_test.go +++ b/pkg/storage/donut/donut_test.go @@ -91,10 +91,11 @@ func (s *MySuite) TestEmptyBucket(c *C) { c.Assert(donut.MakeBucket("foo", "private"), IsNil) // check if bucket is empty - objects, _, istruncated, err := donut.ListObjects("foo", "", "", "", 1) + listObjects, err := donut.ListObjects("foo", "", "", "", 1) c.Assert(err, IsNil) - c.Assert(objects, IsNil) - c.Assert(istruncated, Equals, false) + c.Assert(listObjects.Objects, IsNil) + c.Assert(listObjects.CommonPrefixes, IsNil) + c.Assert(listObjects.IsTruncated, Equals, false) } // test bucket list @@ -303,23 +304,23 @@ func (s *MySuite) TestMultipleNewObjects(c *C) { /// test list of objects // test list objects with prefix and delimiter - listObjects, prefixes, isTruncated, err := donut.ListObjects("foo", "o", "", "1", 10) + listObjects, err := donut.ListObjects("foo", "o", "", "1", 10) c.Assert(err, IsNil) - c.Assert(isTruncated, Equals, false) - c.Assert(prefixes[0], Equals, "obj1") + c.Assert(listObjects.IsTruncated, Equals, false) + c.Assert(listObjects.CommonPrefixes[0], Equals, "obj1") // test list objects with only delimiter - listObjects, prefixes, isTruncated, err = donut.ListObjects("foo", "", "", "1", 10) + listObjects, err = donut.ListObjects("foo", "", "", "1", 10) c.Assert(err, IsNil) - c.Assert(listObjects[0], Equals, "obj2") - c.Assert(isTruncated, Equals, false) - c.Assert(prefixes[0], Equals, "obj1") + c.Assert(listObjects.Objects[0], Equals, "obj2") + c.Assert(listObjects.IsTruncated, Equals, false) + c.Assert(listObjects.CommonPrefixes[0], Equals, "obj1") // test list objects with only prefix - listObjects, _, isTruncated, err = donut.ListObjects("foo", "o", "", "", 10) + listObjects, err = donut.ListObjects("foo", "o", "", "", 10) c.Assert(err, IsNil) - c.Assert(isTruncated, Equals, false) - c.Assert(listObjects, DeepEquals, []string{"obj1", "obj2"}) + c.Assert(listObjects.IsTruncated, Equals, false) + c.Assert(listObjects.Objects, DeepEquals, []string{"obj1", "obj2"}) three := ioutil.NopCloser(bytes.NewReader([]byte("three"))) metadata["contentLength"] = strconv.Itoa(len("three")) @@ -336,8 +337,8 @@ func (s *MySuite) TestMultipleNewObjects(c *C) { c.Assert(readerBuffer3.Bytes(), DeepEquals, []byte("three")) // test list objects with maxkeys - listObjects, _, isTruncated, err = donut.ListObjects("foo", "o", "", "", 2) + listObjects, err = donut.ListObjects("foo", "o", "", "", 2) c.Assert(err, IsNil) - c.Assert(isTruncated, Equals, true) - c.Assert(len(listObjects), Equals, 2) + c.Assert(listObjects.IsTruncated, Equals, true) + c.Assert(len(listObjects.Objects), Equals, 2) } diff --git a/pkg/storage/donut/interfaces.go b/pkg/storage/donut/interfaces.go index 5cd8648c8..c5e78e96f 100644 --- a/pkg/storage/donut/interfaces.go +++ b/pkg/storage/donut/interfaces.go @@ -35,7 +35,7 @@ type ObjectStorage interface { MakeBucket(bucket, acl string) error // Bucket operations - ListObjects(bucket, prefix, marker, delim string, maxKeys int) (objects []string, prefixes []string, isTruncated bool, err error) + ListObjects(bucket, prefix, marker, delim string, maxKeys int) (ListObjects, error) // Object operations GetObject(bucket, object string) (io.ReadCloser, int64, error) diff --git a/pkg/storage/drivers/donut/donut.go b/pkg/storage/drivers/donut/donut.go index 270d936f7..069d095fb 100644 --- a/pkg/storage/drivers/donut/donut.go +++ b/pkg/storage/drivers/donut/donut.go @@ -327,12 +327,6 @@ func (d donutDriver) GetObjectMetadata(bucketName, objectName string) (drivers.O return objectMetadata, nil } -type byObjectKey []drivers.ObjectMetadata - -func (b byObjectKey) Len() int { return len(b) } -func (b byObjectKey) Swap(i, j int) { b[i], b[j] = b[j], b[i] } -func (b byObjectKey) Less(i, j int) bool { return b[i].Key < b[j].Key } - // ListObjects - returns list of objects func (d donutDriver) ListObjects(bucketName string, resources drivers.BucketResourcesMetadata) ([]drivers.ObjectMetadata, drivers.BucketResourcesMetadata, error) { d.lock.RLock() @@ -349,30 +343,30 @@ func (d donutDriver) ListObjects(bucketName string, resources drivers.BucketReso if !drivers.IsValidObjectName(resources.Prefix) { return nil, drivers.BucketResourcesMetadata{}, iodine.New(drivers.ObjectNameInvalid{Object: resources.Prefix}, nil) } - actualObjects, commonPrefixes, isTruncated, err := d.donut.ListObjects(bucketName, resources.Prefix, resources.Marker, resources.Delimiter, - resources.Maxkeys) + listObjects, err := d.donut.ListObjects(bucketName, resources.Prefix, resources.Marker, resources.Delimiter, resources.Maxkeys) if err != nil { return nil, drivers.BucketResourcesMetadata{}, iodine.New(err, errParams) } - resources.CommonPrefixes = commonPrefixes - resources.IsTruncated = isTruncated + resources.CommonPrefixes = listObjects.CommonPrefixes + resources.IsTruncated = listObjects.IsTruncated if resources.IsTruncated && resources.IsDelimiterSet() { - resources.NextMarker = actualObjects[len(actualObjects)-1] + resources.NextMarker = listObjects.Objects[len(listObjects.Objects)-1] } - var results []drivers.ObjectMetadata - for _, objectName := range actualObjects { + // make sure to keep the lexical order same as returned by donut + // we do not have to sort here again + results := make([]drivers.ObjectMetadata, len(listObjects.Objects)) + for i, objectName := range listObjects.Objects { objectMetadata, err := d.donut.GetObjectMetadata(bucketName, objectName) if err != nil { return nil, drivers.BucketResourcesMetadata{}, iodine.New(err, errParams) } metadata := drivers.ObjectMetadata{ - Key: objectName, + Key: objectMetadata.Object, Created: objectMetadata.Created, Size: objectMetadata.Size, } - results = append(results, metadata) + results[i] = metadata } - sort.Sort(byObjectKey(results)) return results, resources, nil }