Merge pull request #1214 from brendanashworth/improve-listbuckets

ListBuckets test & improvement, IsValid{Bucket,Object}Name fix, test, docs

pkg/fs/fs-bucket.go

@@ -69,7 +69,7 @@ func (fs Filesystem) ListBuckets() ([]BucketMetadata, *probe.Error) {
 		}
 		// If directories are found with odd names, skip them.
 		dirName := strings.ToLower(file.Name())
-		if file.IsDir() && !IsValidBucketName(dirName) {
+		if !IsValidBucketName(dirName) {
 			continue
 		}
 		metadata := BucketMetadata{

pkg/fs/fs-bucket_test.go

@@ -19,10 +19,51 @@ package fs
 import (
 	"io/ioutil"
 	"os"
+	"strconv"
 	"strings"
 	"testing"
 )
 
+func TestListBuckets(t *testing.T) {
+	// Make a temporary directory to use as the filesystem.
+	directory, fserr := ioutil.TempDir("", "minio-benchmark")
+	if fserr != nil {
+		t.Fatal(fserr)
+	}
+	defer os.RemoveAll(directory)
+
+	// Create the filesystem.
+	filesystem, err := New(directory, 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Create a few buckets.
+	for i := 0; i < 10; i++ {
+		err = filesystem.MakeBucket("testbucket."+strconv.Itoa(i), "public-read")
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	// List, and ensure that they are all there.
+	metadatas, err := filesystem.ListBuckets()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if len(metadatas) != 10 {
+		t.Errorf("incorrect length of metadatas (%i)\n", len(metadatas))
+	}
+
+	// Iterate over the buckets, ensuring that the name is correct.
+	for i := 0; i < len(metadatas); i++ {
+		if !strings.Contains(metadatas[i].Name, "testbucket") {
+			t.Fail()
+		}
+	}
+}
+
 func TestDeleteBucket(t *testing.T) {
 	// Make a temporary directory to use as the filesystem.
 	directory, fserr := ioutil.TempDir("", "minio-benchmark")
@@ -44,6 +85,39 @@ func TestDeleteBucket(t *testing.T) {
 	}
 }
 
+func BenchmarkListBuckets(b *testing.B) {
+	// Make a temporary directory to use as the filesystem.
+	directory, fserr := ioutil.TempDir("", "minio-benchmark")
+	if fserr != nil {
+		b.Fatal(fserr)
+	}
+	defer os.RemoveAll(directory)
+
+	// Create the filesystem.
+	filesystem, err := New(directory, 0)
+	if err != nil {
+		b.Fatal(err)
+	}
+
+	// Create a few buckets.
+	for i := 0; i < 20; i++ {
+		err = filesystem.MakeBucket("bucket."+strconv.Itoa(i), "public-read")
+		if err != nil {
+			b.Fatal(err)
+		}
+	}
+
+	b.ResetTimer()
+
+	// List the buckets over and over and over.
+	for i := 0; i < b.N; i++ {
+		_, err = filesystem.ListBuckets()
+		if err != nil {
+			b.Fatal(err)
+		}
+	}
+}
+
 func BenchmarkDeleteBucket(b *testing.B) {
 	// Make a temporary directory to use as the filesystem.
 	directory, fserr := ioutil.TempDir("", "minio-benchmark")

pkg/fs/fs-utils.go

@@ -24,12 +24,11 @@ import (
 // validBucket regexp.
 var validBucket = regexp.MustCompile(`^[a-z0-9][a-z0-9\.\-]{1,61}[a-z0-9]$`)
 
-// IsValidBucketName - verify bucket name in accordance with
-//  - http://docs.aws.amazon.com/AmazonS3/latest/dev/UsingBucket.html
+// IsValidBucketName verifies a bucket name in accordance with Amazon's
+// requirements. It must be 3-63 characters long, can contain dashes
+// and periods, but must begin and end with a lowercase letter or a number.
+// See: http://docs.aws.amazon.com/AmazonS3/latest/dev/BucketRestrictions.html
 func IsValidBucketName(bucket string) bool {
-	if bucket == "" {
-		return false
-	}
 	if len(bucket) < 3 || len(bucket) > 63 {
 		return false
 	}
@@ -39,12 +38,11 @@ func IsValidBucketName(bucket string) bool {
 	return validBucket.MatchString(bucket)
 }
 
-// IsValidObjectName - verify object name in accordance with
-//   - http://docs.aws.amazon.com/AmazonS3/latest/dev/UsingMetadata.html
+// IsValidObjectName verifies an object name in accordance with Amazon's
+// requirements. It cannot exceed 1024 characters and must be a valid UTF8
+// string.
+// See: http://docs.aws.amazon.com/AmazonS3/latest/dev/UsingMetadata.html
 func IsValidObjectName(object string) bool {
-	if object == "" {
-		return true
-	}
 	if len(object) > 1024 || len(object) == 0 {
 		return false
 	}

pkg/fs/fs-utils_test.go

@@ -0,0 +1,72 @@
+/*
+ * Minio Cloud Storage, (C) 2016 Minio, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package fs
+
+import (
+	"testing"
+)
+
+func ensureBucketName(name string, t *testing.T, pass bool) {
+	if pass && IsValidBucketName(name) {
+		return
+	}
+	if !pass && !IsValidBucketName(name) {
+		return
+	}
+
+	t.Errorf("\"%s\" should have passed [%t]\n", name, pass)
+}
+
+func TestIsValidBucketName(t *testing.T) {
+	ensureBucketName("lol", t, true)
+	ensureBucketName("s3-eu-west-1.amazonaws.com", t, true)
+	ensureBucketName("ideas-are-more-powerful-than-guns", t, true)
+	ensureBucketName("testbucket", t, true)
+	ensureBucketName("1bucket", t, true)
+	ensureBucketName("bucket1", t, true)
+
+	ensureBucketName("testing.", t, false)
+	ensureBucketName("", t, false)
+	ensureBucketName("......", t, false)
+	ensureBucketName("THIS-IS-UPPERCASE", t, false)
+	ensureBucketName("una ñina", t, false)
+	ensureBucketName("lalalallalallalalalallalallalallalallalallalallalallalallallalala", t, false)
+}
+
+func ensureObjectName(name string, t *testing.T, pass bool) {
+	if pass && IsValidObjectName(name) {
+		return
+	}
+	if !pass && !IsValidObjectName(name) {
+		return
+	}
+
+	t.Errorf("\"%s\" should have passed [%t]\n", name, pass)
+}
+
+func TestIsValidObjectName(t *testing.T) {
+	ensureObjectName("object", t, true)
+	ensureObjectName("The Shining Script <v1>.pdf", t, true)
+	ensureObjectName("Cost Benefit Analysis (2009-2010).pptx", t, true)
+	ensureObjectName("117Gn8rfHL2ACARPAhaFd0AGzic9pUbIA/5OCn5A", t, true)
+	ensureObjectName("SHØRT", t, true)
+	ensureObjectName("There are far too many object names, and far too few bucket names!", t, true)
+
+	ensureObjectName("", t, false)
+	// Bad UTF8 strings should not pass.
+	ensureObjectName(string([]byte{0xff, 0xfe, 0xfd}), t, false)
+}