From 0d94af25034eae47f4408a6830a5cccfb78c74b1 Mon Sep 17 00:00:00 2001 From: Marwan Sulaiman Date: Sat, 5 Jan 2019 15:04:46 -0500 Subject: [PATCH] catalog: clean up unused interfaces (#1027) * catalog: clean up unused interfaces * fix op name --- cmd/proxy/actions/app_proxy.go | 3 +-- .../proxy/actions/catalog.go | 26 +++++++++---------- pkg/download/addons/with_pool.go | 19 -------------- pkg/download/addons/with_pool_test.go | 5 ---- pkg/download/protocol.go | 17 ------------ pkg/storage/azureblob/cataloger.go | 15 ----------- pkg/storage/backend.go | 1 - pkg/storage/compliance/tests.go | 26 +++++-------------- pkg/storage/minio/cataloger.go | 16 ------------ 9 files changed, 20 insertions(+), 108 deletions(-) rename pkg/storage/handler.go => cmd/proxy/actions/catalog.go (66%) delete mode 100644 pkg/storage/azureblob/cataloger.go delete mode 100644 pkg/storage/minio/cataloger.go diff --git a/cmd/proxy/actions/app_proxy.go b/cmd/proxy/actions/app_proxy.go index b6cf25cb..86427516 100644 --- a/cmd/proxy/actions/app_proxy.go +++ b/cmd/proxy/actions/app_proxy.go @@ -23,6 +23,7 @@ func addProxyRoutes( r.HandleFunc("/healthz", healthHandler) r.HandleFunc("/readyz", getReadinessHandler(s)) r.HandleFunc("/version", versionHandler) + r.HandleFunc("/catalog", catalogHandler(s)) // Download Protocol // the download.Protocol and the stash.Stasher interfaces are composable @@ -65,7 +66,5 @@ func addProxyRoutes( handlerOpts := &download.HandlerOpts{Protocol: dp, Logger: l} download.RegisterHandlers(r, handlerOpts) - storage.RegisterHandlers(r, s) - return nil } diff --git a/pkg/storage/handler.go b/cmd/proxy/actions/catalog.go similarity index 66% rename from pkg/storage/handler.go rename to cmd/proxy/actions/catalog.go index 49f69cda..fb566b59 100644 --- a/pkg/storage/handler.go +++ b/cmd/proxy/actions/catalog.go @@ -1,4 +1,4 @@ -package storage +package actions import ( "encoding/json" @@ -8,6 +8,7 @@ import ( "github.com/gomods/athens/pkg/errors" "github.com/gomods/athens/pkg/log" "github.com/gomods/athens/pkg/paths" + "github.com/gomods/athens/pkg/storage" "github.com/gorilla/mux" ) @@ -18,16 +19,15 @@ type catalogRes struct { NextPageToken string `json:"next,omitempty"` } -// RegisterHandlers is a convenience method that registers -// all the storage routes. -func RegisterHandlers(r *mux.Router, s Backend) { - r.Handle("/catalog", CatalogHandler(s)).Methods(http.MethodGet) -} - -// CatalogHandler implements GET baseURL/catalog -func CatalogHandler(s Backend) http.Handler { - const op errors.Op = "download.CatalogHandler" +// catalogHandler implements GET baseURL/catalog +func catalogHandler(s storage.Backend) http.HandlerFunc { + const op errors.Op = "actions.CatalogHandler" + cs, isCataloger := s.(storage.Cataloger) f := func(w http.ResponseWriter, r *http.Request) { + if !isCataloger { + w.WriteHeader(errors.KindNotImplemented) + return + } lggr := log.EntryFromContext(r.Context()) vars := mux.Vars(r) token := vars["token"] @@ -38,11 +38,9 @@ func CatalogHandler(s Backend) http.Handler { return } - modulesAndVersions, newToken, err := s.Catalog(r.Context(), token, pageSize) + modulesAndVersions, newToken, err := cs.Catalog(r.Context(), token, pageSize) if err != nil { - if errors.Kind(err) != errors.KindNotImplemented { - lggr.SystemErr(errors.E(op, err)) - } + lggr.SystemErr(errors.E(op, err)) w.WriteHeader(errors.Kind(err)) return } diff --git a/pkg/download/addons/with_pool.go b/pkg/download/addons/with_pool.go index 6d36c786..605dad80 100644 --- a/pkg/download/addons/with_pool.go +++ b/pkg/download/addons/with_pool.go @@ -6,7 +6,6 @@ import ( "github.com/gomods/athens/pkg/download" "github.com/gomods/athens/pkg/errors" - "github.com/gomods/athens/pkg/paths" "github.com/gomods/athens/pkg/storage" ) @@ -128,21 +127,3 @@ func (p *withpool) Zip(ctx context.Context, mod, ver string) (io.ReadCloser, err } return zip, nil } - -func (p *withpool) Catalog(ctx context.Context, token string, pageSize int) ([]paths.AllPathParams, string, error) { - const op errors.Op = "pool.Catalog" - var modsVers []paths.AllPathParams - var nextToken string - var err error - done := make(chan struct{}, 1) - p.jobCh <- func() { - modsVers, nextToken, err = p.dp.Catalog(ctx, token, pageSize) - close(done) - } - <-done - if err != nil { - return nil, "", errors.E(op, err) - } - - return modsVers, nextToken, nil -} diff --git a/pkg/download/addons/with_pool_test.go b/pkg/download/addons/with_pool_test.go index 960d81cd..c0c840f0 100644 --- a/pkg/download/addons/with_pool_test.go +++ b/pkg/download/addons/with_pool_test.go @@ -152,11 +152,6 @@ func (m *mockDP) Zip(ctx context.Context, mod, ver string) (io.ReadCloser, error return m.zip, m.err } -// Catalog implements GET /catalog -func (m *mockDP) Catalog(ctx context.Context, token string, pageSize int) ([]paths.AllPathParams, string, error) { - return m.catalog, "", m.err -} - // Version is a helper method to get Info, GoMod, and Zip together. func (m *mockDP) Version(ctx context.Context, mod, ver string) (*storage.Version, error) { panic("skipped") diff --git a/pkg/download/protocol.go b/pkg/download/protocol.go index bc89bb34..f4acd257 100644 --- a/pkg/download/protocol.go +++ b/pkg/download/protocol.go @@ -7,7 +7,6 @@ import ( "github.com/gomods/athens/pkg/errors" "github.com/gomods/athens/pkg/observ" - "github.com/gomods/athens/pkg/paths" "github.com/gomods/athens/pkg/stash" "github.com/gomods/athens/pkg/storage" ) @@ -29,9 +28,6 @@ type Protocol interface { // Zip implements GET /{module}/@v/{version}.zip Zip(ctx context.Context, mod, ver string) (io.ReadCloser, error) - - // Catalog implements GET /catalog - Catalog(ctx context.Context, token string, pageSize int) ([]paths.AllPathParams, string, error) } // Wrapper helps extend the main protocol's functionality with addons. @@ -177,19 +173,6 @@ func (p *protocol) Zip(ctx context.Context, mod, ver string) (io.ReadCloser, err return zip, nil } -func (p *protocol) Catalog(ctx context.Context, token string, pageSize int) ([]paths.AllPathParams, string, error) { - const op errors.Op = "protocol.Catalog" - ctx, span := observ.StartSpan(ctx, op.String()) - defer span.End() - modulesAndVersions, newToken, err := p.storage.Catalog(ctx, token, pageSize) - - if err != nil { - return nil, "", errors.E(op, err) - } - - return modulesAndVersions, newToken, err -} - // union concatenates two version lists and removes duplicates func union(list1, list2 []string) []string { if list1 == nil { diff --git a/pkg/storage/azureblob/cataloger.go b/pkg/storage/azureblob/cataloger.go deleted file mode 100644 index 0f75bb6e..00000000 --- a/pkg/storage/azureblob/cataloger.go +++ /dev/null @@ -1,15 +0,0 @@ -package azureblob - -import ( - "context" - - "github.com/gomods/athens/pkg/errors" - "github.com/gomods/athens/pkg/paths" -) - -// Catalog implements the (./pkg/storage).Cataloger interface -// It returns a list of modules and versions contained in the storage -func (s *Storage) Catalog(ctx context.Context, token string, pageSize int) ([]paths.AllPathParams, string, error) { - const op errors.Op = "azure.Catalog" - return nil, "", errors.E(op, errors.KindNotImplemented) -} diff --git a/pkg/storage/backend.go b/pkg/storage/backend.go index 285e92f0..9c789afb 100644 --- a/pkg/storage/backend.go +++ b/pkg/storage/backend.go @@ -7,5 +7,4 @@ type Backend interface { Checker Saver Deleter - Cataloger } diff --git a/pkg/storage/compliance/tests.go b/pkg/storage/compliance/tests.go index 0407b6f9..cc8644b3 100644 --- a/pkg/storage/compliance/tests.go +++ b/pkg/storage/compliance/tests.go @@ -23,9 +23,7 @@ func RunTests(t *testing.T, b storage.Backend, clearBackend func() error) { testList(t, b) testDelete(t, b) testGet(t, b) - if isCatalogImplemented(b) { - testCatalog(t, b) - } + testCatalog(t, b) } // testNotFound ensures that a storage Backend @@ -131,6 +129,10 @@ func testDelete(t *testing.T, b storage.Backend) { } func testCatalog(t *testing.T, b storage.Backend) { + cs, ok := b.(storage.Cataloger) + if !ok { + t.Skip() + } ctx := context.Background() mock := getMockModule() @@ -140,19 +142,13 @@ func testCatalog(t *testing.T, b storage.Backend) { ver := fmt.Sprintf("v1.2.%04d", i) b.Save(ctx, modname, ver, mock.Mod, bytes.NewReader(zipBts), mock.Info) } - defer func() { - for i := 0; i < 1005; i++ { - ver := fmt.Sprintf("v1.2.%04d", i) - b.Delete(ctx, modname, ver) - } - }() - allres, next, err := b.Catalog(ctx, "", 1001) + allres, next, err := cs.Catalog(ctx, "", 1001) require.NoError(t, err) require.Equal(t, 1001, len(allres)) - res, next, err := b.Catalog(ctx, next, 50) + res, next, err := cs.Catalog(ctx, next, 50) allres = append(allres, res...) require.NoError(t, err) require.Equal(t, 4, len(res)) @@ -180,11 +176,3 @@ func getMockModule() *storage.Version { Zip: ioutil.NopCloser(bytes.NewReader([]byte("789"))), } } - -func isCatalogImplemented(b storage.Backend) bool { - ctx := context.Background() - if _, _, err := b.Catalog(ctx, "", 1); errors.Kind(err) == errors.KindNotImplemented { - return false - } - return true -} diff --git a/pkg/storage/minio/cataloger.go b/pkg/storage/minio/cataloger.go deleted file mode 100644 index 372ad012..00000000 --- a/pkg/storage/minio/cataloger.go +++ /dev/null @@ -1,16 +0,0 @@ -package minio - -import ( - "context" - - "github.com/gomods/athens/pkg/paths" - - "github.com/gomods/athens/pkg/errors" -) - -// Catalog implements the (./pkg/storage).Cataloger interface -// It returns a list of modules and versions contained in the storage -func (s *storageImpl) Catalog(ctx context.Context, token string, pageSize int) ([]paths.AllPathParams, string, error) { - const op errors.Op = "minio.Catalog" - return nil, "", errors.E(op, errors.KindNotImplemented) -}