public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[v8.5.x] API: Fix snapshot responses (#53301)

Browse Source 
* API: Fix snapshot responses (#52998)

* API: Fix response status when snapshots are not found

* API: Fix response status when snapshot key is empty

* Apply suggestions from code review

(cherry picked from commit 5fec6cc4f5)
This commit is contained in:
Sofia Papagiannaki
2022-08-04 19:21:39 +03:00
committed by GitHub
parent 6c8768020e
commit dc52320fc9
5 changed files with 90 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/api/dashboard_snapshot.go
+14 -4
View File
@@ -3,6 +3,7 @@ package api
import (
"bytes"
"encoding/json"
"errors"
"fmt"
"net/http"
"time"
@@ -152,14 +153,17 @@ func (hs *HTTPServer) CreateDashboardSnapshot(c *models.ReqContext) response.Res
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 := &models.GetDashboardSnapshotQuery{Key: key}
err := hs.DashboardsnapshotsService.GetDashboardSnapshot(c.Req.Context(), query)
if err != nil {
return response.Error(500, "Failed to get dashboard snapshot", err)
if errors.Is(err, models.ErrDashboardSnapshotNotFound) {
return response.Error(http.StatusNotFound, "Failed to find dashboard snapshot", err)
}
return response.Error(http.StatusInternalServerError, "Failed to get dashboard snapshot", err)
}
snapshot := query.Result
@@ -226,7 +230,10 @@ func (hs *HTTPServer) DeleteDashboardSnapshotByDeleteKey(c *models.ReqContext) r
query := &models.GetDashboardSnapshotQuery{DeleteKey: key}
err := hs.DashboardsnapshotsService.GetDashboardSnapshot(c.Req.Context(), query)
if err != nil {
return response.Error(500, "Failed to get dashboard snapshot", err)
if errors.Is(err, models.ErrDashboardSnapshotNotFound) {
return response.Error(http.StatusNotFound, "Failed to find dashboard snapshot", err)
}
return response.Error(http.StatusInternalServerError, "Failed to get dashboard snapshot", err)
}
if query.Result.External {
@@ -259,7 +266,10 @@ 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)
if errors.Is(err, models.ErrDashboardSnapshotNotFound) {
return response.Error(http.StatusNotFound, "Failed to find dashboard snapshot", err)
}
return response.Error(http.StatusInternalServerError, "Failed to get dashboard snapshot", err)
}
if query.Result == nil {
return response.Error(404, "Failed to get dashboard snapshot", nil)
pkg/api/dashboard_snapshot_test.go
+69
View File
@@ -1,6 +1,7 @@
package api
import (
"errors"
"fmt"
"net/http"
"net/http/httptest"
@@ -233,3 +234,71 @@ func TestDashboardSnapshotAPIEndpoint_singleSnapshot(t *testing.T) {
}, sqlmock)
})
}
func TestGetDashboardSnapshotNotFound(t *testing.T) {
sqlmock := mockstore.NewSQLStoreMock()
sqlmock.ExpectedTeamsByUser = []*models.TeamDTO{}
sqlmock.ExpectedError = models.ErrDashboardSnapshotNotFound
hs := &HTTPServer{DashboardsnapshotsService: &dashboardsnapshots.Service{SQLStore: sqlmock}}
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) {
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) {
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) {
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{}
sqlmock.ExpectedError = errors.New("something went wrong")
hs := &HTTPServer{DashboardsnapshotsService: &dashboardsnapshots.Service{SQLStore: sqlmock}}
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) {
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) {
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) {
sc.handlerFunc = hs.DeleteDashboardSnapshotByDeleteKey
sc.fakeReqWithParams("DELETE", sc.url, map[string]string{"deleteKey": "12345"}).exec()
assert.Equal(t, http.StatusInternalServerError, sc.resp.Code)
}, sqlmock)
}
pkg/api/docs/definitions/snapshot.go
+1
View File
@@ -30,6 +30,7 @@ import (
//
// Responses:
// 200: snapshotResponse
// 400: badRequestError
// 404: notFoundError
// 500: internalServerError
public/api-merged.json
+3
View File
@@ -7897,6 +7897,9 @@
"200": {
"$ref": "#/responses/snapshotResponse"
},
"400": {
"$ref": "#/responses/badRequestError"
},
"404": {
"$ref": "#/responses/notFoundError"
},
public/api-spec.json
+3
View File
@@ -6294,6 +6294,9 @@
"200": {
"$ref": "#/responses/snapshotResponse"
},
"400": {
"$ref": "#/responses/badRequestError"
},
"404": {
"$ref": "#/responses/notFoundError"
},