IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-02-25 18:55:37 -06:00
Code Issues Packages Projects Releases Wiki Activity

API: Fix snapshot responses (#52998)

Browse Source 
* API: Fix response status when snapshots are not found

* API: Fix response status when snapshot key is empty

* Apply suggestions from code review
This commit is contained in:
Sofia Papagiannaki 2022-08-03 17:31:23 +03:00 committed by GitHub
parent 54217a2037
commit 5fec6cc4f5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 117 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
pkg/api/dashboard_snapshot.go
View File

@ -175,19 +175,20 @@ func (hs *HTTPServer) CreateDashboardSnapshot(c *models.ReqContext) response.Res
//
// Responses:
// 200: getDashboardSnapshotResponse
// 400: badRequestError
// 404: notFoundError
// 500: internalServerError
func (hs *HTTPServer) GetDashboardSnapshot(c *models.ReqContext) response.Response {
key := web.Params(c.Req)[":key"]
if len(key) == 0 {
return response.Error(404, "Snapshot not found", nil)
return response.Error(http.StatusBadRequest, "Empty snapshot key", nil)
}
query := &dashboardsnapshots.GetDashboardSnapshotQuery{Key: key}
err := hs.dashboardsnapshotsService.GetDashboardSnapshot(c.Req.Context(), query)
if err != nil {
return response.Error(500, "Failed to get dashboard snapshot", err)
return response.Err(err)
}
snapshot := query.Result
@ -265,7 +266,7 @@ func (hs *HTTPServer) DeleteDashboardSnapshotByDeleteKey(c *models.ReqContext) r
query := &dashboardsnapshots.GetDashboardSnapshotQuery{DeleteKey: key}
err := hs.dashboardsnapshotsService.GetDashboardSnapshot(c.Req.Context(), query)
if err != nil {
return response.Error(500, "Failed to get dashboard snapshot", err)
return response.Err(err)
}
if query.Result.External {
@ -306,7 +307,7 @@ func (hs *HTTPServer) DeleteDashboardSnapshot(c *models.ReqContext) response.Res
err := hs.dashboardsnapshotsService.GetDashboardSnapshot(c.Req.Context(), query)
if err != nil {
return response.Error(500, "Failed to get dashboard snapshot", err)
return response.Err(err)
}
if query.Result == nil {
return response.Error(404, "Failed to get dashboard snapshot", nil)

101
pkg/api/dashboard_snapshot_test.go
View File

@ -1,6 +1,7 @@
package api
import (
"errors"
"fmt"
"net/http"
"net/http/httptest"
@ -223,3 +224,103 @@ func TestDashboardSnapshotAPIEndpoint_singleSnapshot(t *testing.T) {
}, sqlmock)
})
}
func TestGetDashboardSnapshotNotFound(t *testing.T) {
sqlmock := mockstore.NewSQLStoreMock()
sqlmock.ExpectedTeamsByUser = []*models.TeamDTO{}
setUpSnapshotTest := func(t *testing.T) dashboardsnapshots.Service {
t.Helper()
dashSnapSvc := dashboardsnapshots.NewMockService(t)
dashSnapSvc.
On("GetDashboardSnapshot", mock.Anything, mock.AnythingOfType("*dashboardsnapshots.GetDashboardSnapshotQuery")).
Run(func(args mock.Arguments) {}).
Return(dashboardsnapshots.ErrBaseNotFound.Errorf(""))
return dashSnapSvc
}
loggedInUserScenarioWithRole(t,
"GET /snapshots/{key} should return 404 when the snapshot does not exist", "GET",
"/api/snapshots/12345", "/api/snapshots/:key", models.ROLE_EDITOR, func(sc *scenarioContext) {
d := setUpSnapshotTest(t)
hs := &HTTPServer{dashboardsnapshotsService: d}
sc.handlerFunc = hs.GetDashboardSnapshot
sc.fakeReqWithParams("GET", sc.url, map[string]string{"key": "12345"}).exec()
assert.Equal(t, http.StatusNotFound, sc.resp.Code)
}, sqlmock)
loggedInUserScenarioWithRole(t,
"DELETE /snapshots/{key} should return 404 when the snapshot does not exist", "DELETE",
"/api/snapshots/12345", "/api/snapshots/:key", models.ROLE_EDITOR, func(sc *scenarioContext) {
d := setUpSnapshotTest(t)
hs := &HTTPServer{dashboardsnapshotsService: d}
sc.handlerFunc = hs.DeleteDashboardSnapshot
sc.fakeReqWithParams("DELETE", sc.url, map[string]string{"key": "12345"}).exec()
assert.Equal(t, http.StatusNotFound, sc.resp.Code)
}, sqlmock)
loggedInUserScenarioWithRole(t,
"GET /snapshots-delete/{deleteKey} should return 404 when the snapshot does not exist", "DELETE",
"/api/snapshots-delete/12345", "/api/snapshots-delete/:deleteKey", models.ROLE_EDITOR, func(sc *scenarioContext) {
d := setUpSnapshotTest(t)
hs := &HTTPServer{dashboardsnapshotsService: d}
sc.handlerFunc = hs.DeleteDashboardSnapshotByDeleteKey
sc.fakeReqWithParams("DELETE", sc.url, map[string]string{"deleteKey": "12345"}).exec()
assert.Equal(t, http.StatusNotFound, sc.resp.Code)
}, sqlmock)
}
func TestGetDashboardSnapshotFailure(t *testing.T) {
sqlmock := mockstore.NewSQLStoreMock()
sqlmock.ExpectedTeamsByUser = []*models.TeamDTO{}
setUpSnapshotTest := func(t *testing.T) dashboardsnapshots.Service {
t.Helper()
dashSnapSvc := dashboardsnapshots.NewMockService(t)
dashSnapSvc.
On("GetDashboardSnapshot", mock.Anything, mock.AnythingOfType("*dashboardsnapshots.GetDashboardSnapshotQuery")).
Run(func(args mock.Arguments) {}).
Return(errors.New("something went wrong"))
return dashSnapSvc
}
loggedInUserScenarioWithRole(t,
"GET /snapshots/{key} should return 404 when the snapshot does not exist", "GET",
"/api/snapshots/12345", "/api/snapshots/:key", models.ROLE_EDITOR, func(sc *scenarioContext) {
d := setUpSnapshotTest(t)
hs := &HTTPServer{dashboardsnapshotsService: d}
sc.handlerFunc = hs.GetDashboardSnapshot
sc.fakeReqWithParams("GET", sc.url, map[string]string{"key": "12345"}).exec()
assert.Equal(t, http.StatusInternalServerError, sc.resp.Code)
}, sqlmock)
loggedInUserScenarioWithRole(t,
"DELETE /snapshots/{key} should return 404 when the snapshot does not exist", "DELETE",
"/api/snapshots/12345", "/api/snapshots/:key", models.ROLE_EDITOR, func(sc *scenarioContext) {
d := setUpSnapshotTest(t)
hs := &HTTPServer{dashboardsnapshotsService: d}
sc.handlerFunc = hs.DeleteDashboardSnapshot
sc.fakeReqWithParams("DELETE", sc.url, map[string]string{"key": "12345"}).exec()
assert.Equal(t, http.StatusInternalServerError, sc.resp.Code)
}, sqlmock)
loggedInUserScenarioWithRole(t,
"GET /snapshots-delete/{deleteKey} should return 404 when the snapshot does not exist", "DELETE",
"/api/snapshots-delete/12345", "/api/snapshots-delete/:deleteKey", models.ROLE_EDITOR, func(sc *scenarioContext) {
d := setUpSnapshotTest(t)
hs := &HTTPServer{dashboardsnapshotsService: d}
sc.handlerFunc = hs.DeleteDashboardSnapshotByDeleteKey
sc.fakeReqWithParams("DELETE", sc.url, map[string]string{"deleteKey": "12345"}).exec()
assert.Equal(t, http.StatusInternalServerError, sc.resp.Code)
}, sqlmock)
}

2
pkg/services/dashboardsnapshots/database/database.go
View File

@ -91,7 +91,7 @@ func (d *DashboardSnapshotStore) GetDashboardSnapshot(ctx context.Context, query
if err != nil {
return err
} else if !has {
return dashboardsnapshots.ErrDashboardSnapshotNotFound
return dashboardsnapshots.ErrBaseNotFound.Errorf("dashboard snapshot not found")
}
query.Result = &snapshot

9
pkg/services/dashboardsnapshots/errors.go
View File

@ -1,8 +1,7 @@
package dashboardsnapshots
import "github.com/grafana/grafana/pkg/services/dashboards"
import (
"github.com/grafana/grafana/pkg/util/errutil"
)
var ErrDashboardSnapshotNotFound = dashboards.DashboardErr{
Reason: "Dashboard snapshot not found",
StatusCode: 404,
}
var ErrBaseNotFound = errutil.NewBase(errutil.StatusNotFound, "dashboardsnapshots.not-found", errutil.WithPublicMessage("Snapshot not found"))

3
public/api-merged.json
View File

@ -8909,6 +8909,9 @@
"200": {
"$ref": "#/responses/getDashboardSnapshotResponse"
},
"400": {
"$ref": "#/responses/badRequestError"
},
"404": {
"$ref": "#/responses/notFoundError"
},

3
public/api-spec.json
View File

@ -8262,6 +8262,9 @@
"200": {
"$ref": "#/responses/getDashboardSnapshotResponse"
},
"400": {
"$ref": "#/responses/badRequestError"
},
"404": {
"$ref": "#/responses/notFoundError"
},