From b6bcf0e191dd028ce3caba58764482deb2da7493 Mon Sep 17 00:00:00 2001 From: Yi Tang Date: Fri, 2 Aug 2019 01:16:52 +0800 Subject: [PATCH] storage/fs,minio: fix list module with same prefix (#1312) --- go.mod | 1 + go.sum | 4 +++ pkg/storage/compliance/tests.go | 62 +++++++++++++++++---------------- pkg/storage/fs/lister.go | 10 ++++-- pkg/storage/minio/lister.go | 14 +++----- 5 files changed, 50 insertions(+), 41 deletions(-) diff --git a/go.mod b/go.mod index 95070770..e5a1105e 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 6881904d..48578cd3 100644 --- a/go.sum +++ b/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= diff --git a/pkg/storage/compliance/tests.go b/pkg/storage/compliance/tests.go index 48c6fb45..c843c645 100644 --- a/pkg/storage/compliance/tests.go +++ b/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 diff --git a/pkg/storage/fs/lister.go b/pkg/storage/fs/lister.go index c832ef61..04458035 100644 --- a/pkg/storage/fs/lister.go +++ b/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 diff --git a/pkg/storage/minio/lister.go b/pkg/storage/minio/lister.go index 08a6e0ed..e573af9e 100644 --- a/pkg/storage/minio/lister.go +++ b/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 }