Adds before/after app get/list. And some bug fixes/cleanup. (#610)

* Adds before/after app get/list. And some bug fixes/cleanup.

* Fix test
This commit is contained in:
Travis Reeder
2017-12-21 09:32:03 -08:00
committed by GitHub
parent 0a1b180158
commit fdb4188146
26 changed files with 212 additions and 65 deletions

View File

@@ -77,3 +77,50 @@ 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 {
err := l.BeforeAppGet(ctx, appName)
if err != nil {
return err
}
}
return nil
}
// FireAfterAppGet runs AppListener's AfterAppGet method.
func (s *Server) FireAfterAppGet(ctx context.Context, app *models.App) error {
for _, l := range s.appListeners {
err := l.AfterAppGet(ctx, app)
if err != nil {
return err
}
}
return nil
}
// FireBeforeAppsList runs AppListener's BeforeAppsList method.
func (s *Server) FireBeforeAppsList(ctx context.Context, filter *models.AppFilter) error {
for _, l := range s.appListeners {
err := l.BeforeAppsList(ctx, filter)
if err != nil {
return err
}
}
return nil
}
// FireAfterAppsList runs AppListener's AfterAppsList method.
func (s *Server) FireAfterAppsList(ctx context.Context, apps []*models.App) error {
for _, l := range s.appListeners {
err := l.AfterAppsList(ctx, apps)
if err != nil {
return err
}
}
return nil
}

View File

