Skip to content

Commit d9cdcb5

Browse files
zsergezserge
authored
Chore: Refactor api handlers to use web.Bind (grafana#42199)
* Chore: Refactor api handlers to use web.Bind * fix comments * fix comment * trying to fix most of the tests and force routing.Wrap type check * fix library panels tests * fix frontend logging tests * allow passing nil as a response to skip writing * return nil instead of the response * rewrite login handler function types * remove handlerFuncCtx * make linter happy * remove old bindings from the libraryelements * restore comments
1 parent 9cbc872 commit d9cdcb5

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

54 files changed

+739
-299
lines changed

‎pkg/api/admin_users.go

+22-4
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package api
33
import (
44
"errors"
55
"fmt"
6+
"net/http"
67

78
"github.com/grafana/grafana/pkg/api/dtos"
89
"github.com/grafana/grafana/pkg/api/response"
@@ -11,9 +12,14 @@ import (
1112
"github.com/grafana/grafana/pkg/models"
1213
"github.com/grafana/grafana/pkg/services/sqlstore"
1314
"github.com/grafana/grafana/pkg/util"
15+
"github.com/grafana/grafana/pkg/web"
1416
)
1517

16-
func (hs *HTTPServer) AdminCreateUser(c *models.ReqContext, form dtos.AdminCreateUserForm) response.Response {
18+
func (hs *HTTPServer) AdminCreateUser(c *models.ReqContext) response.Response {
19+
form := dtos.AdminCreateUserForm{}
20+
if err := web.Bind(c.Req, &form); err != nil {
21+
return response.Error(http.StatusBadRequest, "bad request data", err)
22+
}
1723
cmd := models.CreateUserCommand{
1824
Login: form.Login,
1925
Email: form.Email,
@@ -56,7 +62,11 @@ func (hs *HTTPServer) AdminCreateUser(c *models.ReqContext, form dtos.AdminCreat
5662
return response.JSON(200, result)
5763
}
5864

59-
func AdminUpdateUserPassword(c *models.ReqContext, form dtos.AdminUpdateUserPasswordForm) response.Response {
65+
func AdminUpdateUserPassword(c *models.ReqContext) response.Response {
66+
form := dtos.AdminUpdateUserPasswordForm{}
67+
if err := web.Bind(c.Req, &form); err != nil {
68+
return response.Error(http.StatusBadRequest, "bad request data", err)
69+
}
6070
userID := c.ParamsInt64(":id")
6171

6272
if len(form.Password) < 4 {
@@ -87,7 +97,11 @@ func AdminUpdateUserPassword(c *models.ReqContext, form dtos.AdminUpdateUserPass
8797
}
8898

8999
// PUT /api/admin/users/:id/permissions
90-
func (hs *HTTPServer) AdminUpdateUserPermissions(c *models.ReqContext, form dtos.AdminUpdateUserPermissionsForm) response.Response {
100+
func (hs *HTTPServer) AdminUpdateUserPermissions(c *models.ReqContext) response.Response {
101+
form := dtos.AdminUpdateUserPermissionsForm{}
102+
if err := web.Bind(c.Req, &form); err != nil {
103+
return response.Error(http.StatusBadRequest, "bad request data", err)
104+
}
91105
userID := c.ParamsInt64(":id")
92106

93107
err := updateUserPermissions(hs.SQLStore, userID, form.IsGrafanaAdmin)
@@ -182,7 +196,11 @@ func (hs *HTTPServer) AdminGetUserAuthTokens(c *models.ReqContext) response.Resp
182196
}
183197

184198
// POST /api/admin/users/:id/revoke-auth-token
185-
func (hs *HTTPServer) AdminRevokeUserAuthToken(c *models.ReqContext, cmd models.RevokeAuthTokenCmd) response.Response {
199+
func (hs *HTTPServer) AdminRevokeUserAuthToken(c *models.ReqContext) response.Response {
200+
cmd := models.RevokeAuthTokenCmd{}
201+
if err := web.Bind(c.Req, &cmd); err != nil {
202+
return response.Error(http.StatusBadRequest, "bad request data", err)
203+
}
186204
userID := c.ParamsInt64(":id")
187205
return hs.revokeUserAuthTokenInternal(c, userID, cmd)
188206
}

‎pkg/api/admin_users_test.go

+6-3
Original file line numberDiff line numberDiff line change
@@ -315,12 +315,13 @@ func putAdminScenario(t *testing.T, desc string, url string, routePattern string
315315

316316
sc := setupScenarioContext(t, url)
317317
sc.defaultHandler = routing.Wrap(func(c *models.ReqContext) response.Response {
318+
c.Req.Body = mockRequestBody(cmd)
318319
sc.context = c
319320
sc.context.UserId = testUserID
320321
sc.context.OrgId = testOrgID
321322
sc.context.OrgRole = role
322323

323-
return hs.AdminUpdateUserPermissions(c, cmd)
324+
return hs.AdminUpdateUserPermissions(c)
324325
})
325326

326327
sc.m.Put(routePattern, sc.defaultHandler)
@@ -370,12 +371,13 @@ func adminRevokeUserAuthTokenScenario(t *testing.T, desc string, url string, rou
370371
sc := setupScenarioContext(t, url)
371372
sc.userAuthTokenService = fakeAuthTokenService
372373
sc.defaultHandler = routing.Wrap(func(c *models.ReqContext) response.Response {
374+
c.Req.Body = mockRequestBody(cmd)
373375
sc.context = c
374376
sc.context.UserId = testUserID
375377
sc.context.OrgId = testOrgID
376378
sc.context.OrgRole = models.ROLE_ADMIN
377379

378-
return hs.AdminRevokeUserAuthToken(c, cmd)
380+
return hs.AdminRevokeUserAuthToken(c)
379381
})
380382

381383
sc.m.Post(routePattern, sc.defaultHandler)
@@ -470,10 +472,11 @@ func adminCreateUserScenario(t *testing.T, desc string, url string, routePattern
470472

471473
sc := setupScenarioContext(t, url)
472474
sc.defaultHandler = routing.Wrap(func(c *models.ReqContext) response.Response {
475+
c.Req.Body = mockRequestBody(cmd)
473476
sc.context = c
474477
sc.context.UserId = testUserID
475478

476-
return hs.AdminCreateUser(c, cmd)
479+
return hs.AdminCreateUser(c)
477480
})
478481

479482
sc.m.Post(routePattern, sc.defaultHandler)

‎pkg/api/alerting.go

+36-7
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"errors"
66
"fmt"
7+
"net/http"
78
"strconv"
89

910
"github.com/grafana/grafana/pkg/api/dtos"
@@ -132,7 +133,11 @@ func GetAlerts(c *models.ReqContext) response.Response {
132133
}
133134

134135
// POST /api/alerts/test
135-
func (hs *HTTPServer) AlertTest(c *models.ReqContext, dto dtos.AlertTestCommand) response.Response {
136+
func (hs *HTTPServer) AlertTest(c *models.ReqContext) response.Response {
137+
dto := dtos.AlertTestCommand{}
138+
if err := web.Bind(c.Req, &dto); err != nil {
139+
return response.Error(http.StatusBadRequest, "bad request data", err)
140+
}
136141
if _, idErr := dto.Dashboard.Get("id").Int64(); idErr != nil {
137142
return response.Error(400, "The dashboard needs to be saved at least once before you can test an alert rule", nil)
138143
}
@@ -276,7 +281,11 @@ func GetAlertNotificationByUID(c *models.ReqContext) response.Response {
276281
return response.JSON(200, dtos.NewAlertNotification(query.Result))
277282
}
278283

279-
func CreateAlertNotification(c *models.ReqContext, cmd models.CreateAlertNotificationCommand) response.Response {
284+
func CreateAlertNotification(c *models.ReqContext) response.Response {
285+
cmd := models.CreateAlertNotificationCommand{}
286+
if err := web.Bind(c.Req, &cmd); err != nil {
287+
return response.Error(http.StatusBadRequest, "bad request data", err)
288+
}
280289
cmd.OrgId = c.OrgId
281290

282291
if err := bus.DispatchCtx(c.Req.Context(), &cmd); err != nil {
@@ -293,7 +302,11 @@ func CreateAlertNotification(c *models.ReqContext, cmd models.CreateAlertNotific
293302
return response.JSON(200, dtos.NewAlertNotification(cmd.Result))
294303
}
295304

296-
func (hs *HTTPServer) UpdateAlertNotification(c *models.ReqContext, cmd models.UpdateAlertNotificationCommand) response.Response {
305+
func (hs *HTTPServer) UpdateAlertNotification(c *models.ReqContext) response.Response {
306+
cmd := models.UpdateAlertNotificationCommand{}
307+
if err := web.Bind(c.Req, &cmd); err != nil {
308+
return response.Error(http.StatusBadRequest, "bad request data", err)
309+
}
297310
cmd.OrgId = c.OrgId
298311

299312
err := hs.fillWithSecureSettingsData(c.Req.Context(), &cmd)
@@ -324,7 +337,11 @@ func (hs *HTTPServer) UpdateAlertNotification(c *models.ReqContext, cmd models.U
324337
return response.JSON(200, dtos.NewAlertNotification(query.Result))
325338
}
326339

327-
func (hs *HTTPServer) UpdateAlertNotificationByUID(c *models.ReqContext, cmd models.UpdateAlertNotificationWithUidCommand) response.Response {
340+
func (hs *HTTPServer) UpdateAlertNotificationByUID(c *models.ReqContext) response.Response {
341+
cmd := models.UpdateAlertNotificationWithUidCommand{}
342+
if err := web.Bind(c.Req, &cmd); err != nil {
343+
return response.Error(http.StatusBadRequest, "bad request data", err)
344+
}
328345
cmd.OrgId = c.OrgId
329346
cmd.Uid = web.Params(c.Req)[":uid"]
330347

@@ -444,7 +461,11 @@ func DeleteAlertNotificationByUID(c *models.ReqContext) response.Response {
444461
}
445462

446463
// POST /api/alert-notifications/test
447-
func NotificationTest(c *models.ReqContext, dto dtos.NotificationTestCommand) response.Response {
464+
func NotificationTest(c *models.ReqContext) response.Response {
465+
dto := dtos.NotificationTestCommand{}
466+
if err := web.Bind(c.Req, &dto); err != nil {
467+
return response.Error(http.StatusBadRequest, "bad request data", err)
468+
}
448469
cmd := &alerting.NotificationTestCommand{
449470
OrgID: c.OrgId,
450471
ID: dto.ID,
@@ -470,7 +491,11 @@ func NotificationTest(c *models.ReqContext, dto dtos.NotificationTestCommand) re
470491
}
471492

472493
// POST /api/alerts/:alertId/pause
473-
func PauseAlert(c *models.ReqContext, dto dtos.PauseAlertCommand) response.Response {
494+
func PauseAlert(c *models.ReqContext) response.Response {
495+
dto := dtos.PauseAlertCommand{}
496+
if err := web.Bind(c.Req, &dto); err != nil {
497+
return response.Error(http.StatusBadRequest, "bad request data", err)
498+
}
474499
alertID := c.ParamsInt64(":alertId")
475500
result := make(map[string]interface{})
476501
result["alertId"] = alertID
@@ -523,7 +548,11 @@ func PauseAlert(c *models.ReqContext, dto dtos.PauseAlertCommand) response.Respo
523548
}
524549

525550
// POST /api/admin/pause-all-alerts
526-
func PauseAllAlerts(c *models.ReqContext, dto dtos.PauseAllAlertsCommand) response.Response {
551+
func PauseAllAlerts(c *models.ReqContext) response.Response {
552+
dto := dtos.PauseAllAlertsCommand{}
553+
if err := web.Bind(c.Req, &dto); err != nil {
554+
return response.Error(http.StatusBadRequest, "bad request data", err)
555+
}
527556
updateCmd := models.PauseAllAlertCommand{
528557
Paused: dto.Paused,
529558
}

‎pkg/api/alerting_test.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -166,12 +166,13 @@ func postAlertScenario(t *testing.T, desc string, url string, routePattern strin
166166

167167
sc := setupScenarioContext(t, url)
168168
sc.defaultHandler = routing.Wrap(func(c *models.ReqContext) response.Response {
169+
c.Req.Body = mockRequestBody(cmd)
169170
sc.context = c
170171
sc.context.UserId = testUserID
171172
sc.context.OrgId = testOrgID
172173
sc.context.OrgRole = role
173174

174-
return PauseAlert(c, cmd)
175+
return PauseAlert(c)
175176
})
176177

177178
sc.m.Post(routePattern, sc.defaultHandler)

‎pkg/api/annotations.go

+27-5
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package api
22

33
import (
44
"errors"
5+
"net/http"
56
"strings"
67

78
"github.com/grafana/grafana/pkg/api/dtos"
@@ -10,6 +11,7 @@ import (
1011
"github.com/grafana/grafana/pkg/services/annotations"
1112
"github.com/grafana/grafana/pkg/services/guardian"
1213
"github.com/grafana/grafana/pkg/util"
14+
"github.com/grafana/grafana/pkg/web"
1315
)
1416

1517
func GetAnnotations(c *models.ReqContext) response.Response {
@@ -51,7 +53,11 @@ func (e *CreateAnnotationError) Error() string {
5153
return e.message
5254
}
5355

54-
func PostAnnotation(c *models.ReqContext, cmd dtos.PostAnnotationsCmd) response.Response {
56+
func PostAnnotation(c *models.ReqContext) response.Response {
57+
cmd := dtos.PostAnnotationsCmd{}
58+
if err := web.Bind(c.Req, &cmd); err != nil {
59+
return response.Error(http.StatusBadRequest, "bad request data", err)
60+
}
5561
if canSave, err := canSaveByDashboardID(c, cmd.DashboardId); err != nil || !canSave {
5662
return dashboardGuardianResponse(err)
5763
}
@@ -98,7 +104,11 @@ func formatGraphiteAnnotation(what string, data string) string {
98104
return text
99105
}
100106

101-
func PostGraphiteAnnotation(c *models.ReqContext, cmd dtos.PostGraphiteAnnotationsCmd) response.Response {
107+
func PostGraphiteAnnotation(c *models.ReqContext) response.Response {
108+
cmd := dtos.PostGraphiteAnnotationsCmd{}
109+
if err := web.Bind(c.Req, &cmd); err != nil {
110+
return response.Error(http.StatusBadRequest, "bad request data", err)
111+
}
102112
repo := annotations.GetRepository()
103113

104114
if cmd.What == "" {
@@ -149,7 +159,11 @@ func PostGraphiteAnnotation(c *models.ReqContext, cmd dtos.PostGraphiteAnnotatio
149159
})
150160
}
151161

152-
func UpdateAnnotation(c *models.ReqContext, cmd dtos.UpdateAnnotationsCmd) response.Response {
162+
func UpdateAnnotation(c *models.ReqContext) response.Response {
163+
cmd := dtos.UpdateAnnotationsCmd{}
164+
if err := web.Bind(c.Req, &cmd); err != nil {
165+
return response.Error(http.StatusBadRequest, "bad request data", err)
166+
}
153167
annotationID := c.ParamsInt64(":annotationId")
154168

155169
repo := annotations.GetRepository()
@@ -175,7 +189,11 @@ func UpdateAnnotation(c *models.ReqContext, cmd dtos.UpdateAnnotationsCmd) respo
175189
return response.Success("Annotation updated")
176190
}
177191

178-
func PatchAnnotation(c *models.ReqContext, cmd dtos.PatchAnnotationsCmd) response.Response {
192+
func PatchAnnotation(c *models.ReqContext) response.Response {
193+
cmd := dtos.PatchAnnotationsCmd{}
194+
if err := web.Bind(c.Req, &cmd); err != nil {
195+
return response.Error(http.StatusBadRequest, "bad request data", err)
196+
}
179197
annotationID := c.ParamsInt64(":annotationId")
180198

181199
repo := annotations.GetRepository()
@@ -223,7 +241,11 @@ func PatchAnnotation(c *models.ReqContext, cmd dtos.PatchAnnotationsCmd) respons
223241
return response.Success("Annotation patched")
224242
}
225243

226-
func DeleteAnnotations(c *models.ReqContext, cmd dtos.DeleteAnnotationsCmd) response.Response {
244+
func DeleteAnnotations(c *models.ReqContext) response.Response {
245+
cmd := dtos.DeleteAnnotationsCmd{}
246+
if err := web.Bind(c.Req, &cmd); err != nil {
247+
return response.Error(http.StatusBadRequest, "bad request data", err)
248+
}
227249
repo := annotations.GetRepository()
228250

229251
err := repo.Delete(&annotations.DeleteParams{

‎pkg/api/annotations_test.go

+8-4
Original file line numberDiff line numberDiff line change
@@ -275,12 +275,13 @@ func postAnnotationScenario(t *testing.T, desc string, url string, routePattern
275275

276276
sc := setupScenarioContext(t, url)
277277
sc.defaultHandler = routing.Wrap(func(c *models.ReqContext) response.Response {
278+
c.Req.Body = mockRequestBody(cmd)
278279
sc.context = c
279280
sc.context.UserId = testUserID
280281
sc.context.OrgId = testOrgID
281282
sc.context.OrgRole = role
282283

283-
return PostAnnotation(c, cmd)
284+
return PostAnnotation(c)
284285
})
285286

286287
fakeAnnoRepo = &fakeAnnotationsRepo{}
@@ -299,12 +300,13 @@ func putAnnotationScenario(t *testing.T, desc string, url string, routePattern s
299300

300301
sc := setupScenarioContext(t, url)
301302
sc.defaultHandler = routing.Wrap(func(c *models.ReqContext) response.Response {
303+
c.Req.Body = mockRequestBody(cmd)
302304
sc.context = c
303305
sc.context.UserId = testUserID
304306
sc.context.OrgId = testOrgID
305307
sc.context.OrgRole = role
306308

307-
return UpdateAnnotation(c, cmd)
309+
return UpdateAnnotation(c)
308310
})
309311

310312
fakeAnnoRepo = &fakeAnnotationsRepo{}
@@ -322,12 +324,13 @@ func patchAnnotationScenario(t *testing.T, desc string, url string, routePattern
322324

323325
sc := setupScenarioContext(t, url)
324326
sc.defaultHandler = routing.Wrap(func(c *models.ReqContext) response.Response {
327+
c.Req.Body = mockRequestBody(cmd)
325328
sc.context = c
326329
sc.context.UserId = testUserID
327330
sc.context.OrgId = testOrgID
328331
sc.context.OrgRole = role
329332

330-
return PatchAnnotation(c, cmd)
333+
return PatchAnnotation(c)
331334
})
332335

333336
fakeAnnoRepo = &fakeAnnotationsRepo{}
@@ -346,12 +349,13 @@ func deleteAnnotationsScenario(t *testing.T, desc string, url string, routePatte
346349

347350
sc := setupScenarioContext(t, url)
348351
sc.defaultHandler = routing.Wrap(func(c *models.ReqContext) response.Response {
352+
c.Req.Body = mockRequestBody(cmd)
349353
sc.context = c
350354
sc.context.UserId = testUserID
351355
sc.context.OrgId = testOrgID
352356
sc.context.OrgRole = role
353357

354-
return DeleteAnnotations(c, cmd)
358+
return DeleteAnnotations(c)
355359
})
356360

357361
fakeAnnoRepo = &fakeAnnotationsRepo{}

0 commit comments

Comments
 (0)