Skip to content

Commit

Permalink
fn: improve UX (#325)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
ccirello authored and seiflotfy committed Nov 21, 2016
1 parent e2e8208 commit fe845e1
Show file tree
Hide file tree
Showing 10 changed files with 189 additions and 96 deletions.
13 changes: 10 additions & 3 deletions api/server/routes_delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package server
import (
"context"
"net/http"
"path"

"github.com/gin-gonic/gin"
"github.com/iron-io/functions/api/models"
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion api/server/routes_get.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package server
import (
"context"
"net/http"
"path"

"github.com/Sirupsen/logrus"
"github.com/gin-gonic/gin"
Expand All @@ -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 {
Expand Down
7 changes: 6 additions & 1 deletion api/server/routes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)

Expand Down
3 changes: 2 additions & 1 deletion api/server/routes_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package server
import (
"context"
"net/http"
"path"

"github.com/gin-gonic/gin"
"github.com/iron-io/functions/api/models"
Expand Down Expand Up @@ -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{
Expand Down
41 changes: 40 additions & 1 deletion docs/swagger.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
77 changes: 46 additions & 31 deletions fn/apps.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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: "set",
Aliases: []string{"s"},
Usage: "store a configuration key for this application",
ArgsUsage: "`app` <key> <value>",
Action: a.configSet,
},
{
Name: "unset",
Description: "remove a configuration key for this application",
Usage: "<app> <key> <value>",
Action: a.configUnset,
Name: "unset",
Aliases: []string{"u"},
Usage: "remove a configuration key for this application",
ArgsUsage: "`app` <key>",
Action: a.configUnset,
},
},
},
Expand All @@ -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)
}

Expand All @@ -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)
}

Expand All @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand Down
6 changes: 3 additions & 3 deletions fn/glide.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions fn/glide.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -20,3 +18,5 @@ import:
subpackages:
- ssh/terminal
- package: gopkg.in/yaml.v2
- package: github.com/iron-io/functions_go
version: 190cb886f033aac6d41e657eb91649b27cb52736
2 changes: 1 addition & 1 deletion fn/publish.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
Loading

0 comments on commit fe845e1

Please sign in to comment.