diff --git a/api/datastore/internal/datastoretest/test.go b/api/datastore/internal/datastoretest/test.go index 94b2ff685..1b22ec373 100644 --- a/api/datastore/internal/datastoretest/test.go +++ b/api/datastore/internal/datastoretest/test.go @@ -280,8 +280,8 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { } _, err = ds.InsertApp(ctx, &models.App{}) - if err != models.ErrDatastoreEmptyAppName { - t.Fatalf("Test InsertApp(&{}): expected error `%v`, but it was `%v`", models.ErrDatastoreEmptyAppName, err) + if err != models.ErrAppsMissingName { + t.Fatalf("Test InsertApp(&{}): expected error `%v`, but it was `%v`", models.ErrAppsMissingName, err) } inserted, err := ds.InsertApp(ctx, testApp) @@ -333,8 +333,8 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { // Testing get app _, err = ds.GetApp(ctx, "") - if err != models.ErrDatastoreEmptyAppName { - t.Fatalf("Test GetApp: expected error to be %v, but it was %s", models.ErrDatastoreEmptyAppName, err) + if err != models.ErrAppsMissingName { + t.Fatalf("Test GetApp: expected error to be %v, but it was %s", models.ErrAppsMissingName, err) } app, err := ds.GetApp(ctx, testApp.Name) @@ -419,8 +419,8 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { // Testing app delete err = ds.RemoveApp(ctx, "") - if err != models.ErrDatastoreEmptyAppName { - t.Fatalf("Test RemoveApp: expected error `%v`, but it was `%v`", models.ErrDatastoreEmptyAppName, err) + if err != models.ErrAppsMissingName { + t.Fatalf("Test RemoveApp: expected error `%v`, but it was `%v`", models.ErrAppsMissingName, err) } err = ds.RemoveApp(ctx, testApp.Name) @@ -462,7 +462,9 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { t.Fatalf("Test InsertRoute(nil): expected error `%v`, but it was `%v`", models.ErrDatastoreEmptyRoute, err) } - _, err = ds.InsertRoute(ctx, &models.Route{AppName: "notreal", Path: "/test"}) + copyRoute := *testRoute + copyRoute.AppName = "notreal" + _, err = ds.InsertRoute(ctx, ©Route) if err != models.ErrAppsNotFound { t.Fatalf("Test InsertRoute: expected error `%v`, but it was `%v`", models.ErrAppsNotFound, err) } @@ -481,13 +483,13 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { // Testing get { _, err = ds.GetRoute(ctx, "a", "") - if err != models.ErrDatastoreEmptyRoutePath { - t.Fatalf("Test GetRoute(empty route path): expected error `%v`, but it was `%v`", models.ErrDatastoreEmptyRoutePath, err) + if err != models.ErrRoutesMissingPath { + t.Fatalf("Test GetRoute(empty route path): expected error `%v`, but it was `%v`", models.ErrRoutesMissingPath, err) } _, err = ds.GetRoute(ctx, "", "a") - if err != models.ErrDatastoreEmptyAppName { - t.Fatalf("Test GetRoute(empty app name): expected error `%v`, but it was `%v`", models.ErrDatastoreEmptyAppName, err) + if err != models.ErrAppsMissingName { + t.Fatalf("Test GetRoute(empty app name): expected error `%v`, but it was `%v`", models.ErrAppsMissingName, err) } route, err := ds.GetRoute(ctx, testApp.Name, testRoute.Path) @@ -682,13 +684,13 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { // Testing route delete err = ds.RemoveRoute(ctx, "", "") - if err != models.ErrDatastoreEmptyAppName { - t.Fatalf("Test RemoveRoute(empty app name): expected error `%v`, but it was `%v`", models.ErrDatastoreEmptyAppName, err) + if err != models.ErrAppsMissingName { + t.Fatalf("Test RemoveRoute(empty app name): expected error `%v`, but it was `%v`", models.ErrAppsMissingName, err) } err = ds.RemoveRoute(ctx, "a", "") - if err != models.ErrDatastoreEmptyRoutePath { - t.Fatalf("Test RemoveRoute(empty route path): expected error `%v`, but it was `%v`", models.ErrDatastoreEmptyRoutePath, err) + if err != models.ErrRoutesMissingPath { + t.Fatalf("Test RemoveRoute(empty route path): expected error `%v`, but it was `%v`", models.ErrRoutesMissingPath, err) } err = ds.RemoveRoute(ctx, testRoute.AppName, testRoute.Path) diff --git a/api/datastore/internal/datastoreutil/validator.go b/api/datastore/internal/datastoreutil/validator.go index 8b08c489a..bd669289d 100644 --- a/api/datastore/internal/datastoreutil/validator.go +++ b/api/datastore/internal/datastoreutil/validator.go @@ -20,7 +20,7 @@ type validator struct { // name will never be empty. func (v *validator) GetApp(ctx context.Context, name string) (app *models.App, err error) { if name == "" { - return nil, models.ErrDatastoreEmptyAppName + return nil, models.ErrAppsMissingName } return v.Datastore.GetApp(ctx, name) } @@ -34,8 +34,10 @@ func (v *validator) InsertApp(ctx context.Context, app *models.App) (*models.App if app == nil { return nil, models.ErrDatastoreEmptyApp } - if app.Name == "" { - return nil, models.ErrDatastoreEmptyAppName + + app.SetDefaults() + if err := app.Validate(); err != nil { + return nil, err } return v.Datastore.InsertApp(ctx, app) @@ -47,7 +49,7 @@ func (v *validator) UpdateApp(ctx context.Context, app *models.App) (*models.App return nil, models.ErrDatastoreEmptyApp } if app.Name == "" { - return nil, models.ErrDatastoreEmptyAppName + return nil, models.ErrAppsMissingName } return v.Datastore.UpdateApp(ctx, app) } @@ -55,7 +57,7 @@ func (v *validator) UpdateApp(ctx context.Context, app *models.App) (*models.App // name will never be empty. func (v *validator) RemoveApp(ctx context.Context, name string) error { if name == "" { - return models.ErrDatastoreEmptyAppName + return models.ErrAppsMissingName } return v.Datastore.RemoveApp(ctx, name) @@ -64,10 +66,10 @@ func (v *validator) RemoveApp(ctx context.Context, name string) error { // appName and routePath will never be empty. func (v *validator) GetRoute(ctx context.Context, appName, routePath string) (*models.Route, error) { if appName == "" { - return nil, models.ErrDatastoreEmptyAppName + return nil, models.ErrAppsMissingName } if routePath == "" { - return nil, models.ErrDatastoreEmptyRoutePath + return nil, models.ErrRoutesMissingPath } return v.Datastore.GetRoute(ctx, appName, routePath) @@ -76,7 +78,7 @@ func (v *validator) GetRoute(ctx context.Context, appName, routePath string) (*m // appName will never be empty func (v *validator) GetRoutesByApp(ctx context.Context, appName string, routeFilter *models.RouteFilter) (routes []*models.Route, err error) { if appName == "" { - return nil, models.ErrDatastoreEmptyAppName + return nil, models.ErrAppsMissingName } return v.Datastore.GetRoutesByApp(ctx, appName, routeFilter) } @@ -86,11 +88,10 @@ func (v *validator) InsertRoute(ctx context.Context, route *models.Route) (*mode if route == nil { return nil, models.ErrDatastoreEmptyRoute } - if route.AppName == "" { - return nil, models.ErrDatastoreEmptyAppName - } - if route.Path == "" { - return nil, models.ErrDatastoreEmptyRoutePath + + route.SetDefaults() + if err := route.Validate(); err != nil { + return nil, err } return v.Datastore.InsertRoute(ctx, route) @@ -102,10 +103,10 @@ func (v *validator) UpdateRoute(ctx context.Context, newroute *models.Route) (*m return nil, models.ErrDatastoreEmptyRoute } if newroute.AppName == "" { - return nil, models.ErrDatastoreEmptyAppName + return nil, models.ErrAppsMissingName } if newroute.Path == "" { - return nil, models.ErrDatastoreEmptyRoutePath + return nil, models.ErrRoutesMissingPath } return v.Datastore.UpdateRoute(ctx, newroute) } @@ -113,10 +114,10 @@ func (v *validator) UpdateRoute(ctx context.Context, newroute *models.Route) (*m // appName and routePath will never be empty. func (v *validator) RemoveRoute(ctx context.Context, appName, routePath string) error { if appName == "" { - return models.ErrDatastoreEmptyAppName + return models.ErrAppsMissingName } if routePath == "" { - return models.ErrDatastoreEmptyRoutePath + return models.ErrRoutesMissingPath } return v.Datastore.RemoveRoute(ctx, appName, routePath) diff --git a/api/models/app.go b/api/models/app.go index 46dedd500..6bba0a4ea 100644 --- a/api/models/app.go +++ b/api/models/app.go @@ -2,6 +2,7 @@ package models import ( "time" + "unicode" "github.com/go-openapi/strfmt" ) @@ -34,7 +35,7 @@ func (a *App) Validate() error { return ErrAppsTooLongName } for _, c := range a.Name { - if (c < '0' || '9' < c) && (c < 'A' || 'Z' > c) && (c < 'a' || 'z' < c) && c != '_' && c != '-' { + if !(unicode.IsLetter(c) || unicode.IsNumber(c) || c == '_' || c == '-') { return ErrAppsInvalidName } } diff --git a/api/models/error.go b/api/models/error.go index e3cde7877..9517c8756 100644 --- a/api/models/error.go +++ b/api/models/error.go @@ -56,14 +56,6 @@ var ( code: http.StatusConflict, error: errors.New("Cannot remove apps with routes"), } - ErrDatastoreEmptyAppName = err{ - code: http.StatusBadRequest, - error: errors.New("Missing app name"), - } - ErrDatastoreEmptyRoutePath = err{ - code: http.StatusBadRequest, - error: errors.New("Missing route name"), - } ErrDatastoreEmptyApp = err{ code: http.StatusBadRequest, error: errors.New("Missing app"), diff --git a/api/server/apps_create.go b/api/server/apps_create.go index 8de723baa..aa7d702da 100644 --- a/api/server/apps_create.go +++ b/api/server/apps_create.go @@ -28,12 +28,6 @@ func (s *Server) handleAppCreate(c *gin.Context) { return } - app.SetDefaults() - if err = app.Validate(); err != nil { - handleErrorResponse(c, err) - return - } - err = s.FireBeforeAppCreate(ctx, app) if err != nil { handleErrorResponse(c, err) diff --git a/api/server/routes_create_update.go b/api/server/routes_create_update.go index 03039c838..8c2e2c79c 100644 --- a/api/server/routes_create_update.go +++ b/api/server/routes_create_update.go @@ -49,11 +49,6 @@ func (s *Server) handleRoutesPostPutPatch(c *gin.Context) { } func (s *Server) submitRoute(ctx context.Context, wroute *models.RouteWrapper) error { - wroute.Route.SetDefaults() - err := wroute.Route.Validate() - if err != nil { - return err - } r, err := s.datastore.InsertRoute(ctx, wroute.Route) if err != nil { return err @@ -114,9 +109,6 @@ func (s *Server) ensureApp(ctx context.Context, wroute *models.RouteWrapper, met } else if app == nil { // Create a new application newapp := &models.App{Name: wroute.Route.AppName} - if err = newapp.Validate(); err != nil { - return err - } err = s.FireBeforeAppCreate(ctx, newapp) if err != nil {