Merge pull request #681 from harshavardhana/pr_out_keeping_the_lexical_order_same_add_optimizations_provide_a_comprehensive_response_from_listobjects_

Keeping the lexical order same add optimizations, provide a comprehensive response from ListObjects()

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

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"`
+}

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

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(listObjects.Objects, IsNil)
-	c.Assert(istruncated, Equals, false)
+	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(listObjects.IsTruncated, Equals, false)
-	c.Assert(prefixes[0], Equals, "obj1")
+	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(listObjects.Objects[0], Equals, "obj2")
-	c.Assert(isTruncated, Equals, false)
+	c.Assert(listObjects.IsTruncated, Equals, false)
-	c.Assert(prefixes[0], Equals, "obj1")
+	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.IsTruncated, Equals, false)
-	c.Assert(listObjects, DeepEquals, []string{"obj1", "obj2"})
+	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(listObjects.IsTruncated, Equals, true)
-	c.Assert(len(listObjects), Equals, 2)
+	c.Assert(len(listObjects.Objects), Equals, 2)
 }

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)

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,
+	listObjects, err := d.donut.ListObjects(bucketName, resources.Prefix, resources.Marker, resources.Delimiter, resources.Maxkeys)
-		resources.Maxkeys)
 	if err != nil {
 		return nil, drivers.BucketResourcesMetadata{}, iodine.New(err, errParams)
 	}
-	resources.CommonPrefixes = commonPrefixes
+	resources.CommonPrefixes = listObjects.CommonPrefixes
-	resources.IsTruncated = isTruncated
+	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
+	// make sure to keep the lexical order same as returned by donut
-	for _, objectName := range actualObjects {
+	// 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
 }