@@ -34,7 +34,7 @@ func (s *Server) handleAppCreate(c *gin.Context) {
return
}
app, err := s.Datastore.InsertApp(ctx, wapp.App)
app, err := s.Datastore().InsertApp(ctx, wapp.App)
if err != nil {
handleErrorResponse(c, err)
return

View File

@@ -17,7 +17,7 @@ func (s *Server) handleAppDelete(c *gin.Context) {
err := s.FireBeforeAppDelete(ctx, app)
err = s.Datastore.RemoveApp(ctx, app.Name)
err = s.Datastore().RemoveApp(ctx, app.Name)
if err != nil {
handleErrorResponse(c, err)
return

View File

@@ -11,7 +11,19 @@ func (s *Server) handleAppGet(c *gin.Context) {
ctx := c.Request.Context()
appName := c.MustGet(api.AppName).(string)
app, err := s.Datastore.GetApp(ctx, appName)
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

View File

@@ -11,10 +11,21 @@ import (
func (s *Server) handleAppList(c *gin.Context) {
ctx := c.Request.Context()
var filter models.AppFilter
filter := &models.AppFilter{}
filter.Cursor, filter.PerPage = pageParams(c, true)
apps, err := s.Datastore.GetApps(ctx, &filter)
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

View File

@@ -31,13 +31,13 @@ func (s *Server) handleAppUpdate(c *gin.Context) {
wapp.App.Name = c.MustGet(api.AppName).(string)
err = s.FireAfterAppUpdate(ctx, wapp.App)
err = s.FireBeforeAppUpdate(ctx, wapp.App)
if err != nil {
handleErrorResponse(c, err)
return
}
app, err := s.Datastore.UpdateApp(ctx, wapp.App)
app, err := s.Datastore().UpdateApp(ctx, wapp.App)
if err != nil {
handleErrorResponse(c, err)
return

View File

@@ -12,7 +12,7 @@ func (s *Server) handleCallGet(c *gin.Context) {
appName := c.MustGet(api.AppName).(string)
callID := c.Param(api.Call)
callObj, err := s.Datastore.GetCall(ctx, appName, callID)
callObj, err := s.Datastore().GetCall(ctx, appName, callID)
if err != nil {
handleErrorResponse(c, err)
return

View File

@@ -27,11 +27,11 @@ func (s *Server) handleCallList(c *gin.Context) {
return
}
calls, err := s.Datastore.GetCalls(ctx, &filter)
calls, err := s.Datastore().GetCalls(ctx, &filter)
if len(calls) == 0 {
// TODO this should be done in front of this handler to even get here...
_, err = s.Datastore.GetApp(c, appName)
_, err = s.Datastore().GetApp(c, appName)
}
if err != nil {

View File

@@ -20,7 +20,7 @@ func (s *Server) apiAppHandlerWrapperFunc(apiHandler fnext.ApiAppHandler) gin.Ha
return func(c *gin.Context) {
// get the app
appName := c.Param(api.CApp)
app, err := s.Datastore.GetApp(c.Request.Context(), appName)
app, err := s.Datastore().GetApp(c.Request.Context(), appName)
if err != nil {
handleErrorResponse(c, err)
c.Abort()
@@ -41,7 +41,7 @@ func (s *Server) apiRouteHandlerWrapperFunc(apiHandler fnext.ApiRouteHandler) gi
context := c.Request.Context()
// get the app
appName := c.Param(api.CApp)
app, err := s.Datastore.GetApp(context, appName)
app, err := s.Datastore().GetApp(context, appName)
if err != nil {
handleErrorResponse(c, err)
c.Abort()
@@ -54,7 +54,7 @@ func (s *Server) apiRouteHandlerWrapperFunc(apiHandler fnext.ApiRouteHandler) gi
}
// get the route TODO
routePath := "/" + c.Param(api.CRoute)
route, err := s.Datastore.GetRoute(context, appName, routePath)
route, err := s.Datastore().GetRoute(context, appName, routePath)
if err != nil {
handleErrorResponse(c, err)
c.Abort()

View File

@@ -10,6 +10,7 @@ import (
"github.com/fnproject/fn/api"
"github.com/fnproject/fn/api/common"
"github.com/fnproject/fn/api/models"
"github.com/fnproject/fn/fnext"
"github.com/gin-contrib/cors"
"github.com/gin-gonic/gin"
opentracing "github.com/opentracing/opentracing-go"
@@ -85,7 +86,16 @@ func loggerWrap(c *gin.Context) {
c.Next()
}
func appWrap(c *gin.Context) {
func setAppNameInCtx(c *gin.Context) {
// add appName to context
appName := c.GetString(api.AppName)
if appName != "" {
c.Request = c.Request.WithContext(context.WithValue(c.Request.Context(), fnext.AppNameKey, appName))
}
c.Next()
}
func appNameCheck(c *gin.Context) {
appName := c.GetString(api.AppName)
if appName == "" {
handleErrorResponse(c, models.ErrAppsMissingName)

View File

@@ -44,7 +44,7 @@ func (s *Server) handleRunnerEnqueue(c *gin.Context) {
// runner and enter into 'running' state before we can insert it in the db as
// 'queued' state. we can ignore any error inserting into db here and Start
// will ensure the call exists in the db in 'running' state there.
// s.Datastore.InsertCall(ctx, &call)
// s.Datastore().InsertCall(ctx, &call)
c.JSON(200, struct {
M string `json:"msg"`
@@ -112,7 +112,7 @@ func (s *Server) handleRunnerStart(c *gin.Context) {
// TODO do this client side and validate it here?
//call.Status = "running"
//call.StartedAt = strfmt.DateTime(time.Now())
//err := s.Datastore.UpdateCall(c.Request.Context(), &call)
//err := s.Datastore().UpdateCall(c.Request.Context(), &call)
//if err != nil {
//if err == InvalidStatusChange {
//// TODO we could either let UpdateCall handle setting to error or do it
@@ -153,7 +153,7 @@ func (s *Server) handleRunnerFinish(c *gin.Context) {
// TODO this needs UpdateCall functionality to work for async and should only work if:
// running->error|timeout|success
// TODO all async will fail here :( all sync will work fine :) -- *feeling conflicted*
if err := s.Datastore.InsertCall(ctx, &call); err != nil {
if err := s.Datastore().InsertCall(ctx, &call); err != nil {
common.Logger(ctx).WithError(err).Error("error inserting call into datastore")
// note: Not returning err here since the job could have already finished successfully.
}

View File

@@ -69,13 +69,15 @@ func (s *Server) runMiddleware(c *gin.Context, ms []fnext.Middleware) {
ctx := context.WithValue(c.Request.Context(), fnext.MiddlewareControllerKey, s.newMiddlewareController(c))
last := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// fmt.Println("final handler called")
ctx := r.Context()
mctx := fnext.GetMiddlewareController(ctx)
// check for bypass
mctx := fnext.GetMiddlewareController(r.Context())
if mctx.FunctionCalled() {
// fmt.Println("func already called, skipping")
c.Abort()
return
}
c.Request = c.Request.WithContext(ctx)
c.Next()
})

View File

@@ -57,7 +57,7 @@ func (s *Server) submitRoute(ctx context.Context, wroute *models.RouteWrapper) e
if err != nil {
return err
}
r, err := s.Datastore.InsertRoute(ctx, wroute.Route)
r, err := s.Datastore().InsertRoute(ctx, wroute.Route)
if err != nil {
return err
}
@@ -66,7 +66,7 @@ func (s *Server) submitRoute(ctx context.Context, wroute *models.RouteWrapper) e
}
func (s *Server) changeRoute(ctx context.Context, wroute *models.RouteWrapper) error {
r, err := s.Datastore.UpdateRoute(ctx, wroute.Route)
r, err := s.Datastore().UpdateRoute(ctx, wroute.Route)
if err != nil {
return err
}
@@ -86,7 +86,7 @@ func (s *Server) ensureRoute(ctx context.Context, method string, wroute *models.
}
return routeResponse{"Route successfully created", wroute.Route}, nil
case http.MethodPut:
_, err := s.Datastore.GetRoute(ctx, wroute.Route.AppName, wroute.Route.Path)
_, err := s.Datastore().GetRoute(ctx, wroute.Route.AppName, wroute.Route.Path)
if err != nil && err == models.ErrRoutesNotFound {
err := s.submitRoute(ctx, wroute)
if err != nil {
@@ -111,7 +111,7 @@ func (s *Server) ensureRoute(ctx context.Context, method string, wroute *models.
// ensureApp will only execute if it is on post or put. Patch is not allowed to create apps.
func (s *Server) ensureApp(ctx context.Context, wroute *models.RouteWrapper, method string) error {
app, err := s.Datastore.GetApp(ctx, wroute.Route.AppName)
app, err := s.Datastore().GetApp(ctx, wroute.Route.AppName)
if err != nil && err != models.ErrAppsNotFound {
return err
} else if app == nil {
@@ -126,7 +126,7 @@ func (s *Server) ensureApp(ctx context.Context, wroute *models.RouteWrapper, met
return err
}
_, err = s.Datastore.InsertApp(ctx, newapp)
_, err = s.Datastore().InsertApp(ctx, newapp)
if err != nil {
return err
}

View File

@@ -14,12 +14,12 @@ func (s *Server) handleRouteDelete(c *gin.Context) {
appName := c.MustGet(api.AppName).(string)
routePath := path.Clean(c.MustGet(api.Path).(string))
if _, err := s.Datastore.GetRoute(ctx, appName, routePath); err != nil {
if _, err := s.Datastore().GetRoute(ctx, appName, routePath); err != nil {
handleErrorResponse(c, err)
return
}
if err := s.Datastore.RemoveRoute(ctx, appName, routePath); err != nil {
if err := s.Datastore().RemoveRoute(ctx, appName, routePath); err != nil {
handleErrorResponse(c, err)
return
}

View File

@@ -13,7 +13,7 @@ func (s *Server) handleRouteGet(c *gin.Context) {
appName := c.MustGet(api.AppName).(string)
routePath := path.Clean("/" + c.MustGet(api.Path).(string))
route, err := s.Datastore.GetRoute(ctx, appName, routePath)
route, err := s.Datastore().GetRoute(ctx, appName, routePath)
if err != nil {
handleErrorResponse(c, err)
return

View File

@@ -19,13 +19,13 @@ func (s *Server) handleRouteList(c *gin.Context) {
// filter.PathPrefix = c.Query("path_prefix") TODO not hooked up
filter.Cursor, filter.PerPage = pageParams(c, true)
routes, err := s.Datastore.GetRoutesByApp(ctx, appName, &filter)
routes, err := s.Datastore().GetRoutesByApp(ctx, appName, &filter)
// if there are no routes for the app, check if the app exists to return
// 404 if it does not
// TODO this should be done in front of this handler to even get here...
if err == nil && len(routes) == 0 {
_, err = s.Datastore.GetApp(ctx, appName)
_, err = s.Datastore().GetApp(ctx, appName)
}
if err != nil {

View File

@@ -19,7 +19,7 @@ func testRouterAsync(ds models.Datastore, mq models.MessageQueue, rnr agent.Agen
s := &Server{
Agent: rnr,
Router: gin.New(),
Datastore: ds,
datastore: ds,
MQ: mq,
nodeType: ServerTypeFull,
}

View File

@@ -66,7 +66,7 @@ func (s ServerNodeType) String() string {
type Server struct {
Router *gin.Engine
Agent agent.Agent
Datastore models.Datastore
datastore models.Datastore
MQ models.MessageQueue
LogDB models.LogStore
nodeType ServerNodeType
@@ -164,7 +164,7 @@ func WithType(t ServerNodeType) ServerOption {
}
func WithDatastore(ds models.Datastore) ServerOption {
return func(s *Server) { s.Datastore = ds }
return func(s *Server) { s.datastore = ds }
}
func WithMQ(mq models.MessageQueue) ServerOption {
@@ -195,7 +195,7 @@ func New(ctx context.Context, opts ...ServerOption) *Server {
}
if s.LogDB == nil { // TODO seems weird?
s.LogDB = s.Datastore
s.LogDB = s.Datastore()
}
// TODO we maybe should use the agent.DirectDataAccess in the /runner endpoints server side?
@@ -209,13 +209,13 @@ func New(ctx context.Context, opts ...ServerOption) *Server {
}
default:
s.nodeType = ServerTypeFull
if s.Datastore == nil || s.LogDB == nil || s.MQ == nil {
if s.Datastore() == nil || s.LogDB == nil || s.MQ == nil {
logrus.Fatal("Full nodes must configure FN_DB_URL, FN_LOG_URL, FN_MQ_URL.")
}
// TODO force caller to use WithAgent option ?
// TODO for tests we don't want cache, really, if we force WithAgent this can be fixed. cache needs to be moved anyway so that runner nodes can use it...
s.Agent = agent.New(agent.NewCachedDataAccess(agent.NewDirectDataAccess(s.Datastore, s.LogDB, s.MQ)))
s.Agent = agent.New(agent.NewCachedDataAccess(agent.NewDirectDataAccess(s.Datastore(), s.LogDB, s.MQ)))
}
setMachineID()
@@ -376,13 +376,14 @@ func (s *Server) bindHandlers(ctx context.Context) {
if s.nodeType != ServerTypeRunner {
v1 := engine.Group("/v1")
v1.Use(setAppNameInCtx)
v1.Use(s.apiMiddlewareWrapper())
v1.GET("/apps", s.handleAppList)
v1.POST("/apps", s.handleAppCreate)
{
apps := v1.Group("/apps/:app")
apps.Use(appWrap)
apps.Use(appNameCheck)
apps.GET("", s.handleAppGet)
apps.PATCH("", s.handleAppUpdate)
@@ -413,7 +414,7 @@ func (s *Server) bindHandlers(ctx context.Context) {
if s.nodeType != ServerTypeAPI {
runner := engine.Group("/r")
runner.Use(appWrap)
runner.Use(appNameCheck)
runner.Any("/:app", s.handleFunctionCall)
runner.Any("/:app/*route", s.handleFunctionCall)
}
@@ -433,6 +434,10 @@ func (s *Server) bindHandlers(ctx context.Context) {
})
}
func (s *Server) Datastore() models.Datastore {
return s.datastore
}
// returns the unescaped ?cursor and ?perPage values
// pageParams clamps 0 < ?perPage <= 100 and defaults to 30 if 0
// ignores parsing errors and falls back to defaults.