storage/fs,minio: fix list module with same prefix (#1312)

go.mod

@@ -50,6 +50,7 @@ require (
 	go.etcd.io/etcd v0.0.0-20190215181705-784daa04988c
 	go.mongodb.org/mongo-driver v1.0.0
 	go.opencensus.io v0.17.0
+	golang.org/x/mod v0.1.0
 	golang.org/x/oauth2 v0.0.0-20180620175406-ef147856a6dd
 	golang.org/x/sync v0.0.0-20190423024810-112230192c58
 	google.golang.org/api v0.0.0-20180910000450-7ca32eb868bf

go.sum

@@ -248,6 +248,10 @@ golang.org/x/crypto v0.0.0-20181029103014-dab2b1051b5d/go.mod h1:6SG95UA2DQfeDnf
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20190426145343-a29dc8fdc734 h1:p/H982KKEjUnLJkM3tt/LemDnOc1GiZL5FCVlORJ5zo=
 golang.org/x/crypto v0.0.0-20190426145343-a29dc8fdc734/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
+golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529 h1:iMGN4xG0cnqj3t+zOM8wUB0BiPKHEwSxEZCvzcbZuvk=
+golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
+golang.org/x/mod v0.1.0 h1:sfUMP1Gu8qASkorDVjnMuvgJzwFbTZSeXFiGBYAVdl4=
+golang.org/x/mod v0.1.0/go.mod h1:0QHyrYULN0/3qlju5TqG8bIK38QM8yzMo5ekMj3DlcY=
 golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180811021610-c39426892332/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180906233101-161cd47e91fd h1:nTDtHvHSdCn1m6ITfMRqtOd/9+7a3s8RBNOZ3eYZzJA=

pkg/storage/compliance/tests.go

@@ -63,40 +63,42 @@ func testNotFound(t *testing.T, b storage.Backend) {
 func testListSuffix(t *testing.T, b storage.Backend) {
 	ctx := context.Background()
 
-	otherMod := "github.com/one/two-other"
-	mock := getMockModule()
-	err := b.Save(
-		ctx,
-		otherMod,
-		"v0.9.0",
-		mock.Mod,
-		mock.Zip,
-		mock.Info,
-	)
-	require.NoError(t, err, "Save for storage failed")
-	modname := "github.com/one/two"
-	versions := []string{"v1.1.0", "v1.2.0", "v1.3.0"}
-	for _, version := range versions {
-		mock := getMockModule()
-		err := b.Save(
-			ctx,
-			modname,
-			version,
-			mock.Mod,
-			mock.Zip,
-			mock.Info,
-		)
-		require.NoError(t, err, "Save for storage failed")
+	modVers := map[string][]string{
+		"github.com/one/two":       {"v1.1.0", "v1.2.0", "v1.3.0"},
+		"github.com/one/two/v2":    {"v2.1.0"},
+		"github.com/one/two-other": {"v0.9.0"},
+		"github.com/one":           {}, // not a module but a valid query, so no versions
+	}
+	for modname, versions := range modVers {
+		for _, version := range versions {
+			mock := getMockModule()
+			err := b.Save(
+				ctx,
+				modname,
+				version,
+				mock.Mod,
+				mock.Zip,
+				mock.Info,
+			)
+			require.NoError(t, err, "Save for storage failed")
+		}
 	}
 	defer func() {
-		b.Delete(ctx, otherMod, "v0.9.0")
-		for _, ver := range versions {
-			b.Delete(ctx, modname, ver)
+		for modname, versions := range modVers {
+			for _, version := range versions {
+				b.Delete(ctx, modname, version)
+			}
 		}
 	}()
-	retVersions, err := b.List(ctx, modname)
-	require.NoError(t, err)
-	require.Equal(t, versions, retVersions)
+	for modname, versions := range modVers {
+		retVersions, err := b.List(ctx, modname)
+		require.NoError(t, err)
+		if len(versions) == 0 {
+			require.Empty(t, retVersions)
+		} else {
+			require.Equal(t, versions, retVersions)
+		}
+	}
 }
 
 // testList tests that a storage Backend returns

pkg/storage/fs/lister.go

@@ -3,10 +3,12 @@ package fs
 import (
 	"context"
 	"os"
+	"strings"
 
 	"github.com/gomods/athens/pkg/errors"
 	"github.com/gomods/athens/pkg/observ"
 	"github.com/spf13/afero"
+	"golang.org/x/mod/semver"
 )
 
 func (l *storageImpl) List(ctx context.Context, module string) ([]string, error) {
@@ -24,8 +26,12 @@ func (l *storageImpl) List(ctx context.Context, module string) ([]string, error)
 	}
 	ret := []string{}
 	for _, fileInfo := range fileInfos {
-		if fileInfo.IsDir() {
-			ret = append(ret, fileInfo.Name())
+		if !fileInfo.IsDir() {
+			continue
+		}
+		ver := fileInfo.Name()
+		if v := semver.Canonical(ver); v != "" && strings.HasPrefix(ver, v) {
+			ret = append(ret, ver)
 		}
 	}
 	return ret, nil

pkg/storage/minio/lister.go

@@ -2,6 +2,7 @@ package minio
 
 import (
 	"context"
+	"fmt"
 	"sort"
 	"strings"
 
@@ -13,8 +14,6 @@ func (l *storageImpl) List(ctx context.Context, module string) ([]string, error)
 	const op errors.Op = "minio.List"
 	ctx, span := observ.StartSpan(ctx, op.String())
 	defer span.End()
-	dict := make(map[string]struct{})
-
 	doneCh := make(chan struct{})
 	defer close(doneCh)
 	searchPrefix := module + "/"
@@ -23,21 +22,18 @@ func (l *storageImpl) List(ctx context.Context, module string) ([]string, error)
 	if err != nil {
 		return nil, errors.E(op, err, errors.M(module))
 	}
+	ret := []string{}
 	for _, object := range objectCh.Contents {
 		if object.Err != nil {
 			return nil, errors.E(op, object.Err, errors.M(module))
 		}
 		parts := strings.Split(object.Key, "/")
 		ver := parts[len(parts)-2]
-		if _, ok := dict[ver]; !ok {
-			dict[ver] = struct{}{}
+		goModKey := fmt.Sprintf("%s/go.mod", l.versionLocation(module, ver))
+		if goModKey == object.Key {
+			ret = append(ret, ver)
 		}
 	}
-
-	ret := []string{}
-	for ver := range dict {
-		ret = append(ret, ver)
-	}
 	sort.Strings(ret)
 	return ret, nil
 }