alihan/fn-serverless
1
0
Fork 0
mirror of https://github.com/fnproject/fn.git synced 2022-10-28 21:29:17 +03:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #224 from fnproject/222

Browse Source 
Fix route update procedure
This commit is contained in:
Denis Makogon
2017-08-18 01:47:01 +03:00
committed by GitHub
parent ddfc71196d 1f0f9c7a46
commit 31eb3852c0
11 changed files with 258 additions and 120 deletions
Download Patch File Download Diff File Expand all files Collapse all files

129
api/server/routes_create_update.go
View File

@@ -22,41 +22,97 @@ import (
update only
Patch accepts partial updates / skips validation of zero values.
*/
func (s *Server) handleRouteCreateOrUpdate(c *gin.Context) {
func (s *Server) handleRoutesPostPutPatch(c *gin.Context) {
ctx := c.Request.Context()
method := strings.ToUpper(c.Request.Method)
var wroute models.RouteWrapper
err := s.bindAndValidate(c, method, &wroute)
err := bindRoute(c, method, &wroute)
if err != nil {
handleErrorResponse(c, err)
return
}
// Create the app if it does not exist.
err = s.ensureApp(ctx, &wroute, method)
if err != nil {
handleErrorResponse(c, err)
return
if method != http.MethodPatch {
err = s.ensureApp(ctx, &wroute, method)
if err != nil {
handleErrorResponse(c, err)
return
}
}
resp, err := s.updateOrInsertRoute(ctx, method, wroute)
resp, err := s.ensureRoute(ctx, method, &wroute)
if err != nil {
handleErrorResponse(c, err)
return
}
s.cachedelete(resp.Route.AppName, resp.Route.Path)
c.JSON(http.StatusOK, resp)
}
func (s *Server) submitRoute(ctx context.Context, wroute *models.RouteWrapper) error {
err := s.setDefaultsAndValidate(wroute)
if err != nil {
return err
}
r, err := s.Datastore.InsertRoute(ctx, wroute.Route)
if err != nil {
return err
}
wroute.Route = r
return nil
}
func (s *Server) changeRoute(ctx context.Context, wroute *models.RouteWrapper) error {
err := wroute.Validate(true)
if err != nil {
return err
}
r, err := s.Datastore.UpdateRoute(ctx, wroute.Route)
if err != nil {
return err
}
wroute.Route = r
return nil
}
// ensureApp will only execute if it is on put
func (s *Server) ensureRoute(ctx context.Context, method string, wroute *models.RouteWrapper) (routeResponse, error) {
bad := new(routeResponse)
switch method {
case http.MethodPost:
err := s.submitRoute(ctx, wroute)
if err != nil {
return *bad, err
}
return routeResponse{"Route successfully created", wroute.Route}, nil
case http.MethodPut:
_, 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 {
return *bad, err
}
return routeResponse{"Route successfully created", wroute.Route}, nil
} else {
err := s.changeRoute(ctx, wroute)
if err != nil {
return *bad, err
}
return routeResponse{"Route successfully updated", wroute.Route}, nil
}
case http.MethodPatch:
err := s.changeRoute(ctx, wroute)
if err != nil {
return *bad, err
}
return routeResponse{"Route successfully updated", wroute.Route}, nil
}
return *bad, nil
}
// 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 {
if !(method == http.MethodPost || method == http.MethodPut) {
return nil
}
app, err := s.Datastore.GetApp(ctx, wroute.Route.AppName)
if err != nil && err != models.ErrAppsNotFound {
return err
@@ -86,11 +142,8 @@ func (s *Server) ensureApp(ctx context.Context, wroute *models.RouteWrapper, met
return nil
}
/* bindAndValidate binds the RouteWrapper to the json from the request and validates that it is correct.
If it is a put or patch it makes sure that the path in the url matches the provideed one in the body.
Defaults are set and if patch skipZero is true for validating the RouteWrapper
*/
func (s *Server) bindAndValidate(c *gin.Context, method string, wroute *models.RouteWrapper) error {
// bindRoute binds the RouteWrapper to the json from the request.
func bindRoute(c *gin.Context, method string, wroute *models.RouteWrapper) error {
err := c.BindJSON(wroute)
if err != nil {
return models.ErrInvalidJSON
@@ -109,33 +162,15 @@ func (s *Server) bindAndValidate(c *gin.Context, method string, wroute *models.R
}
wroute.Route.Path = p
}
wroute.Route.SetDefaults()
return wroute.Validate(method == http.MethodPatch)
}
// updateOrInsertRoute will either update or insert the route respective the method.
func (s *Server) updateOrInsertRoute(ctx context.Context, method string, wroute models.RouteWrapper) (routeResponse, error) {
var route *models.Route
var err error
resp := routeResponse{"Route successfully created", nil}
up := routeResponse{"Route successfully updated", nil}
switch method {
case http.MethodPost:
route, err = s.Datastore.InsertRoute(ctx, wroute.Route)
case http.MethodPut:
route, err = s.Datastore.UpdateRoute(ctx, wroute.Route)
if err == models.ErrRoutesNotFound {
// try insert then
route, err = s.Datastore.InsertRoute(ctx, wroute.Route)
if method == http.MethodPost {
if wroute.Route.Path == "" {
return models.ErrRoutesValidationMissingPath
}
case http.MethodPatch:
// When patching if there is an error around the app we will return one and the update fails.
route, err = s.Datastore.UpdateRoute(ctx, wroute.Route)
resp = up
}
resp.Route = route
return resp, err
return nil
}
func (s *Server) setDefaultsAndValidate(wroute *models.RouteWrapper) error {
wroute.Route.SetDefaults()
return wroute.Validate(false)
}

43
api/server/routes_test.go
View File

@@ -57,13 +57,13 @@ func TestRouteCreate(t *testing.T) {
for i, test := range []routeTestCase{
// errors
{datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", ``, http.StatusBadRequest, models.ErrInvalidJSON},
{datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ }`, http.StatusBadRequest, models.ErrRoutesMissingNew},
{datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "path": "/myroute" }`, http.StatusBadRequest, models.ErrRoutesMissingNew},
{datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "type": "sync" }`, http.StatusBadRequest, models.ErrRoutesMissingNew},
{datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "path": "/myroute", "type": "sync" }`, http.StatusBadRequest, models.ErrRoutesMissingNew},
{datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingPath},
{datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "path": "/myroute" } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingImage},
{datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "funcy/hello" } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingPath},
{datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "funcy/hello", "path": "myroute" } }`, http.StatusBadRequest, models.ErrRoutesValidationInvalidPath},
{datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/$/routes", `{ "route": { "image": "funcy/hello", "path": "/myroute" } }`, http.StatusBadRequest, models.ErrAppsValidationInvalidName},
{datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "path": "/myroute", "type": "sync" } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingImage},
{datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "funcy/hello", "type": "sync" } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingPath},
{datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "funcy/hello", "path": "myroute", "type": "sync" } }`, http.StatusBadRequest, models.ErrRoutesValidationInvalidPath},
{datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/$/routes", `{ "route": { "image": "funcy/hello", "path": "/myroute", "type": "sync" } }`, http.StatusBadRequest, models.ErrAppsValidationInvalidName},
{datastore.NewMockInit(nil,
[]*models.Route{
{
@@ -71,10 +71,10 @@ func TestRouteCreate(t *testing.T) {
Path: "/myroute",
},
}, nil, nil,
), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "funcy/hello", "path": "/myroute" } }`, http.StatusConflict, models.ErrRoutesAlreadyExists},
), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "funcy/hello", "path": "/myroute", "type": "sync" } }`, http.StatusConflict, models.ErrRoutesAlreadyExists},
// success
{datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "funcy/hello", "path": "/myroute" } }`, http.StatusOK, nil},
{datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "funcy/hello", "path": "/myroute", "type": "sync" } }`, http.StatusOK, nil},
} {
test.run(t, i, buf)
}
@@ -86,29 +86,20 @@ func TestRoutePut(t *testing.T) {
for i, test := range []routeTestCase{
// errors (NOTE: this route doesn't exist yet)
{datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ }`, http.StatusBadRequest, models.ErrRoutesMissingNew},
{datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "path": "/myroute" }`, http.StatusBadRequest, models.ErrRoutesMissingNew},
{datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingImage},
{datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "path": "/myroute" } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingImage},
{datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "myroute" } }`, http.StatusConflict, models.ErrRoutesPathImmutable},
{datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "diffRoute" } }`, http.StatusConflict, models.ErrRoutesPathImmutable},
{datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/$/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "/myroute" } }`, http.StatusBadRequest, models.ErrAppsValidationInvalidName},
{datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "path": "/myroute", "type": "sync" }`, http.StatusBadRequest, models.ErrRoutesMissingNew},
{datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "type": "sync" } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingImage},
{datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "path": "/myroute", "type": "sync" } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingImage},
{datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "myroute", "type": "sync" } }`, http.StatusConflict, models.ErrRoutesPathImmutable},
{datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "diffRoute", "type": "sync" } }`, http.StatusConflict, models.ErrRoutesPathImmutable},
{datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/$/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "/myroute", "type": "sync" } }`, http.StatusBadRequest, models.ErrAppsValidationInvalidName},
{datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "/myroute", "type": "invalid-type" } }`, http.StatusBadRequest, models.ErrRoutesValidationInvalidType},
{datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "/myroute", "format": "invalid-format" } }`, http.StatusBadRequest, models.ErrRoutesValidationInvalidFormat},
{datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "/myroute", "format": "invalid-format", "type": "sync" } }`, http.StatusBadRequest, models.ErrRoutesValidationInvalidFormat},
// success
{datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "/myroute" } }`, http.StatusOK, nil},
{datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello" } }`, http.StatusOK, nil},
{datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "/myroute", "type": "sync" } }`, http.StatusOK, nil},
{datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello", "type": "sync" } }`, http.StatusOK, nil},
} {
test.run(t, i, buf)
test.ds = datastore.NewMockInit(nil,
[]*models.Route{
{
AppName: "a",
Path: "/myroute",
},
}, nil, nil,
)
test.run(t, i, buf)
}
}

