API improvements (#410)

* api improvements, remove global Api object and reduce gin dependency

* requested changes
This commit is contained in:
Pedro Nasser
2016-12-09 15:24:35 -02:00
committed by GitHub
parent 0c3c9ff2ee
commit 49a7712e6b
18 changed files with 126 additions and 107 deletions

View File

@@ -34,21 +34,21 @@ func (s *Server) handleAppCreate(c *gin.Context) {
return
}
err = Api.FireBeforeAppCreate(ctx, wapp.App)
err = s.FireBeforeAppCreate(ctx, wapp.App)
if err != nil {
log.WithError(err).Errorln(models.ErrAppsCreate)
c.JSON(http.StatusInternalServerError, simpleError(err))
return
}
app, err := Api.Datastore.InsertApp(ctx, wapp.App)
app, err := s.Datastore.InsertApp(ctx, wapp.App)
if err != nil {
log.WithError(err).Errorln(models.ErrAppsCreate)
c.JSON(http.StatusInternalServerError, simpleError(err))
return
}
err = Api.FireAfterAppCreate(ctx, wapp.App)
err = s.FireAfterAppCreate(ctx, wapp.App)
if err != nil {
log.WithError(err).Errorln(models.ErrAppsCreate)
c.JSON(http.StatusInternalServerError, simpleError(err))

View File

@@ -9,13 +9,13 @@ import (
"github.com/iron-io/runner/common"
)
func handleAppDelete(c *gin.Context) {
func (s *Server) handleAppDelete(c *gin.Context) {
ctx := c.MustGet("ctx").(context.Context)
log := common.Logger(ctx)
appName := c.Param("app")
appName := ctx.Value("appName").(string)
routes, err := Api.Datastore.GetRoutesByApp(ctx, appName, &models.RouteFilter{})
routes, err := s.Datastore.GetRoutesByApp(ctx, appName, &models.RouteFilter{})
if err != nil {
log.WithError(err).Debug(models.ErrAppsRemoving)
c.JSON(http.StatusInternalServerError, simpleError(models.ErrAppsRemoving))
@@ -28,20 +28,24 @@ func handleAppDelete(c *gin.Context) {
return
}
err = Api.FireBeforeAppDelete(ctx, appName)
err = s.FireBeforeAppDelete(ctx, appName)
if err != nil {
log.WithError(err).Errorln(models.ErrAppsRemoving)
c.JSON(http.StatusInternalServerError, simpleError(err))
return
}
if err = Api.Datastore.RemoveApp(ctx, appName); err != nil {
if err = s.Datastore.RemoveApp(ctx, appName); err != nil {
log.WithError(err).Debug(models.ErrAppsRemoving)
c.JSON(http.StatusInternalServerError, simpleError(models.ErrAppsRemoving))
if err == models.ErrAppsNotFound {
c.JSON(http.StatusNotFound, simpleError(models.ErrAppsNotFound))
} else {
c.JSON(http.StatusInternalServerError, simpleError(models.ErrAppsRemoving))
}
return
}
err = Api.FireAfterAppDelete(ctx, appName)
err = s.FireAfterAppDelete(ctx, appName)
if err != nil {
log.WithError(err).Errorln(models.ErrAppsRemoving)
c.JSON(http.StatusInternalServerError, simpleError(err))

View File

@@ -9,12 +9,12 @@ import (
"github.com/iron-io/runner/common"
)
func handleAppGet(c *gin.Context) {
func (s *Server) handleAppGet(c *gin.Context) {
ctx := c.MustGet("ctx").(context.Context)
log := common.Logger(ctx)
appName := c.Param("app")
app, err := Api.Datastore.GetApp(ctx, appName)
appName := ctx.Value("appName").(string)
app, err := s.Datastore.GetApp(ctx, appName)
if err != nil && err != models.ErrAppsNotFound {
log.WithError(err).Error(models.ErrAppsGet)

View File

@@ -9,13 +9,13 @@ import (
"github.com/iron-io/runner/common"
)
func handleAppList(c *gin.Context) {
func (s *Server) handleAppList(c *gin.Context) {
ctx := c.MustGet("ctx").(context.Context)
log := common.Logger(ctx)
filter := &models.AppFilter{}
apps, err := Api.Datastore.GetApps(ctx, filter)
apps, err := s.Datastore.GetApps(ctx, filter)
if err != nil {
log.WithError(err).Debug(models.ErrAppsList)
c.JSON(http.StatusInternalServerError, simpleError(models.ErrAppsList))

View File

@@ -94,7 +94,7 @@ func TestAppDelete(t *testing.T) {
expectedCode int
expectedError error
}{
{&datastore.Mock{}, "/v1/apps", "", http.StatusNotFound, nil},
{&datastore.Mock{}, "/v1/apps/myapp", "", http.StatusNotFound, nil},
{&datastore.Mock{
Apps: []*models.App{{
Name: "myapp",

View File

@@ -9,7 +9,7 @@ import (
"github.com/iron-io/runner/common"
)
func handleAppUpdate(c *gin.Context) {
func (s *Server) handleAppUpdate(c *gin.Context) {
ctx := c.MustGet("ctx").(context.Context)
log := common.Logger(ctx)
@@ -34,23 +34,23 @@ func handleAppUpdate(c *gin.Context) {
return
}
wapp.App.Name = c.Param("app")
wapp.App.Name = ctx.Value("appName").(string)
err = Api.FireAfterAppUpdate(ctx, wapp.App)
err = s.FireAfterAppUpdate(ctx, wapp.App)
if err != nil {
log.WithError(err).Errorln(models.ErrAppsUpdate)
c.JSON(http.StatusInternalServerError, simpleError(err))
return
}
app, err := Api.Datastore.UpdateApp(ctx, wapp.App)
app, err := s.Datastore.UpdateApp(ctx, wapp.App)
if err != nil {
log.WithError(err).Debug(models.ErrAppsUpdate)
c.JSON(http.StatusInternalServerError, simpleError(models.ErrAppsUpdate))
return
}
err = Api.FireAfterAppUpdate(ctx, wapp.App)
err = s.FireAfterAppUpdate(ctx, wapp.App)
if err != nil {
log.WithError(err).Errorln(models.ErrAppsUpdate)
c.JSON(http.StatusInternalServerError, simpleError(err))

View File

@@ -29,7 +29,7 @@ func (s *Server) handleRouteCreate(c *gin.Context) {
return
}
wroute.Route.AppName = c.Param("app")
wroute.Route.AppName = ctx.Value("appName").(string)
if err := wroute.Validate(); err != nil {
log.Error(err)
@@ -41,7 +41,7 @@ func (s *Server) handleRouteCreate(c *gin.Context) {
c.JSON(http.StatusBadRequest, simpleError(models.ErrRoutesValidationMissingImage))
return
}
err = Api.Runner.EnsureImageExists(ctx, &task.Config{
err = s.Runner.EnsureImageExists(ctx, &task.Config{
Image: wroute.Route.Image,
})
if err != nil {
@@ -49,7 +49,7 @@ func (s *Server) handleRouteCreate(c *gin.Context) {
return
}
app, err := Api.Datastore.GetApp(ctx, wroute.Route.AppName)
app, err := s.Datastore.GetApp(ctx, wroute.Route.AppName)
if err != nil && err != models.ErrAppsNotFound {
log.WithError(err).Error(models.ErrAppsGet)
c.JSON(http.StatusInternalServerError, simpleError(models.ErrAppsGet))
@@ -63,21 +63,21 @@ func (s *Server) handleRouteCreate(c *gin.Context) {
return
}
err = Api.FireBeforeAppCreate(ctx, newapp)
err = s.FireBeforeAppCreate(ctx, newapp)
if err != nil {
log.WithError(err).Errorln(models.ErrAppsCreate)
c.JSON(http.StatusInternalServerError, simpleError(err))
return
}
_, err = Api.Datastore.InsertApp(ctx, newapp)
_, err = s.Datastore.InsertApp(ctx, newapp)
if err != nil {
log.WithError(err).Error(models.ErrAppsCreate)
c.JSON(http.StatusInternalServerError, simpleError(models.ErrAppsCreate))
return
}
err = Api.FireAfterAppCreate(ctx, newapp)
err = s.FireAfterAppCreate(ctx, newapp)
if err != nil {
log.WithError(err).Errorln(models.ErrAppsCreate)
c.JSON(http.StatusInternalServerError, simpleError(err))
@@ -86,7 +86,7 @@ func (s *Server) handleRouteCreate(c *gin.Context) {
}
route, err := Api.Datastore.InsertRoute(ctx, wroute.Route)
route, err := s.Datastore.InsertRoute(ctx, wroute.Route)
if err != nil {
log.WithError(err).Error(models.ErrRoutesCreate)
c.JSON(http.StatusInternalServerError, simpleError(models.ErrRoutesCreate))

View File

@@ -14,19 +14,23 @@ func (s *Server) handleRouteDelete(c *gin.Context) {
ctx := c.MustGet("ctx").(context.Context)
log := common.Logger(ctx)
appName := c.Param("app")
routePath := path.Clean(c.Param("route"))
appName := ctx.Value("appName").(string)
routePath := path.Clean(ctx.Value("routePath").(string))
route, err := Api.Datastore.GetRoute(ctx, appName, routePath)
route, err := s.Datastore.GetRoute(ctx, appName, routePath)
if err != nil || route == nil {
log.Error(models.ErrRoutesNotFound)
c.JSON(http.StatusNotFound, simpleError(models.ErrRoutesNotFound))
return
}
if err := Api.Datastore.RemoveRoute(ctx, appName, routePath); err != nil {
if err := s.Datastore.RemoveRoute(ctx, appName, routePath); err != nil {
log.WithError(err).Debug(models.ErrRoutesRemoving)
c.JSON(http.StatusInternalServerError, simpleError(models.ErrRoutesRemoving))
if err == models.ErrRoutesNotFound {
c.JSON(http.StatusNotFound, simpleError(models.ErrRoutesNotFound))
} else {
c.JSON(http.StatusInternalServerError, simpleError(models.ErrRoutesRemoving))
}
return
}

View File

@@ -11,14 +11,14 @@ import (
"github.com/iron-io/runner/common"
)
func handleRouteGet(c *gin.Context) {
func (s *Server) handleRouteGet(c *gin.Context) {
ctx := c.MustGet("ctx").(context.Context)
log := common.Logger(ctx)
appName := c.Param("app")
routePath := path.Clean(c.Param("route"))
appName := ctx.Value("appName").(string)
routePath := path.Clean(ctx.Value("routePath").(string))
route, err := Api.Datastore.GetRoute(ctx, appName, routePath)
route, err := s.Datastore.GetRoute(ctx, appName, routePath)
if err != nil && err != models.ErrRoutesNotFound {
log.WithError(err).Error(models.ErrRoutesGet)

View File

@@ -10,7 +10,7 @@ import (
"github.com/iron-io/runner/common"
)
func handleRouteList(c *gin.Context) {
func (s *Server) handleRouteList(c *gin.Context) {
ctx := c.MustGet("ctx").(context.Context)
log := common.Logger(ctx)
@@ -22,10 +22,10 @@ func handleRouteList(c *gin.Context) {
var routes []*models.Route
var err error
if app := c.Param("app"); app != "" {
routes, err = Api.Datastore.GetRoutesByApp(ctx, app, filter)
if appName, ok := ctx.Value("appName").(string); ok && appName != "" {
routes, err = s.Datastore.GetRoutesByApp(ctx, appName, filter)
} else {
routes, err = Api.Datastore.GetRoutes(ctx, filter)
routes, err = s.Datastore.GetRoutes(ctx, filter)
}
if err != nil {

View File

@@ -73,7 +73,6 @@ func TestRouteDelete(t *testing.T) {
expectedCode int
expectedError error
}{
{&datastore.Mock{}, "/v1/apps/a/routes", "", http.StatusTemporaryRedirect, nil},
{&datastore.Mock{}, "/v1/apps/a/routes/missing", "", http.StatusNotFound, nil},
{&datastore.Mock{
Routes: []*models.Route{

View File

@@ -11,7 +11,7 @@ import (
"github.com/iron-io/runner/common"
)
func handleRouteUpdate(c *gin.Context) {
func (s *Server) handleRouteUpdate(c *gin.Context) {
ctx := c.MustGet("ctx").(context.Context)
log := common.Logger(ctx)
@@ -36,11 +36,11 @@ func handleRouteUpdate(c *gin.Context) {
return
}
wroute.Route.AppName = c.Param("app")
wroute.Route.Path = path.Clean(c.Param("route"))
wroute.Route.AppName = ctx.Value("appName").(string)
wroute.Route.Path = path.Clean(ctx.Value("routePath").(string))
if wroute.Route.Image != "" {
err = Api.Runner.EnsureImageExists(ctx, &task.Config{
err = s.Runner.EnsureImageExists(ctx, &task.Config{
Image: wroute.Route.Image,
})
if err != nil {
@@ -49,7 +49,7 @@ func handleRouteUpdate(c *gin.Context) {
}
}
route, err := Api.Datastore.UpdateRoute(ctx, wroute.Route)
route, err := s.Datastore.UpdateRoute(ctx, wroute.Route)
if err != nil {
log.WithError(err).Debug(models.ErrRoutesUpdate)
c.JSON(http.StatusInternalServerError, simpleError(models.ErrRoutesUpdate))

View File

@@ -3,10 +3,12 @@ package server
import (
"bytes"
"context"
"errors"
"fmt"
"io"
"io/ioutil"
"net/http"
"path"
"strings"
"time"
@@ -19,15 +21,30 @@ import (
uuid "github.com/satori/go.uuid"
)
func handleSpecial(c *gin.Context) {
func (s *Server) handleSpecial(c *gin.Context) {
ctx := c.MustGet("ctx").(context.Context)
log := common.Logger(ctx)
err := Api.UseSpecialHandlers(c)
ctx = context.WithValue(ctx, "appName", "")
ctx = context.WithValue(ctx, "routePath", c.Request.URL.Path)
c.Set("ctx", ctx)
err := s.UseSpecialHandlers(c)
if err != nil {
log.WithError(err).Errorln("Error using special handler!")
// todo: what do we do here? Should probably return a 500 or something
c.JSON(http.StatusInternalServerError, simpleError(errors.New("Failed to run function")))
return
}
ctx = c.MustGet("ctx").(context.Context)
if ctx.Value("appName").(string) == "" {
log.WithError(err).Errorln("Specialhandler returned empty app name")
c.JSON(http.StatusBadRequest, simpleError(models.ErrRunnerRouteNotFound))
return
}
// now call the normal runner call
s.handleRequest(c, nil)
}
func ToEnvName(envtype, name string) string {
@@ -44,8 +61,6 @@ func (s *Server) handleRequest(c *gin.Context, enqueue models.Enqueue) {
ctx := c.MustGet("ctx").(context.Context)
reqID := uuid.NewV5(uuid.Nil, fmt.Sprintf("%s%s%d", c.Request.RemoteAddr, c.Request.URL.Path, time.Now().Unix())).String()
c.Set("reqID", reqID) // todo: put this in the ctx instead of gin's
ctx, log := common.LoggerWithFields(ctx, logrus.Fields{"call_id": reqID})
var err error
@@ -63,26 +78,10 @@ func (s *Server) handleRequest(c *gin.Context, enqueue models.Enqueue) {
payload = strings.NewReader(reqPayload)
}
appName := c.Param("app")
if appName == "" {
// check context, app can be added via special handlers
a, ok := c.Get("app")
if ok {
appName = a.(string)
}
}
// if still no appName, we gotta exit
if appName == "" {
log.WithError(err).Error("Invalid app, blank")
c.JSON(http.StatusBadRequest, simpleError(models.ErrAppsNotFound))
return
}
path := c.Param("route")
if path == "" {
path = c.Request.URL.Path
}
appName := ctx.Value("appName").(string)
path := path.Clean(ctx.Value("routePath").(string))
app, err := Api.Datastore.GetApp(ctx, appName)
app, err := s.Datastore.GetApp(ctx, appName)
if err != nil || app == nil {
log.WithError(err).Error(models.ErrAppsNotFound)
c.JSON(http.StatusNotFound, simpleError(models.ErrAppsNotFound))
@@ -119,7 +118,7 @@ func (s *Server) loadroutes(ctx context.Context, filter models.RouteFilter) ([]*
resp, err := s.singleflight.do(
filter,
func() (interface{}, error) {
return Api.Datastore.GetRoutesByApp(ctx, filter.AppName, &filter)
return s.Datastore.GetRoutesByApp(ctx, filter.AppName, &filter)
},
)
return resp.([]*models.Route), err

View File

@@ -14,7 +14,6 @@ import (
"github.com/iron-io/functions/api/mqs"
"github.com/iron-io/functions/api/runner"
"github.com/iron-io/functions/api/runner/task"
"github.com/iron-io/runner/common"
)
func testRouterAsync(ds models.Datastore, mq models.MessageQueue, rnr *runner.Runner, tasks chan task.Request, enqueue models.Enqueue) *gin.Engine {
@@ -23,11 +22,7 @@ func testRouterAsync(ds models.Datastore, mq models.MessageQueue, rnr *runner.Ru
r := s.Router
r.Use(gin.Logger())
r.Use(func(c *gin.Context) {
ctx, _ := common.LoggerWithFields(ctx, extractFields(c))
c.Set("ctx", ctx)
c.Next()
})
r.Use(prepareMiddleware(ctx))
s.bindHandlers()
return r
}

View File

@@ -42,7 +42,7 @@ func TestRouteRunnerGet(t *testing.T) {
expectedCode int
expectedError error
}{
{"/route", "", http.StatusBadRequest, models.ErrAppsNotFound},
{"/route", "", http.StatusBadRequest, models.ErrRunnerRouteNotFound},
{"/r/app/route", "", http.StatusNotFound, models.ErrAppsNotFound},
{"/r/myapp/route", "", http.StatusNotFound, models.ErrRunnerRouteNotFound},
} {
@@ -85,7 +85,7 @@ func TestRouteRunnerPost(t *testing.T) {
expectedCode int
expectedError error
}{
{"/route", `{ "payload": "" }`, http.StatusBadRequest, models.ErrAppsNotFound},
{"/route", `{ "payload": "" }`, http.StatusBadRequest, models.ErrRunnerRouteNotFound},
{"/r/app/route", `{ "payload": "" }`, http.StatusNotFound, models.ErrAppsNotFound},
{"/r/myapp/route", `{ "payload": "" }`, http.StatusNotFound, models.ErrRunnerRouteNotFound},
} {

View File

@@ -17,10 +17,6 @@ import (
"github.com/iron-io/runner/common"
)
// Would be nice to not have this is a global, but hard to pass things around to the
// handlers in Gin without it.
var Api *Server
type Server struct {
Runner *runner.Runner
Router *gin.Engine
@@ -38,7 +34,7 @@ type Server struct {
}
func New(ctx context.Context, ds models.Datastore, mq models.MessageQueue, r *runner.Runner, tasks chan task.Request, enqueue models.Enqueue) *Server {
Api = &Server{
s := &Server{
Runner: r,
Router: gin.New(),
Datastore: ds,
@@ -47,13 +43,26 @@ func New(ctx context.Context, ds models.Datastore, mq models.MessageQueue, r *ru
Enqueue: enqueue,
}
Api.Router.Use(func(c *gin.Context) {
s.Router.Use(prepareMiddleware(ctx))
return s
}
func prepareMiddleware(ctx context.Context) gin.HandlerFunc {
return func(c *gin.Context) {
ctx, _ := common.LoggerWithFields(ctx, extractFields(c))
if appName := c.Param("app"); appName != "" {
ctx = context.WithValue(ctx, "appName", appName)
}
if routePath := c.Param("route"); routePath != "" {
ctx = context.WithValue(ctx, "routePath", routePath)
}
c.Set("ctx", ctx)
c.Next()
})
return Api
}
}
func (s *Server) AddSpecialHandler(handler ifaces.SpecialHandler) {
@@ -71,8 +80,6 @@ func (s *Server) UseSpecialHandlers(ginC *gin.Context) error {
return err
}
}
// now call the normal runner call
s.handleRequest(ginC, nil)
return nil
}
@@ -143,21 +150,21 @@ func (s *Server) bindHandlers() {
v1 := engine.Group("/v1")
{
v1.GET("/apps", handleAppList)
v1.GET("/apps", s.handleAppList)
v1.POST("/apps", s.handleAppCreate)
v1.GET("/apps/:app", handleAppGet)
v1.PUT("/apps/:app", handleAppUpdate)
v1.DELETE("/apps/:app", handleAppDelete)
v1.GET("/apps/:app", s.handleAppGet)
v1.PUT("/apps/:app", s.handleAppUpdate)
v1.DELETE("/apps/:app", s.handleAppDelete)
v1.GET("/routes", handleRouteList)
v1.GET("/routes", s.handleRouteList)
apps := v1.Group("/apps/:app")
{
apps.GET("/routes", handleRouteList)
apps.GET("/routes", s.handleRouteList)
apps.POST("/routes", s.handleRouteCreate)
apps.GET("/routes/*route", handleRouteGet)
apps.PUT("/routes/*route", handleRouteUpdate)
apps.GET("/routes/*route", s.handleRouteGet)
apps.PUT("/routes/*route", s.handleRouteUpdate)
apps.DELETE("/routes/*route", s.handleRouteDelete)
}
}
@@ -167,7 +174,7 @@ func (s *Server) bindHandlers() {
engine.Any("/r/:app/*route", s.handleRunnerRequest)
// This final route is used for extensions, see Server.Add
engine.NoRoute(handleSpecial)
engine.NoRoute(s.handleSpecial)
}
var ErrInternalServerError = errors.New("Something unexpected happened on the server")

View File

@@ -17,7 +17,6 @@ import (
"github.com/iron-io/functions/api/mqs"
"github.com/iron-io/functions/api/runner"
"github.com/iron-io/functions/api/runner/task"
"github.com/iron-io/runner/common"
)
var tmpBolt = "/tmp/func_test_bolt.db"
@@ -28,11 +27,7 @@ func testRouter(ds models.Datastore, mq models.MessageQueue, rnr *runner.Runner,
r := s.Router
r.Use(gin.Logger())
r.Use(func(c *gin.Context) {
ctx, _ := common.LoggerWithFields(ctx, extractFields(c))
c.Set("ctx", ctx)
c.Next()
})
r.Use(prepareMiddleware(ctx))
s.bindHandlers()
return r
}

View File

@@ -6,7 +6,7 @@ swagger: '2.0'
info:
title: IronFunctions
description: The open source serverless platform.
version: "0.1.24"
version: "0.1.25"
# the domain of the service
host: "127.0.0.1:8080"
# array of all schemes that your API supports
@@ -91,6 +91,14 @@ paths:
responses:
200:
description: Apps successfully deleted.
404:
description: App does not exist.
schema:
$ref: '#/definitions/Error'
default:
description: Unexpected error
schema:
$ref: '#/definitions/Error'
get:
summary: "Get information for a app."
description: "This gives more details about a app, such as statistics."
@@ -296,6 +304,14 @@ paths:
responses:
200:
description: Route successfully deleted.
404:
description: Route does not exist.
schema:
$ref: '#/definitions/Error'
default:
description: Unexpected error
schema:
$ref: '#/definitions/Error'
/tasks:
get: