From d63d17012dc27d73ff2641561a145a00766e39f0 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Thu, 21 Apr 2016 23:40:01 -0700 Subject: [PATCH] tests: Add API suite tests back for object api. (#1352) --- fs.go | 12 +-- object-api-getobjectinfo_test.go | 176 +++++++++++++++++++++++++++++++ object-api.go | 5 + object-api_test.go | 164 ++++------------------------ object_api_suite_test.go | 8 +- 5 files changed, 206 insertions(+), 159 deletions(-) create mode 100644 object-api-getobjectinfo_test.go diff --git a/fs.go b/fs.go index 91eb3e351..06b39fa15 100644 --- a/fs.go +++ b/fs.go @@ -43,7 +43,6 @@ type listParams struct { // fsStorage - implements StorageAPI interface. type fsStorage struct { diskPath string - diskInfo disk.Info minFreeDisk int64 listObjectMap map[listParams][]*treeWalker listObjectMapMutex *sync.Mutex @@ -79,20 +78,15 @@ func newFS(diskPath string) (StorageAPI, error) { if diskPath == "" { return nil, errInvalidArgument } - st, e := os.Stat(diskPath) - if e != nil { - return nil, e + st, err := os.Stat(diskPath) + if err != nil { + return nil, err } if !st.IsDir() { return nil, syscall.ENOTDIR } - diskInfo, e := disk.GetInfo(diskPath) - if e != nil { - return nil, e - } fs := fsStorage{ diskPath: diskPath, - diskInfo: diskInfo, minFreeDisk: 5, // Minimum 5% disk should be free. listObjectMap: make(map[listParams][]*treeWalker), listObjectMapMutex: &sync.Mutex{}, diff --git a/object-api-getobjectinfo_test.go b/object-api-getobjectinfo_test.go new file mode 100644 index 000000000..82a473e89 --- /dev/null +++ b/object-api-getobjectinfo_test.go @@ -0,0 +1,176 @@ +/* + * 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 main + +import ( + "bytes" + "crypto/md5" + "encoding/hex" + "io" + "io/ioutil" + "os" + "strconv" + "testing" + + "github.com/minio/minio/pkg/probe" +) + +// Testing GetObjectInfo(). +func TestGetObjectInfo(t *testing.T) { + directory, e := ioutil.TempDir("", "minio-get-objinfo-test") + if e != nil { + t.Fatal(e) + } + defer os.RemoveAll(directory) + + // Create the obj. + fs, e := newFS(directory) + if e != nil { + t.Fatal(e) + } + + obj := newObjectLayer(fs) + var err *probe.Error + + // This bucket is used for testing getObjectInfo operations. + err = obj.MakeBucket("test-getobjectinfo") + if err != nil { + t.Fatal(err) + } + _, err = obj.PutObject("test-getobjectinfo", "Asia/asiapics.jpg", int64(len("asiapics")), bytes.NewBufferString("asiapics"), nil) + if err != nil { + t.Fatal(err) + } + resultCases := []ObjectInfo{ + // ObjectInfo -1. + // ObjectName set to a existing object in the test case (Test case 14). + {Bucket: "test-getobjectinfo", Name: "Asia/asiapics.jpg", ContentType: "image/jpeg", IsDir: false}, + } + testCases := []struct { + bucketName string + objectName string + + // Expected output of GetObjectInfo. + result ObjectInfo + err error + // Flag indicating whether the test is expected to pass or not. + shouldPass bool + }{ + // Test cases with invalid bucket names ( Test number 1-4 ). + {".test", "", ObjectInfo{}, BucketNameInvalid{Bucket: ".test"}, false}, + {"Test", "", ObjectInfo{}, BucketNameInvalid{Bucket: "Test"}, false}, + {"---", "", ObjectInfo{}, BucketNameInvalid{Bucket: "---"}, false}, + {"ad", "", ObjectInfo{}, BucketNameInvalid{Bucket: "ad"}, false}, + // Test cases with valid but non-existing bucket names (Test number 5-7). + {"abcdefgh", "abc", ObjectInfo{}, BucketNotFound{Bucket: "abcdefgh"}, false}, + {"ijklmnop", "efg", ObjectInfo{}, BucketNotFound{Bucket: "ijklmnop"}, false}, + // Test cases with valid but non-existing bucket names and invalid object name (Test number 8-9). + {"abcdefgh", "", ObjectInfo{}, ObjectNameInvalid{Bucket: "abcdefgh", Object: ""}, false}, + {"ijklmnop", "", ObjectInfo{}, ObjectNameInvalid{Bucket: "ijklmnop", Object: ""}, false}, + // Test cases with non-existing object name with existing bucket (Test number 10-12). + {"test-getobjectinfo", "Africa", ObjectInfo{}, ObjectNotFound{Bucket: "test-getobjectinfo", Object: "Africa"}, false}, + {"test-getobjectinfo", "Antartica", ObjectInfo{}, ObjectNotFound{Bucket: "test-getobjectinfo", Object: "Antartica"}, false}, + {"test-getobjectinfo", "Asia/myfile", ObjectInfo{}, ObjectNotFound{Bucket: "test-getobjectinfo", Object: "Asia/myfile"}, false}, + // Test case with existing bucket but object name set to a directory (Test number 13). + {"test-getobjectinfo", "Asia", ObjectInfo{}, ObjectNotFound{Bucket: "test-getobjectinfo", Object: "Asia"}, false}, + // Valid case with existing object (Test number 14). + {"test-getobjectinfo", "Asia/asiapics.jpg", resultCases[0], nil, true}, + } + for i, testCase := range testCases { + result, err := obj.GetObjectInfo(testCase.bucketName, testCase.objectName) + if err != nil && testCase.shouldPass { + t.Errorf("Test %d: Expected to pass, but failed with: %s", i+1, err.Cause.Error()) + } + if err == nil && !testCase.shouldPass { + t.Errorf("Test %d: Expected to fail with \"%s\", but passed instead", i+1, testCase.err.Error()) + } + // Failed as expected, but does it fail for the expected reason. + if err != nil && !testCase.shouldPass { + if testCase.err.Error() != err.Cause.Error() { + t.Errorf("Test %d: Expected to fail with error \"%s\", but instead failed with error \"%s\" instead", i+1, testCase.err.Error(), err.Cause.Error()) + } + } + + // Test passes as expected, but the output values are verified for correctness here. + if err == nil && testCase.shouldPass { + if testCase.result.Bucket != result.Bucket { + t.Fatalf("Test %d: Expected Bucket name to be '%s', but found '%s' instead", i+1, testCase.result.Bucket, result.Bucket) + } + if testCase.result.Name != result.Name { + t.Errorf("Test %d: Expected Object name to be %s, but instead found it to be %s", i+1, testCase.result.Name, result.Name) + } + if testCase.result.ContentType != result.ContentType { + t.Errorf("Test %d: Expected Content Type of the object to be %v, but instead found it to be %v", i+1, testCase.result.ContentType, result.ContentType) + } + if testCase.result.IsDir != result.IsDir { + t.Errorf("Test %d: Expected IsDir flag of the object to be %v, but instead found it to be %v", i+1, testCase.result.IsDir, result.IsDir) + } + } + } +} + +func BenchmarkGetObject(b *testing.B) { + // Make a temporary directory to use as the obj. + directory, e := ioutil.TempDir("", "minio-benchmark-getobject") + if e != nil { + b.Fatal(e) + } + defer os.RemoveAll(directory) + + // Create the obj. + fs, e := newFS(directory) + if e != nil { + b.Fatal(e) + } + + obj := newObjectLayer(fs) + var err *probe.Error + + // Make a bucket and put in a few objects. + err = obj.MakeBucket("bucket") + if err != nil { + b.Fatal(err) + } + + text := "Jack and Jill went up the hill / To fetch a pail of water." + hasher := md5.New() + hasher.Write([]byte(text)) + metadata := make(map[string]string) + for i := 0; i < 10; i++ { + metadata["md5Sum"] = hex.EncodeToString(hasher.Sum(nil)) + _, err = obj.PutObject("bucket", "object"+strconv.Itoa(i), int64(len(text)), bytes.NewBufferString(text), metadata) + if err != nil { + b.Fatal(err) + } + } + + b.ResetTimer() + for i := 0; i < b.N; i++ { + var buffer = new(bytes.Buffer) + r, err := obj.GetObject("bucket", "object"+strconv.Itoa(i%10), 0) + if err != nil { + b.Error(err) + } + if _, e := io.Copy(buffer, r); e != nil { + b.Error(e) + } + if buffer.Len() != len(text) { + b.Errorf("GetObject returned incorrect length %d (should be %d)\n", buffer.Len(), len(text)) + } + r.Close() + } +} diff --git a/object-api.go b/object-api.go index 9504bd378..0b1082abc 100644 --- a/object-api.go +++ b/object-api.go @@ -135,6 +135,11 @@ func (o objectAPI) GetObject(bucket, object string, startOffset int64) (io.ReadC return nil, probe.NewError(BucketNotFound{Bucket: bucket}) } else if e == errFileNotFound { return nil, probe.NewError(ObjectNotFound{Bucket: bucket, Object: object}) + } else if e == errIsNotRegular { + return nil, probe.NewError(ObjectExistsAsPrefix{ + Bucket: bucket, + Object: object, + }) } return nil, probe.NewError(e) } diff --git a/object-api_test.go b/object-api_test.go index 82a473e89..4292e8f90 100644 --- a/object-api_test.go +++ b/object-api_test.go @@ -1,5 +1,5 @@ /* - * Minio Cloud Storage, (C) 2016 Minio, Inc. + * Minio Cloud Storage, (C) 2015, 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. @@ -17,160 +17,32 @@ package main import ( - "bytes" - "crypto/md5" - "encoding/hex" - "io" "io/ioutil" "os" - "strconv" - "testing" - "github.com/minio/minio/pkg/probe" + . "gopkg.in/check.v1" ) -// Testing GetObjectInfo(). -func TestGetObjectInfo(t *testing.T) { - directory, e := ioutil.TempDir("", "minio-get-objinfo-test") - if e != nil { - t.Fatal(e) - } - defer os.RemoveAll(directory) - - // Create the obj. - fs, e := newFS(directory) - if e != nil { - t.Fatal(e) - } +type MySuite struct{} - obj := newObjectLayer(fs) - var err *probe.Error - - // This bucket is used for testing getObjectInfo operations. - err = obj.MakeBucket("test-getobjectinfo") - if err != nil { - t.Fatal(err) - } - _, err = obj.PutObject("test-getobjectinfo", "Asia/asiapics.jpg", int64(len("asiapics")), bytes.NewBufferString("asiapics"), nil) - if err != nil { - t.Fatal(err) - } - resultCases := []ObjectInfo{ - // ObjectInfo -1. - // ObjectName set to a existing object in the test case (Test case 14). - {Bucket: "test-getobjectinfo", Name: "Asia/asiapics.jpg", ContentType: "image/jpeg", IsDir: false}, - } - testCases := []struct { - bucketName string - objectName string - - // Expected output of GetObjectInfo. - result ObjectInfo - err error - // Flag indicating whether the test is expected to pass or not. - shouldPass bool - }{ - // Test cases with invalid bucket names ( Test number 1-4 ). - {".test", "", ObjectInfo{}, BucketNameInvalid{Bucket: ".test"}, false}, - {"Test", "", ObjectInfo{}, BucketNameInvalid{Bucket: "Test"}, false}, - {"---", "", ObjectInfo{}, BucketNameInvalid{Bucket: "---"}, false}, - {"ad", "", ObjectInfo{}, BucketNameInvalid{Bucket: "ad"}, false}, - // Test cases with valid but non-existing bucket names (Test number 5-7). - {"abcdefgh", "abc", ObjectInfo{}, BucketNotFound{Bucket: "abcdefgh"}, false}, - {"ijklmnop", "efg", ObjectInfo{}, BucketNotFound{Bucket: "ijklmnop"}, false}, - // Test cases with valid but non-existing bucket names and invalid object name (Test number 8-9). - {"abcdefgh", "", ObjectInfo{}, ObjectNameInvalid{Bucket: "abcdefgh", Object: ""}, false}, - {"ijklmnop", "", ObjectInfo{}, ObjectNameInvalid{Bucket: "ijklmnop", Object: ""}, false}, - // Test cases with non-existing object name with existing bucket (Test number 10-12). - {"test-getobjectinfo", "Africa", ObjectInfo{}, ObjectNotFound{Bucket: "test-getobjectinfo", Object: "Africa"}, false}, - {"test-getobjectinfo", "Antartica", ObjectInfo{}, ObjectNotFound{Bucket: "test-getobjectinfo", Object: "Antartica"}, false}, - {"test-getobjectinfo", "Asia/myfile", ObjectInfo{}, ObjectNotFound{Bucket: "test-getobjectinfo", Object: "Asia/myfile"}, false}, - // Test case with existing bucket but object name set to a directory (Test number 13). - {"test-getobjectinfo", "Asia", ObjectInfo{}, ObjectNotFound{Bucket: "test-getobjectinfo", Object: "Asia"}, false}, - // Valid case with existing object (Test number 14). - {"test-getobjectinfo", "Asia/asiapics.jpg", resultCases[0], nil, true}, - } - for i, testCase := range testCases { - result, err := obj.GetObjectInfo(testCase.bucketName, testCase.objectName) - if err != nil && testCase.shouldPass { - t.Errorf("Test %d: Expected to pass, but failed with: %s", i+1, err.Cause.Error()) - } - if err == nil && !testCase.shouldPass { - t.Errorf("Test %d: Expected to fail with \"%s\", but passed instead", i+1, testCase.err.Error()) - } - // Failed as expected, but does it fail for the expected reason. - if err != nil && !testCase.shouldPass { - if testCase.err.Error() != err.Cause.Error() { - t.Errorf("Test %d: Expected to fail with error \"%s\", but instead failed with error \"%s\" instead", i+1, testCase.err.Error(), err.Cause.Error()) - } - } +var _ = Suite(&MySuite{}) - // Test passes as expected, but the output values are verified for correctness here. - if err == nil && testCase.shouldPass { - if testCase.result.Bucket != result.Bucket { - t.Fatalf("Test %d: Expected Bucket name to be '%s', but found '%s' instead", i+1, testCase.result.Bucket, result.Bucket) - } - if testCase.result.Name != result.Name { - t.Errorf("Test %d: Expected Object name to be %s, but instead found it to be %s", i+1, testCase.result.Name, result.Name) - } - if testCase.result.ContentType != result.ContentType { - t.Errorf("Test %d: Expected Content Type of the object to be %v, but instead found it to be %v", i+1, testCase.result.ContentType, result.ContentType) - } - if testCase.result.IsDir != result.IsDir { - t.Errorf("Test %d: Expected IsDir flag of the object to be %v, but instead found it to be %v", i+1, testCase.result.IsDir, result.IsDir) - } - } +func (s *MySuite) TestAPISuite(c *C) { + var storageList []string + create := func() *objectAPI { + path, err := ioutil.TempDir(os.TempDir(), "minio-") + c.Check(err, IsNil) + storageAPI, err := newFS(path) + objAPI := newObjectLayer(storageAPI) + storageList = append(storageList, path) + return objAPI } + APITestSuite(c, create) + defer removeRoots(c, storageList) } -func BenchmarkGetObject(b *testing.B) { - // Make a temporary directory to use as the obj. - directory, e := ioutil.TempDir("", "minio-benchmark-getobject") - if e != nil { - b.Fatal(e) - } - defer os.RemoveAll(directory) - - // Create the obj. - fs, e := newFS(directory) - if e != nil { - b.Fatal(e) - } - - obj := newObjectLayer(fs) - var err *probe.Error - - // Make a bucket and put in a few objects. - err = obj.MakeBucket("bucket") - if err != nil { - b.Fatal(err) - } - - text := "Jack and Jill went up the hill / To fetch a pail of water." - hasher := md5.New() - hasher.Write([]byte(text)) - metadata := make(map[string]string) - for i := 0; i < 10; i++ { - metadata["md5Sum"] = hex.EncodeToString(hasher.Sum(nil)) - _, err = obj.PutObject("bucket", "object"+strconv.Itoa(i), int64(len(text)), bytes.NewBufferString(text), metadata) - if err != nil { - b.Fatal(err) - } - } - - b.ResetTimer() - for i := 0; i < b.N; i++ { - var buffer = new(bytes.Buffer) - r, err := obj.GetObject("bucket", "object"+strconv.Itoa(i%10), 0) - if err != nil { - b.Error(err) - } - if _, e := io.Copy(buffer, r); e != nil { - b.Error(e) - } - if buffer.Len() != len(text) { - b.Errorf("GetObject returned incorrect length %d (should be %d)\n", buffer.Len(), len(text)) - } - r.Close() +func removeRoots(c *C, roots []string) { + for _, root := range roots { + os.RemoveAll(root) } } diff --git a/object_api_suite_test.go b/object_api_suite_test.go index 859533a7f..2fa5ef245 100644 --- a/object_api_suite_test.go +++ b/object_api_suite_test.go @@ -390,22 +390,22 @@ func testGetDirectoryReturnsObjectNotFound(c *check.C, create func() *objectAPI) _, err = obj.GetObject("bucket", "dir1", 0) switch err := err.ToGoError().(type) { - case ObjectNotFound: + case ObjectExistsAsPrefix: c.Assert(err.Bucket, check.Equals, "bucket") c.Assert(err.Object, check.Equals, "dir1") default: // force a failure with a line number - c.Assert(err, check.Equals, "ObjectNotFound") + c.Assert(err, check.Equals, "ObjectExistsAsPrefix") } _, err = obj.GetObject("bucket", "dir1/", 0) switch err := err.ToGoError().(type) { - case ObjectNotFound: + case ObjectExistsAsPrefix: c.Assert(err.Bucket, check.Equals, "bucket") c.Assert(err.Object, check.Equals, "dir1/") default: // force a failure with a line number - c.Assert(err, check.Equals, "ObjectNotFound") + c.Assert(err, check.Equals, "ObjectExistsAsPrefix") } }