6
api/server/server.go
View File

@@ -365,10 +365,10 @@ func (s *Server) bindHandlers(ctx context.Context) {
apps := v1.Group("/apps/:app")
{
apps.GET("/routes", s.handleRouteList)
apps.POST("/routes", s.handleRouteCreateOrUpdate)
apps.POST("/routes", s.handleRoutesPostPutPatch)
apps.GET("/routes/*route", s.handleRouteGet)
apps.PATCH("/routes/*route", s.handleRouteCreateOrUpdate)
apps.PUT("/routes/*route", s.handleRouteCreateOrUpdate)
apps.PATCH("/routes/*route", s.handleRoutesPostPutPatch)
apps.PUT("/routes/*route", s.handleRoutesPostPutPatch)
apps.DELETE("/routes/*route", s.handleRouteDelete)
apps.GET("/calls", s.handleCallList)

4
api/server/server_test.go
View File

@@ -116,8 +116,8 @@ func TestFullStack(t *testing.T) {
{"list apps", "GET", "/v1/apps", ``, http.StatusOK, 0},
{"get app", "GET", "/v1/apps/myapp", ``, http.StatusOK, 0},
// NOTE: cache is lazy, loads when a request comes in for the route, not when added
{"add myroute", "POST", "/v1/apps/myapp/routes", `{ "route": { "name": "myroute", "path": "/myroute", "image": "funcy/hello" } }`, http.StatusOK, 0},
{"add myroute2", "POST", "/v1/apps/myapp/routes", `{ "route": { "name": "myroute2", "path": "/myroute2", "image": "funcy/error" } }`, http.StatusOK, 0},
{"add myroute", "POST", "/v1/apps/myapp/routes", `{ "route": { "name": "myroute", "path": "/myroute", "image": "funcy/hello", "type": "sync" } }`, http.StatusOK, 0},
{"add myroute2", "POST", "/v1/apps/myapp/routes", `{ "route": { "name": "myroute2", "path": "/myroute2", "image": "funcy/error", "type": "sync" } }`, http.StatusOK, 0},
{"get myroute", "GET", "/v1/apps/myapp/routes/myroute", ``, http.StatusOK, 0},
{"get myroute2", "GET", "/v1/apps/myapp/routes/myroute2", ``, http.StatusOK, 0},
{"get all routes", "GET", "/v1/apps/myapp/routes", ``, http.StatusOK, 0},