Skip to content

Commit

Permalink
client: Avoid returning metadata with Get()
Browse files Browse the repository at this point in the history 
client-s3.Get() calls Stat() method because Get() is supposed to return
object metadata. This PR makes a change in client interface so metadata
is returned in Stat() method. This is beneficial for us to avoid calling
HEAD method in S3 when run `mc cat`
  • Loading branch information
@vadmeste
vadmeste committed Jun 2, 2017
1 parent fed9ebe commit 219d800
Show file tree
Hide file tree
Showing 11 changed files with 60 additions and 64 deletions.
23 changes: 12 additions & 11 deletions cmd/client-fs.go
View file
Expand Up @@ -451,7 +451,7 @@ func (f *fsClient) Copy(source string, size int64, progress io.Reader) *probe.Er

// get - get wrapper returning reader and additional metadata if any.
// currently only returns metadata.
func (f *fsClient) get() (io.Reader, map[string][]string, *probe.Error) {
func (f *fsClient) get() (io.Reader, *probe.Error) {
tmppath := f.PathURL.Path
// Golang strips trailing / if you clean(..) or
// EvalSymlinks(..). Adding '.' prevents it from doing so.
Expand All @@ -463,21 +463,18 @@ func (f *fsClient) get() (io.Reader, map[string][]string, *probe.Error) {
_, e := filepath.EvalSymlinks(tmppath)
if e != nil {
err := f.toClientError(e, f.PathURL.Path)
return nil, nil, err.Trace(f.PathURL.Path)
return nil, err.Trace(f.PathURL.Path)
}
fileData, e := os.Open(f.PathURL.Path)
if e != nil {
err := f.toClientError(e, f.PathURL.Path)
return nil, nil, err.Trace(f.PathURL.Path)
return nil, err.Trace(f.PathURL.Path)
}
metadata := map[string][]string{
"Content-Type": {guessURLContentType(f.PathURL.Path)},
}
return fileData, metadata, nil
return fileData, nil
}

// Get returns reader and any additional metadata.
func (f *fsClient) Get() (io.Reader, map[string][]string, *probe.Error) {
func (f *fsClient) Get() (io.Reader, *probe.Error) {
return f.get()
}

Expand Down Expand Up @@ -1011,17 +1008,21 @@ func (f *fsClient) SetAccess(access string) *probe.Error {
}

// Stat - get metadata from path.
func (f *fsClient) Stat(isIncomplete bool) (content *clientContent, err *probe.Error) {
func (f *fsClient) Stat(isIncomplete bool) (content *clientContent, metadata map[string][]string, err *probe.Error) {
st, err := f.fsStat(isIncomplete)
if err != nil {
return nil, err.Trace(f.PathURL.String())
return nil, nil, err.Trace(f.PathURL.String())
}
content = &clientContent{}
content.URL = *f.PathURL
content.Size = st.Size()
content.Time = st.ModTime()
content.Type = st.Mode()
return content, nil

metadata = map[string][]string{
"Content-Type": {guessURLContentType(f.PathURL.Path)},
}
return content, metadata, nil
}

// toClientError error constructs a typed client error for known filesystem errors.
Expand Down
8 changes: 4 additions & 4 deletions cmd/client-fs_test.go
View file
Expand Up @@ -209,7 +209,7 @@ func (s *TestSuite) TestStatBucket(c *C) {
c.Assert(err, IsNil)
err = fsClient.MakeBucket("us-east-1", true)
c.Assert(err, IsNil)
_, err = fsClient.Stat(false)
_, _, err = fsClient.Stat(false)
c.Assert(err, IsNil)
}

Expand Down Expand Up @@ -274,7 +274,7 @@ func (s *TestSuite) TestGet(c *C) {
c.Assert(err, IsNil)
c.Assert(n, Equals, int64(len(data)))

reader, _, err = fsClient.Get()
reader, err = fsClient.Get()
c.Assert(err, IsNil)
var results bytes.Buffer
_, e = io.Copy(&results, reader)
Expand Down Expand Up @@ -302,7 +302,7 @@ func (s *TestSuite) TestGetRange(c *C) {
c.Assert(err, IsNil)
c.Assert(n, Equals, int64(len(data)))

reader, _, err = fsClient.Get()
reader, err = fsClient.Get()
c.Assert(err, IsNil)
var results bytes.Buffer
buf := make([]byte, 5)
Expand Down Expand Up @@ -333,7 +333,7 @@ func (s *TestSuite) TestStatObject(c *C) {
c.Assert(err, IsNil)
c.Assert(n, Equals, int64(len(data)))

content, err := fsClient.Stat(false)
content, _, err := fsClient.Stat(false)
c.Assert(err, IsNil)
c.Assert(content.Size, Equals, int64(dataLen))
}
Expand Down
67 changes: 29 additions & 38 deletions cmd/client-s3.go
View file
Expand Up @@ -497,40 +497,27 @@ func (c *s3Client) Watch(params watchParams) (*watchObject, *probe.Error) {
}

// Get - get object with metadata.
func (c *s3Client) Get() (io.Reader, map[string][]string, *probe.Error) {
func (c *s3Client) Get() (io.Reader, *probe.Error) {
bucket, object := c.url2BucketAndObject()
reader, e := c.api.GetObject(bucket, object)
if e != nil {
errResponse := minio.ToErrorResponse(e)
if errResponse.Code == "NoSuchBucket" {
return nil, nil, probe.NewError(BucketDoesNotExist{
return nil, probe.NewError(BucketDoesNotExist{
Bucket: bucket,
})
}
if errResponse.Code == "InvalidBucketName" {
return nil, nil, probe.NewError(BucketInvalid{
return nil, probe.NewError(BucketInvalid{
Bucket: bucket,
})
}
if errResponse.Code == "NoSuchKey" || errResponse.Code == "InvalidArgument" {
return nil, nil, probe.NewError(ObjectMissing{})
}
return nil, nil, probe.NewError(e)
}
objInfo, e := reader.Stat()
if e != nil {
errResponse := minio.ToErrorResponse(e)
if errResponse.Code == "AccessDenied" {
return nil, nil, probe.NewError(PathInsufficientPermission{Path: c.targetURL.String()})
}
if errResponse.Code == "NoSuchKey" || errResponse.Code == "InvalidArgument" {
return nil, nil, probe.NewError(ObjectMissing{})
return nil, probe.NewError(ObjectMissing{})
}
return nil, nil, probe.NewError(e)
return nil, probe.NewError(e)
}
metadata := objInfo.Metadata
metadata.Set("Content-Type", objInfo.ContentType)
return reader, metadata, nil
return reader, nil
}

// Copy - copy object
Expand Down Expand Up @@ -829,28 +816,28 @@ func (c *s3Client) listObjectWrapper(bucket, object string, isRecursive bool, do
}

// Stat - send a 'HEAD' on a bucket or object to fetch its metadata.
func (c *s3Client) Stat(isIncomplete bool) (*clientContent, *probe.Error) {
func (c *s3Client) Stat(isIncomplete bool) (*clientContent, map[string][]string, *probe.Error) {
c.mutex.Lock()
defer c.mutex.Unlock()
bucket, object := c.url2BucketAndObject()
// Bucket name cannot be empty, stat on URL has no meaning.
if bucket == "" {
return nil, probe.NewError(BucketNameEmpty{})
return nil, nil, probe.NewError(BucketNameEmpty{})
}

if object == "" {
exists, e := c.api.BucketExists(bucket)
if e != nil {
return nil, probe.NewError(e)
return nil, nil, probe.NewError(e)
}
if !exists {
return nil, probe.NewError(BucketDoesNotExist{Bucket: bucket})
return nil, nil, probe.NewError(BucketDoesNotExist{Bucket: bucket})
}
bucketMetadata := &clientContent{}
bucketMetadata.URL = *c.targetURL
bucketMetadata.Type = os.ModeDir

return bucketMetadata, nil
return bucketMetadata, nil, nil
}

// Remove trailing slashes needed for the following ListObjects call.
Expand All @@ -864,71 +851,75 @@ func (c *s3Client) Stat(isIncomplete bool) (*clientContent, *probe.Error) {
if isIncomplete {
for objectMultipartInfo := range c.api.ListIncompleteUploads(bucket, object, nonRecursive, nil) {
if objectMultipartInfo.Err != nil {
return nil, probe.NewError(objectMultipartInfo.Err)
return nil, nil, probe.NewError(objectMultipartInfo.Err)
}

if objectMultipartInfo.Key == object {
objectMetadata.URL = *c.targetURL
objectMetadata.Time = objectMultipartInfo.Initiated
objectMetadata.Size = objectMultipartInfo.Size
objectMetadata.Type = os.FileMode(0664)
return objectMetadata, nil
return objectMetadata, nil, nil
}

if strings.HasSuffix(objectMultipartInfo.Key, string(c.targetURL.Separator)) {
objectMetadata.URL = *c.targetURL
objectMetadata.Type = os.ModeDir
return objectMetadata, nil
return objectMetadata, nil, nil
}
}
return nil, probe.NewError(ObjectMissing{})
return nil, nil, probe.NewError(ObjectMissing{})
}

for objectStat := range c.listObjectWrapper(bucket, object, nonRecursive, nil) {
if objectStat.Err != nil {
return nil, probe.NewError(objectStat.Err)
return nil, nil, probe.NewError(objectStat.Err)
}

if objectStat.Key == object {
objectMetadata.URL = *c.targetURL
objectMetadata.Time = objectStat.LastModified
objectMetadata.Size = objectStat.Size
objectMetadata.Type = os.FileMode(0664)
return objectMetadata, nil
return objectMetadata, map[string][]string{}, nil
}

if strings.HasSuffix(objectStat.Key, string(c.targetURL.Separator)) {
objectMetadata.URL = *c.targetURL
objectMetadata.Type = os.ModeDir
return objectMetadata, nil
return objectMetadata, map[string][]string{}, nil
}
}
objectStat, e := c.api.StatObject(bucket, object)
if e != nil {
errResponse := minio.ToErrorResponse(e)
if errResponse.Code == "AccessDenied" {
return nil, probe.NewError(PathInsufficientPermission{Path: c.targetURL.String()})
return nil, nil, probe.NewError(PathInsufficientPermission{Path: c.targetURL.String()})
}
if errResponse.Code == "NoSuchBucket" {
return nil, probe.NewError(BucketDoesNotExist{
return nil, nil, probe.NewError(BucketDoesNotExist{
Bucket: bucket,
})
}
if errResponse.Code == "InvalidBucketName" {
return nil, probe.NewError(BucketInvalid{
return nil, nil, probe.NewError(BucketInvalid{
Bucket: bucket,
})
}
if errResponse.Code == "NoSuchKey" || errResponse.Code == "InvalidArgument" {
return nil, probe.NewError(ObjectMissing{})
return nil, nil, probe.NewError(ObjectMissing{})
}
return nil, probe.NewError(e)
return nil, nil, probe.NewError(e)
}
objectMetadata.URL = *c.targetURL
objectMetadata.Time = objectStat.LastModified
objectMetadata.Size = objectStat.Size
objectMetadata.Type = os.FileMode(0664)
return objectMetadata, nil

metadata := objectStat.Metadata
metadata.Set("Content-Type", objectStat.ContentType)

return objectMetadata, metadata, nil
}

func isAmazon(host string) bool {
Expand Down Expand Up @@ -1348,7 +1339,7 @@ func (c *s3Client) listRecursiveInRoutineDirOpt(contentCh chan *clientContent, d
} else if strings.HasSuffix(object, string(c.targetURL.Separator)) {
// Get stat of given object is a directory.
isIncomplete := false
content, perr := c.Stat(isIncomplete)
content, _, perr := c.Stat(isIncomplete)
cContent = content
if perr != nil {
contentCh <- &clientContent{URL: *c.targetURL, Err: perr}
Expand Down
2 changes: 1 addition & 1 deletion cmd/client-s3_test.go
View file
Expand Up @@ -229,7 +229,7 @@ func (s *TestSuite) TestObjectOperations(c *C) {
c.Assert(err, IsNil)
c.Assert(n, Equals, int64(len(object.data)))

reader, _, err = s3c.Get()
reader, err = s3c.Get()
c.Assert(err, IsNil)
var buffer bytes.Buffer
{
Expand Down
2 changes: 1 addition & 1 deletion cmd/client-url.go
View file
Expand Up @@ -176,7 +176,7 @@ func url2Stat(urlStr string) (client Client, content *clientContent, err *probe.
if err != nil {
return nil, nil, err.Trace(urlStr)
}
content, err = client.Stat(false)
content, _, err = client.Stat(false)
if err != nil {
return nil, nil, err.Trace(urlStr)
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/client.go
View file
Expand Up @@ -39,7 +39,7 @@ const (
// Client - client interface
type Client interface {
// Common operations
Stat(isIncomplete bool) (content *clientContent, err *probe.Error)
Stat(isIncomplete bool) (content *clientContent, metadata map[string][]string, err *probe.Error)
List(isRecursive, isIncomplete bool, showDir DirOpt) <-chan *clientContent

// Bucket operations
Expand All @@ -54,7 +54,7 @@ type Client interface {
Copy(source string, size int64, progress io.Reader) *probe.Error

// I/O operations with metadata.
Get() (reader io.Reader, metadata map[string][]string, err *probe.Error)
Get() (reader io.Reader, err *probe.Error)
Put(reader io.Reader, size int64, metadata map[string][]string, progress io.Reader) (n int64, err *probe.Error)

// I/O operations with expiration
Expand Down
6 changes: 5 additions & 1 deletion cmd/common-methods.go
View file
Expand Up @@ -80,7 +80,11 @@ func getSourceStreamFromAlias(alias string, urlStr string) (reader io.Reader, me
if err != nil {
return nil, nil, err.Trace(alias, urlStr)
}
reader, metadata, err = sourceClnt.Get()
reader, err = sourceClnt.Get()
if err != nil {
return nil, nil, err.Trace(alias, urlStr)
}
_, metadata, err = sourceClnt.Stat(false)
if err != nil {
return nil, nil, err.Trace(alias, urlStr)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/ls-main.go
View file
Expand Up @@ -132,7 +132,7 @@ func mainList(ctx *cli.Context) error {
fatalIf(err.Trace(targetURL), "Unable to initialize target `"+targetURL+"`.")

var st *clientContent
if st, err = clnt.Stat(isIncomplete); err != nil {
if st, _, err = clnt.Stat(isIncomplete); err != nil {
switch err.ToGoError().(type) {
case BucketNameEmpty:
// For aliases like ``mc ls s3`` it's acceptable to receive BucketNameEmpty error.
Expand Down
6 changes: 3 additions & 3 deletions cmd/mirror-main.go
View file
Expand Up @@ -317,7 +317,7 @@ func (mj *mirrorJob) watchMirror() {
mj.statusCh <- mirrorURL.WithError(err)
continue
}
sourceContent, err := sourceClient.Stat(false)
sourceContent, _, err := sourceClient.Stat(false)
if err != nil {
// source doesn't exist anymore
mj.statusCh <- mirrorURL.WithError(err)
Expand All @@ -331,7 +331,7 @@ func (mj *mirrorJob) watchMirror() {
}
shouldQueue := false
if !mj.isForce {
_, err = targetClient.Stat(false)
_, _, err = targetClient.Stat(false)
if err == nil {
continue
} // doesn't exist
Expand All @@ -354,7 +354,7 @@ func (mj *mirrorJob) watchMirror() {
mj.statusCh <- mirrorURL.WithError(err)
return
}
_, err = targetClient.Stat(false)
_, _, err = targetClient.Stat(false)
if err == nil {
continue
} // doesn't exist
Expand Down
2 changes: 1 addition & 1 deletion cmd/rm-main.go
View file
Expand Up @@ -142,7 +142,7 @@ func removeSingle(url string, isIncomplete bool, isFake bool, older int) error {
return exitStatus(globalErrorExitStatus) // End of journey.
}

content, pErr := clnt.Stat(isIncomplete)
content, _, pErr := clnt.Stat(isIncomplete)
if pErr != nil {
errorIf(pErr.Trace(url), "Failed to remove `"+url+"`.")
return exitStatus(globalErrorExitStatus)
Expand Down
2 changes: 1 addition & 1 deletion cmd/share-download-main.go
View file
Expand Up @@ -128,7 +128,7 @@ func doShareDownloadURL(targetURL string, isRecursive bool, expiry time.Duration

if !isRecursive {
// Share thr url of only one exact prefix if it exists
content, err := clnt.Stat(isIncomplete)
content, _, err := clnt.Stat(isIncomplete)
if err != nil {
return err.Trace(clnt.GetURL().String())
}
Expand Down

0 comments on commit 219d800

Please sign in to comment.