Repository: peak/s5cmd (go)

Issue:
Wildcard mv causes infinite repetition if target destination also qualifies wildcart
Suppose you have objects like this:
```
...
s3://sandbox/single-4k.blob.4
s3://sandbox/single-4k.blob.5
s3://sandbox/single-4k.blob.6
...
s3://sandbox/single-16k.blob.1
s3://sandbox/single-16k.blob.2
...
```
Trying to move them to a new prefix that also matches the glob causes infinite repetition. For example:
```
s5cmd mv 's3://sandbox/single*.blob*' s3://sandbox/single/
```
Produces this after some time:
```
...
mv s3://sandbox/single/single/single/single-4k.blob.1 s3://sandbox/single/single/single/single/single-4k.blob.1
...
```
Naive solution is to have the application make a full pass of the bucket for keys matching the wildcard, keep state of the initial list of qualified keys, and pop them off the queue as the individual `mv`s complete. I am guessing that right now it is doing a lazy evaluation of qualified keys one at a time.

For others who are experiencing this, temporary workaround right now is to pipe a `s5cmd ls` wildcard output into `xargs` and individually `s5cmd mv`.

Pull request description:
storage/s3: fix wildcard issue (#168)
Resolves #168 

Patch:
diff --git a/.circleci/config.yml b/.circleci/config.yml
deleted file mode 100644
index 5519221..0000000
--- a/.circleci/config.yml
+++ /dev/null
@@ -1,46 +0,0 @@
-version: 2.1
-
-jobs:
-  build-unix:
-    docker:
-      - image: circleci/golang:1.14
-
-    working_directory: /go/src/github.com/peak/s5cmd
-
-    steps:
-      - checkout
-      - run: go mod verify
-      - run: go build -mod=vendor .
-
-  test:
-    docker:
-      - image: circleci/golang:1.14
-
-    working_directory: /go/src/github.com/peak/s5cmd
-
-    steps:
-      - checkout
-      - run: make test
-
-  qa:
-    docker:
-      - image: circleci/golang:1.14
-
-    working_directory: /go/src/github.com/peak/s5cmd
-
-    steps:
-      - checkout
-      - run: make check-fmt
-      - run: make vet
-      - run: go get honnef.co/go/tools/cmd/staticcheck
-      - run: go get mvdan.cc/unparam
-      - run: make staticcheck
-      - run: make unparam
-
-workflows:
-  version: 2.1
-  build:
-    jobs:
-      - build-unix
-      - test
-      - qa
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 1b2cc86..640b3ea 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -41,14 +41,7 @@ jobs:
     - run: make test
 
   qa:
-    strategy:
-      matrix:
-        os:
-          - macos
-          - ubuntu
-          - windows
-
-    runs-on: ${{ matrix.os }}-latest
+    runs-on: ubuntu-latest
 
     steps:
     - uses: actions/checkout@v2
diff --git a/CHANGELOG.md b/CHANGELOG.md
index a2f90e2..26f684e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,9 +2,17 @@
 
 ## not released yet
 
+#### Breaking changes
+- Dropped storage class short codes display from default behaviour of `ls` operation. Instead, use `-s` flag with `ls`
+to see full names of the storage classes when listing objects.
+
+#### Features
+- Added an option to show full form of [storage class](https://aws.amazon.com/s3/storage-classes/) when listing objects. ([#165](https://github.com/peak/s5cmd/issues/165))
 
 #### Bugfixes
 
+- Fixed infinite repetition issue on mv/cp operations which would occur
+ if the destination matched the source wildcard. ([#168](https://github.com/peak/s5cmd/issues/168))
 - Fixed windows filepath issue, where backslashes should be treated as the path delimiter. ([#178](https://github.com/peak/s5cmd/issues/178))
 - All tests pass on windows, by converting and treating file paths to UNIX filepath format.
 - Fixed a transfer issue where the object path contains particular regex metacharacters. ([#111](https://github.com/peak/s5cmd/pull/111)) [@brendan-matroid](https://github.com/brendan-matroid)
diff --git a/command/ls.go b/command/ls.go
index 8d4c0f9..2a1214d 100644
--- a/command/ls.go
+++ b/command/ls.go
@@ -53,6 +53,11 @@ var listCommand = &cli.Command{
 			Aliases: []string{"H"},
 			Usage:   "human-readable output for object sizes",
 		},
+		&cli.BoolFlag{
+			Name:    "storage-class",
+			Aliases: []string{"s"},
+			Usage:   "display full name of the object class",
+		},
 	},
 	Before: func(c *cli.Context) error {
 		if c.Args().Len() > 1 {
@@ -67,12 +72,14 @@ var listCommand = &cli.Command{
 
 		showEtag := c.Bool("etag")
 		humanize := c.Bool("humanize")
+		showStorageClass := c.Bool("storage-class")
 
 		return List(
 			c.Context,
 			c.Args().First(),
 			showEtag,
 			humanize,
+			showStorageClass,
 		)
 	},
 }
@@ -102,6 +109,7 @@ func List(
 	// flags
 	showEtag bool,
 	humanize bool,
+	showStorageClass bool,
 ) error {
 	srcurl, err := url.New(src)
 	if err != nil {
@@ -123,9 +131,10 @@ func List(
 		}
 
 		msg := ListMessage{
-			Object:        object,
-			showEtag:      showEtag,
-			showHumanized: humanize,
+			Object:           object,
+			showEtag:         showEtag,
+			showHumanized:    humanize,
+			showStorageClass: showStorageClass,
 		}
 
 		log.Info(msg)
@@ -138,8 +147,9 @@ func List(
 type ListMessage struct {
 	Object *storage.Object `json:"object"`
 
-	showEtag      bool
-	showHumanized bool
+	showEtag         bool
+	showHumanized    bool
+	showStorageClass bool
 }
 
 // humanize is a helper function to humanize bytes.
@@ -178,10 +188,15 @@ func (l ListMessage) String() string {
 		return s
 	}
 
+	stclass := ""
+	if l.showStorageClass {
+		stclass = fmt.Sprintf("%v", l.Object.StorageClass)
+	}
+
 	s := fmt.Sprintf(
 		listFormat,
 		l.Object.ModTime.Format(dateFormat),
-		l.Object.StorageClass.ShortCode(),
+		stclass,
 		etag,
 		l.humanize(),
 		l.Object.URL.Relative(),
diff --git a/storage/s3.go b/storage/s3.go
index 729549b..5d9a0c1 100644
--- a/storage/s3.go
+++ b/storage/s3.go
@@ -12,6 +12,7 @@ import (
 	"strconv"
 	"strings"
 	"sync"
+	"time"
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/aws/awserr"
@@ -156,6 +157,10 @@ func (s *S3) listObjectsV2(ctx context.Context, url *url.URL) <-chan *Object {
 		defer close(objCh)
 		objectFound := false
 
+		// keep track of unix timestamp, which is used not to iterate
+		// files, which have already been passed through
+		now := time.Now().UTC().Unix()
+
 		err := s.api.ListObjectsV2PagesWithContext(ctx, &listInput, func(p *s3.ListObjectsV2Output, lastPage bool) bool {
 			for _, c := range p.CommonPrefixes {
 				prefix := aws.StringValue(c.Prefix)
@@ -188,6 +193,12 @@ func (s *S3) listObjectsV2(ctx context.Context, url *url.URL) <-chan *Object {
 				newurl.Path = aws.StringValue(c.Key)
 				etag := aws.StringValue(c.ETag)
 				mod := aws.TimeValue(c.LastModified)
+
+				if mod.UTC().Unix() > now {
+					objectFound = true
+					continue
+				}
+
 				objCh <- &Object{
 					URL:          newurl,
 					Etag:         strings.Trim(etag, `"`),
@@ -234,6 +245,10 @@ func (s *S3) listObjects(ctx context.Context, url *url.URL) <-chan *Object {
 		defer close(objCh)
 		objectFound := false
 
+		// keep track of unix timestamp, which is used not to iterate
+		// files, which have already been passed through
+		now := time.Now().UTC().Unix()
+
 		err := s.api.ListObjectsPagesWithContext(ctx, &listInput, func(p *s3.ListObjectsOutput, lastPage bool) bool {
 			for _, c := range p.CommonPrefixes {
 				prefix := aws.StringValue(c.Prefix)
@@ -266,6 +281,12 @@ func (s *S3) listObjects(ctx context.Context, url *url.URL) <-chan *Object {
 				newurl.Path = aws.StringValue(c.Key)
 				etag := aws.StringValue(c.ETag)
 				mod := aws.TimeValue(c.LastModified)
+
+				if mod.UTC().Unix() > now {
+					objectFound = true
+					continue
+				}
+
 				objCh <- &Object{
 					URL:          newurl,
 					Etag:         strings.Trim(etag, `"`),
diff --git a/storage/storage.go b/storage/storage.go
index f8e9845..22aa23e 100644
--- a/storage/storage.go
+++ b/storage/storage.go
@@ -161,30 +161,6 @@ func (b Bucket) JSON() string {
 // StorageClass represents the storage used to store an object.
 type StorageClass string
 
-// ShortCode returns the short code of Amazon S3 Storage Class.
-func (s StorageClass) ShortCode() string {
-	var code string
-	switch s {
-	case "STANDARD":
-		code = "ST"
-	case "REDUCED_REDUNDANCY":
-		code = "RR"
-	case "STANDARD_IA":
-		code = "SI"
-	case "ONEZONE_IA":
-		code = "OI"
-	case "INTELLIGENT_TIERING":
-		code = "IT"
-	case "GLACIER":
-		code = "GL"
-	case "DEEP_ARCHIVE":
-		code = "DA"
-	default:
-		code = " ?"
-	}
-	return code
-}
-
 func (s StorageClass) IsGlacier() bool {
 	return s == "GLACIER"
 }


Test patch:
diff --git a/e2e/cp_test.go b/e2e/cp_test.go
index fe077e2..a7d06a4 100644
--- a/e2e/cp_test.go
+++ b/e2e/cp_test.go
@@ -838,7 +838,6 @@ func TestCopyDirBackslashedToS3(t *testing.T) {
 
 	result.Assert(t, icmd.Success)
 
-
 	assertLines(t, result.Stdout(), map[int]compareFunc{
 		0: equals(`cp %v/readme.md %vreadme.md`, srcpath, dstpath),
 		1: equals(`cp %v/t\est/filetest.txt %vt\est/filetest.txt`, srcpath, dstpath),
diff --git a/e2e/ls_test.go b/e2e/ls_test.go
index 9a34c58..8524df0 100644
--- a/e2e/ls_test.go
+++ b/e2e/ls_test.go
@@ -139,6 +139,26 @@ func TestListSingleWildcardS3Object(t *testing.T) {
 	}, alignment(true))
 }
 
+// ls -s bucket/object
+func TestListS3ObjectsWithDashS(t *testing.T) {
+	t.Parallel()
+
+	bucket := s3BucketFromTestName(t)
+
+	s3client, s5cmd, cleanup := setup(t)
+	defer cleanup()
+
+	createBucket(t, s3client, bucket)
+	putFile(t, s3client, bucket, "testfile1.txt", "this is a file content")
+
+	cmd := s5cmd("ls -s", "s3://"+bucket+"/testfile1.txt")
+	result := icmd.RunCmd(cmd)
+
+	result.Assert(t, icmd.Success)
+
+	// TODO: test if full form of storage class is displayed (it can be done when and if gofakes3 supports storage classes)
+}
+
 // ls bucket/*/object*.ext
 func TestListMultipleWildcardS3Object(t *testing.T) {
 	t.Parallel()
@@ -243,8 +263,8 @@ func TestListS3ObjectsAndFolders(t *testing.T) {
 		3: suffix("DIR d/"),
 		4: suffix("DIR e/"),
 		5: suffix("DIR f/"),
-		6: suffix("? 298 report.gz"),
-		7: suffix("? 302 testfile1.txt"),
+		6: suffix("298 report.gz"),
+		7: suffix("302 testfile1.txt"),
 	}, alignment(true))
 }
 
@@ -271,7 +291,7 @@ func TestListS3ObjectsAndFoldersWithPrefix(t *testing.T) {
 
 	assertLines(t, result.Stdout(), map[int]compareFunc{
 		0: suffix("DIR t/"),
-		1: suffix("? 302 testfile1.txt"),
+		1: suffix("302 testfile1.txt"),
 	}, alignment(true))
 }
 
@@ -342,8 +362,8 @@ func TestListS3ObjectsWithDashE(t *testing.T) {
 	result.Assert(t, icmd.Success)
 
 	assertLines(t, result.Stdout(), map[int]compareFunc{
-		0: match(`^ \? \w+ \d+ testfile1.txt$`),
-		1: match(`^ \? \w+ \d+ testfile2.txt$`),
+		0: match(`^ \w+ \d+ testfile1.txt$`),
+		1: match(`^ \w+ \d+ testfile2.txt$`),
 	}, trimMatch(dateRe), alignment(true))
 }
 
@@ -367,7 +387,7 @@ func TestListS3ObjectsWithDashH(t *testing.T) {
 	result.Assert(t, icmd.Success)
 
 	assertLines(t, result.Stdout(), map[int]compareFunc{
-		0: match(`^ \? 215.1K testfile1.txt$`),
-		1: match(`^ \? 264.0K testfile2.txt$`),
+		0: match(`^ 215.1K testfile1.txt$`),
+		1: match(`^ 264.0K testfile2.txt$`),
 	}, trimMatch(dateRe), alignment(true))
 }
diff --git a/storage/s3_test.go b/storage/s3_test.go
index c189422..b64c636 100644
--- a/storage/s3_test.go
+++ b/storage/s3_test.go
@@ -3,10 +3,14 @@ package storage
 import (
 	"context"
 	"fmt"
+	"io/ioutil"
+	"math/rand"
 	"net/http"
 	urlpkg "net/url"
 	"os"
+	"strings"
 	"testing"
+	"time"
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/aws/awserr"
@@ -14,6 +18,7 @@ import (
 	"github.com/aws/aws-sdk-go/awstesting/unit"
 	"github.com/aws/aws-sdk-go/service/s3"
 	"github.com/google/go-cmp/cmp"
+	"github.com/stretchr/testify/assert"
 
 	"github.com/peak/s5cmd/storage/url"
 )
@@ -398,3 +403,85 @@ func TestS3Retry(t *testing.T) {
 		})
 	}
 }
+
+func TestS3listObjectsV2(t *testing.T) {
+	const (
+		numObjectsToReturn = 10100
+		numObjectsToIgnore = 1127
+
+		pre = "s3://bucket/key"
+	)
+
+	u, err := url.New(pre)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
+	mapReturnObjNameToModtime := map[string]time.Time{}
+	mapIgnoreObjNameToModtime := map[string]time.Time{}
+
+	s3objs := make([]*s3.Object, 0, numObjectsToIgnore+numObjectsToReturn)
+
+	for i := 0; i < numObjectsToReturn; i++ {
+		fname := fmt.Sprintf("%s/%d", pre, i)
+		now := time.Now()
+
+		mapReturnObjNameToModtime[pre+"/"+fname] = now
+		s3objs = append(s3objs, &s3.Object{
+			Key:          aws.String("key/" + fname),
+			LastModified: aws.Time(now),
+		})
+	}
+
+	for i := 0; i < numObjectsToIgnore; i++ {
+		fname := fmt.Sprintf("%s/%d", pre, numObjectsToReturn+i)
+		later := time.Now().Add(time.Second * 10)
+
+		mapIgnoreObjNameToModtime[pre+"/"+fname] = later
+		s3objs = append(s3objs, &s3.Object{
+			Key:          aws.String("key/" + fname),
+			LastModified: aws.Time(later),
+		})
+	}
+
+	// shuffle the objects array to remove possible assumptions about how objects
+	// are stored.
+	rand.Shuffle(len(s3objs), func(i, j int) {
+		s3objs[i], s3objs[j] = s3objs[j], s3objs[i]
+	})
+
+	mockApi := s3.New(unit.Session)
+
+	mockApi.Handlers.Unmarshal.Clear()
+	mockApi.Handlers.UnmarshalMeta.Clear()
+	mockApi.Handlers.UnmarshalError.Clear()
+	mockApi.Handlers.Send.Clear()
+
+	mockApi.Handlers.Send.PushBack(func(r *request.Request) {
+
+		r.HTTPResponse = &http.Response{
+			StatusCode: http.StatusOK,
+			Body:       ioutil.NopCloser(strings.NewReader("")),
+		}
+
+		r.Data = &s3.ListObjectsV2Output{
+			Contents: s3objs,
+		}
+
+	})
+
+	mockS3 := &S3{
+		api: mockApi,
+	}
+
+	ouputCh := mockS3.listObjectsV2(context.Background(), u)
+
+	for obj := range ouputCh {
+		if _, ok := mapReturnObjNameToModtime[obj.String()]; ok {
+			delete(mapReturnObjNameToModtime, obj.String())
+			continue
+		}
+		t.Errorf("%v should not have been returned\n", obj)
+	}
+	assert.Equal(t, len(mapReturnObjNameToModtime), 0)
+}