storage: return custom errors on storage.{Stat,List} (#64)

peak · Feb 24, 2020 · 64dc884 · 64dc884
1 parent b28b180
commit 64dc884
Show file tree

Hide file tree

Showing 9 changed files with 54 additions and 36 deletions.
diff --git a/complete/complete.go b/complete/complete.go
@@ -32,7 +32,6 @@ func ParseFlagsAndRun() (bool, error) {
 
 	completer := cmp.Command{
 		Flags: cmp.Flags{
-			"-f": cmp.PredictOr(cmp.PredictSet("-"), cmp.PredictFiles("*")),
 			"-numworkers": cmp.PredictFunc(func(a cmp.Args) []string {
 				// add some sensible defaults...
 				ret := []string{"-1", "-2", "-4"}
@@ -45,15 +44,18 @@ func ParseFlagsAndRun() (bool, error) {
 				}
 				return ret
 			}),
-			"-cs":            cmp.PredictSet("5", "16", "64", "128", "256"),
-			"-dlp":           cmp.PredictSet("5", "16", "64", "128", "256"),
-			"-dlw":           cmp.PredictSet("5", "8", "16", "32", "64"),
+			"-f":             cmp.PredictOr(cmp.PredictSet("-"), cmp.PredictFiles("*")),
+			"-ds":            cmp.PredictSet("5", "16", "64", "128", "256"),
+			"-dw":            cmp.PredictSet("5", "8", "16", "32", "64"),
+			"-us":            cmp.PredictSet("5", "16", "64", "128", "256"),
+			"-uw":            cmp.PredictSet("5", "8", "16", "32", "64"),
 			"-cmp-install":   cmp.PredictSet("assume-yes"),
 			"-cmp-uninstall": cmp.PredictSet("assume-yes"),
 			"-h":             cmp.PredictNothing,
 			"-r":             cmp.PredictSet("0", "1", "2", "10", "100"),
 			"-stats":         cmp.PredictNothing,
-			"-ulw":           cmp.PredictSet("5", "8", "16", "32", "64"),
+			"-endpoint-url":  cmp.PredictNothing,
+			"-no-verify-ssl": cmp.PredictNothing,
 			"-version":       cmp.PredictNothing,
 			"-gops":          cmp.PredictNothing,
 			"-vv":            cmp.PredictNothing,

diff --git a/core/job_test.go b/core/job_test.go
@@ -101,6 +101,7 @@ func createFile(filename, contents string) error {
 		return err
 	}
 	defer f.Close()
+
 	f.WriteString(contents)
 	return nil
 }
@@ -140,6 +141,7 @@ func TestJobRunLocalDelete(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
+
 	err = createFile(filename, "contents")
 	if err != nil {
 		t.Fatal(err)

diff --git a/core/op_local.go b/core/op_local.go
@@ -76,7 +76,7 @@ func BatchLocalCopy(job *Job, wp *WorkerParams) (stats.StatType, *JobResponse) {
 	// src.url could contain glob, or could be a directory.
 	// err is not important here
 	obj, err := client.Stat(wp.ctx, src.url)
-	walkMode := err == nil && obj.Type.IsDir()
+	walkMode := err == nil && obj.Mode.IsDir()
 
 	trimPrefix := src.url.Absolute()
 	globStart := src.url.Absolute()
@@ -104,7 +104,7 @@ func BatchLocalCopy(job *Job, wp *WorkerParams) (stats.StatType, *JobResponse) {
 	isRecursive := job.opts.Has(opt.Recursive)
 
 	err = wildOperation(client, globurl, isRecursive, wp, func(object *storage.Object) *Job {
-		if object.IsMarker() || object.Type.IsDir() {
+		if object.IsMarker() || object.Mode.IsDir() {
 			return nil
 		}
 
@@ -149,7 +149,7 @@ func BatchLocalUpload(job *Job, wp *WorkerParams) (stats.StatType, *JobResponse)
 	// src.url could contain glob, or could be a directory.
 	// err is not important here
 	obj, err := client.Stat(wp.ctx, src.url)
-	walkMode := err == nil && obj.Type.IsDir()
+	walkMode := err == nil && obj.Mode.IsDir()
 
 	trimPrefix := src.url.Absolute()
 	if !walkMode {
@@ -167,7 +167,7 @@ func BatchLocalUpload(job *Job, wp *WorkerParams) (stats.StatType, *JobResponse)
 	}
 
 	err = wildOperation(client, src.url, true, wp, func(object *storage.Object) *Job {
-		if object.IsMarker() || object.Type.IsDir() {
+		if object.IsMarker() || object.Mode.IsDir() {
 			return nil
 		}
 

diff --git a/core/op_s3.go b/core/op_s3.go
@@ -96,7 +96,7 @@ func S3BatchDelete(job *Job, wp *WorkerParams) (stats.StatType, *JobResponse) {
 	}
 
 	err = wildOperation(client, src.url, true, wp, func(object *storage.Object) *Job {
-		if object.Type.IsDir() {
+		if object.Mode.IsDir() {
 			return nil
 		}
 
@@ -163,7 +163,7 @@ func S3BatchDownload(job *Job, wp *WorkerParams) (stats.StatType, *JobResponse)
 		return opType, jobResponse(err)
 	}
 	err = wildOperation(client, src.url, true, wp, func(object *storage.Object) *Job {
-		if object.IsMarker() || object.Type.IsDir() {
+		if object.IsMarker() || object.Mode.IsDir() {
 			return nil
 		}
 
@@ -294,7 +294,7 @@ func S3BatchCopy(job *Job, wp *WorkerParams) (stats.StatType, *JobResponse) {
 	}
 
 	err = wildOperation(client, src.url, true, wp, func(object *storage.Object) *Job {
-		if object.IsMarker() || object.StorageClass.IsGlacier() || object.Type.IsDir() {
+		if object.IsMarker() || object.StorageClass.IsGlacier() || object.Mode.IsDir() {
 			return nil
 		}
 
@@ -359,7 +359,7 @@ func S3List(job *Job, wp *WorkerParams) (stats.StatType, *JobResponse) {
 			continue
 		}
 
-		if object.Type.IsDir() {
+		if object.Mode.IsDir() {
 			msg = append(msg, fmt.Sprintf("%19s %1s %-38s  %12s  %s", "", "", "", "DIR", object.URL.Relative()))
 		} else {
 			var cls, etag, size string
@@ -420,7 +420,7 @@ func S3Size(job *Job, wp *WorkerParams) (stats.StatType, *JobResponse) {
 	}
 
 	err = wildOperation(client, src.url, true, wp, func(object *storage.Object) *Job {
-		if object.IsMarker() || object.Type.IsDir() {
+		if object.IsMarker() || object.Mode.IsDir() {
 			return nil
 		}
 		storageClass := string(object.StorageClass)

diff --git a/main.go b/main.go
@@ -53,7 +53,7 @@ func main() {
 	var (
 		flagCommandFile         = flag.String("f", "", "Commands-file or - for stdin")
 		flagEndpointURL         = flag.String("endpoint-url", "", "Override default URL with the given one")
-		flagWorkerCount         = flag.Int("numworkers", defaultWorkerCount, fmt.Sprintf("Number of worker goroutines. Negative numbers mean multiples of the CPU core count."))
+		flagWorkerCount         = flag.Int("numworkers", defaultWorkerCount, "Number of worker goroutines. Negative numbers mean multiples of the CPU core count")
 		flagDownloadConcurrency = flag.Int("dw", defaultDownloadConcurrency, "Download concurrency for each file")
 		flagDownloadPartSize    = flag.Int("ds", 50, "Multipart chunk size in MB for downloads")
 		flagUploadConcurrency   = flag.Int("uw", defaultUploadConcurrency, "Upload concurrency for each file")

diff --git a/storage/fs.go b/storage/fs.go
@@ -24,12 +24,15 @@ func NewFilesystem() *Filesystem {
 func (f *Filesystem) Stat(ctx context.Context, url *objurl.ObjectURL) (*Object, error) {
 	st, err := os.Stat(url.Absolute())
 	if err != nil {
+		if os.IsNotExist(err) {
+			return nil, ErrGivenObjectNotFound
+		}
 		return nil, err
 	}
 
 	return &Object{
 		URL:     url,
-		Type:    st.Mode(),
+		Mode:    st.Mode(),
 		Size:    st.Size(),
 		ModTime: st.ModTime(),
 		Etag:    "",
@@ -38,7 +41,7 @@ func (f *Filesystem) Stat(ctx context.Context, url *objurl.ObjectURL) (*Object,
 
 func (f *Filesystem) List(ctx context.Context, url *objurl.ObjectURL, isRecursive bool, _ int64) <-chan *Object {
 	obj, err := f.Stat(ctx, url)
-	isDir := err == nil && obj.Type.IsDir()
+	isDir := err == nil && obj.Mode.IsDir()
 
 	if isDir {
 		return f.walkDir(ctx, url, isRecursive)
@@ -74,7 +77,7 @@ func (f *Filesystem) expandGlob(ctx context.Context, url *objurl.ObjectURL, isRe
 			url, _ := objurl.New(filename)
 			obj, _ := f.Stat(ctx, url)
 
-			if !obj.Type.IsDir() {
+			if !obj.Mode.IsDir() {
 				sendObject(ctx, obj, ch)
 			}
 
@@ -97,7 +100,7 @@ func (f *Filesystem) expandGlob(ctx context.Context, url *objurl.ObjectURL, isRe
 
 					obj := &Object{
 						URL:  url,
-						Type: dirent.ModeType(),
+						Mode: dirent.ModeType(),
 					}
 
 					sendObject(ctx, obj, ch)
@@ -143,7 +146,7 @@ func (f *Filesystem) walkDir(ctx context.Context, url *objurl.ObjectURL, isRecur
 
 				obj := &Object{
 					URL:  url,
-					Type: dirent.ModeType(),
+					Mode: dirent.ModeType(),
 				}
 
 				sendObject(ctx, obj, ch)

diff --git a/storage/s3.go b/storage/s3.go
@@ -26,11 +26,6 @@ import (
 
 var _ Storage = (*S3)(nil)
 
-var (
-	// ErrNoItemFound is a error type for marking empty list results.
-	ErrNoItemFound = fmt.Errorf("s3: no item found")
-)
-
 const (
 	// ListAllItems is a type to paginate all S3 keys.
 	ListAllItems = -1
@@ -86,8 +81,10 @@ func (s *S3) Stat(ctx context.Context, url *objurl.ObjectURL) (*Object, error) {
 		Bucket: aws.String(url.Bucket),
 		Key:    aws.String(url.Path),
 	})
-
 	if err != nil {
+		if errHasCode(err, "NotFound") {
+			return nil, ErrGivenObjectNotFound
+		}
 		return nil, err
 	}
 
@@ -135,7 +132,7 @@ func (s *S3) List(ctx context.Context, url *objurl.ObjectURL, _ bool, maxKeys in
 				newurl.Path = prefix
 				objCh <- &Object{
 					URL:  newurl,
-					Type: os.ModeDir,
+					Mode: os.ModeDir,
 				}
 
 				itemFound = true
@@ -158,7 +155,7 @@ func (s *S3) List(ctx context.Context, url *objurl.ObjectURL, _ bool, maxKeys in
 					URL:          newurl,
 					Etag:         aws.StringValue(c.ETag),
 					ModTime:      aws.TimeValue(c.LastModified),
-					Type:         objtype,
+					Mode:         objtype,
 					Size:         aws.Int64Value(c.Size),
 					StorageClass: storageClass(aws.StringValue(c.StorageClass)),
 				}
@@ -179,7 +176,7 @@ func (s *S3) List(ctx context.Context, url *objurl.ObjectURL, _ bool, maxKeys in
 		}
 
 		if !itemFound {
-			objCh <- &Object{Err: ErrNoItemFound}
+			objCh <- &Object{Err: ErrNoObjectFound}
 		}
 	}()
 
@@ -382,15 +379,21 @@ func newAWSSession(opts S3Opts) (*session.Session, error) {
 	return ses, err
 }
 
-func IsCancelationError(err error) bool {
-	if err == nil {
+func errHasCode(err error, code string) bool {
+	if code == "" || err == nil {
 		return false
 	}
+
 	var awsErr awserr.Error
 	if errors.As(err, &awsErr) {
-		if awsErr.Code() == request.CanceledErrorCode {
+		if awsErr.Code() == code {
 			return true
 		}
 	}
 	return false
+
+}
+
+func IsCancelationError(err error) bool {
+	return errHasCode(err, request.CanceledErrorCode)
 }
diff --git a/storage/s3_test.go b/storage/s3_test.go
@@ -82,7 +82,7 @@ func TestS3_List_success(t *testing.T) {
 		}
 
 		want := responses[index]
-		if diff := cmp.Diff(want.isDir, got.Type.IsDir()); diff != "" {
+		if diff := cmp.Diff(want.isDir, got.Mode.IsDir()); diff != "" {
 			t.Errorf("(-want +got):\n%v", diff)
 		}
 		if diff := cmp.Diff(want.url, got.URL.Absolute()); diff != "" {
@@ -153,8 +153,8 @@ func TestS3_List_no_item_found(t *testing.T) {
 	})
 
 	for got := range mockS3.List(context.Background(), url, true, ListAllItems) {
-		if got.Err != ErrNoItemFound {
-			t.Errorf("error got = %v, want %v", got.Err, ErrNoItemFound)
+		if got.Err != ErrNoObjectFound {
+			t.Errorf("error got = %v, want %v", got.Err, ErrNoObjectFound)
 		}
 	}
 }

diff --git a/storage/storage.go b/storage/storage.go
@@ -13,6 +13,14 @@ import (
 
 const dateFormat = "2006/01/02 15:04:05"
 
+var (
+	// ErrGivenObjectNotFound indicates a specified object is not found.
+	ErrGivenObjectNotFound = fmt.Errorf("given object not found")
+
+	// ErrNoObjectFound indicates there are no objects found from a given directory.
+	ErrNoObjectFound = fmt.Errorf("no object found")
+)
+
 // Storage is an interface for storage operations.
 type Storage interface {
 	Stat(context.Context, *objurl.ObjectURL) (*Object, error)
@@ -31,7 +39,7 @@ type Object struct {
 	URL          *objurl.ObjectURL
 	Etag         string
 	ModTime      time.Time
-	Type         os.FileMode
+	Mode         os.FileMode
 	Size         int64
 	StorageClass storageClass
 	Err          error