diff --git a/cmd/olympus/actions/storage.go b/cmd/olympus/actions/storage.go index bae7b693..984d97b8 100644 --- a/cmd/olympus/actions/storage.go +++ b/cmd/olympus/actions/storage.go @@ -12,8 +12,8 @@ import ( "github.com/spf13/afero" ) -// GetStorage returns storage.BackendConnector implementation -func GetStorage() (storage.BackendConnector, error) { +// GetStorage returns storage.Backend implementation +func GetStorage() (storage.Backend, error) { storageType := env.StorageTypeWithDefault("memory") switch storageType { case "memory": @@ -27,19 +27,19 @@ func GetStorage() (storage.BackendConnector, error) { if err != nil { return nil, fmt.Errorf("could not create new storage from os fs (%s)", err) } - return storage.NoOpBackendConnector(s), nil + return s, nil case "mongo": mongoURI, err := env.MongoURI() if err != nil { return nil, err } - return mongo.NewStorage(mongoURI), nil + return mongo.NewStorage(mongoURI) case "postgres", "sqlite", "cockroach", "mysql": connectionName, err := env.RdbmsName() if err != nil { return nil, err } - return rdbms.NewRDBMSStorage(connectionName), nil + return rdbms.NewRDBMSStorage(connectionName) default: return nil, fmt.Errorf("storage type %s is unknown", storageType) } diff --git a/cmd/olympus/main.go b/cmd/olympus/main.go index bed91880..f40ff55b 100644 --- a/cmd/olympus/main.go +++ b/cmd/olympus/main.go @@ -48,8 +48,5 @@ func getStorage() (storage.Backend, error) { if err != nil { return nil, fmt.Errorf("error creating storage (%s)", err) } - if err := storage.Connect(); err != nil { - return nil, fmt.Errorf("unable to connect to backing store: %v", err) - } return storage, nil } diff --git a/cmd/proxy/actions/app.go b/cmd/proxy/actions/app.go index 1d2d975b..4c16c4fa 100644 --- a/cmd/proxy/actions/app.go +++ b/cmd/proxy/actions/app.go @@ -66,10 +66,6 @@ func App() (*buffalo.App, error) { err = fmt.Errorf("error getting storage configuration (%s)", err) return nil, err } - if err := store.Connect(); err != nil { - err = fmt.Errorf("error connecting to storage (%s)", err) - return nil, err - } worker, err := getWorker(ctx, store, mf) if err != nil { diff --git a/cmd/proxy/actions/storage.go b/cmd/proxy/actions/storage.go index 6c71a455..a1fbe750 100644 --- a/cmd/proxy/actions/storage.go +++ b/cmd/proxy/actions/storage.go @@ -17,7 +17,7 @@ import ( ) // GetStorage returns storage backend based on env configuration -func GetStorage() (storage.BackendConnector, error) { +func GetStorage() (storage.Backend, error) { storageType := env.StorageTypeWithDefault("memory") var storageRoot string var err error @@ -30,7 +30,7 @@ func GetStorage() (storage.BackendConnector, error) { if err != nil { return nil, err } - return mongo.NewStorage(storageRoot), nil + return mongo.NewStorage(storageRoot) case "disk": storageRoot, err = env.DiskRoot() if err != nil { @@ -40,13 +40,13 @@ func GetStorage() (storage.BackendConnector, error) { if err != nil { return nil, fmt.Errorf("could not create new storage from os fs (%s)", err) } - return storage.NoOpBackendConnector(s), nil + return s, nil case "postgres", "sqlite", "cockroach", "mysql": storageRoot, err = env.RdbmsName() if err != nil { return nil, err } - return rdbms.NewRDBMSStorage(storageRoot), nil + return rdbms.NewRDBMSStorage(storageRoot) case "minio": endpoint, err := env.MinioEndpoint() if err != nil { @@ -65,8 +65,7 @@ func GetStorage() (storage.BackendConnector, error) { if useSSLVar := env.MinioSSLWithDefault("yes"); strings.ToLower(useSSLVar) == "no" { useSSL = false } - s, err := minio.NewStorage(endpoint, accessKeyID, secretAccessKey, bucketName, useSSL) - return storage.NoOpBackendConnector(s), err + return minio.NewStorage(endpoint, accessKeyID, secretAccessKey, bucketName, useSSL) case "gcp": return gcp.New(context.Background()) default: diff --git a/pkg/storage/backendconnector.go b/pkg/storage/backendconnector.go deleted file mode 100644 index b35cf845..00000000 --- a/pkg/storage/backendconnector.go +++ /dev/null @@ -1,48 +0,0 @@ -package storage - -import ( - "context" - "io" -) - -// BackendConnector is a regular storage backend with Connect functionality -type BackendConnector interface { - Backend - Connect() error -} - -type noOpConnectedBackend struct { - backend Backend -} - -// NoOpBackendConnector wraps storage backend with Connect functionality -func NoOpBackendConnector(b Backend) BackendConnector { - return noOpConnectedBackend{backend: b} -} - -func (n noOpConnectedBackend) Connect() error { - return nil -} - -func (n noOpConnectedBackend) Exists(ctx context.Context, module, version string) bool { - return n.backend.Exists(ctx, module, version) -} - -func (n noOpConnectedBackend) Info(ctx context.Context, module, vsn string) ([]byte, error) { - return n.backend.Info(ctx, module, vsn) -} -func (n noOpConnectedBackend) GoMod(ctx context.Context, module, vsn string) ([]byte, error) { - return n.backend.GoMod(ctx, module, vsn) -} -func (n noOpConnectedBackend) Zip(ctx context.Context, module, vsn string) (io.ReadCloser, error) { - return n.backend.Zip(ctx, module, vsn) -} -func (n noOpConnectedBackend) List(ctx context.Context, module string) ([]string, error) { - return n.backend.List(ctx, module) -} -func (n noOpConnectedBackend) Save(ctx context.Context, module, version string, mod []byte, zip io.Reader, info []byte) error { - return n.backend.Save(ctx, module, version, mod, zip, info) -} -func (n noOpConnectedBackend) Delete(ctx context.Context, module, version string) error { - return n.backend.Delete(ctx, module, version) -} diff --git a/pkg/storage/connector.go b/pkg/storage/connector.go deleted file mode 100644 index 0c21a623..00000000 --- a/pkg/storage/connector.go +++ /dev/null @@ -1,7 +0,0 @@ -package storage - -// Connector connects storage to its backing engine -type Connector interface { - // Connect connects storage to its backing engine - Connect() error -} diff --git a/pkg/storage/gcp/gcp.go b/pkg/storage/gcp/gcp.go index 2c58acf7..35bffc75 100644 --- a/pkg/storage/gcp/gcp.go +++ b/pkg/storage/gcp/gcp.go @@ -88,8 +88,3 @@ func (s *Storage) BaseURL() *url.URL { func (s *Storage) Close() error { return s.closeStorage() } - -// Connect is noop. -func (s *Storage) Connect() error { - return nil -} diff --git a/pkg/storage/mem/mem.go b/pkg/storage/mem/mem.go index 6124b09b..2df63f70 100644 --- a/pkg/storage/mem/mem.go +++ b/pkg/storage/mem/mem.go @@ -12,11 +12,11 @@ import ( var ( l sync.Mutex - memStorage storage.BackendConnector + memStorage storage.Backend ) // NewStorage creates new in-memory storage using the afero.NewMemMapFs() in memory file system -func NewStorage() (storage.BackendConnector, error) { +func NewStorage() (storage.Backend, error) { const op errors.Op = "mem.NewStorage" l.Lock() defer l.Unlock() @@ -35,6 +35,5 @@ func NewStorage() (storage.BackendConnector, error) { if err != nil { return nil, errors.E(op, fmt.Errorf("could not create storage from memory fs (%s)", err)) } - memStorage = storage.NoOpBackendConnector(s) - return memStorage, nil + return s, nil } diff --git a/pkg/storage/mongo/all_test.go b/pkg/storage/mongo/all_test.go index c0e3f7ab..59e92cb5 100644 --- a/pkg/storage/mongo/all_test.go +++ b/pkg/storage/mongo/all_test.go @@ -9,12 +9,13 @@ import ( type MongoTests struct { suite.Suite - storage storage.BackendConnector + storage storage.Backend } func (d *MongoTests) SetupTest() { - store := NewStorage("mongodb://127.0.0.1:27017") - store.Connect() + store, err := NewStorage("mongodb://127.0.0.1:27017") + + d.NoError(err) store.s.DB(store.d).C(store.c).RemoveAll(nil) d.storage = store diff --git a/pkg/storage/mongo/mongo.go b/pkg/storage/mongo/mongo.go index 8004c8cf..d86a91c3 100644 --- a/pkg/storage/mongo/mongo.go +++ b/pkg/storage/mongo/mongo.go @@ -16,18 +16,25 @@ type ModuleStore struct { url string } -// NewStorage returns an unconnected Mongo backed storage -// that satisfies the Backend interface. You must call -// Connect() on the returned store before using it. -func NewStorage(url string) *ModuleStore { - return &ModuleStore{url: url} +// NewStorage returns a connected Mongo backed storage +// that satisfies the Backend interface. +func NewStorage(url string) (*ModuleStore, error) { + const op errors.Op = "fs.NewStorage" + ms := &ModuleStore{url: url} + + err := ms.connect() + if err != nil { + return nil, errors.E(op, err) + } + return ms, nil + } -// Connect conntect the the newly created mongo backend. -func (m *ModuleStore) Connect() error { - const op errors.Op = "mongo.Connect" +func (m *ModuleStore) connect() error { + const op errors.Op = "mongo.connect" timeout := env.MongoConnectionTimeoutSecWithDefault(1) s, err := mgo.DialWithTimeout(m.url, timeout) + if err != nil { return errors.E(op, err) } diff --git a/pkg/storage/mongo/mongo_test.go b/pkg/storage/mongo/mongo_test.go index d595e4d1..fd11f1cf 100644 --- a/pkg/storage/mongo/mongo_test.go +++ b/pkg/storage/mongo/mongo_test.go @@ -3,9 +3,9 @@ package mongo func (m *MongoTests) TestNewMongoStorage() { r := m.Require() url := "mongodb://127.0.0.1:27017" - getterSaver := NewStorage(url) - getterSaver.Connect() + getterSaver, err := NewStorage(url) + r.NoError(err) r.NotNil(getterSaver.c) r.NotNil(getterSaver.d) r.NotNil(getterSaver.s) diff --git a/pkg/storage/mongo/test_suite.go b/pkg/storage/mongo/test_suite.go index eb2ee8fe..44733e61 100644 --- a/pkg/storage/mongo/test_suite.go +++ b/pkg/storage/mongo/test_suite.go @@ -33,12 +33,12 @@ func newTestStore() (*ModuleStore, error) { return nil, err } - mongoStore := NewStorage(muri) - if mongoStore == nil { - return nil, fmt.Errorf("Mongo storage is nil") + mongoStore, err := NewStorage(muri) + if err != nil { + return nil, fmt.Errorf("Not able to connect to mongo storage") } - return mongoStore, mongoStore.Connect() + return mongoStore, nil } // Storage retrieves initialized storage backend diff --git a/pkg/storage/rdbms/all_test.go b/pkg/storage/rdbms/all_test.go index 28d30820..1d143775 100644 --- a/pkg/storage/rdbms/all_test.go +++ b/pkg/storage/rdbms/all_test.go @@ -9,7 +9,7 @@ import ( type RDBMSTestSuite struct { *suite.Model - storage storage.BackendConnector + storage storage.Backend } func (rd *RDBMSTestSuite) SetupTest() { diff --git a/pkg/storage/rdbms/rdbms.go b/pkg/storage/rdbms/rdbms.go index 6faec133..816982c2 100644 --- a/pkg/storage/rdbms/rdbms.go +++ b/pkg/storage/rdbms/rdbms.go @@ -2,6 +2,7 @@ package rdbms import ( "github.com/gobuffalo/pop" + "github.com/gomods/athens/pkg/errors" ) // ModuleStore represents a rdbms(postgres, mysql, sqlite, cockroachdb) backed storage backend. @@ -10,28 +11,31 @@ type ModuleStore struct { connectionName string // settings name from database.yml } -// NewRDBMSStorage returns an unconnected RDBMS Module Storage -// that satisfies the Storage interface. You must call -// Connect() on the returned store before using it. -// connectionName -func NewRDBMSStorage(connectionName string) *ModuleStore { - return &ModuleStore{ +// NewRDBMSStorage returns a connected RDBMS Module Storage +// that satisfies the Storage interface. +func NewRDBMSStorage(connectionName string) (*ModuleStore, error) { + const op errors.Op = "rdbms.NewRDBMSStorage" + + ms := &ModuleStore{ connectionName: connectionName, } + err := ms.connect() + if err != nil { + return nil, errors.E(op, err) + } + return ms, nil } // NewRDBMSStorageWithConn returns a connected RDBMS Module Storage -// that satisfies the Storage interface. You must call -// Connect() on the returned store before using it. -// connectionName +// that satisfies the Storage interface. func NewRDBMSStorageWithConn(connection *pop.Connection) *ModuleStore { - return &ModuleStore{ + ms := &ModuleStore{ conn: connection, } + return ms } -// Connect creates connection to rdmbs backend. -func (r *ModuleStore) Connect() error { +func (r *ModuleStore) connect() error { c, err := pop.Connect(r.connectionName) if err != nil { return err diff --git a/pkg/storage/rdbms/rdbms_test.go b/pkg/storage/rdbms/rdbms_test.go index 3af6b1d6..34d11652 100644 --- a/pkg/storage/rdbms/rdbms_test.go +++ b/pkg/storage/rdbms/rdbms_test.go @@ -3,8 +3,8 @@ package rdbms func (rd *RDBMSTestSuite) TestNewRDBMSStorage() { r := rd.Require() e := "development" - getterSaver := NewRDBMSStorage(e) - getterSaver.Connect() + getterSaver, err := NewRDBMSStorage(e) + r.NoError(err) r.NotNil(getterSaver.conn) r.Equal(getterSaver.connectionName, e)