From 4845ddb1d4045194537238d54a9813d8e5aab8ce Mon Sep 17 00:00:00 2001 From: James Jeffrey Date: Wed, 5 Jul 2017 12:30:18 -0700 Subject: [PATCH] Only use 200 follows what others do. Less switching. Remove defense --- api/server/routes_create_update.go | 29 +++++------------------- api/server/routes_test.go | 36 +++++++++++++++--------------- docs/swagger.yml | 8 ++----- 3 files changed, 26 insertions(+), 47 deletions(-) diff --git a/api/server/routes_create_update.go b/api/server/routes_create_update.go index 43f8fbf49..b68e840d5 100644 --- a/api/server/routes_create_update.go +++ b/api/server/routes_create_update.go @@ -2,7 +2,6 @@ package server import ( "context" - "fmt" "net/http" "path" "strings" @@ -25,12 +24,7 @@ func (s *Server) handleRouteCreateOrUpdate(c *gin.Context) { ctx := c.MustGet("ctx").(context.Context) log := common.Logger(ctx) 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 err := c.BindJSON(&wroute) @@ -110,22 +104,21 @@ func (s *Server) handleRouteCreateOrUpdate(c *gin.Context) { var route *models.Route - var createdOrUpdated int + resp := routeResponse{"Route successfully created", route} + up := routeResponse{"Route successfully updated", route} + switch method { case http.MethodPost: route, err = s.Datastore.InsertRoute(ctx, wroute.Route) - createdOrUpdated = 1 case http.MethodPut: route, err = s.Datastore.UpdateRoute(ctx, wroute.Route) - createdOrUpdated = 2 if err == models.ErrRoutesNotFound { // try insert then route, err = s.Datastore.InsertRoute(ctx, wroute.Route) - createdOrUpdated = 1 } case http.MethodPatch: route, err = s.Datastore.UpdateRoute(ctx, wroute.Route) - createdOrUpdated = 2 + resp = up } if err != nil { @@ -135,15 +128,5 @@ func (s *Server) handleRouteCreateOrUpdate(c *gin.Context) { s.cacheRefresh(route) - var msg string - 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}) + c.JSON(http.StatusOK, resp) } diff --git a/api/server/routes_test.go b/api/server/routes_test.go index 5394e5bf9..a4ef6b24c 100644 --- a/api/server/routes_test.go +++ b/api/server/routes_test.go @@ -7,9 +7,9 @@ import ( "testing" "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/mqs" - "gitlab-odx.oracle.com/odx/functions/api/logs" ) func TestRouteCreate(t *testing.T) { @@ -24,17 +24,17 @@ func TestRouteCreate(t *testing.T) { expectedError error }{ // errors - {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", `{ "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": { "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", "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/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", `{ "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": { "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", "path": "myroute" } }`, http.StatusBadRequest, models.ErrRoutesValidationInvalidPath}, + {datastore.NewMock(), logs.NewMock(), "/v1/apps/$/routes", `{ "route": { "image": "funcy/hello", "path": "/myroute" } }`, http.StatusInternalServerError, models.ErrAppsValidationInvalidName}, // 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) srv := testServer(test.mock, &mqs.Mock{}, test.logDB, rnr) @@ -75,12 +75,12 @@ func TestRouteDelete(t *testing.T) { expectedCode int 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, []*models.Route{ {Path: "/myroute", AppName: "a"}, }, 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) srv := testServer(test.ds, &mqs.Mock{}, test.logDB, rnr) @@ -195,10 +195,10 @@ func TestRouteUpdate(t *testing.T) { expectedError error }{ // 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.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": { "format": "invalid-format" } }`, http.StatusBadRequest, nil}, + {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", `{ "route": { "type": "invalid-type" } }`, http.StatusBadRequest, nil}, + {datastore.NewMock(), logs.NewMock(), "/v1/apps/a/routes/myroute/do", `{ "route": { "format": "invalid-format" } }`, http.StatusBadRequest, nil}, // success {datastore.NewMockInit(nil, @@ -208,7 +208,7 @@ func TestRouteUpdate(t *testing.T) { Path: "/myroute/do", }, }, 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 {datastore.NewMockInit(nil, @@ -218,7 +218,7 @@ func TestRouteUpdate(t *testing.T) { Path: "/myroute/do", }, }, 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) diff --git a/docs/swagger.yml b/docs/swagger.yml index 4cc9886ea..20cf97f82 100644 --- a/docs/swagger.yml +++ b/docs/swagger.yml @@ -177,7 +177,7 @@ paths: schema: $ref: '#/definitions/RouteWrapper' responses: - 201: + 200: description: Route created schema: $ref: '#/definitions/RouteWrapper' @@ -244,11 +244,7 @@ paths: $ref: '#/definitions/RouteWrapper' responses: 200: - description: Route updated - schema: - $ref: '#/definitions/RouteWrapper' - 201: - description: Route created + description: Route created or updated schema: $ref: '#/definitions/RouteWrapper' 400: