Cleanup - Comments and readability fixes (#1386)

This commit is contained in:
karthic rao 2016-04-28 07:58:13 +05:30 committed by Harshavardhana
parent f87a19a15c
commit 1813e9c070
7 changed files with 85 additions and 56 deletions

3
fs.go
View file

@ -467,10 +467,11 @@ func (s fsStorage) ListFiles(volume, prefix, marker string, recursive bool, coun
if err == nil {
// Prefix does not exist, not an error just respond empty list response.
return nil, true, nil
} else if strings.Contains(err.Error(), "not a directory") {
} else if err.Error() == syscall.ENOTDIR.Error() {
// Prefix exists as a file.
return nil, true, nil
}
log.WithFields(logrus.Fields{
"volumeDir": volumeDir,
"prefixRootDir": prefixRootDir,

View file

@ -36,18 +36,6 @@ const (
minioMetaVolume = ".minio"
)
// checks whether bucket exists.
func (o objectAPI) isBucketExist(bucketName string) (bool, error) {
// Check whether bucket exists.
if _, e := o.storage.StatVol(bucketName); e != nil {
if e == errVolumeNotFound {
return false, nil
}
return false, e
}
return true, nil
}
// listLeafEntries - lists all entries if a given prefixPath is a leaf
// directory, returns error if any - returns empty list if prefixPath
// is not a leaf directory.

View file

@ -213,7 +213,7 @@ func TestObjectAPIPutObjectPart(t *testing.T) {
// Test case - 11.
// Input to replicate Md5 mismatch.
{bucket, object, uploadID, 1, "", "a35", 0, false, "",
fmt.Errorf("%s", "Bad digest expected a35 is not valid with what we calculated "+"d41d8cd98f00b204e9800998ecf8427e")},
fmt.Errorf("%s", "Bad digest: Expected a35 is not valid with what we calculated "+"d41d8cd98f00b204e9800998ecf8427e")},
// Test case - 12.
// Input with size more than the size of actual data inside the reader.
{bucket, object, uploadID, 1, "abcd", "a35", int64(len("abcd") + 1), false, "", fmt.Errorf("%s", "EOF")},

View file

@ -38,6 +38,18 @@ func newObjectLayer(storage StorageAPI) objectAPI {
return objectAPI{storage}
}
// checks whether bucket exists.
func (o objectAPI) isBucketExist(bucketName string) (bool, error) {
// Check whether bucket exists.
if _, e := o.storage.StatVol(bucketName); e != nil {
if e == errVolumeNotFound {
return false, nil
}
return false, e
}
return true, nil
}
/// Bucket operations
// MakeBucket - make a bucket.
@ -199,6 +211,15 @@ func (o objectAPI) PutObject(bucket string, object string, size int64, data io.R
Object: object,
})
}
// Check whether the bucket exists.
isExist, err := o.isBucketExist(bucket)
if err != nil {
return "", probe.NewError(err)
}
if !isExist {
return "", probe.NewError(BucketNotFound{Bucket: bucket})
}
fileWriter, e := o.storage.CreateFile(bucket, object)
if e != nil {
return "", probe.NewError(toObjectErr(e, bucket, object))

View file

@ -60,7 +60,7 @@ func toObjectErr(err error, params ...string) error {
return err
}
// StorageFull storage ran out of space
// StorageFull storage ran out of space.
type StorageFull struct{}
func (e StorageFull) Error() string {
@ -87,21 +87,21 @@ type GenericError struct {
Object string
}
// BucketNotFound bucket does not exist
// BucketNotFound bucket does not exist.
type BucketNotFound GenericError
func (e BucketNotFound) Error() string {
return "Bucket not found: " + e.Bucket
}
// BucketNotEmpty bucket is not empty
// BucketNotEmpty bucket is not empty.
type BucketNotEmpty GenericError
func (e BucketNotEmpty) Error() string {
return "Bucket not empty: " + e.Bucket
}
// ObjectNotFound object does not exist
// ObjectNotFound object does not exist.
type ObjectNotFound GenericError
func (e ObjectNotFound) Error() string {
@ -115,7 +115,7 @@ func (e ObjectExistsAsPrefix) Error() string {
return "Object exists on : " + e.Bucket + " as prefix " + e.Object
}
// BucketExists bucket exists
// BucketExists bucket exists.
type BucketExists GenericError
func (e BucketExists) Error() string {
@ -129,7 +129,7 @@ type BadDigest struct {
}
func (e BadDigest) Error() string {
return "Bad digest expected " + e.ExpectedMD5 + " is not valid with what we calculated " + e.CalculatedMD5
return "Bad digest: Expected " + e.ExpectedMD5 + " is not valid with what we calculated " + e.CalculatedMD5
}
// UnsupportedDelimiter - unsupported delimiter.
@ -166,22 +166,22 @@ func (e BucketPolicyNotFound) Error() string {
return "No bucket policy found for bucket: " + e.Bucket
}
/// Bucket related errors
/// Bucket related errors.
// BucketNameInvalid - bucketname provided is invalid
// BucketNameInvalid - bucketname provided is invalid.
type BucketNameInvalid GenericError
// Return string an error formatted as the given text
// Return string an error formatted as the given text.
func (e BucketNameInvalid) Error() string {
return "Bucket name invalid: " + e.Bucket
}
/// Object related errors
/// Object related errors.
// ObjectNameInvalid - object name provided is invalid
// ObjectNameInvalid - object name provided is invalid.
type ObjectNameInvalid GenericError
// Return string an error formatted as the given text
// Return string an error formatted as the given text.
func (e ObjectNameInvalid) Error() string {
return "Object name invalid: " + e.Bucket + "#" + e.Object
}
@ -195,15 +195,15 @@ func (e UnExpectedDataSize) Error() string {
return fmt.Sprintf("Contains more data than specified size of %d bytes.", e.Size)
}
// IncompleteBody You did not provide the number of bytes specified by the Content-Length HTTP header
// IncompleteBody You did not provide the number of bytes specified by the Content-Length HTTP header.
type IncompleteBody GenericError
// Return string an error formatted as the given text
// Return string an error formatted as the given text.
func (e IncompleteBody) Error() string {
return e.Bucket + "#" + e.Object + "has incomplete body"
}
/// Multipart related errors
/// Multipart related errors.
// MalformedUploadID malformed upload id.
type MalformedUploadID struct {

View file

@ -30,8 +30,8 @@ func TestIsValidBucketName(t *testing.T) {
bucketName string
shouldPass bool
}{
//cases which should pass the test
//passing in valid bucket names
// cases which should pass the test.
// passing in valid bucket names.
{"lol", true},
{"1-this-is-valid", true},
{"1-this-too-is-valid-1", true},
@ -43,8 +43,8 @@ func TestIsValidBucketName(t *testing.T) {
{"testbucket", true},
{"1bucket", true},
{"bucket1", true},
//cases for which test should fail
//passing invalid bucket names
// cases for which test should fail.
// passing invalid bucket names.
{"------", false},
{"$this-is-not-valid-too", false},
{"contains-$-dollar", false},
@ -83,16 +83,16 @@ func TestIsValidObjectName(t *testing.T) {
objectName string
shouldPass bool
}{
//cases which should pass the test
//passing in valid object name
// cases which should pass the test.
// passing in valid object name.
{"object", true},
{"The Shining Script <v1>.pdf", true},
{"Cost Benefit Analysis (2009-2010).pptx", true},
{"117Gn8rfHL2ACARPAhaFd0AGzic9pUbIA/5OCn5A", true},
{"SHØRT", true},
{"There are far too many object names, and far too few bucket names!", true},
//cases for which test should fail
//passing invalid object names
// cases for which test should fail.
// passing invalid object names.
{"", false},
{string([]byte{0xff, 0xfe, 0xfd}), false},
}

View file

@ -30,7 +30,7 @@ import (
// TODO - enable all the commented tests.
// APITestSuite - collection of API tests
// APITestSuite - collection of API tests.
func APITestSuite(c *check.C, create func() objectAPI) {
testMakeBucket(c, create)
testMultipleObjectCreation(c, create)
@ -49,12 +49,15 @@ func APITestSuite(c *check.C, create func() objectAPI) {
testMultipartObjectAbort(c, create)
}
// Tests validate bucket creation.
func testMakeBucket(c *check.C, create func() objectAPI) {
obj := create()
err := obj.MakeBucket("bucket-unknown")
c.Assert(err, check.IsNil)
}
// Tests validate creation of part files during Multipart operation.
func testMultipartObjectCreation(c *check.C, create func() objectAPI) {
obj := create()
err := obj.MakeBucket("bucket")
@ -79,6 +82,7 @@ func testMultipartObjectCreation(c *check.C, create func() objectAPI) {
c.Assert(md5Sum, check.Equals, "7dd76eded6f7c3580a78463a7cf539bd-10")
}
// Tests validate abortion of Multipart operation.
func testMultipartObjectAbort(c *check.C, create func() objectAPI) {
obj := create()
err := obj.MakeBucket("bucket")
@ -110,6 +114,7 @@ func testMultipartObjectAbort(c *check.C, create func() objectAPI) {
c.Assert(err, check.IsNil)
}
// Tests validate object creation.
func testMultipleObjectCreation(c *check.C, create func() objectAPI) {
objects := make(map[string][]byte)
obj := create()
@ -151,6 +156,7 @@ func testMultipleObjectCreation(c *check.C, create func() objectAPI) {
}
}
// Tests validate creation of objects and the order of listing using various filters for ListObjects operation.
func testPaging(c *check.C, create func() objectAPI) {
obj := create()
obj.MakeBucket("bucket")
@ -158,7 +164,7 @@ func testPaging(c *check.C, create func() objectAPI) {
c.Assert(err, check.IsNil)
c.Assert(len(result.Objects), check.Equals, 0)
c.Assert(result.IsTruncated, check.Equals, false)
// check before paging occurs
// check before paging occurs.
for i := 0; i < 5; i++ {
key := "obj" + strconv.Itoa(i)
_, err = obj.PutObject("bucket", key, int64(len("The specified multipart upload does not exist. The upload ID might be invalid, or the multipart upload might have been aborted or completed.")), bytes.NewBufferString("The specified multipart upload does not exist. The upload ID might be invalid, or the multipart upload might have been aborted or completed."), nil)
@ -169,7 +175,7 @@ func testPaging(c *check.C, create func() objectAPI) {
c.Assert(len(result.Objects), check.Equals, i+1)
c.Assert(result.IsTruncated, check.Equals, false)
}
// check after paging occurs pages work
// check after paging occurs pages work.
for i := 6; i <= 10; i++ {
key := "obj" + strconv.Itoa(i)
_, err = obj.PutObject("bucket", key, int64(len("The specified multipart upload does not exist. The upload ID might be invalid, or the multipart upload might have been aborted or completed.")), bytes.NewBufferString("The specified multipart upload does not exist. The upload ID might be invalid, or the multipart upload might have been aborted or completed."), nil)
@ -179,7 +185,7 @@ func testPaging(c *check.C, create func() objectAPI) {
c.Assert(len(result.Objects), check.Equals, 5)
c.Assert(result.IsTruncated, check.Equals, true)
}
// check paging with prefix at end returns less objects
// check paging with prefix at end returns less objects.
{
_, err = obj.PutObject("bucket", "newPrefix", int64(len("The specified multipart upload does not exist. The upload ID might be invalid, or the multipart upload might have been aborted or completed.")), bytes.NewBufferString("The specified multipart upload does not exist. The upload ID might be invalid, or the multipart upload might have been aborted or completed."), nil)
c.Assert(err, check.IsNil)
@ -190,7 +196,7 @@ func testPaging(c *check.C, create func() objectAPI) {
c.Assert(len(result.Objects), check.Equals, 2)
}
// check ordering of pages
// check ordering of pages.
{
result, err = obj.ListObjects("bucket", "", "", "", 1000)
c.Assert(err, check.IsNil)
@ -201,7 +207,7 @@ func testPaging(c *check.C, create func() objectAPI) {
c.Assert(result.Objects[4].Name, check.Equals, "obj10")
}
// check delimited results with delimiter and prefix
// check delimited results with delimiter and prefix.
{
_, err = obj.PutObject("bucket", "this/is/delimited", int64(len("The specified multipart upload does not exist. The upload ID might be invalid, or the multipart upload might have been aborted or completed.")), bytes.NewBufferString("The specified multipart upload does not exist. The upload ID might be invalid, or the multipart upload might have been aborted or completed."), nil)
c.Assert(err, check.IsNil)
@ -213,7 +219,7 @@ func testPaging(c *check.C, create func() objectAPI) {
c.Assert(result.Prefixes[0], check.Equals, "this/is/also/")
}
// check delimited results with delimiter without prefix
// check delimited results with delimiter without prefix.
{
result, err = obj.ListObjects("bucket", "", "", "/", 1000)
c.Assert(err, check.IsNil)
@ -225,7 +231,7 @@ func testPaging(c *check.C, create func() objectAPI) {
c.Assert(result.Prefixes[0], check.Equals, "this/")
}
// check results with Marker
// check results with Marker.
{
result, err = obj.ListObjects("bucket", "", "newPrefix", "", 3)
@ -235,7 +241,7 @@ func testPaging(c *check.C, create func() objectAPI) {
c.Assert(result.Objects[2].Name, check.Equals, "obj1")
}
// check ordering of results with prefix
// check ordering of results with prefix.
{
result, err = obj.ListObjects("bucket", "obj", "", "", 1000)
c.Assert(err, check.IsNil)
@ -245,7 +251,7 @@ func testPaging(c *check.C, create func() objectAPI) {
c.Assert(result.Objects[3].Name, check.Equals, "obj2")
c.Assert(result.Objects[4].Name, check.Equals, "obj3")
}
// check ordering of results with prefix and no paging
// check ordering of results with prefix and no paging.
{
result, err = obj.ListObjects("bucket", "new", "", "", 5)
c.Assert(err, check.IsNil)
@ -254,6 +260,7 @@ func testPaging(c *check.C, create func() objectAPI) {
}
}
// Tests validate overwriting of an existing object.
func testObjectOverwriteWorks(c *check.C, create func() objectAPI) {
obj := create()
err := obj.MakeBucket("bucket")
@ -276,20 +283,25 @@ func testObjectOverwriteWorks(c *check.C, create func() objectAPI) {
c.Assert(r.Close(), check.IsNil)
}
// Tests validate that bucket operation on non-existent bucket fails.
func testNonExistantBucketOperations(c *check.C, create func() objectAPI) {
obj := create()
_, err := obj.PutObject("bucket1", "object", int64(len("one")), bytes.NewBufferString("one"), nil)
c.Assert(err, check.Not(check.IsNil))
c.Assert(err.ToGoError().Error(), check.Equals, "Bucket not found: bucket1")
}
// Tests validate that recreation of the bucket fails.
func testBucketRecreateFails(c *check.C, create func() objectAPI) {
obj := create()
err := obj.MakeBucket("string")
c.Assert(err, check.IsNil)
err = obj.MakeBucket("string")
c.Assert(err, check.Not(check.IsNil))
c.Assert(err.ToGoError().Error(), check.Equals, "Bucket exists: string")
}
// Tests validate PutObject with subdirectory prefix.
func testPutObjectInSubdir(c *check.C, create func() objectAPI) {
obj := create()
err := obj.MakeBucket("bucket")
@ -308,15 +320,16 @@ func testPutObjectInSubdir(c *check.C, create func() objectAPI) {
c.Assert(r.Close(), check.IsNil)
}
// Tests validate ListBuckets.
func testListBuckets(c *check.C, create func() objectAPI) {
obj := create()
// test empty list
// test empty list.
buckets, err := obj.ListBuckets()
c.Assert(err, check.IsNil)
c.Assert(len(buckets), check.Equals, 0)
// add one and test exists
// add one and test exists.
err = obj.MakeBucket("bucket1")
c.Assert(err, check.IsNil)
@ -324,7 +337,7 @@ func testListBuckets(c *check.C, create func() objectAPI) {
c.Assert(len(buckets), check.Equals, 1)
c.Assert(err, check.IsNil)
// add two and test exists
// add two and test exists.
err = obj.MakeBucket("bucket2")
c.Assert(err, check.IsNil)
@ -332,7 +345,7 @@ func testListBuckets(c *check.C, create func() objectAPI) {
c.Assert(len(buckets), check.Equals, 2)
c.Assert(err, check.IsNil)
// add three and test exists + prefix
// add three and test exists + prefix.
err = obj.MakeBucket("bucket22")
buckets, err = obj.ListBuckets()
@ -340,12 +353,13 @@ func testListBuckets(c *check.C, create func() objectAPI) {
c.Assert(err, check.IsNil)
}
// Tests validate the order of result of ListBuckets.
func testListBucketsOrder(c *check.C, create func() objectAPI) {
// if implementation contains a map, order of map keys will vary.
// this ensures they return in the same order each time
// this ensures they return in the same order each time.
for i := 0; i < 10; i++ {
obj := create()
// add one and test exists
// add one and test exists.
err := obj.MakeBucket("bucket1")
c.Assert(err, check.IsNil)
err = obj.MakeBucket("bucket2")
@ -358,14 +372,17 @@ func testListBucketsOrder(c *check.C, create func() objectAPI) {
}
}
// Tests validate that ListObjects operation on a non-existent bucket fails as expected.
func testListObjectsTestsForNonExistantBucket(c *check.C, create func() objectAPI) {
obj := create()
result, err := obj.ListObjects("bucket", "", "", "", 1000)
c.Assert(err, check.Not(check.IsNil))
c.Assert(result.IsTruncated, check.Equals, false)
c.Assert(len(result.Objects), check.Equals, 0)
c.Assert(err.ToGoError().Error(), check.Equals, "Bucket not found: bucket")
}
// Tests validate that GetObject fails on a non-existent bucket as expected.
func testNonExistantObjectInBucket(c *check.C, create func() objectAPI) {
obj := create()
err := obj.MakeBucket("bucket")
@ -381,6 +398,7 @@ func testNonExistantObjectInBucket(c *check.C, create func() objectAPI) {
}
}
// Tests validate that GetObject on an existing directory fails as expected.
func testGetDirectoryReturnsObjectNotFound(c *check.C, create func() objectAPI) {
obj := create()
err := obj.MakeBucket("bucket")
@ -395,7 +413,7 @@ func testGetDirectoryReturnsObjectNotFound(c *check.C, create func() objectAPI)
c.Assert(err.Bucket, check.Equals, "bucket")
c.Assert(err.Object, check.Equals, "dir1")
default:
// force a failure with a line number
// force a failure with a line number.
c.Assert(err, check.Equals, "ObjectNotFound")
}
@ -405,17 +423,18 @@ func testGetDirectoryReturnsObjectNotFound(c *check.C, create func() objectAPI)
c.Assert(err.Bucket, check.Equals, "bucket")
c.Assert(err.Object, check.Equals, "dir1/")
default:
// force a failure with a line number
// force a failure with a line number.
c.Assert(err, check.Equals, "ObjectNotFound")
}
}
// Tests valdiate the default ContentType.
func testDefaultContentType(c *check.C, create func() objectAPI) {
obj := create()
err := obj.MakeBucket("bucket")
c.Assert(err, check.IsNil)
// Test empty
// Test empty.
_, err = obj.PutObject("bucket", "one", int64(len("The specified multipart upload does not exist. The upload ID might be invalid, or the multipart upload might have been aborted or completed.")), bytes.NewBufferString("The specified multipart upload does not exist. The upload ID might be invalid, or the multipart upload might have been aborted or completed."), nil)
c.Assert(err, check.IsNil)
objInfo, err := obj.GetObjectInfo("bucket", "one")