diff --git a/pkg/fs/fs-object.go b/pkg/fs/fs-object.go index dd45a90c3..748bcd621 100644 --- a/pkg/fs/fs-object.go +++ b/pkg/fs/fs-object.go @@ -54,50 +54,53 @@ func (fs Filesystem) GetObject(w io.Writer, bucket, object string, start, length // normalize buckets. bucket = fs.denormalizeBucket(bucket) - bucketPath := filepath.Join(fs.path, bucket) - if _, e := os.Stat(bucketPath); e != nil { + objectPath := filepath.Join(fs.path, bucket, object) + + file, e := os.Open(objectPath) + if e != nil { + // If the object doesn't exist, the bucket might not exist either. Stat for + // the bucket and give a better error message if that is true. if os.IsNotExist(e) { - return 0, probe.NewError(BucketNotFound{Bucket: bucket}) + _, e = os.Stat(filepath.Join(fs.path, bucket)) + if os.IsNotExist(e) { + return 0, probe.NewError(BucketNotFound{Bucket: bucket}) + } + + return 0, probe.NewError(ObjectNotFound{Bucket: bucket, Object: object}) } return 0, probe.NewError(e) } + defer file.Close() - objectPath := filepath.Join(bucketPath, object) - filestat, err := os.Stat(objectPath) - switch err := err.(type) { - case nil: - if filestat.IsDir() { - return 0, probe.NewError(ObjectNotFound{Bucket: bucket, Object: object}) - } - default: - if os.IsNotExist(err) { + _, e = file.Seek(start, os.SEEK_SET) + if e != nil { + // When the "handle is invalid", the file might be a directory on Windows. + if runtime.GOOS == "windows" && strings.Contains(e.Error(), "handle is invalid") { return 0, probe.NewError(ObjectNotFound{Bucket: bucket, Object: object}) } - return 0, probe.NewError(err) - } - file, err := os.Open(objectPath) - if err != nil { - return 0, probe.NewError(err) - } - defer file.Close() - _, err = file.Seek(start, os.SEEK_SET) - if err != nil { - return 0, probe.NewError(err) + return 0, probe.NewError(e) } var count int64 + // Copy over the whole file if the length is non-positive. if length > 0 { - count, err = io.CopyN(w, file, length) - if err != nil { - return count, probe.NewError(err) - } + count, e = io.CopyN(w, file, length) } else { - count, err = io.Copy(w, file) - if err != nil { - return count, probe.NewError(err) + count, e = io.Copy(w, file) + } + + if e != nil { + // This call will fail if the object is a directory. Stat the file to see if + // this is true, if so, return an ObjectNotFound error. + stat, e := os.Stat(objectPath) + if e == nil && stat.IsDir() { + return count, probe.NewError(ObjectNotFound{Bucket: bucket, Object: object}) } + + return count, probe.NewError(e) } + return count, nil } diff --git a/pkg/fs/fs-object_test.go b/pkg/fs/fs-object_test.go new file mode 100644 index 000000000..4c1de5f44 --- /dev/null +++ b/pkg/fs/fs-object_test.go @@ -0,0 +1,73 @@ +/* + * 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 ( + "bytes" + "crypto/md5" + "encoding/base64" + "io/ioutil" + "os" + "strconv" + "testing" +) + +func BenchmarkGetObject(b *testing.B) { + // Make a temporary directory to use as the filesystem. + directory, e := ioutil.TempDir("", "minio-benchmark-getobject") + if e != nil { + b.Fatal(e) + } + defer os.RemoveAll(directory) + + // Create the filesystem. + filesystem, err := New(directory, 0) + if err != nil { + b.Fatal(err) + } + + // Make a bucket and put in a few objects. + err = filesystem.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)) + sum := base64.StdEncoding.EncodeToString(hasher.Sum(nil)) + for i := 0; i < 10; i++ { + _, err = filesystem.CreateObject("bucket", "object"+strconv.Itoa(i), sum, int64(len(text)), bytes.NewBufferString(text), nil) + if err != nil { + b.Fatal(err) + } + } + + var w bytes.Buffer + + b.ResetTimer() + + for i := 0; i < b.N; i++ { + n, err := filesystem.GetObject(&w, "bucket", "object"+strconv.Itoa(i%10), 0, 0) + if err != nil { + b.Error(err) + } + if n != int64(len(text)) { + b.Errorf("GetObject returned incorrect length %d (should be %d)\n", n, int64(len(text))) + } + } +}