Skip to content

Commit

Permalink
functions: application updates no longer accept name in the body (#391)
Browse files Browse the repository at this point in the history
* functions: application updates no longer accept name in the body

AppUpdate was initially conceived as an upsert endpoint for apps.
It turns out that it created an inconsistency regarding updates:
updates with names divergent with URL would not actually change
application's name.

This commit atempts to address the issue by returning an HTTP
error when trying to update an application name. In swagger.yml,
application names are already `readOnly:true`. Thus there is no
change from expected behavior.

Fixes #380

* functions: use specific error value for name change
  • Loading branch information
ccirello authored Dec 7, 2016
1 parent 66d446b commit 3b16b7f
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 3 deletions.
3 changes: 2 additions & 1 deletion api/models/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@ var (
ErrAppsGet = errors.New("Could not get app from datastore")
ErrAppsList = errors.New("Could not list apps from datastore")
ErrAppsMissingNew = errors.New("Missing new application")
ErrAppsNameImmutable = errors.New("Could not update app - name is immutable")
ErrAppsNotFound = errors.New("App not found")
ErrAppsNothingToUpdate = errors.New("Nothing to update")
ErrAppsRemoving = errors.New("Could not remove app from datastore")
ErrDeleteAppsWithRoutes = errors.New("Cannot remove apps with routes")
ErrAppsUpdate = errors.New("Could not update app")
ErrDeleteAppsWithRoutes = errors.New("Cannot remove apps with routes")
ErrUsableImage = errors.New("Image not found")
)

Expand Down
7 changes: 7 additions & 0 deletions api/server/apps_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,13 @@ func TestAppUpdate(t *testing.T) {
Name: "myapp",
}},
}, "/v1/apps/myapp", `{ "app": { "config": { "test": "1" } } }`, http.StatusOK, nil},

// Addresses #380
{&datastore.Mock{
Apps: []*models.App{{
Name: "myapp",
}},
}, "/v1/apps/myapp", `{ "app": { "name": "othername" } }`, http.StatusForbidden, nil},
} {
rnr, cancel := testRunner(t)
router := testRouter(test.mock, &mqs.Mock{}, rnr, tasks)
Expand Down
6 changes: 6 additions & 0 deletions api/server/apps_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ func handleAppUpdate(c *gin.Context) {
return
}

if wapp.App.Name != "" {
log.Debug(models.ErrAppsNameImmutable)
c.JSON(http.StatusForbidden, simpleError(models.ErrAppsNameImmutable))
return
}

wapp.App.Name = c.Param("app")

err = Api.FireAfterAppUpdate(ctx, wapp.App)
Expand Down
7 changes: 6 additions & 1 deletion docs/apps.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,9 @@ curl -H "Content-Type: application/json" -X POST -d '{
}
}
}' http://localhost:8080/v1/apps
```
```

## Notes

App names are immutable. When doing `PUT` calls, keep in mind that although you
are able to update an app's configuration set, you cannot really rename it.
2 changes: 1 addition & 1 deletion docs/swagger.yml
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ paths:
schema:
$ref: '#/definitions/Error'
put:
summary: "Create/update a app."
summary: "Updates an app."
description: "You can set app level settings here. "
tags:
- Apps
Expand Down

0 comments on commit 3b16b7f

Please # to comment.