push down app listeners to a datastore (#742)

* push down app listeners to a datastore

fnext.NewDatastore returns a datastore that wraps the appropriate methods for
AppListener in a Datastore implementation. this is more future proof than
needing to wrap every call of GetApp/UpdateApp/etc with the listeners, there
are a few places where this can happen and it seems like the AppListener
behavior is supposed to wrap the datastore, not just the front end methods
surrounding CRUD ops on an app. the hairy case that came up was when fiddling
with the create/update route business.

this changes the FireBeforeApp* ops to be an AppListener implementation itself
rather than having the Server itself expose certain methods to fire off the
app listeners, now they're on the datastore itself, which the server can
return the instance of.

small change to BeforeAppDelete/AfterAppDelete -- we were passing in a half
baked struct with only the name filled in and not filling in the fields
anywhere. this is mostly just misleading, we could fill in the app, but we
weren't and don't really want to, it's more to notify of an app deletion event
so that an extension can behave accordingly instead of letting a user inspect
the app. i know of 3 extensions and the changes required to update are very
small.

cleans up all the front end implementations FireBefore/FireAfter.

this seems potentially less flexible than previous version if we do want to
allow users some way to call the database methods without using the
extensions, but that's exactly the trade off, as far as the AppListener's are
described it seems heavily implied that this should be the case.

mostly a feeler, for the above reasons, but this was kind of odorous so just
went for it. we do need to lock in the extension api stuff.

* hand em an app that's been smokin the reefer
This commit is contained in:
Reed Allman
2018-02-28 17:04:00 -08:00
committed by GitHub
parent 10e313e524
commit a2ed1dfb2d
9 changed files with 128 additions and 109 deletions

View File

@@ -7,14 +7,17 @@ import (
"github.com/fnproject/fn/fnext"
)
// AddAppListener adds a listener that will be notified on App created.
type appListeners []fnext.AppListener
var _ fnext.AppListener = new(appListeners)
// AddAppListener adds an AppListener for the server to use.
func (s *Server) AddAppListener(listener fnext.AppListener) {
s.appListeners = append(s.appListeners, listener)
*s.appListeners = append(*s.appListeners, listener)
}
// FireBeforeAppCreate is used to call all the server's Listeners BeforeAppCreate functions.
func (s *Server) FireBeforeAppCreate(ctx context.Context, app *models.App) error {
for _, l := range s.appListeners {
func (a *appListeners) BeforeAppCreate(ctx context.Context, app *models.App) error {
for _, l := range *a {
err := l.BeforeAppCreate(ctx, app)
if err != nil {
return err
@@ -23,9 +26,8 @@ func (s *Server) FireBeforeAppCreate(ctx context.Context, app *models.App) error
return nil
}
// FireAfterAppCreate is used to call all the server's Listeners AfterAppCreate functions.
func (s *Server) FireAfterAppCreate(ctx context.Context, app *models.App) error {
for _, l := range s.appListeners {
func (a *appListeners) AfterAppCreate(ctx context.Context, app *models.App) error {
for _, l := range *a {
err := l.AfterAppCreate(ctx, app)
if err != nil {
return err
@@ -34,9 +36,8 @@ func (s *Server) FireAfterAppCreate(ctx context.Context, app *models.App) error
return nil
}
// FireBeforeAppUpdate is used to call all the server's Listeners BeforeAppUpdate functions.
func (s *Server) FireBeforeAppUpdate(ctx context.Context, app *models.App) error {
for _, l := range s.appListeners {
func (a *appListeners) BeforeAppUpdate(ctx context.Context, app *models.App) error {
for _, l := range *a {
err := l.BeforeAppUpdate(ctx, app)
if err != nil {
return err
@@ -45,9 +46,8 @@ func (s *Server) FireBeforeAppUpdate(ctx context.Context, app *models.App) error
return nil
}
// FireAfterAppUpdate is used to call all the server's Listeners AfterAppUpdate functions.
func (s *Server) FireAfterAppUpdate(ctx context.Context, app *models.App) error {
for _, l := range s.appListeners {
func (a *appListeners) AfterAppUpdate(ctx context.Context, app *models.App) error {
for _, l := range *a {
err := l.AfterAppUpdate(ctx, app)
if err != nil {
return err
@@ -56,9 +56,8 @@ func (s *Server) FireAfterAppUpdate(ctx context.Context, app *models.App) error
return nil
}
// FireBeforeAppDelete is used to call all the server's Listeners BeforeAppDelete functions.
func (s *Server) FireBeforeAppDelete(ctx context.Context, app *models.App) error {
for _, l := range s.appListeners {
func (a *appListeners) BeforeAppDelete(ctx context.Context, app *models.App) error {
for _, l := range *a {
err := l.BeforeAppDelete(ctx, app)
if err != nil {
return err
@@ -67,9 +66,8 @@ func (s *Server) FireBeforeAppDelete(ctx context.Context, app *models.App) error
return nil
}
// FireAfterAppDelete is used to call all the server's Listeners AfterAppDelete functions.
func (s *Server) FireAfterAppDelete(ctx context.Context, app *models.App) error {
for _, l := range s.appListeners {
func (a *appListeners) AfterAppDelete(ctx context.Context, app *models.App) error {
for _, l := range *a {
err := l.AfterAppDelete(ctx, app)
if err != nil {
return err
@@ -78,12 +76,8 @@ func (s *Server) FireAfterAppDelete(ctx context.Context, app *models.App) error
return nil
}
// FireBeforeAppGet runs AppListener's BeforeAppGet method.
// todo: All of these listener methods could/should return the 2nd param rather than modifying in place. For instance,
// if a listener were to change the appName here (maybe prefix it or something for the database), it wouldn't be reflected anywhere else.
// If this returned appName, then keep passing along the returned appName, it would work.
func (s *Server) FireBeforeAppGet(ctx context.Context, appName string) error {
for _, l := range s.appListeners {
func (a *appListeners) BeforeAppGet(ctx context.Context, appName string) error {
for _, l := range *a {
err := l.BeforeAppGet(ctx, appName)
if err != nil {
return err
@@ -92,9 +86,8 @@ func (s *Server) FireBeforeAppGet(ctx context.Context, appName string) error {
return nil
}
// FireAfterAppGet runs AppListener's AfterAppGet method.
func (s *Server) FireAfterAppGet(ctx context.Context, app *models.App) error {
for _, l := range s.appListeners {
func (a *appListeners) AfterAppGet(ctx context.Context, app *models.App) error {
for _, l := range *a {
err := l.AfterAppGet(ctx, app)
if err != nil {
return err
@@ -103,9 +96,8 @@ func (s *Server) FireAfterAppGet(ctx context.Context, app *models.App) error {
return nil
}
// FireBeforeAppsList runs AppListener's BeforeAppsList method.
func (s *Server) FireBeforeAppsList(ctx context.Context, filter *models.AppFilter) error {
for _, l := range s.appListeners {
func (a *appListeners) BeforeAppsList(ctx context.Context, filter *models.AppFilter) error {
for _, l := range *a {
err := l.BeforeAppsList(ctx, filter)
if err != nil {
return err
@@ -114,9 +106,8 @@ func (s *Server) FireBeforeAppsList(ctx context.Context, filter *models.AppFilte
return nil
}
// FireAfterAppsList runs AppListener's AfterAppsList method.
func (s *Server) FireAfterAppsList(ctx context.Context, apps []*models.App) error {
for _, l := range s.appListeners {
func (a *appListeners) AfterAppsList(ctx context.Context, apps []*models.App) error {
for _, l := range *a {
err := l.AfterAppsList(ctx, apps)
if err != nil {
return err

View File

@@ -28,23 +28,11 @@ func (s *Server) handleAppCreate(c *gin.Context) {
return
}
err = s.FireBeforeAppCreate(ctx, app)
if err != nil {
handleErrorResponse(c, err)
return
}
app, err = s.datastore.InsertApp(ctx, app)
if err != nil {
handleErrorResponse(c, err)
return
}
err = s.FireAfterAppCreate(ctx, app)
if err != nil {
handleErrorResponse(c, err)
return
}
c.JSON(http.StatusOK, appResponse{"App successfully created", app})
}

View File

@@ -4,31 +4,18 @@ import (
"net/http"
"github.com/fnproject/fn/api"
"github.com/fnproject/fn/api/common"
"github.com/fnproject/fn/api/models"
"github.com/gin-gonic/gin"
)
func (s *Server) handleAppDelete(c *gin.Context) {
ctx := c.Request.Context()
log := common.Logger(ctx)
app := &models.App{Name: c.MustGet(api.AppName).(string)}
err := s.FireBeforeAppDelete(ctx, app)
err = s.datastore.RemoveApp(ctx, app.Name)
appName := c.MustGet(api.AppName).(string)
err := s.datastore.RemoveApp(ctx, appName)
if err != nil {
handleErrorResponse(c, err)
return
}
err = s.FireAfterAppDelete(ctx, app)
if err != nil {
log.WithError(err).Error("error firing after app delete")
handleErrorResponse(c, err)
return
}
c.JSON(http.StatusOK, gin.H{"message": "App deleted"})
}

View File

@@ -12,22 +12,11 @@ func (s *Server) handleAppGet(c *gin.Context) {
appName := c.MustGet(api.AppName).(string)
err := s.FireBeforeAppGet(ctx, appName)
if err != nil {
handleErrorResponse(c, err)
return
}
app, err := s.datastore.GetApp(ctx, appName)
if err != nil {
handleErrorResponse(c, err)
return
}
err = s.FireAfterAppGet(ctx, app)
if err != nil {
handleErrorResponse(c, err)
return
}
c.JSON(http.StatusOK, appResponse{"Successfully loaded app", app})
}

View File

@@ -14,22 +14,11 @@ func (s *Server) handleAppList(c *gin.Context) {
filter := &models.AppFilter{}
filter.Cursor, filter.PerPage = pageParams(c, true)
err := s.FireBeforeAppsList(ctx, filter)
if err != nil {
handleErrorResponse(c, err)
return
}
apps, err := s.datastore.GetApps(ctx, filter)
if err != nil {
handleErrorResponse(c, err)
return
}
err = s.FireAfterAppsList(ctx, apps)
if err != nil {
handleErrorResponse(c, err)
return
}
var nextCursor string
if len(apps) > 0 && len(apps) == filter.PerPage {

View File

@@ -35,23 +35,11 @@ func (s *Server) handleAppUpdate(c *gin.Context) {
wapp.App.Name = c.MustGet(api.AppName).(string)
err = s.FireBeforeAppUpdate(ctx, wapp.App)
if err != nil {
handleErrorResponse(c, err)
return
}
app, err := s.datastore.UpdateApp(ctx, wapp.App)
if err != nil {
handleErrorResponse(c, err)
return
}
err = s.FireAfterAppUpdate(ctx, wapp.App)
if err != nil {
handleErrorResponse(c, err)
return
}
c.JSON(http.StatusOK, appResponse{"App successfully updated", app})
}

View File

@@ -109,22 +109,10 @@ func (s *Server) ensureApp(ctx context.Context, wroute *models.RouteWrapper, met
} else if app == nil {
// Create a new application
newapp := &models.App{Name: wroute.Route.AppName}
err = s.FireBeforeAppCreate(ctx, newapp)
if err != nil {
return err
}
_, err = s.datastore.InsertApp(ctx, newapp)
if err != nil {
return err
}
err = s.FireAfterAppCreate(ctx, newapp)
if err != nil {
return err
}
}
return nil
}

View File

@@ -76,7 +76,7 @@ type Server struct {
mq models.MessageQueue
logstore models.LogStore
nodeType ServerNodeType
appListeners []fnext.AppListener
appListeners *appListeners
rootMiddlewares []fnext.Middleware
apiMiddlewares []fnext.Middleware
}
@@ -279,6 +279,10 @@ func New(ctx context.Context, opts ...ServerOption) *Server {
s.Router.Use(loggerWrap, traceWrap, panicWrap) // TODO should be opts
optionalCorsWrap(s.Router) // TODO should be an opt
s.bindHandlers(ctx)
s.appListeners = new(appListeners)
s.datastore = fnext.NewDatastore(s.datastore, s.appListeners)
return s
}

95
fnext/datastore.go Normal file
View File

@@ -0,0 +1,95 @@
package fnext
import (
"context"
"github.com/fnproject/fn/api/models"
)
func NewDatastore(ds models.Datastore, al AppListener) models.Datastore {
return &extds{
Datastore: ds,
al: al,
}
}
type extds struct {
models.Datastore
al AppListener
}
func (e *extds) GetApp(ctx context.Context, appName string) (*models.App, error) {
err := e.al.BeforeAppGet(ctx, appName)
if err != nil {
return nil, err
}
app, err := e.Datastore.GetApp(ctx, appName)
if err != nil {
return nil, err
}
err = e.al.AfterAppGet(ctx, app)
return app, err
}
func (e *extds) InsertApp(ctx context.Context, app *models.App) (*models.App, error) {
err := e.al.BeforeAppCreate(ctx, app)
if err != nil {
return nil, err
}
app, err = e.Datastore.InsertApp(ctx, app)
if err != nil {
return nil, err
}
err = e.al.AfterAppCreate(ctx, app)
return app, err
}
func (e *extds) UpdateApp(ctx context.Context, app *models.App) (*models.App, error) {
err := e.al.BeforeAppUpdate(ctx, app)
if err != nil {
return nil, err
}
app, err = e.Datastore.UpdateApp(ctx, app)
if err != nil {
return nil, err
}
err = e.al.AfterAppUpdate(ctx, app)
return app, err
}
func (e *extds) RemoveApp(ctx context.Context, appName string) error {
var app models.App
app.Name = appName
err := e.al.BeforeAppDelete(ctx, &app)
if err != nil {
return err
}
err = e.Datastore.RemoveApp(ctx, appName)
if err != nil {
return err
}
return e.al.AfterAppDelete(ctx, &app)
}
func (e *extds) GetApps(ctx context.Context, filter *models.AppFilter) ([]*models.App, error) {
err := e.al.BeforeAppsList(ctx, filter)
if err != nil {
return nil, err
}
apps, err := e.Datastore.GetApps(ctx, filter)
if err != nil {
return nil, err
}
err = e.al.AfterAppsList(ctx, apps)
return apps, err
}