fn: improve UX (#325)

* fn: make UX more consistent with regards to app name position

* fn: improve detection of missing routes

* fn: fix update operations

- No longer delete-than-add for configuration updates
- Path cleaning before most of routes operations

api/server/routes_delete.go

@@ -3,6 +3,7 @@ package server
 import (
 	"context"
 	"net/http"
+	"path"
 
 	"github.com/gin-gonic/gin"
 	"github.com/iron-io/functions/api/models"
@@ -14,10 +15,16 @@ func (s *Server) handleRouteDelete(c *gin.Context) {
 	log := common.Logger(ctx)
 
 	appName := c.Param("app")
-	routePath := c.Param("route")
-	err := Api.Datastore.RemoveRoute(appName, routePath)
+	routePath := path.Clean(c.Param("route"))
 
-	if err != nil {
+	route, err := Api.Datastore.GetRoute(appName, routePath)
+	if err != nil || route == nil {
+		log.Error(models.ErrRoutesNotFound)
+		c.JSON(http.StatusNotFound, simpleError(models.ErrRoutesNotFound))
+		return
+	}
+
+	if err := Api.Datastore.RemoveRoute(appName, routePath); err != nil {
 		log.WithError(err).Debug(models.ErrRoutesRemoving)
 		c.JSON(http.StatusInternalServerError, simpleError(models.ErrRoutesRemoving))
 		return

api/server/routes_get.go

@@ -3,6 +3,7 @@ package server
 import (
 	"context"
 	"net/http"
+	"path"
 
 	"github.com/Sirupsen/logrus"
 	"github.com/gin-gonic/gin"
@@ -15,7 +16,7 @@ func handleRouteGet(c *gin.Context) {
 	log := common.Logger(ctx)
 
 	appName := c.Param("app")
-	routePath := c.Param("route")
+	routePath := path.Clean(c.Param("route"))
 
 	route, err := Api.Datastore.GetRoute(appName, routePath)
 	if err != nil {

api/server/routes_test.go

@@ -64,7 +64,11 @@ func TestRouteDelete(t *testing.T) {
 	tasks := mockTasksConduit()
 	defer close(tasks)
 
-	router := testRouter(&datastore.Mock{}, &mqs.Mock{}, testRunner(t), tasks)
+	router := testRouter(&datastore.Mock{
+		FakeRoutes: []*models.Route{
+			&models.Route{AppName: "a", Path: "/myroute"},
+		},
+	}, &mqs.Mock{}, testRunner(t), tasks)
 
 	for i, test := range []struct {
 		path          string
@@ -74,6 +78,7 @@ func TestRouteDelete(t *testing.T) {
 	}{
 		{"/v1/apps/a/routes", "", http.StatusTemporaryRedirect, nil},
 		{"/v1/apps/a/routes/myroute", "", http.StatusOK, nil},
+		{"/v1/apps/a/routes/missing", "", http.StatusNotFound, nil},
 	} {
 		_, rec := routerRequest(t, router, "DELETE", test.path, nil)
 

api/server/routes_update.go

@@ -3,6 +3,7 @@ package server
 import (
 	"context"
 	"net/http"
+	"path"
 
 	"github.com/gin-gonic/gin"
 	"github.com/iron-io/functions/api/models"
@@ -30,7 +31,7 @@ func handleRouteUpdate(c *gin.Context) {
 	}
 
 	wroute.Route.AppName = c.Param("app")
-	wroute.Route.Path = c.Param("route")
+	wroute.Route.Path = path.Clean(c.Param("route"))
 
 	if wroute.Route.Image != "" {
 		err = Api.Runner.EnsureImageExists(ctx, &runner.Config{

docs/swagger.yml

@@ -6,7 +6,7 @@ swagger: '2.0'
 info:
   title: IronFunctions
   description: The open source serverless platform.
-  version: "0.1.0"
+  version: "0.1.16"
 # the domain of the service
 host: "127.0.0.1:8080"
 # array of all schemes that your API supports
@@ -184,6 +184,45 @@ paths:
             $ref: '#/definitions/Error'
 
   /apps/{app}/routes/{route}:
+    put:
+      summary: Update a Route
+      description: Update a route
+      tags:
+        - Routes
+      parameters:
+        - name: app
+          in: path
+          description: name of the app.
+          required: true
+          type: string
+        - name: route
+          in: path
+          description: route path.
+          required: true
+          type: string
+        - name: body
+          in: body
+          description: One route to post.
+          required: true
+          schema:
+            $ref: '#/definitions/RouteWrapper'
+      responses:
+        201:
+          description: Route updated
+          schema:
+            $ref: '#/definitions/RouteWrapper'
+        400:
+          description: One or more of the routes were invalid due to parameters being missing or invalid.
+          schema:
+            $ref: '#/definitions/Error'
+        500:
+          description: Could not accept routes due to internal error.
+          schema:
+            $ref: '#/definitions/Error'
+        default:
+          description: Unexpected error
+          schema:
+            $ref: '#/definitions/Error'
     get:
       summary: Gets route by name
       description: Gets a route by name.

fn/apps.go

@@ -20,14 +20,15 @@ func apps() cli.Command {
 
 	return cli.Command{
 		Name:      "apps",
-		Usage:     "list apps",
+		Usage:     "operate applications",
 		ArgsUsage: "fn apps",
-		Action:    a.list,
 		Subcommands: []cli.Command{
 			{
-				Name:   "create",
-				Usage:  "create a new app",
-				Action: a.create,
+				Name:      "create",
+				Aliases:   []string{"c"},
+				Usage:     "create a new app",
+				ArgsUsage: "`app`",
+				Action:    a.create,
 				Flags: []cli.Flag{
 					cli.StringSliceFlag{
 						Name:  "config",
@@ -36,31 +37,45 @@ func apps() cli.Command {
 				},
 			},
 			{
-				Name:   "config",
-				Usage:  "operate an application configuration set",
-				Action: a.configList,
-				Flags: []cli.Flag{
-					cli.BoolFlag{
-						Name:  "shell",
-						Usage: "output in shell format",
-					},
-					cli.BoolFlag{
-						Name:  "json",
-						Usage: "output in JSON format",
-					},
-				},
+				Name:    "list",
+				Aliases: []string{"l"},
+				Usage:   "list all apps",
+				Action:  a.list,
+			},
+			{
+				Name:  "config",
+				Usage: "operate an application configuration set",
 				Subcommands: []cli.Command{
 					{
-						Name:        "set",
-						Description: "store a configuration key for this application",
-						Usage:       "<app> <key> <value>",
-						Action:      a.configSet,
+						Name:      "view",
+						Aliases:   []string{"v"},
+						Usage:     "view all configuration keys for this app",
+						ArgsUsage: "`app`",
+						Action:    a.configList,
+						Flags: []cli.Flag{
+							cli.BoolFlag{
+								Name:  "shell,s",
+								Usage: "output in shell format",
+							},
+							cli.BoolFlag{
+								Name:  "json,j",
+								Usage: "output in JSON format",
+							},
+						},
 					},
 					{
-						Name:        "unset",
-						Description: "remove a configuration key for this application",
-						Usage:       "<app> <key> <value>",
-						Action:      a.configUnset,
+						Name:      "set",
+						Aliases:   []string{"s"},
+						Usage:     "store a configuration key for this application",
+						ArgsUsage: "`app` <key> <value>",
+						Action:    a.configSet,
+					},
+					{
+						Name:      "unset",
+						Aliases:   []string{"u"},
+						Usage:     "remove a configuration key for this application",
+						ArgsUsage: "`app` <key>",
+						Action:    a.configUnset,
 					},
 				},
 			},
@@ -69,7 +84,7 @@ func apps() cli.Command {
 }
 
 func (a *appsCmd) list(c *cli.Context) error {
-	if err := resetBasePath(&a.Configuration); err != nil {
+	if err := resetBasePath(a.Configuration); err != nil {
 		return fmt.Errorf("error setting endpoint: %v", err)
 	}
 
@@ -95,7 +110,7 @@ func (a *appsCmd) create(c *cli.Context) error {
 		return errors.New("error: app creating takes one argument, an app name")
 	}
 
-	if err := resetBasePath(&a.Configuration); err != nil {
+	if err := resetBasePath(a.Configuration); err != nil {
 		return fmt.Errorf("error setting endpoint: %v", err)
 	}
 
@@ -117,7 +132,7 @@ func (a *appsCmd) configList(c *cli.Context) error {
 		return errors.New("error: app description takes one argument, an app name")
 	}
 
-	if err := resetBasePath(&a.Configuration); err != nil {
+	if err := resetBasePath(a.Configuration); err != nil {
 		return fmt.Errorf("error setting endpoint: %v", err)
 	}
 
@@ -157,7 +172,7 @@ func (a *appsCmd) configSet(c *cli.Context) error {
 		return errors.New("error: application configuration setting takes three arguments: an app name, a key and a value")
 	}
 
-	if err := resetBasePath(&a.Configuration); err != nil {
+	if err := resetBasePath(a.Configuration); err != nil {
 		return fmt.Errorf("error setting endpoint: %v", err)
 	}
 
@@ -191,7 +206,7 @@ func (a *appsCmd) configUnset(c *cli.Context) error {
 		return errors.New("error: application configuration setting takes three arguments: an app name, a key and a value")
 	}
 
-	if err := resetBasePath(&a.Configuration); err != nil {
+	if err := resetBasePath(a.Configuration); err != nil {
 		return fmt.Errorf("error setting endpoint: %v", err)
 	}
 

fn/glide.lock

@@ -1,5 +1,5 @@
-hash: a7faac39f56e73fb3987d7a00062a52817432ba5e9620cc83dca15b75496f926
-updated: 2016-11-09T20:59:19.840009806+01:00
+hash: 30e4e556fec49a0c5bba030f272201870cb067d2b2c09d2e02240725e5bc09a2
+updated: 2016-11-21T19:58:04.868845654+01:00
 imports:
 - name: github.com/aws/aws-sdk-go
   version: 90dec2183a5f5458ee79cbaf4b8e9ab910bc81a6
@@ -77,7 +77,7 @@ imports:
 - name: github.com/hashicorp/go-cleanhttp
   version: ad28ea4487f05916463e2423a55166280e8254b5
 - name: github.com/iron-io/functions_go
-  version: 7f5bf75abece5380e916b594e17a552be365276d
+  version: 190cb886f033aac6d41e657eb91649b27cb52736
 - name: github.com/iron-io/iron_go3
   version: b50ecf8ff90187fc5fabccd9d028dd461adce4ee
   subpackages:

fn/glide.yaml

@@ -7,8 +7,6 @@ import:
   subpackages:
   - bump
   - storage
-- package: github.com/iron-io/functions_go
-  version: 7f5bf75abece5380e916b594e17a552be365276d
 - package: github.com/iron-io/iron_go3
   subpackages:
   - config
@@ -20,3 +18,5 @@ import:
   subpackages:
   - ssh/terminal
 - package: gopkg.in/yaml.v2
+- package: github.com/iron-io/functions_go
+  version: 190cb886f033aac6d41e657eb91649b27cb52736

fn/publish.go

@@ -88,7 +88,7 @@ func (p publishcmd) dockerpush(ff *funcfile) error {
 }
 
 func (p *publishcmd) route(path string, ff *funcfile) error {
-	if err := resetBasePath(&p.Configuration); err != nil {
+	if err := resetBasePath(p.Configuration); err != nil {
 		return fmt.Errorf("error setting endpoint: %v", err)
 	}
 

fn/routes.go

@@ -26,71 +26,87 @@ func routes() cli.Command {
 
 	return cli.Command{
 		Name:      "routes",
-		Usage:     "list routes",
+		Usage:     "operate routes",
 		ArgsUsage: "fn routes",
-		Action:    r.list,
 		Subcommands: []cli.Command{
 			{
 				Name:      "call",
 				Usage:     "call a route",
-				ArgsUsage: "appName /path",
+				ArgsUsage: "`app` /path",
 				Action:    r.call,
 				Flags:     runflags(),
 			},
+			{
+				Name:      "list",
+				Aliases:   []string{"l"},
+				Usage:     "list routes for `app`",
+				ArgsUsage: "`app`",
+				Action:    r.list,
+			},
 			{
 				Name:      "create",
-				Usage:     "create a route",
-				ArgsUsage: "appName /path image/name",
+				Aliases:   []string{"c"},
+				Usage:     "create a route in an `app`",
+				ArgsUsage: "`app` /path image/name",
 				Action:    r.create,
 				Flags: []cli.Flag{
 					cli.Int64Flag{
-						Name:  "memory",
+						Name:  "memory,m",
 						Usage: "memory in MiB",
 						Value: 128,
 					},
 					cli.StringFlag{
-						Name:  "type",
+						Name:  "type,t",
 						Usage: "route type - sync or async",
 						Value: "sync",
 					},
 					cli.StringSliceFlag{
-						Name:  "config",
+						Name:  "config,c",
 						Usage: "route configuration",
 					},
 				},
 			},
 			{
 				Name:      "delete",
-				Usage:     "delete a route",
-				ArgsUsage: "appName /path",
+				Aliases:   []string{"d"},
+				Usage:     "delete a route from `app`",
+				ArgsUsage: "`app` /path",
 				Action:    r.delete,
 			},
 			{
-				Name:   "config",
-				Usage:  "operate a route configuration set",
-				Action: r.configList,
-				Flags: []cli.Flag{
-					cli.BoolFlag{
-						Name:  "shell",
-						Usage: "output in shell format",
-					},
-					cli.BoolFlag{
-						Name:  "json",
-						Usage: "output in JSON format",
-					},
-				},
+				Name:  "config",
+				Usage: "operate a route configuration set",
 				Subcommands: []cli.Command{
 					{
-						Name:        "set",
-						Description: "store a configuration key for this route",
-						Usage:       "<app> <key> <value>",
-						Action:      r.configSet,
+						Name:      "view",
+						Aliases:   []string{"v"},
+						Usage:     "view all configuration keys for this route",
+						ArgsUsage: "`app` /path",
+						Action:    r.configList,
+						Flags: []cli.Flag{
+							cli.BoolFlag{
+								Name:  "shell,s",
+								Usage: "output in shell format",
+							},
+							cli.BoolFlag{
+								Name:  "json,j",
+								Usage: "output in JSON format",
+							},
+						},
 					},
 					{
-						Name:        "unset",
-						Description: "remove a configuration key for this route",
-						Usage:       "<app> <key> <value>",
-						Action:      r.configUnset,
+						Name:      "set",
+						Aliases:   []string{"s"},
+						Usage:     "store a configuration key for this route",
+						ArgsUsage: "`app` /path <key> <value>",
+						Action:    r.configSet,
+					},
+					{
+						Name:      "unset",
+						Aliases:   []string{"u"},
+						Usage:     "remove a configuration key for this route",
+						ArgsUsage: "`app` /path <key>",
+						Action:    r.configUnset,
 					},
 				},
 			},
@@ -104,7 +120,7 @@ func call() cli.Command {
 	return cli.Command{
 		Name:      "call",
 		Usage:     "call a remote function",
-		ArgsUsage: "appName /path",
+		ArgsUsage: "`app` /path",
 		Flags:     runflags(),
 		Action:    r.call,
 	}
@@ -115,7 +131,7 @@ func (a *routesCmd) list(c *cli.Context) error {
 		return errors.New("error: routes listing takes one argument, an app name")
 	}
 
-	if err := resetBasePath(&a.Configuration); err != nil {
+	if err := resetBasePath(a.Configuration); err != nil {
 		return fmt.Errorf("error setting endpoint: %v", err)
 	}
 
@@ -151,7 +167,7 @@ func (a *routesCmd) call(c *cli.Context) error {
 		return errors.New("error: routes listing takes three arguments: an app name and a route")
 	}
 
-	if err := resetBasePath(&a.Configuration); err != nil {
+	if err := resetBasePath(a.Configuration); err != nil {
 		return fmt.Errorf("error setting endpoint: %v", err)
 	}
 
@@ -206,7 +222,7 @@ func (a *routesCmd) create(c *cli.Context) error {
 		return errors.New("error: routes creation takes three arguments: an app name, a route path and an image")
 	}
 
-	if err := resetBasePath(&a.Configuration); err != nil {
+	if err := resetBasePath(a.Configuration); err != nil {
 		return fmt.Errorf("error setting endpoint: %v", err)
 	}
 
@@ -253,17 +269,22 @@ func (a *routesCmd) delete(c *cli.Context) error {
 		return errors.New("error: routes listing takes three arguments: an app name and a path")
 	}
 
-	if err := resetBasePath(&a.Configuration); err != nil {
+	if err := resetBasePath(a.Configuration); err != nil {
 		return fmt.Errorf("error setting endpoint: %v", err)
 	}
 
 	appName := c.Args().Get(0)
 	route := c.Args().Get(1)
-	_, err := a.AppsAppRoutesRouteDelete(appName, route)
+
+	resp, err := a.AppsAppRoutesRouteDelete(appName, route)
 	if err != nil {
 		return fmt.Errorf("error deleting route: %v", err)
 	}
 
+	if resp.StatusCode >= http.StatusBadRequest {
+		return fmt.Errorf("route not found: %s", route)
+	}
+
 	fmt.Println(route, "deleted")
 	return nil
 }
@@ -273,7 +294,7 @@ func (a *routesCmd) configList(c *cli.Context) error {
 		return errors.New("error: route configuration description takes two arguments: an app name and a route")
 	}
 
-	if err := resetBasePath(&a.Configuration); err != nil {
+	if err := resetBasePath(a.Configuration); err != nil {
 		return fmt.Errorf("error setting endpoint: %v", err)
 	}
 
@@ -284,6 +305,10 @@ func (a *routesCmd) configList(c *cli.Context) error {
 		return fmt.Errorf("error loading route information: %v", err)
 	}
 
+	if msg := wrapper.Error_.Message; msg != "" {
+		return errors.New(msg)
+	}
+
 	config := wrapper.Route.Config
 	if len(config) == 0 {
 		return errors.New("this route has no configurations")
@@ -314,7 +339,7 @@ func (a *routesCmd) configSet(c *cli.Context) error {
 		return errors.New("error: route configuration setting takes four arguments: an app name, a route, a key and a value")
 	}
 
-	if err := resetBasePath(&a.Configuration); err != nil {
+	if err := resetBasePath(a.Configuration); err != nil {
 		return fmt.Errorf("error setting endpoint: %v", err)
 	}
 
@@ -325,7 +350,11 @@ func (a *routesCmd) configSet(c *cli.Context) error {
 
 	wrapper, _, err := a.AppsAppRoutesRouteGet(appName, route)
 	if err != nil {
-		return fmt.Errorf("error creating app: %v", err)
+		return fmt.Errorf("error loading route: %v", err)
+	}
+
+	if msg := wrapper.Error_.Message; msg != "" {
+		return errors.New(msg)
 	}
 
 	config := wrapper.Route.Config
@@ -337,11 +366,7 @@ func (a *routesCmd) configSet(c *cli.Context) error {
 	config[key] = value
 	wrapper.Route.Config = config
 
-	if _, err := a.AppsAppRoutesRouteDelete(appName, route); err != nil {
-		return fmt.Errorf("error deleting to force update route: %v", err)
-	}
-
-	if _, _, err := a.AppsAppRoutesPost(appName, *wrapper); err != nil {
+	if _, _, err := a.AppsAppRoutesRoutePut(appName, route, *wrapper); err != nil {
 		return fmt.Errorf("error updating route configuration: %v", err)
 	}
 
@@ -354,7 +379,7 @@ func (a *routesCmd) configUnset(c *cli.Context) error {
 		return errors.New("error: route configuration setting takes four arguments: an app name, a route and a key")
 	}
 
-	if err := resetBasePath(&a.Configuration); err != nil {
+	if err := resetBasePath(a.Configuration); err != nil {
 		return fmt.Errorf("error setting endpoint: %v", err)
 	}
 
@@ -364,7 +389,11 @@ func (a *routesCmd) configUnset(c *cli.Context) error {
 
 	wrapper, _, err := a.AppsAppRoutesRouteGet(appName, route)
 	if err != nil {
-		return fmt.Errorf("error creating app: %v", err)
+		return fmt.Errorf("error loading app: %v", err)
+	}
+
+	if msg := wrapper.Error_.Message; msg != "" {
+		return errors.New(msg)
 	}
 
 	config := wrapper.Route.Config
@@ -380,11 +409,7 @@ func (a *routesCmd) configUnset(c *cli.Context) error {
 	delete(config, key)
 	wrapper.Route.Config = config
 
-	if _, err := a.AppsAppRoutesRouteDelete(appName, route); err != nil {
-		return fmt.Errorf("error deleting to force update route: %v", err)
-	}
-
-	if _, _, err := a.AppsAppRoutesPost(appName, *wrapper); err != nil {
+	if _, _, err := a.AppsAppRoutesRoutePut(appName, route, *wrapper); err != nil {
 		return fmt.Errorf("error updating route configuration: %v", err)
 	}