diff --git a/api/datastore/bolt/bolt.go b/api/datastore/bolt/bolt.go index 247107497..75a42b169 100644 --- a/api/datastore/bolt/bolt.go +++ b/api/datastore/bolt/bolt.go @@ -15,6 +15,7 @@ import ( "github.com/Sirupsen/logrus" "github.com/boltdb/bolt" "github.com/iron-io/functions/api/models" + "github.com/iron-io/functions/api/datastore/internal/datastoreutil" ) type BoltDatastore struct { @@ -74,18 +75,10 @@ func New(url *url.URL) (models.Datastore, error) { } log.WithFields(logrus.Fields{"prefix": bucketPrefix, "file": url.Path}).Debug("BoltDB initialized") - return ds, nil + return datastoreutil.NewValidator(ds), nil } func (ds *BoltDatastore) InsertApp(ctx context.Context, app *models.App) (*models.App, error) { - if app == nil { - return nil, models.ErrDatastoreEmptyApp - } - - if app.Name == "" { - return nil, models.ErrDatastoreEmptyAppName - } - appname := []byte(app.Name) err := ds.db.Update(func(tx *bolt.Tx) error { @@ -117,14 +110,6 @@ func (ds *BoltDatastore) InsertApp(ctx context.Context, app *models.App) (*model } func (ds *BoltDatastore) UpdateApp(ctx context.Context, newapp *models.App) (*models.App, error) { - if newapp == nil { - return nil, models.ErrDatastoreEmptyApp - } - - if newapp.Name == "" { - return nil, models.ErrDatastoreEmptyAppName - } - var app *models.App appname := []byte(newapp.Name) @@ -164,10 +149,6 @@ func (ds *BoltDatastore) UpdateApp(ctx context.Context, newapp *models.App) (*mo } func (ds *BoltDatastore) RemoveApp(ctx context.Context, appName string) error { - if appName == "" { - return models.ErrDatastoreEmptyAppName - } - err := ds.db.Update(func(tx *bolt.Tx) error { bIm := tx.Bucket(ds.appsBucket) err := bIm.Delete([]byte(appName)) @@ -211,10 +192,6 @@ func (ds *BoltDatastore) GetApps(ctx context.Context, filter *models.AppFilter) } func (ds *BoltDatastore) GetApp(ctx context.Context, name string) (*models.App, error) { - if name == "" { - return nil, models.ErrDatastoreEmptyAppName - } - var res *models.App err := ds.db.View(func(tx *bolt.Tx) error { b := tx.Bucket(ds.appsBucket) @@ -248,18 +225,6 @@ func (ds *BoltDatastore) getRouteBucketForApp(tx *bolt.Tx, appName string) (*bol } func (ds *BoltDatastore) InsertRoute(ctx context.Context, route *models.Route) (*models.Route, error) { - if route == nil { - return nil, models.ErrDatastoreEmptyRoute - } - - if route.AppName == "" { - return nil, models.ErrDatastoreEmptyAppName - } - - if route.Path == "" { - return nil, models.ErrDatastoreEmptyRoutePath - } - routePath := []byte(route.Path) err := ds.db.Update(func(tx *bolt.Tx) error { @@ -291,18 +256,6 @@ func (ds *BoltDatastore) InsertRoute(ctx context.Context, route *models.Route) ( } func (ds *BoltDatastore) UpdateRoute(ctx context.Context, newroute *models.Route) (*models.Route, error) { - if newroute == nil { - return nil, models.ErrDatastoreEmptyRoute - } - - if newroute.AppName == "" { - return nil, models.ErrDatastoreEmptyAppName - } - - if newroute.Path == "" { - return nil, models.ErrDatastoreEmptyRoutePath - } - routePath := []byte(newroute.Path) var route *models.Route @@ -339,14 +292,6 @@ func (ds *BoltDatastore) UpdateRoute(ctx context.Context, newroute *models.Route } func (ds *BoltDatastore) RemoveRoute(ctx context.Context, appName, routePath string) error { - if appName == "" { - return models.ErrDatastoreEmptyAppName - } - - if routePath == "" { - return models.ErrDatastoreEmptyRoutePath - } - err := ds.db.Update(func(tx *bolt.Tx) error { b, err := ds.getRouteBucketForApp(tx, appName) if err != nil { @@ -366,14 +311,6 @@ func (ds *BoltDatastore) RemoveRoute(ctx context.Context, appName, routePath str } func (ds *BoltDatastore) GetRoute(ctx context.Context, appName, routePath string) (*models.Route, error) { - if appName == "" { - return nil, models.ErrDatastoreEmptyAppName - } - - if routePath == "" { - return nil, models.ErrDatastoreEmptyRoutePath - } - var route *models.Route err := ds.db.View(func(tx *bolt.Tx) error { b, err := ds.getRouteBucketForApp(tx, appName) @@ -466,10 +403,6 @@ func (ds *BoltDatastore) GetRoutes(ctx context.Context, filter *models.RouteFilt } func (ds *BoltDatastore) Put(ctx context.Context, key, value []byte) error { - if key == nil || len(key) == 0 { - return models.ErrDatastoreEmptyKey - } - ds.db.Update(func(tx *bolt.Tx) error { b := tx.Bucket(ds.extrasBucket) // todo: maybe namespace by app? err := b.Put(key, value) @@ -479,10 +412,6 @@ func (ds *BoltDatastore) Put(ctx context.Context, key, value []byte) error { } func (ds *BoltDatastore) Get(ctx context.Context, key []byte) ([]byte, error) { - if key == nil || len(key) == 0 { - return nil, models.ErrDatastoreEmptyKey - } - var ret []byte ds.db.View(func(tx *bolt.Tx) error { b := tx.Bucket(ds.extrasBucket) diff --git a/api/datastore/internal/datastoreutil/validator.go b/api/datastore/internal/datastoreutil/validator.go new file mode 100644 index 000000000..28831c417 --- /dev/null +++ b/api/datastore/internal/datastoreutil/validator.go @@ -0,0 +1,167 @@ +package datastoreutil + +import ( + "context" + + "github.com/iron-io/functions/api/models" +) + +// Datastore is a copy of models.Datastore, with additional comments on parameter guarantees. +type Datastore interface { + // name will never be empty. + GetApp(ctx context.Context, name string) (*models.App, error) + + GetApps(ctx context.Context, appFilter *models.AppFilter) ([]*models.App, error) + + // app and app.Name will never be nil/empty. + InsertApp(ctx context.Context, app *models.App) (*models.App, error) + UpdateApp(ctx context.Context, app *models.App) (*models.App, error) + + // name will never be empty. + RemoveApp(ctx context.Context, name string) error + + // appName and routePath will never be empty. + GetRoute(ctx context.Context, appName, routePath string) (*models.Route, error) + RemoveRoute(ctx context.Context, appName, routePath string) error + + GetRoutes(ctx context.Context, filter *models.RouteFilter) (routes []*models.Route, err error) + + // appName will never be empty + GetRoutesByApp(ctx context.Context, appName string, filter *models.RouteFilter) (routes []*models.Route, err error) + + // route will never be nil and route's AppName and Path will never be empty. + InsertRoute(ctx context.Context, route *models.Route) (*models.Route, error) + UpdateRoute(ctx context.Context, route *models.Route) (*models.Route, error) + + // key will never be nil/empty + Put(ctx context.Context, key, val []byte) error + Get(ctx context.Context, key []byte) ([]byte, error) +} + +// NewValidator returns a models.Datastore which validates certain arguments before delegating to ds. +func NewValidator(ds Datastore) models.Datastore { + return &validator{ds} +} + +type validator struct { + ds Datastore +} + +func (v *validator) GetApp(ctx context.Context, name string) (app *models.App, err error) { + if name == "" { + return nil, models.ErrDatastoreEmptyAppName + } + return v.ds.GetApp(ctx, name) +} + +func (v *validator) GetApps(ctx context.Context, appFilter *models.AppFilter) ([]*models.App, error) { + return v.ds.GetApps(ctx, appFilter) +} + +func (v *validator) InsertApp(ctx context.Context, app *models.App) (*models.App, error) { + if app == nil { + return nil, models.ErrDatastoreEmptyApp + } + if app.Name == "" { + return nil, models.ErrDatastoreEmptyAppName + } + + return v.ds.InsertApp(ctx, app) +} + +func (v *validator) UpdateApp(ctx context.Context, app *models.App) (*models.App, error) { + if app == nil { + return nil, models.ErrDatastoreEmptyApp + } + if app.Name == "" { + return nil, models.ErrDatastoreEmptyAppName + } + return v.ds.UpdateApp(ctx, app) +} + +func (v *validator) RemoveApp(ctx context.Context, name string) error { + if name == "" { + return models.ErrDatastoreEmptyAppName + } + + return v.ds.RemoveApp(ctx, name) +} + +func (v *validator) GetRoute(ctx context.Context, appName, routePath string) (*models.Route, error) { + if appName == "" { + return nil, models.ErrDatastoreEmptyAppName + } + if routePath == "" { + return nil, models.ErrDatastoreEmptyRoutePath + } + + return v.ds.GetRoute(ctx, appName, routePath) +} + +func (v *validator) GetRoutes(ctx context.Context, routeFilter *models.RouteFilter) (routes []*models.Route, err error) { + if routeFilter != nil && routeFilter.AppName != "" { + return v.ds.GetRoutesByApp(ctx, routeFilter.AppName, routeFilter) + } + + return v.ds.GetRoutes(ctx, routeFilter) +} + +func (v *validator) GetRoutesByApp(ctx context.Context, appName string, routeFilter *models.RouteFilter) (routes []*models.Route, err error) { + if appName == "" { + return nil, models.ErrDatastoreEmptyAppName + } + return v.ds.GetRoutesByApp(ctx, appName, routeFilter) +} + +func (v *validator) InsertRoute(ctx context.Context, route *models.Route) (*models.Route, error) { + if route == nil { + return nil, models.ErrDatastoreEmptyRoute + } + if route.AppName == "" { + return nil, models.ErrDatastoreEmptyAppName + } + if route.Path == "" { + return nil, models.ErrDatastoreEmptyRoutePath + } + + return v.ds.InsertRoute(ctx, route) +} + +func (v *validator) UpdateRoute(ctx context.Context, newroute *models.Route) (*models.Route, error) { + if newroute == nil { + return nil, models.ErrDatastoreEmptyRoute + } + if newroute.AppName == "" { + return nil, models.ErrDatastoreEmptyAppName + } + if newroute.Path == "" { + return nil, models.ErrDatastoreEmptyRoutePath + } + return v.ds.UpdateRoute(ctx, newroute) +} + +func (v *validator) RemoveRoute(ctx context.Context, appName, routePath string) error { + if appName == "" { + return models.ErrDatastoreEmptyAppName + } + if routePath == "" { + return models.ErrDatastoreEmptyRoutePath + } + + return v.ds.RemoveRoute(ctx, appName, routePath) +} + +func (v *validator) Put(ctx context.Context, key, value []byte) error { + if len(key) == 0 { + return models.ErrDatastoreEmptyKey + } + + return v.ds.Put(ctx, key, value) +} + +func (v *validator) Get(ctx context.Context, key []byte) ([]byte, error) { + if len(key) == 0 { + return nil, models.ErrDatastoreEmptyKey + } + return v.ds.Get(ctx, key) +} \ No newline at end of file diff --git a/api/datastore/mock.go b/api/datastore/mock.go index e1d466742..991312274 100644 --- a/api/datastore/mock.go +++ b/api/datastore/mock.go @@ -1,31 +1,33 @@ package datastore import ( - "github.com/iron-io/functions/api/models" - "context" + + "github.com/iron-io/functions/api/datastore/internal/datastoreutil" + "github.com/iron-io/functions/api/models" ) -type Mock struct { +type mock struct { Apps []*models.App Routes []*models.Route data map[string][]byte } -func NewMock(apps []*models.App, routes []*models.Route) *Mock { +func NewMock() models.Datastore { + return NewMockInit(nil, nil) +} + +func NewMockInit(apps []*models.App, routes []*models.Route) models.Datastore { if apps == nil { apps = []*models.App{} } if routes == nil { routes = []*models.Route{} } - return &Mock{apps, routes, make(map[string][]byte)} + return datastoreutil.NewValidator(&mock{apps, routes, make(map[string][]byte)}) } -func (m *Mock) GetApp(ctx context.Context, appName string) (app *models.App, err error) { - if appName == "" { - return nil, models.ErrDatastoreEmptyAppName - } +func (m *mock) GetApp(ctx context.Context, appName string) (app *models.App, err error) { for _, a := range m.Apps { if a.Name == appName { return a, nil @@ -35,18 +37,11 @@ func (m *Mock) GetApp(ctx context.Context, appName string) (app *models.App, err return nil, models.ErrAppsNotFound } -func (m *Mock) GetApps(ctx context.Context, appFilter *models.AppFilter) ([]*models.App, error) { +func (m *mock) GetApps(ctx context.Context, appFilter *models.AppFilter) ([]*models.App, error) { return m.Apps, nil } -func (m *Mock) InsertApp(ctx context.Context, app *models.App) (*models.App, error) { - if app == nil { - return nil, models.ErrDatastoreEmptyApp - } - if app.Name == "" { - return nil, models.ErrDatastoreEmptyAppName - } - +func (m *mock) InsertApp(ctx context.Context, app *models.App) (*models.App, error) { if a, _ := m.GetApp(ctx, app.Name); a != nil { return nil, models.ErrAppsAlreadyExists } @@ -54,7 +49,7 @@ func (m *Mock) InsertApp(ctx context.Context, app *models.App) (*models.App, err return app, nil } -func (m *Mock) UpdateApp(ctx context.Context, app *models.App) (*models.App, error) { +func (m *mock) UpdateApp(ctx context.Context, app *models.App) (*models.App, error) { a, err := m.GetApp(ctx, app.Name) if err != nil { return nil, err @@ -64,10 +59,7 @@ func (m *Mock) UpdateApp(ctx context.Context, app *models.App) (*models.App, err return a.Clone(), nil } -func (m *Mock) RemoveApp(ctx context.Context, appName string) error { - if appName == "" { - return models.ErrDatastoreEmptyAppName - } +func (m *mock) RemoveApp(ctx context.Context, appName string) error { for i, a := range m.Apps { if a.Name == appName { m.Apps = append(m.Apps[:i], m.Apps[i+1:]...) @@ -77,13 +69,7 @@ func (m *Mock) RemoveApp(ctx context.Context, appName string) error { return models.ErrAppsNotFound } -func (m *Mock) GetRoute(ctx context.Context, appName, routePath string) (*models.Route, error) { - if appName == "" { - return nil, models.ErrDatastoreEmptyAppName - } - if routePath == "" { - return nil, models.ErrDatastoreEmptyRoutePath - } +func (m *mock) GetRoute(ctx context.Context, appName, routePath string) (*models.Route, error) { for _, r := range m.Routes { if r.AppName == appName && r.Path == routePath { return r, nil @@ -92,14 +78,14 @@ func (m *Mock) GetRoute(ctx context.Context, appName, routePath string) (*models return nil, models.ErrRoutesNotFound } -func (m *Mock) GetRoutes(ctx context.Context, routeFilter *models.RouteFilter) (routes []*models.Route, err error) { +func (m *mock) GetRoutes(ctx context.Context, routeFilter *models.RouteFilter) (routes []*models.Route, err error) { for _, r := range m.Routes { routes = append(routes, r) } return } -func (m *Mock) GetRoutesByApp(ctx context.Context, appName string, routeFilter *models.RouteFilter) (routes []*models.Route, err error) { +func (m *mock) GetRoutesByApp(ctx context.Context, appName string, routeFilter *models.RouteFilter) (routes []*models.Route, err error) { for _, r := range m.Routes { if r.AppName == appName && (routeFilter.Path == "" || r.Path == routeFilter.Path) && (routeFilter.AppName == "" || r.AppName == routeFilter.AppName) { routes = append(routes, r) @@ -108,19 +94,7 @@ func (m *Mock) GetRoutesByApp(ctx context.Context, appName string, routeFilter * return } -func (m *Mock) InsertRoute(ctx context.Context, route *models.Route) (*models.Route, error) { - if route == nil { - return nil, models.ErrDatastoreEmptyRoute - } - - if route.AppName == "" { - return nil, models.ErrDatastoreEmptyAppName - } - - if route.Path == "" { - return nil, models.ErrDatastoreEmptyRoutePath - } - +func (m *mock) InsertRoute(ctx context.Context, route *models.Route) (*models.Route, error) { if _, err := m.GetApp(ctx, route.AppName); err != nil { return nil, err } @@ -132,7 +106,7 @@ func (m *Mock) InsertRoute(ctx context.Context, route *models.Route) (*models.Ro return route, nil } -func (m *Mock) UpdateRoute(ctx context.Context, route *models.Route) (*models.Route, error) { +func (m *mock) UpdateRoute(ctx context.Context, route *models.Route) (*models.Route, error) { r, err := m.GetRoute(ctx, route.AppName, route.Path) if err != nil { return nil, err @@ -141,13 +115,7 @@ func (m *Mock) UpdateRoute(ctx context.Context, route *models.Route) (*models.Ro return r.Clone(), nil } -func (m *Mock) RemoveRoute(ctx context.Context, appName, routePath string) error { - if appName == "" { - return models.ErrDatastoreEmptyAppName - } - if routePath == "" { - return models.ErrDatastoreEmptyRoutePath - } +func (m *mock) RemoveRoute(ctx context.Context, appName, routePath string) error { for i, r := range m.Routes { if r.AppName == appName && r.Path == routePath { m.Routes = append(m.Routes[:i], m.Routes[i+1:]...) @@ -157,10 +125,7 @@ func (m *Mock) RemoveRoute(ctx context.Context, appName, routePath string) error return models.ErrRoutesNotFound } -func (m *Mock) Put(ctx context.Context, key, value []byte) error { - if len(key) == 0 { - return models.ErrDatastoreEmptyKey - } +func (m *mock) Put(ctx context.Context, key, value []byte) error { if len(value) == 0 { delete(m.data, string(key)) } else { @@ -169,9 +134,6 @@ func (m *Mock) Put(ctx context.Context, key, value []byte) error { return nil } -func (m *Mock) Get(ctx context.Context, key []byte) ([]byte, error) { - if len(key) == 0 { - return nil, models.ErrDatastoreEmptyKey - } +func (m *mock) Get(ctx context.Context, key []byte) ([]byte, error) { return m.data[string(key)], nil } diff --git a/api/datastore/mock_test.go b/api/datastore/mock_test.go index 77e6b4814..fc9591323 100644 --- a/api/datastore/mock_test.go +++ b/api/datastore/mock_test.go @@ -7,5 +7,5 @@ import ( ) func TestDatastore(t *testing.T) { - datastoretest.Test(t, NewMock(nil, nil)) + datastoretest.Test(t, NewMock()) } \ No newline at end of file diff --git a/api/datastore/postgres/postgres.go b/api/datastore/postgres/postgres.go index 3474f3aa3..d3d967beb 100644 --- a/api/datastore/postgres/postgres.go +++ b/api/datastore/postgres/postgres.go @@ -13,6 +13,7 @@ import ( "github.com/lib/pq" _ "github.com/lib/pq" "bytes" + "github.com/iron-io/functions/api/datastore/internal/datastoreutil" ) const routesTableCreate = ` @@ -80,7 +81,7 @@ func New(url *url.URL) (models.Datastore, error) { } } - return pg, nil + return datastoreutil.NewValidator(pg), nil } func (ds *PostgresDatastore) InsertApp(ctx context.Context, app *models.App) (*models.App, error) { @@ -119,10 +120,6 @@ func (ds *PostgresDatastore) InsertApp(ctx context.Context, app *models.App) (*m } func (ds *PostgresDatastore) UpdateApp(ctx context.Context, newapp *models.App) (*models.App, error) { - if newapp == nil { - return nil, models.ErrAppsNotFound - } - app := &models.App{Name: newapp.Name} err := ds.Tx(func(tx *sql.Tx) error { row := ds.db.QueryRow("SELECT config FROM apps WHERE name=$1", app.Name) @@ -170,10 +167,6 @@ func (ds *PostgresDatastore) UpdateApp(ctx context.Context, newapp *models.App) } func (ds *PostgresDatastore) RemoveApp(ctx context.Context, appName string) error { - if appName == "" { - return models.ErrDatastoreEmptyAppName - } - _, err := ds.db.Exec(` DELETE FROM apps WHERE name = $1 @@ -187,10 +180,6 @@ func (ds *PostgresDatastore) RemoveApp(ctx context.Context, appName string) erro } func (ds *PostgresDatastore) GetApp(ctx context.Context, name string) (*models.App, error) { - if name == "" { - return nil, models.ErrDatastoreEmptyAppName - } - row := ds.db.QueryRow("SELECT name, config FROM apps WHERE name=$1", name) var resName string @@ -256,10 +245,6 @@ func (ds *PostgresDatastore) GetApps(ctx context.Context, filter *models.AppFilt } func (ds *PostgresDatastore) InsertRoute(ctx context.Context, route *models.Route) (*models.Route, error) { - if route == nil { - return nil, models.ErrDatastoreEmptyRoute - } - hbyte, err := json.Marshal(route.Headers) if err != nil { return nil, err @@ -325,10 +310,6 @@ func (ds *PostgresDatastore) InsertRoute(ctx context.Context, route *models.Rout } func (ds *PostgresDatastore) UpdateRoute(ctx context.Context, newroute *models.Route) (*models.Route, error) { - if newroute == nil { - return nil, models.ErrDatastoreEmptyRoute - } - var route models.Route err := ds.Tx(func(tx *sql.Tx) error { row := ds.db.QueryRow(fmt.Sprintf("%s WHERE app_name=$1 AND path=$2", routeSelector), newroute.AppName, newroute.Path) @@ -393,14 +374,6 @@ func (ds *PostgresDatastore) UpdateRoute(ctx context.Context, newroute *models.R } func (ds *PostgresDatastore) RemoveRoute(ctx context.Context, appName, routePath string) error { - if appName == "" { - return models.ErrDatastoreEmptyAppName - } - - if routePath == "" { - return models.ErrDatastoreEmptyRoutePath - } - res, err := ds.db.Exec(` DELETE FROM routes WHERE path = $1 AND app_name = $2 @@ -450,14 +423,6 @@ func scanRoute(scanner rowScanner, route *models.Route) error { } func (ds *PostgresDatastore) GetRoute(ctx context.Context, appName, routePath string) (*models.Route, error) { - if appName == "" { - return nil, models.ErrDatastoreEmptyAppName - } - - if routePath == "" { - return nil, models.ErrDatastoreEmptyRoutePath - } - var route models.Route row := ds.db.QueryRow(fmt.Sprintf("%s WHERE app_name=$1 AND path=$2", routeSelector), appName, routePath) @@ -568,10 +533,6 @@ func buildFilterRouteQuery(filter *models.RouteFilter) (string, []interface{}) { } func (ds *PostgresDatastore) Put(ctx context.Context, key, value []byte) error { - if key == nil || len(key) == 0 { - return models.ErrDatastoreEmptyKey - } - _, err := ds.db.Exec(` INSERT INTO extras ( key, @@ -590,10 +551,6 @@ func (ds *PostgresDatastore) Put(ctx context.Context, key, value []byte) error { } func (ds *PostgresDatastore) Get(ctx context.Context, key []byte) ([]byte, error) { - if key == nil || len(key) == 0 { - return nil, models.ErrDatastoreEmptyKey - } - row := ds.db.QueryRow("SELECT value FROM extras WHERE key=$1", key) var value string diff --git a/api/datastore/redis/redis.go b/api/datastore/redis/redis.go index 883a1f05f..024cda1bc 100644 --- a/api/datastore/redis/redis.go +++ b/api/datastore/redis/redis.go @@ -13,6 +13,7 @@ import ( "github.com/Sirupsen/logrus" "github.com/garyburd/redigo/redis" "github.com/iron-io/functions/api/models" + "github.com/iron-io/functions/api/datastore/internal/datastoreutil" ) type RedisDataStore struct { @@ -43,7 +44,7 @@ func New(url *url.URL) (models.Datastore, error) { ds := &RedisDataStore{ conn: conn, } - return ds, nil + return datastoreutil.NewValidator(ds), nil } func (ds *RedisDataStore) setApp(app *models.App) (*models.App, error) { @@ -59,13 +60,6 @@ func (ds *RedisDataStore) setApp(app *models.App) (*models.App, error) { } func (ds *RedisDataStore) InsertApp(ctx context.Context, app *models.App) (*models.App, error) { - if app == nil { - return nil, models.ErrDatastoreEmptyApp - } - if app.Name == "" { - return nil, models.ErrDatastoreEmptyAppName - } - reply, err := ds.conn.Do("HEXISTS", "apps", app.Name) if err != nil { return nil, err @@ -80,13 +74,6 @@ func (ds *RedisDataStore) InsertApp(ctx context.Context, app *models.App) (*mode } func (ds *RedisDataStore) UpdateApp(ctx context.Context, newapp *models.App) (*models.App, error) { - if newapp == nil { - return nil, models.ErrDatastoreEmptyApp - } - if newapp.Name == "" { - return nil, models.ErrDatastoreEmptyAppName - } - app, err := ds.GetApp(ctx, newapp.Name) if err != nil { return nil, err @@ -98,10 +85,6 @@ func (ds *RedisDataStore) UpdateApp(ctx context.Context, newapp *models.App) (*m } func (ds *RedisDataStore) RemoveApp(ctx context.Context, appName string) error { - if appName == "" { - return models.ErrDatastoreEmptyAppName - } - if _, err := ds.conn.Do("HDEL", "apps", appName); err != nil { return err } @@ -110,10 +93,6 @@ func (ds *RedisDataStore) RemoveApp(ctx context.Context, appName string) error { } func (ds *RedisDataStore) GetApp(ctx context.Context, name string) (*models.App, error) { - if name == "" { - return nil, models.ErrDatastoreEmptyAppName - } - reply, err := ds.conn.Do("HGET", "apps", name) if err != nil { return nil, err @@ -168,18 +147,6 @@ func (ds *RedisDataStore) setRoute(set string, route *models.Route) (*models.Rou } func (ds *RedisDataStore) InsertRoute(ctx context.Context, route *models.Route) (*models.Route, error) { - if route == nil { - return nil, models.ErrDatastoreEmptyRoute - } - - if route.AppName == "" { - return nil, models.ErrDatastoreEmptyAppName - } - - if route.Path == "" { - return nil, models.ErrDatastoreEmptyRoutePath - } - reply, err := ds.conn.Do("HEXISTS", "apps", route.AppName) if err != nil { return nil, err @@ -207,18 +174,6 @@ func (ds *RedisDataStore) InsertRoute(ctx context.Context, route *models.Route) } func (ds *RedisDataStore) UpdateRoute(ctx context.Context, newroute *models.Route) (*models.Route, error) { - if newroute == nil { - return nil, models.ErrDatastoreEmptyRoute - } - - if newroute.AppName == "" { - return nil, models.ErrDatastoreEmptyAppName - } - - if newroute.Path == "" { - return nil, models.ErrDatastoreEmptyRoutePath - } - route, err := ds.GetRoute(ctx, newroute.AppName, newroute.Path) if err != nil { return nil, err @@ -233,14 +188,6 @@ func (ds *RedisDataStore) UpdateRoute(ctx context.Context, newroute *models.Rout } func (ds *RedisDataStore) RemoveRoute(ctx context.Context, appName, routePath string) error { - if appName == "" { - return models.ErrDatastoreEmptyAppName - } - - if routePath == "" { - return models.ErrDatastoreEmptyRoutePath - } - hset := fmt.Sprintf("routes:%s", appName) if n, err := ds.conn.Do("HDEL", hset, routePath); err != nil { return err @@ -252,14 +199,6 @@ func (ds *RedisDataStore) RemoveRoute(ctx context.Context, appName, routePath st } func (ds *RedisDataStore) GetRoute(ctx context.Context, appName, routePath string) (*models.Route, error) { - if appName == "" { - return nil, models.ErrDatastoreEmptyAppName - } - - if routePath == "" { - return nil, models.ErrDatastoreEmptyRoutePath - } - hset := fmt.Sprintf("routes:%s", appName) reply, err := ds.conn.Do("HGET", hset, routePath) if err != nil { @@ -312,9 +251,6 @@ func (ds *RedisDataStore) GetRoutes(ctx context.Context, filter *models.RouteFil } func (ds *RedisDataStore) GetRoutesByApp(ctx context.Context, appName string, filter *models.RouteFilter) ([]*models.Route, error) { - if appName == "" { - return nil, models.ErrDatastoreEmptyAppName - } if filter == nil { filter = new(models.RouteFilter) } @@ -344,10 +280,6 @@ func (ds *RedisDataStore) GetRoutesByApp(ctx context.Context, appName string, fi } func (ds *RedisDataStore) Put(ctx context.Context, key, value []byte) error { - if key == nil || len(key) == 0 { - return models.ErrDatastoreEmptyKey - } - if _, err := ds.conn.Do("HSET", "extras", key, value); err != nil { return err } @@ -356,10 +288,6 @@ func (ds *RedisDataStore) Put(ctx context.Context, key, value []byte) error { } func (ds *RedisDataStore) Get(ctx context.Context, key []byte) ([]byte, error) { - if key == nil || len(key) == 0 { - return nil, models.ErrDatastoreEmptyKey - } - value, err := ds.conn.Do("HGET", "extras", key) if err != nil { return nil, err diff --git a/api/models/datastore.go b/api/models/datastore.go index 68b8c5207..ef28b1a31 100644 --- a/api/models/datastore.go +++ b/api/models/datastore.go @@ -6,18 +6,56 @@ import ( ) type Datastore interface { - // GetApp returns the app called appName or nil if it doesn't exist + + // GetApp gets an App by name. + // Returns ErrDatastoreEmptyAppName for empty appName. + // Returns ErrAppsNotFound if no app is found. GetApp(ctx context.Context, appName string) (*App, error) + + // GetApps gets a slice of Apps, optionally filtered by name. + // Missing filter or empty name will match all Apps. GetApps(ctx context.Context, filter *AppFilter) ([]*App, error) + + // InsertApp inserts an App. Returns ErrDatastoreEmptyApp when app is nil, and + // ErrDatastoreEmptyAppName when app.Name is empty. + // Returns ErrAppsAlreadyExists if an App by the same name already exists. InsertApp(ctx context.Context, app *App) (*App, error) + + // UpdateApp updates an App's Config. Returns ErrDatastoreEmptyApp when app is nil, and + // ErrDatastoreEmptyAppName when app.Name is empty. + // Returns ErrAppsNotFound if an App is not found. UpdateApp(ctx context.Context, app *App) (*App, error) + + // RemoveApp removes the App named appName. Returns ErrDatastoreEmptyAppName if appName is empty. + // Returns ErrAppsNotFound if an App is not found. + //TODO remove routes automatically? #528 RemoveApp(ctx context.Context, appName string) error + // GetRoute looks up a matching Route for appName and the literal request route routePath. + // Returns ErrDatastoreEmptyAppName when appName is empty, and ErrDatastoreEmptyRoutePath when + // routePath is empty. + // Returns ErrRoutesNotFound when no matching route is found. GetRoute(ctx context.Context, appName, routePath string) (*Route, error) + + // GetRoutes gets a slice of Routes, optionally filtered by filter. GetRoutes(ctx context.Context, filter *RouteFilter) (routes []*Route, err error) + + // GetRoutesByApp gets a slice of routes for a appName, optionally filtering on filter (filter.AppName is ignored). + // Returns ErrDatastoreEmptyAppName if appName is empty. GetRoutesByApp(ctx context.Context, appName string, filter *RouteFilter) (routes []*Route, err error) + + // InsertRoute inserts a route. Returns ErrDatastoreEmptyRoute when route is nil, and ErrDatastoreEmptyAppName + // or ErrDatastoreEmptyRoutePath for empty AppName or Path. + // Returns ErrRoutesAlreadyExists if the exact route.Path already exists, or ErrRoutesCreate if a conflicting + // route already exists. InsertRoute(ctx context.Context, route *Route) (*Route, error) + + // UpdateRoute updates route's Config and Header fields. Returns ErrDatastoreEmptyRoute when route is nil, and + // ErrDatastoreEmptyAppName or ErrDatastoreEmptyRoutePath for empty AppName or Path. UpdateRoute(ctx context.Context, route *Route) (*Route, error) + + // RemoveRoute removes a route. Returns ErrDatastoreEmptyAppName when appName is empty, and + // ErrDatastoreEmptyRoutePath when routePath is empty. Returns ErrRoutesNotFound when no route exists. RemoveRoute(ctx context.Context, appName, routePath string) error // The following provide a generic key value store for arbitrary data, can be used by extensions to store extra data diff --git a/api/server/apps_test.go b/api/server/apps_test.go index 2bd928b1f..17f8ed16d 100644 --- a/api/server/apps_test.go +++ b/api/server/apps_test.go @@ -39,23 +39,23 @@ func TestAppCreate(t *testing.T) { tasks := mockTasksConduit() defer close(tasks) for i, test := range []struct { - mock *datastore.Mock + mock models.Datastore path string body string expectedCode int expectedError error }{ // errors - {&datastore.Mock{}, "/v1/apps", ``, http.StatusBadRequest, models.ErrInvalidJSON}, - {&datastore.Mock{}, "/v1/apps", `{}`, http.StatusBadRequest, models.ErrAppsMissingNew}, - {&datastore.Mock{}, "/v1/apps", `{ "name": "Test" }`, http.StatusBadRequest, models.ErrAppsMissingNew}, - {&datastore.Mock{}, "/v1/apps", `{ "app": { "name": "" } }`, http.StatusInternalServerError, models.ErrAppsValidationMissingName}, - {&datastore.Mock{}, "/v1/apps", `{ "app": { "name": "1234567890123456789012345678901" } }`, http.StatusInternalServerError, models.ErrAppsValidationTooLongName}, - {&datastore.Mock{}, "/v1/apps", `{ "app": { "name": "&&%@!#$#@$" } }`, http.StatusInternalServerError, models.ErrAppsValidationInvalidName}, - {&datastore.Mock{}, "/v1/apps", `{ "app": { "name": "&&%@!#$#@$" } }`, http.StatusInternalServerError, models.ErrAppsValidationInvalidName}, + {datastore.NewMock(), "/v1/apps", ``, http.StatusBadRequest, models.ErrInvalidJSON}, + {datastore.NewMock(), "/v1/apps", `{}`, http.StatusBadRequest, models.ErrAppsMissingNew}, + {datastore.NewMock(), "/v1/apps", `{ "name": "Test" }`, http.StatusBadRequest, models.ErrAppsMissingNew}, + {datastore.NewMock(), "/v1/apps", `{ "app": { "name": "" } }`, http.StatusInternalServerError, models.ErrAppsValidationMissingName}, + {datastore.NewMock(), "/v1/apps", `{ "app": { "name": "1234567890123456789012345678901" } }`, http.StatusInternalServerError, models.ErrAppsValidationTooLongName}, + {datastore.NewMock(), "/v1/apps", `{ "app": { "name": "&&%@!#$#@$" } }`, http.StatusInternalServerError, models.ErrAppsValidationInvalidName}, + {datastore.NewMock(), "/v1/apps", `{ "app": { "name": "&&%@!#$#@$" } }`, http.StatusInternalServerError, models.ErrAppsValidationInvalidName}, // success - {&datastore.Mock{}, "/v1/apps", `{ "app": { "name": "teste" } }`, http.StatusOK, nil}, + {datastore.NewMock(), "/v1/apps", `{ "app": { "name": "teste" } }`, http.StatusOK, nil}, } { rnr, cancel := testRunner(t) srv := testServer(test.mock, &mqs.Mock{}, rnr, tasks) @@ -95,12 +95,12 @@ func TestAppDelete(t *testing.T) { expectedCode int expectedError error }{ - {&datastore.Mock{}, "/v1/apps/myapp", "", http.StatusNotFound, nil}, - {&datastore.Mock{ - Apps: []*models.App{{ + {datastore.NewMock(), "/v1/apps/myapp", "", http.StatusNotFound, nil}, + {datastore.NewMockInit( + []*models.App{{ Name: "myapp", - }}, - }, "/v1/apps/myapp", "", http.StatusOK, nil}, + }},nil, + ), "/v1/apps/myapp", "", http.StatusOK, nil}, } { rnr, cancel := testRunner(t) srv := testServer(test.ds, &mqs.Mock{}, rnr, tasks) @@ -133,7 +133,7 @@ func TestAppList(t *testing.T) { rnr, cancel := testRunner(t) defer cancel() - srv := testServer(&datastore.Mock{}, &mqs.Mock{}, rnr, tasks) + srv := testServer(datastore.NewMock(), &mqs.Mock{}, rnr, tasks) for i, test := range []struct { path string @@ -170,7 +170,7 @@ func TestAppGet(t *testing.T) { rnr, cancel := testRunner(t) defer cancel() - srv := testServer(&datastore.Mock{}, &mqs.Mock{}, rnr, tasks) + srv := testServer(datastore.NewMock(), &mqs.Mock{}, rnr, tasks) for i, test := range []struct { path string @@ -206,28 +206,28 @@ func TestAppUpdate(t *testing.T) { defer close(tasks) for i, test := range []struct { - mock *datastore.Mock + mock models.Datastore path string body string expectedCode int expectedError error }{ // errors - {&datastore.Mock{}, "/v1/apps/myapp", ``, http.StatusBadRequest, models.ErrInvalidJSON}, + {datastore.NewMock(), "/v1/apps/myapp", ``, http.StatusBadRequest, models.ErrInvalidJSON}, // success - {&datastore.Mock{ - Apps: []*models.App{{ + {datastore.NewMockInit( + []*models.App{{ Name: "myapp", - }}, - }, "/v1/apps/myapp", `{ "app": { "config": { "test": "1" } } }`, http.StatusOK, nil}, + }}, nil, + ), "/v1/apps/myapp", `{ "app": { "config": { "test": "1" } } }`, http.StatusOK, nil}, // Addresses #380 - {&datastore.Mock{ - Apps: []*models.App{{ + {datastore.NewMockInit( + []*models.App{{ Name: "myapp", - }}, - }, "/v1/apps/myapp", `{ "app": { "name": "othername" } }`, http.StatusBadRequest, nil}, + }}, nil, + ), "/v1/apps/myapp", `{ "app": { "name": "othername" } }`, http.StatusBadRequest, nil}, } { rnr, cancel := testRunner(t) srv := testServer(test.mock, &mqs.Mock{}, rnr, tasks) diff --git a/api/server/routes_test.go b/api/server/routes_test.go index 86ec1d77e..8f306eeb8 100644 --- a/api/server/routes_test.go +++ b/api/server/routes_test.go @@ -17,24 +17,24 @@ func TestRouteCreate(t *testing.T) { defer close(tasks) for i, test := range []struct { - mock *datastore.Mock + mock models.Datastore path string body string expectedCode int expectedError error }{ // errors - {&datastore.Mock{}, "/v1/apps/a/routes", ``, http.StatusBadRequest, models.ErrInvalidJSON}, - {&datastore.Mock{}, "/v1/apps/a/routes", `{ }`, http.StatusBadRequest, models.ErrRoutesMissingNew}, - {&datastore.Mock{}, "/v1/apps/a/routes", `{ "path": "/myroute" }`, http.StatusBadRequest, models.ErrRoutesMissingNew}, - {&datastore.Mock{}, "/v1/apps/a/routes", `{ "route": { } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingPath}, - {&datastore.Mock{}, "/v1/apps/a/routes", `{ "route": { "path": "/myroute" } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingImage}, - {&datastore.Mock{}, "/v1/apps/a/routes", `{ "route": { "image": "iron/hello" } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingPath}, - {&datastore.Mock{}, "/v1/apps/a/routes", `{ "route": { "image": "iron/hello", "path": "myroute" } }`, http.StatusBadRequest, models.ErrRoutesValidationInvalidPath}, - {&datastore.Mock{}, "/v1/apps/$/routes", `{ "route": { "image": "iron/hello", "path": "/myroute" } }`, http.StatusInternalServerError, models.ErrAppsValidationInvalidName}, + {datastore.NewMock(), "/v1/apps/a/routes", ``, http.StatusBadRequest, models.ErrInvalidJSON}, + {datastore.NewMock(), "/v1/apps/a/routes", `{ }`, http.StatusBadRequest, models.ErrRoutesMissingNew}, + {datastore.NewMock(), "/v1/apps/a/routes", `{ "path": "/myroute" }`, http.StatusBadRequest, models.ErrRoutesMissingNew}, + {datastore.NewMock(), "/v1/apps/a/routes", `{ "route": { } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingPath}, + {datastore.NewMock(), "/v1/apps/a/routes", `{ "route": { "path": "/myroute" } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingImage}, + {datastore.NewMock(), "/v1/apps/a/routes", `{ "route": { "image": "iron/hello" } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingPath}, + {datastore.NewMock(), "/v1/apps/a/routes", `{ "route": { "image": "iron/hello", "path": "myroute" } }`, http.StatusBadRequest, models.ErrRoutesValidationInvalidPath}, + {datastore.NewMock(), "/v1/apps/$/routes", `{ "route": { "image": "iron/hello", "path": "/myroute" } }`, http.StatusInternalServerError, models.ErrAppsValidationInvalidName}, // success - {&datastore.Mock{}, "/v1/apps/a/routes", `{ "route": { "image": "iron/hello", "path": "/myroute" } }`, http.StatusOK, nil}, + {datastore.NewMock(), "/v1/apps/a/routes", `{ "route": { "image": "iron/hello", "path": "/myroute" } }`, http.StatusOK, nil}, } { rnr, cancel := testRunner(t) srv := testServer(test.mock, &mqs.Mock{}, rnr, tasks) @@ -73,12 +73,12 @@ func TestRouteDelete(t *testing.T) { expectedCode int expectedError error }{ - {&datastore.Mock{}, "/v1/apps/a/routes/missing", "", http.StatusNotFound, nil}, - {&datastore.Mock{ - Routes: []*models.Route{ + {datastore.NewMock(), "/v1/apps/a/routes/missing", "", http.StatusNotFound, nil}, + {datastore.NewMockInit(nil, + []*models.Route{ {Path: "/myroute", AppName: "a"}, }, - }, "/v1/apps/a/routes/myroute", "", http.StatusOK, nil}, + ), "/v1/apps/a/routes/myroute", "", http.StatusOK, nil}, } { rnr, cancel := testRunner(t) srv := testServer(test.ds, &mqs.Mock{}, rnr, tasks) @@ -110,7 +110,7 @@ func TestRouteList(t *testing.T) { rnr, cancel := testRunner(t) defer cancel() - srv := testServer(&datastore.Mock{}, &mqs.Mock{}, rnr, tasks) + srv := testServer(datastore.NewMock(), &mqs.Mock{}, rnr, tasks) for i, test := range []struct { path string @@ -148,7 +148,7 @@ func TestRouteGet(t *testing.T) { rnr, cancel := testRunner(t) defer cancel() - srv := testServer(&datastore.Mock{}, &mqs.Mock{}, rnr, tasks) + srv := testServer(datastore.NewMock(), &mqs.Mock{}, rnr, tasks) for i, test := range []struct { path string @@ -191,28 +191,28 @@ func TestRouteUpdate(t *testing.T) { expectedError error }{ // errors - {&datastore.Mock{}, "/v1/apps/a/routes/myroute/do", ``, http.StatusBadRequest, models.ErrInvalidJSON}, - {&datastore.Mock{}, "/v1/apps/a/routes/myroute/do", `{}`, http.StatusBadRequest, models.ErrRoutesMissingNew}, + {datastore.NewMock(), "/v1/apps/a/routes/myroute/do", ``, http.StatusBadRequest, models.ErrInvalidJSON}, + {datastore.NewMock(), "/v1/apps/a/routes/myroute/do", `{}`, http.StatusBadRequest, models.ErrRoutesMissingNew}, // success - {&datastore.Mock{ - Routes: []*models.Route{ + {datastore.NewMockInit(nil, + []*models.Route{ { AppName: "a", Path: "/myroute/do", }, }, - }, "/v1/apps/a/routes/myroute/do", `{ "route": { "image": "iron/hello" } }`, http.StatusOK, nil}, + ), "/v1/apps/a/routes/myroute/do", `{ "route": { "image": "iron/hello" } }`, http.StatusOK, nil}, // Addresses #381 - {&datastore.Mock{ - Routes: []*models.Route{ + {datastore.NewMockInit(nil, + []*models.Route{ { AppName: "a", Path: "/myroute/do", }, }, - }, "/v1/apps/a/routes/myroute/do", `{ "route": { "path": "/otherpath" } }`, http.StatusBadRequest, nil}, + ), "/v1/apps/a/routes/myroute/do", `{ "route": { "path": "/otherpath" } }`, http.StatusBadRequest, nil}, } { rnr, cancel := testRunner(t) srv := testServer(test.ds, &mqs.Mock{}, rnr, tasks) diff --git a/api/server/runner_async_test.go b/api/server/runner_async_test.go index b4eae89ee..13bd05052 100644 --- a/api/server/runner_async_test.go +++ b/api/server/runner_async_test.go @@ -40,16 +40,16 @@ func testRouterAsync(ds models.Datastore, mq models.MessageQueue, rnr *runner.Ru func TestRouteRunnerAsyncExecution(t *testing.T) { tasks := mockTasksConduit() - ds := &datastore.Mock{ - Apps: []*models.App{ + ds := datastore.NewMockInit( + []*models.App{ {Name: "myapp", Config: map[string]string{"app": "true"}}, }, - Routes: []*models.Route{ + []*models.Route{ {Type: "async", Path: "/myroute", AppName: "myapp", Image: "iron/hello", Config: map[string]string{"test": "true"}}, {Type: "async", Path: "/myerror", AppName: "myapp", Image: "iron/error", Config: map[string]string{"test": "true"}}, {Type: "async", Path: "/myroute/:param", AppName: "myapp", Image: "iron/hello", Config: map[string]string{"test": "true"}}, }, - } + ) mq := &mqs.Mock{} for i, test := range []struct { diff --git a/api/server/runner_test.go b/api/server/runner_test.go index 5c6b3bbe2..34522d5d0 100644 --- a/api/server/runner_test.go +++ b/api/server/runner_test.go @@ -30,11 +30,11 @@ func TestRouteRunnerGet(t *testing.T) { rnr, cancel := testRunner(t) defer cancel() - srv := testServer(&datastore.Mock{ - Apps: []*models.App{ + srv := testServer(datastore.NewMockInit( + []*models.App{ {Name: "myapp", Config: models.Config{}}, - }, - }, &mqs.Mock{}, rnr, tasks) + }, nil, + ), &mqs.Mock{}, rnr, tasks) for i, test := range []struct { path string @@ -73,11 +73,11 @@ func TestRouteRunnerPost(t *testing.T) { rnr, cancel := testRunner(t) defer cancel() - srv := testServer(&datastore.Mock{ - Apps: []*models.App{ + srv := testServer(datastore.NewMockInit( + []*models.App{ {Name: "myapp", Config: models.Config{}}, - }, - }, &mqs.Mock{}, rnr, tasks) + }, nil, + ), &mqs.Mock{}, rnr, tasks) for i, test := range []struct { path string @@ -123,15 +123,15 @@ func TestRouteRunnerExecution(t *testing.T) { go runner.StartWorkers(ctx, rnr, tasks) - srv := testServer(&datastore.Mock{ - Apps: []*models.App{ + srv := testServer(datastore.NewMockInit( + []*models.App{ {Name: "myapp", Config: models.Config{}}, }, - Routes: []*models.Route{ + []*models.Route{ {Path: "/myroute", AppName: "myapp", Image: "iron/hello", Headers: map[string][]string{"X-Function": {"Test"}}}, {Path: "/myerror", AppName: "myapp", Image: "iron/error", Headers: map[string][]string{"X-Function": {"Test"}}}, }, - }, &mqs.Mock{}, rnr, tasks) + ), &mqs.Mock{}, rnr, tasks) for i, test := range []struct { path string @@ -181,14 +181,14 @@ func TestRouteRunnerTimeout(t *testing.T) { defer cancelrnr() go runner.StartWorkers(ctx, rnr, tasks) - srv := testServer(&datastore.Mock{ - Apps: []*models.App{ + srv := testServer(datastore.NewMockInit( + []*models.App{ {Name: "myapp", Config: models.Config{}}, }, - Routes: []*models.Route{ + []*models.Route{ {Path: "/sleeper", AppName: "myapp", Image: "iron/sleeper", Timeout: 1}, }, - }, &mqs.Mock{}, rnr, tasks) + ), &mqs.Mock{}, rnr, tasks) for i, test := range []struct { path string