server updates (#566)

* server updates

- improved route create/update validation/defaults
- improved/added route test cases

* cleanup

* negative concurrency check

api/models/app.go

@@ -87,5 +87,6 @@ func (a *App) UpdateConfig(patch Config) {
 }
 
 type AppFilter struct {
+	// An SQL LIKE query. Empty does not filter.
 	Name string
 }

api/models/route.go

@@ -54,52 +54,23 @@ var (
 	ErrRoutesValidationMissingType           = errors.New("Missing route Type")
 	ErrRoutesValidationPathMalformed         = errors.New("Path malformed")
 	ErrRoutesValidationNegativeTimeout       = errors.New("Negative timeout")
+	ErrRoutesValidationNegativeMaxConcurrency = errors.New("Negative MaxConcurrency")
 )
 
-func (r *Route) Validate() error {
-	var res []error
-
+// SetDefaults sets zeroed field to defaults.
+func (r *Route) SetDefaults() {
 	if r.Memory == 0 {
 		r.Memory = 128
 	}
 
-	if r.AppName == "" {
-		res = append(res, ErrRoutesValidationMissingAppName)
-	}
-
-	if r.Path == "" {
-		res = append(res, ErrRoutesValidationMissingPath)
-	}
-
-	u, err := url.Parse(r.Path)
-	if err != nil {
-		res = append(res, ErrRoutesValidationPathMalformed)
-	}
-
-	if strings.Contains(u.Path, ":") {
-		res = append(res, ErrRoutesValidationFoundDynamicURL)
-	}
-
-	if !path.IsAbs(u.Path) {
-		res = append(res, ErrRoutesValidationInvalidPath)
-	}
-
 	if r.Type == TypeNone {
 		r.Type = TypeSync
 	}
 
-	if r.Type != TypeAsync && r.Type != TypeSync {
-		res = append(res, ErrRoutesValidationInvalidType)
-	}
-
 	if r.Format == "" {
 		r.Format = FormatDefault
 	}
 
-	if r.Format != FormatDefault && r.Format != FormatHTTP {
-		res = append(res, ErrRoutesValidationInvalidFormat)
-	}
-
 	if r.MaxConcurrency == 0 {
 		r.MaxConcurrency = 1
 	}
@@ -114,7 +85,59 @@ func (r *Route) Validate() error {
 
 	if r.Timeout == 0 {
 		r.Timeout = defaultRouteTimeout
-	} else if r.Timeout < 0 {
+	}
+}
+
+// Validate validates field values, skipping zeroed fields if skipZero is true.
+func (r *Route) Validate(skipZero bool) error {
+	var res []error
+
+	if !skipZero {
+		if r.AppName == "" {
+			res = append(res, ErrRoutesValidationMissingAppName)
+		}
+
+		if r.Image == "" {
+			res = append(res, ErrRoutesValidationMissingImage)
+		}
+
+		if r.Path == "" {
+			res = append(res, ErrRoutesValidationMissingPath)
+		}
+	}
+
+	if !skipZero || r.Path != "" {
+		u, err := url.Parse(r.Path)
+		if err != nil {
+			res = append(res, ErrRoutesValidationPathMalformed)
+		}
+
+		if strings.Contains(u.Path, ":") {
+			res = append(res, ErrRoutesValidationFoundDynamicURL)
+		}
+
+		if !path.IsAbs(u.Path) {
+			res = append(res, ErrRoutesValidationInvalidPath)
+		}
+	}
+
+	if !skipZero || r.Type != "" {
+		if r.Type != TypeAsync && r.Type != TypeSync {
+			res = append(res, ErrRoutesValidationInvalidType)
+		}
+	}
+
+	if !skipZero || r.Format != "" {
+		if r.Format != FormatDefault && r.Format != FormatHTTP {
+			res = append(res, ErrRoutesValidationInvalidFormat)
+		}
+	}
+
+	if r.MaxConcurrency < 0 {
+		res = append(res, ErrRoutesValidationNegativeMaxConcurrency)
+	}
+
+	if r.Timeout < 0 {
 		res = append(res, ErrRoutesValidationNegativeTimeout)
 	}
 
@@ -182,6 +205,7 @@ func (r *Route) Update(new *Route) {
 	}
 }
 
+//TODO are these sql LIKE queries? or strict matches?
 type RouteFilter struct {
 	Path    string
 	AppName string

api/models/route_wrapper.go

@@ -6,10 +6,10 @@ type RouteWrapper struct {
 	Route *Route `json:"route"`
 }
 
-func (m *RouteWrapper) Validate() error {
+func (m *RouteWrapper) Validate(skipZero bool) error {
 	var res []error
 
-	if err := m.validateRoute(); err != nil {
+	if err := m.validateRoute(skipZero); err != nil {
 		res = append(res, err)
 	}
 
@@ -19,10 +19,10 @@ func (m *RouteWrapper) Validate() error {
 	return nil
 }
 
-func (m *RouteWrapper) validateRoute() error {
+func (m *RouteWrapper) validateRoute(skipZero bool) error {
 
 	if m.Route != nil {
-		if err := m.Route.Validate(); err != nil {
+		if err := m.Route.Validate(skipZero); err != nil {
 			return err
 		}
 	}

api/server/apps_delete.go

@@ -22,7 +22,7 @@ func (s *Server) handleAppDelete(c *gin.Context) {
 		c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError))
 		return
 	}
-
+	//TODO allow this? #528
 	if len(routes) > 0 {
 		log.WithError(err).Debug(models.ErrDeleteAppsWithRoutes)
 		c.JSON(http.StatusBadRequest, simpleError(models.ErrDeleteAppsWithRoutes))

api/server/routes_create.go

@@ -31,18 +31,14 @@ func (s *Server) handleRouteCreate(c *gin.Context) {
 
 	wroute.Route.AppName = c.MustGet(api.AppName).(string)
 
-	if err := wroute.Validate(); err != nil {
+	wroute.Route.SetDefaults()
+
+	if err := wroute.Validate(false); err != nil {
 		log.WithError(err).Debug(models.ErrRoutesCreate)
 		c.JSON(http.StatusBadRequest, simpleError(err))
 		return
 	}
 
-	if wroute.Route.Image == "" {
-		log.WithError(models.ErrRoutesValidationMissingImage).Debug(models.ErrRoutesCreate)
-		c.JSON(http.StatusBadRequest, simpleError(models.ErrRoutesValidationMissingImage))
-		return
-	}
-
 	// err = s.Runner.EnsureImageExists(ctx, &task.Config{
 	// 	Image: wroute.Route.Image,
 	// })

api/server/routes_test.go

@@ -50,11 +50,14 @@ func TestRouteCreate(t *testing.T) {
 
 		if test.expectedError != nil {
 			resp := getErrorResponse(t, rec)
-
-			if !strings.Contains(resp.Error.Message, test.expectedError.Error()) {
+			if resp.Error == nil {
+				t.Log(buf.String())
+				t.Errorf("Test %d: Expected error message to have `%s`, but it was nil",
+					i, test.expectedError)
+			} else if !strings.Contains(resp.Error.Message, test.expectedError.Error()) {
 				t.Log(buf.String())
 				t.Errorf("Test %d: Expected error message to have `%s`, but it was `%s`",
-					i, test.expectedError.Error(), resp.Error.Message)
+					i, test.expectedError, resp.Error.Message)
 			}
 		}
 		cancel()
@@ -193,6 +196,8 @@ func TestRouteUpdate(t *testing.T) {
 		// errors
 		{datastore.NewMock(), "/v1/apps/a/routes/myroute/do", ``, http.StatusBadRequest, models.ErrInvalidJSON},
 		{datastore.NewMock(), "/v1/apps/a/routes/myroute/do", `{}`, http.StatusBadRequest, models.ErrRoutesMissingNew},
+		{datastore.NewMock(), "/v1/apps/a/routes/myroute/do", `{ "route": { "type": "invalid-type" } }`, http.StatusBadRequest, nil},
+		{datastore.NewMock(), "/v1/apps/a/routes/myroute/do", `{ "route": { "format": "invalid-format" } }`, http.StatusBadRequest, nil},
 
 		// success
 		{datastore.NewMockInit(nil,
@@ -223,8 +228,8 @@ func TestRouteUpdate(t *testing.T) {
 
 		if rec.Code != test.expectedCode {
 			t.Log(buf.String())
-			t.Errorf("Test %d: Expected status code to be %d but was %d",
-				i, test.expectedCode, rec.Code)
+			t.Errorf("Test %d: Expected status code to be %d but was %d: %s",
+				i, test.expectedCode, rec.Code, rec.Body.String())
 		}
 
 		if test.expectedError != nil {

api/server/routes_update.go

@@ -39,6 +39,12 @@ func (s *Server) handleRouteUpdate(c *gin.Context) {
 	wroute.Route.AppName = c.MustGet(api.AppName).(string)
 	wroute.Route.Path = path.Clean(c.MustGet(api.Path).(string))
 
+	if err := wroute.Validate(true); err != nil {
+		log.WithError(err).Debug(models.ErrRoutesUpdate)
+		c.JSON(http.StatusBadRequest, simpleError(err))
+		return
+	}
+
 	if wroute.Route.Image != "" {
 		// err = s.Runner.EnsureImageExists(ctx, &task.Config{
 		// 	Image: wroute.Route.Image,