Skip to content

Commit 165c9f5

Browse files
mirkoCrobumirkoCrobu
andauthored
Feat/455 improve app patch response (#461)
Co-authored-by: mirkoCrobu <mirkocrobu@NB-0531.localdomain>
1 parent d54d550 commit 165c9f5

File tree

8 files changed

+64
-32
lines changed

8 files changed

+64
-32
lines changed

cmd/gendoc/docs.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -396,8 +396,10 @@ Contains a JSON object with the details of an error.
396396
ID string `path:"id" description:"application identifier."`
397397
})(nil),
398398
CustomSuccessResponse: &CustomResponseDef{
399-
Description: "Successful response",
400-
StatusCode: http.StatusOK,
399+
ContentType: "application/json",
400+
DataStructure: orchestrator.AppDetailedInfo{},
401+
Description: "Successful response",
402+
StatusCode: http.StatusOK,
401403
},
402404
Description: "Edit the given application. Is it possible to modify the default status, to add/remove/update bricks and bricks variables.",
403405
Summary: "Update App Details",

internal/api/api.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ func NewHTTPRouter(
4545
mux.Handle("POST /v1/apps", handlers.HandleAppCreate(dockerClient))
4646

4747
mux.Handle("GET /v1/apps/{appID}", handlers.HandleAppDetails(dockerClient, bricksIndex))
48-
mux.Handle("PATCH /v1/apps/{appID}", handlers.HandleAppDetailsEdits())
48+
mux.Handle("PATCH /v1/apps/{appID}", handlers.HandleAppDetailsEdits(dockerClient, bricksIndex))
4949
mux.Handle("GET /v1/apps/{appID}/logs", handlers.HandleAppLogs(dockerClient))
5050
mux.Handle("POST /v1/apps/{appID}/start", handlers.HandleAppStart(dockerClient, provisioner, modelsIndex, bricksIndex))
5151
mux.Handle("POST /v1/apps/{appID}/stop", handlers.HandleAppStop(dockerClient))

internal/api/docs/openapi.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,10 @@ paths:
336336
$ref: '#/components/schemas/EditRequest'
337337
responses:
338338
"200":
339+
content:
340+
application/json:
341+
schema:
342+
$ref: '#/components/schemas/AppDetailedInfo'
339343
description: Successful response
340344
"400":
341345
$ref: '#/components/responses/BadRequest'

internal/api/handlers/app_details.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ type EditRequest struct {
4646
Default *bool `json:"default"`
4747
}
4848

49-
func HandleAppDetailsEdits() http.HandlerFunc {
49+
func HandleAppDetailsEdits(dockerClient *dockerClient.Client, bricksIndex *bricksindex.BricksIndex) http.HandlerFunc {
5050
return func(w http.ResponseWriter, r *http.Request) {
5151
id, err := orchestrator.NewIDFromBase64(r.PathValue("appID"))
5252
if err != nil {
@@ -71,7 +71,6 @@ func HandleAppDetailsEdits() http.HandlerFunc {
7171
render.EncodeResponse(w, http.StatusBadRequest, models.ErrorResponse{Details: "invalid request"})
7272
return
7373
}
74-
7574
err = orchestrator.EditApp(orchestrator.AppEditRequest{
7675
Default: editRequest.Default,
7776
Name: editRequest.Name,
@@ -83,7 +82,12 @@ func HandleAppDetailsEdits() http.HandlerFunc {
8382
render.EncodeResponse(w, http.StatusInternalServerError, models.ErrorResponse{Details: "unable to edit the app"})
8483
return
8584
}
86-
87-
w.WriteHeader(http.StatusOK)
85+
res, err := orchestrator.AppDetails(r.Context(), dockerClient, app, bricksIndex)
86+
if err != nil {
87+
slog.Error("Unable to parse the app.yaml", slog.String("error", err.Error()))
88+
render.EncodeResponse(w, http.StatusInternalServerError, models.ErrorResponse{Details: "unable to find the app"})
89+
return
90+
}
91+
render.EncodeResponse(w, http.StatusOK, res)
8892
}
8993
}

internal/e2e/client/client.gen.go

Lines changed: 8 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

internal/e2e/daemon/app_test.go

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,6 @@ func TestEditApp(t *testing.T) {
115115
httpClient := GetHttpclient(t)
116116

117117
appName := "test-app-to-edit"
118-
var updatedAppId string
119118
createResp, err := httpClient.CreateAppWithResponse(
120119
t.Context(),
121120
&client.CreateAppParams{SkipSketch: f.Ptr(true)},
@@ -132,29 +131,25 @@ func TestEditApp(t *testing.T) {
132131

133132
t.Run("EditAllFields_Success", func(t *testing.T) {
134133
renamedApp := appName + "-renamed"
134+
modifedIcon := "🌟"
135135
editResp, err := httpClient.EditAppWithResponse(
136136
t.Context(),
137137
validAppId,
138138
client.EditRequest{
139139
Description: f.Ptr("new-description"),
140-
Icon: f.Ptr("🌟"),
140+
Icon: f.Ptr(modifedIcon),
141141
Name: f.Ptr(renamedApp),
142142
},
143143
)
144144
require.NoError(t, err)
145145
require.Equal(t, http.StatusOK, editResp.StatusCode())
146-
147-
appList, err := httpClient.GetAppsWithResponse(t.Context(), &client.GetAppsParams{})
146+
require.NotNil(t, editResp.JSON200)
147+
require.NotNil(t, editResp.JSON200.Id)
148+
detailsResp, err := httpClient.GetAppDetailsWithResponse(t.Context(), editResp.JSON200.Id)
148149
require.NoError(t, err)
149-
require.Equal(t, http.StatusOK, appList.StatusCode())
150-
require.NotEmpty(t, appList.JSON200.Apps)
151-
require.Len(t, *appList.JSON200.Apps, 1)
152-
153-
app := (*appList.JSON200.Apps)[0]
154-
require.Equal(t, renamedApp, *app.Name)
155-
require.Equal(t, "new-description", *app.Description)
156-
require.Equal(t, "🌟", *app.Icon)
157-
updatedAppId = *app.Id
150+
require.Equal(t, http.StatusOK, detailsResp.StatusCode())
151+
require.Equal(t, renamedApp, detailsResp.JSON200.Name)
152+
require.Equal(t, modifedIcon, *detailsResp.JSON200.Icon)
158153
})
159154

160155
t.Run("InvalidAppId_Fail", func(t *testing.T) {
@@ -216,11 +211,25 @@ func TestEditApp(t *testing.T) {
216211
})
217212

218213
t.Run("InvalidRequestBody_Fail", func(t *testing.T) {
214+
createResp, err := httpClient.CreateAppWithResponse(
215+
t.Context(),
216+
&client.CreateAppParams{SkipSketch: f.Ptr(true)},
217+
client.CreateAppRequest{
218+
Icon: f.Ptr("💻"),
219+
Name: "new-valid-app",
220+
},
221+
)
222+
require.NoError(t, err)
223+
require.Equal(t, http.StatusCreated, createResp.StatusCode())
224+
require.NotNil(t, createResp.JSON201)
225+
226+
validAppId := *createResp.JSON201.Id
227+
219228
var actualResponseBody models.ErrorResponse
220229
malformedBody := `{"name": "test" "icon": "💻"}`
221230
editResp, err := httpClient.EditAppWithBody(
222231
t.Context(),
223-
updatedAppId,
232+
validAppId,
224233
"application/json",
225234
strings.NewReader(malformedBody),
226235
)

internal/orchestrator/orchestrator.go

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -748,22 +748,26 @@ func EditApp(req AppEditRequest, app *app.ArduinoApp) (editErr error) {
748748
if newPath.Exist() {
749749
return ErrAppAlreadyExists
750750
}
751-
752-
defer func() {
753-
if err := app.FullPath.Rename(newPath); err != nil {
754-
editErr = fmt.Errorf("failed to rename app path: %w", err)
755-
return
756-
}
757-
}()
751+
if err := app.FullPath.Rename(newPath); err != nil {
752+
editErr = fmt.Errorf("failed to rename app path: %w", err)
753+
return editErr
754+
}
755+
app.FullPath = newPath
756+
app.Name = app.Descriptor.Name
758757
}
758+
759759
if req.Icon != nil {
760760
app.Descriptor.Icon = *req.Icon
761761
}
762762
if req.Description != nil {
763763
app.Descriptor.Description = *req.Description
764764
}
765765

766-
return app.Save()
766+
err := app.Save()
767+
if err != nil {
768+
return fmt.Errorf("failed to save app: %w", err)
769+
}
770+
return nil
767771
}
768772

769773
func editAppDefaults(userApp *app.ArduinoApp, isDefault bool) error {

internal/orchestrator/orchestrator_test.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -238,14 +238,15 @@ func TestEditApp(t *testing.T) {
238238
_, err := CreateApp(t.Context(), CreateAppRequest{Name: originalAppName})
239239
require.NoError(t, err)
240240
appDir := orchestratorConfig.AppsDir().Join(originalAppName)
241-
originalApp := f.Must(app.Load(appDir.String()))
241+
userApp := f.Must(app.Load(appDir.String()))
242+
originalPath := userApp.FullPath
242243

243-
err = EditApp(AppEditRequest{Name: f.Ptr("new-name")}, &originalApp)
244+
err = EditApp(AppEditRequest{Name: f.Ptr("new-name")}, &userApp)
244245
require.NoError(t, err)
245246
editedApp, err := app.Load(orchestratorConfig.AppsDir().Join("new-name").String())
246247
require.NoError(t, err)
247248
require.Equal(t, "new-name", editedApp.Name)
248-
require.True(t, originalApp.FullPath.NotExist()) // The original app directory should be removed after renaming
249+
require.True(t, originalPath.NotExist()) // The original app directory should be removed after renaming
249250

250251
t.Run("already existing name", func(t *testing.T) {
251252
existingAppName := "existing-name"

0 commit comments

Comments
 (0)