Only use 200 follows what others do. Less switching. Remove defense

This commit is contained in:
James Jeffrey
2017-07-05 12:30:18 -07:00
parent c66a0d0cc4
commit 4845ddb1d4
3 changed files with 26 additions and 47 deletions

View File

@@ -2,7 +2,6 @@ package server
import ( import (
"context" "context"
"fmt"
"net/http" "net/http"
"path" "path"
"strings" "strings"
@@ -25,12 +24,7 @@ func (s *Server) handleRouteCreateOrUpdate(c *gin.Context) {
ctx := c.MustGet("ctx").(context.Context) ctx := c.MustGet("ctx").(context.Context)
log := common.Logger(ctx) log := common.Logger(ctx)
method := strings.ToUpper(c.Request.Method) method := strings.ToUpper(c.Request.Method)
switch method {
case http.MethodPost, http.MethodPut, http.MethodPatch:
default:
c.JSON(http.StatusMethodNotAllowed, simpleError(fmt.Errorf(http.StatusText(http.StatusMethodNotAllowed))))
return
}
var wroute models.RouteWrapper var wroute models.RouteWrapper
err := c.BindJSON(&wroute) err := c.BindJSON(&wroute)
@@ -110,22 +104,21 @@ func (s *Server) handleRouteCreateOrUpdate(c *gin.Context) {
var route *models.Route var route *models.Route
var createdOrUpdated int resp := routeResponse{"Route successfully created", route}
up := routeResponse{"Route successfully updated", route}
switch method { switch method {
case http.MethodPost: case http.MethodPost:
route, err = s.Datastore.InsertRoute(ctx, wroute.Route) route, err = s.Datastore.InsertRoute(ctx, wroute.Route)
createdOrUpdated = 1
case http.MethodPut: case http.MethodPut:
route, err = s.Datastore.UpdateRoute(ctx, wroute.Route) route, err = s.Datastore.UpdateRoute(ctx, wroute.Route)
createdOrUpdated = 2
if err == models.ErrRoutesNotFound { if err == models.ErrRoutesNotFound {
// try insert then // try insert then
route, err = s.Datastore.InsertRoute(ctx, wroute.Route) route, err = s.Datastore.InsertRoute(ctx, wroute.Route)
createdOrUpdated = 1
} }
case http.MethodPatch: case http.MethodPatch:
route, err = s.Datastore.UpdateRoute(ctx, wroute.Route) route, err = s.Datastore.UpdateRoute(ctx, wroute.Route)
createdOrUpdated = 2 resp = up
} }
if err != nil { if err != nil {
@@ -135,15 +128,5 @@ func (s *Server) handleRouteCreateOrUpdate(c *gin.Context) {
s.cacheRefresh(route) s.cacheRefresh(route)
var msg string c.JSON(http.StatusOK, resp)
var code int
switch createdOrUpdated {
case 1:
msg = "Route successfully created"
code = http.StatusCreated
case 2:
msg = "Route successfully updated"
code = http.StatusOK
}
c.JSON(code, routeResponse{msg, route})
} }

View File

@@ -7,9 +7,9 @@ import (
"testing" "testing"
"gitlab-odx.oracle.com/odx/functions/api/datastore" "gitlab-odx.oracle.com/odx/functions/api/datastore"
"gitlab-odx.oracle.com/odx/functions/api/logs"
"gitlab-odx.oracle.com/odx/functions/api/models" "gitlab-odx.oracle.com/odx/functions/api/models"
"gitlab-odx.oracle.com/odx/functions/api/mqs" "gitlab-odx.oracle.com/odx/functions/api/mqs"
"gitlab-odx.oracle.com/odx/functions/api/logs"
) )
func TestRouteCreate(t *testing.T) { func TestRouteCreate(t *testing.T) {
@@ -24,17 +24,17 @@ func TestRouteCreate(t *testing.T) {
expectedError error expectedError error
}{ }{
// errors // errors
{datastore.NewMock(), logs.NewMock(),"/v1/apps/a/routes", ``, http.StatusBadRequest, models.ErrInvalidJSON}, {datastore.NewMock(), logs.NewMock(), "/v1/apps/a/routes", ``, http.StatusBadRequest, models.ErrInvalidJSON},
{datastore.NewMock(), logs.NewMock(),"/v1/apps/a/routes", `{ }`, http.StatusBadRequest, models.ErrRoutesMissingNew}, {datastore.NewMock(), logs.NewMock(), "/v1/apps/a/routes", `{ }`, http.StatusBadRequest, models.ErrRoutesMissingNew},
{datastore.NewMock(), logs.NewMock(),"/v1/apps/a/routes", `{ "path": "/myroute" }`, http.StatusBadRequest, models.ErrRoutesMissingNew}, {datastore.NewMock(), logs.NewMock(), "/v1/apps/a/routes", `{ "path": "/myroute" }`, http.StatusBadRequest, models.ErrRoutesMissingNew},
{datastore.NewMock(), logs.NewMock(),"/v1/apps/a/routes", `{ "route": { } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingPath}, {datastore.NewMock(), logs.NewMock(), "/v1/apps/a/routes", `{ "route": { } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingPath},
{datastore.NewMock(), logs.NewMock(),"/v1/apps/a/routes", `{ "route": { "path": "/myroute" } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingImage}, {datastore.NewMock(), logs.NewMock(), "/v1/apps/a/routes", `{ "route": { "path": "/myroute" } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingImage},
{datastore.NewMock(), logs.NewMock(),"/v1/apps/a/routes", `{ "route": { "image": "funcy/hello" } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingPath}, {datastore.NewMock(), logs.NewMock(), "/v1/apps/a/routes", `{ "route": { "image": "funcy/hello" } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingPath},
{datastore.NewMock(), logs.NewMock(),"/v1/apps/a/routes", `{ "route": { "image": "funcy/hello", "path": "myroute" } }`, http.StatusBadRequest, models.ErrRoutesValidationInvalidPath}, {datastore.NewMock(), logs.NewMock(), "/v1/apps/a/routes", `{ "route": { "image": "funcy/hello", "path": "myroute" } }`, http.StatusBadRequest, models.ErrRoutesValidationInvalidPath},
{datastore.NewMock(), logs.NewMock(),"/v1/apps/$/routes", `{ "route": { "image": "funcy/hello", "path": "/myroute" } }`, http.StatusInternalServerError, models.ErrAppsValidationInvalidName}, {datastore.NewMock(), logs.NewMock(), "/v1/apps/$/routes", `{ "route": { "image": "funcy/hello", "path": "/myroute" } }`, http.StatusInternalServerError, models.ErrAppsValidationInvalidName},
// success // success
{datastore.NewMock(), logs.NewMock(),"/v1/apps/a/routes", `{ "route": { "image": "funcy/hello", "path": "/myroute" } }`, http.StatusOK, nil}, {datastore.NewMock(), logs.NewMock(), "/v1/apps/a/routes", `{ "route": { "image": "funcy/hello", "path": "/myroute" } }`, http.StatusOK, nil},
} { } {
rnr, cancel := testRunner(t) rnr, cancel := testRunner(t)
srv := testServer(test.mock, &mqs.Mock{}, test.logDB, rnr) srv := testServer(test.mock, &mqs.Mock{}, test.logDB, rnr)
@@ -75,12 +75,12 @@ func TestRouteDelete(t *testing.T) {
expectedCode int expectedCode int
expectedError error expectedError error
}{ }{
{datastore.NewMock(), logs.NewMock(),"/v1/apps/a/routes/missing", "", http.StatusNotFound, nil}, {datastore.NewMock(), logs.NewMock(), "/v1/apps/a/routes/missing", "", http.StatusNotFound, nil},
{datastore.NewMockInit(nil, {datastore.NewMockInit(nil,
[]*models.Route{ []*models.Route{
{Path: "/myroute", AppName: "a"}, {Path: "/myroute", AppName: "a"},
}, nil, nil, }, nil, nil,
), logs.NewMock(),"/v1/apps/a/routes/myroute", "", http.StatusOK, nil}, ), logs.NewMock(), "/v1/apps/a/routes/myroute", "", http.StatusOK, nil},
} { } {
rnr, cancel := testRunner(t) rnr, cancel := testRunner(t)
srv := testServer(test.ds, &mqs.Mock{}, test.logDB, rnr) srv := testServer(test.ds, &mqs.Mock{}, test.logDB, rnr)
@@ -195,10 +195,10 @@ func TestRouteUpdate(t *testing.T) {
expectedError error expectedError error
}{ }{
// errors // errors
{datastore.NewMock(), logs.NewMock(),"/v1/apps/a/routes/myroute/do", ``, http.StatusBadRequest, models.ErrInvalidJSON}, {datastore.NewMock(), logs.NewMock(), "/v1/apps/a/routes/myroute/do", ``, http.StatusBadRequest, models.ErrInvalidJSON},
{datastore.NewMock(), logs.NewMock(),"/v1/apps/a/routes/myroute/do", `{}`, http.StatusBadRequest, models.ErrRoutesMissingNew}, {datastore.NewMock(), logs.NewMock(), "/v1/apps/a/routes/myroute/do", `{}`, http.StatusBadRequest, models.ErrRoutesMissingNew},
{datastore.NewMock(), logs.NewMock(),"/v1/apps/a/routes/myroute/do", `{ "route": { "type": "invalid-type" } }`, http.StatusBadRequest, nil}, {datastore.NewMock(), logs.NewMock(), "/v1/apps/a/routes/myroute/do", `{ "route": { "type": "invalid-type" } }`, http.StatusBadRequest, nil},
{datastore.NewMock(), logs.NewMock(),"/v1/apps/a/routes/myroute/do", `{ "route": { "format": "invalid-format" } }`, http.StatusBadRequest, nil}, {datastore.NewMock(), logs.NewMock(), "/v1/apps/a/routes/myroute/do", `{ "route": { "format": "invalid-format" } }`, http.StatusBadRequest, nil},
// success // success
{datastore.NewMockInit(nil, {datastore.NewMockInit(nil,
@@ -208,7 +208,7 @@ func TestRouteUpdate(t *testing.T) {
Path: "/myroute/do", Path: "/myroute/do",
}, },
}, nil, nil, }, nil, nil,
), logs.NewMock(),"/v1/apps/a/routes/myroute/do", `{ "route": { "image": "funcy/hello" } }`, http.StatusOK, nil}, ), logs.NewMock(), "/v1/apps/a/routes/myroute/do", `{ "route": { "image": "funcy/hello" } }`, http.StatusOK, nil},
// Addresses #381 // Addresses #381
{datastore.NewMockInit(nil, {datastore.NewMockInit(nil,
@@ -218,7 +218,7 @@ func TestRouteUpdate(t *testing.T) {
Path: "/myroute/do", Path: "/myroute/do",
}, },
}, nil, nil, }, nil, nil,
), logs.NewMock(),"/v1/apps/a/routes/myroute/do", `{ "route": { "path": "/otherpath" } }`, http.StatusBadRequest, nil}, ), logs.NewMock(), "/v1/apps/a/routes/myroute/do", `{ "route": { "path": "/otherpath" } }`, http.StatusBadRequest, nil},
} { } {
rnr, cancel := testRunner(t) rnr, cancel := testRunner(t)

View File

@@ -177,7 +177,7 @@ paths:
schema: schema:
$ref: '#/definitions/RouteWrapper' $ref: '#/definitions/RouteWrapper'
responses: responses:
201: 200:
description: Route created description: Route created
schema: schema:
$ref: '#/definitions/RouteWrapper' $ref: '#/definitions/RouteWrapper'
@@ -244,11 +244,7 @@ paths:
$ref: '#/definitions/RouteWrapper' $ref: '#/definitions/RouteWrapper'
responses: responses:
200: 200:
description: Route updated description: Route created or updated
schema:
$ref: '#/definitions/RouteWrapper'
201:
description: Route created
schema: schema:
$ref: '#/definitions/RouteWrapper' $ref: '#/definitions/RouteWrapper'
400: 400: