From 540d540ea93cbc63d38121c01c5dbbd285a78434 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Thu, 16 Nov 2017 14:24:56 +0100 Subject: [PATCH 1/3] fix: return id from api when creating new annotation/graphite annotation, fixes #9798 When creating a region annotation the response will include both id (region start id) and endId (region end id), if not only id. --- docs/sources/http_api/annotations.md | 13 ++++-- pkg/api/annotations.go | 50 ++++++++++++++++++------ pkg/api/api.go | 4 +- pkg/services/sqlstore/annotation_test.go | 6 ++- 4 files changed, 53 insertions(+), 20 deletions(-) diff --git a/docs/sources/http_api/annotations.md b/docs/sources/http_api/annotations.md index 7aab127cb0c..19c2a5c386c 100644 --- a/docs/sources/http_api/annotations.md +++ b/docs/sources/http_api/annotations.md @@ -89,7 +89,7 @@ Content-Type: application/json ## Create Annotation -Creates an annotation in the Grafana database. The `dashboardId` and `panelId` fields are optional. If they are not specified then a global annotation is created and can be queried in any dashboard that adds the Grafana annotations data source. +Creates an annotation in the Grafana database. The `dashboardId` and `panelId` fields are optional. If they are not specified then a global annotation is created and can be queried in any dashboard that adds the Grafana annotations data source. When creating a region annotation the response will include both `id` and `endId`, if not only `id`. `POST /api/annotations` @@ -117,7 +117,11 @@ Content-Type: application/json HTTP/1.1 200 Content-Type: application/json -{"message":"Annotation added"} +{ + "message":"Annotation added", + "id": 1, + "endId": 2 +} ``` ## Create Annotation in Graphite format @@ -148,7 +152,10 @@ Content-Type: application/json HTTP/1.1 200 Content-Type: application/json -{"message":"Graphite annotation added"} +{ + "message":"Graphite annotation added", + "id": 1 +} ``` ## Update Annotation diff --git a/pkg/api/annotations.go b/pkg/api/annotations.go index e6454e9cf86..db1a2f668ae 100644 --- a/pkg/api/annotations.go +++ b/pkg/api/annotations.go @@ -8,6 +8,7 @@ import ( "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/middleware" "github.com/grafana/grafana/pkg/services/annotations" + "github.com/grafana/grafana/pkg/util" ) func GetAnnotations(c *middleware.Context) Response { @@ -48,12 +49,13 @@ func (e *CreateAnnotationError) Error() string { return e.message } -func PostAnnotation(c *middleware.Context, cmd dtos.PostAnnotationsCmd) Response { +func PostAnnotation(c *middleware.Context, cmd dtos.PostAnnotationsCmd) { repo := annotations.GetRepository() if cmd.Text == "" { err := &CreateAnnotationError{"text field should not be empty"} - return ApiError(500, "Failed to save annotation", err) + c.JsonApiErr(500, "Failed to save annotation", err) + return } item := annotations.Item{ @@ -72,30 +74,45 @@ func PostAnnotation(c *middleware.Context, cmd dtos.PostAnnotationsCmd) Response } if err := repo.Save(&item); err != nil { - return ApiError(500, "Failed to save annotation", err) + c.JsonApiErr(500, "Failed to save annotation", err) + return } + startID := item.Id + // handle regions if cmd.IsRegion { - item.RegionId = item.Id + item.RegionId = startID if item.Data == nil { item.Data = simplejson.New() } if err := repo.Update(&item); err != nil { - return ApiError(500, "Failed set regionId on annotation", err) + c.JsonApiErr(500, "Failed set regionId on annotation", err) + return } item.Id = 0 item.Epoch = cmd.TimeEnd / 1000 if err := repo.Save(&item); err != nil { - return ApiError(500, "Failed save annotation for region end time", err) + c.JsonApiErr(500, "Failed save annotation for region end time", err) + return } + + c.JSON(200, util.DynMap{ + "message": "Annotation added", + "id": startID, + "endId": item.Id, + }) + return } - return ApiSuccess("Annotation added") + c.JSON(200, util.DynMap{ + "message": "Annotation added", + "id": startID, + }) } func formatGraphiteAnnotation(what string, data string) string { @@ -106,12 +123,13 @@ func formatGraphiteAnnotation(what string, data string) string { return text } -func PostGraphiteAnnotation(c *middleware.Context, cmd dtos.PostGraphiteAnnotationsCmd) Response { +func PostGraphiteAnnotation(c *middleware.Context, cmd dtos.PostGraphiteAnnotationsCmd) { repo := annotations.GetRepository() if cmd.What == "" { err := &CreateAnnotationError{"what field should not be empty"} - return ApiError(500, "Failed to save Graphite annotation", err) + c.JsonApiErr(500, "Failed to save Graphite annotation", err) + return } if cmd.When == 0 { @@ -134,12 +152,14 @@ func PostGraphiteAnnotation(c *middleware.Context, cmd dtos.PostGraphiteAnnotati tagsArray = append(tagsArray, tagStr) } else { err := &CreateAnnotationError{"tag should be a string"} - return ApiError(500, "Failed to save Graphite annotation", err) + c.JsonApiErr(500, "Failed to save Graphite annotation", err) + return } } default: err := &CreateAnnotationError{"unsupported tags format"} - return ApiError(500, "Failed to save Graphite annotation", err) + c.JsonApiErr(500, "Failed to save Graphite annotation", err) + return } item := annotations.Item{ @@ -151,10 +171,14 @@ func PostGraphiteAnnotation(c *middleware.Context, cmd dtos.PostGraphiteAnnotati } if err := repo.Save(&item); err != nil { - return ApiError(500, "Failed to save Graphite annotation", err) + c.JsonApiErr(500, "Failed to save Graphite annotation", err) + return } - return ApiSuccess("Graphite annotation added") + c.JSON(200, util.DynMap{ + "message": "Graphite annotation added", + "id": item.Id, + }) } func UpdateAnnotation(c *middleware.Context, cmd dtos.UpdateAnnotationsCmd) Response { diff --git a/pkg/api/api.go b/pkg/api/api.go index ba5e7ae4e10..75ad23fbb21 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -288,11 +288,11 @@ func (hs *HttpServer) registerRoutes() { apiRoute.Post("/annotations/mass-delete", reqOrgAdmin, bind(dtos.DeleteAnnotationsCmd{}), wrap(DeleteAnnotations)) apiRoute.Group("/annotations", func(annotationsRoute RouteRegister) { - annotationsRoute.Post("/", bind(dtos.PostAnnotationsCmd{}), wrap(PostAnnotation)) + annotationsRoute.Post("/", bind(dtos.PostAnnotationsCmd{}), PostAnnotation) annotationsRoute.Delete("/:annotationId", wrap(DeleteAnnotationById)) annotationsRoute.Put("/:annotationId", bind(dtos.UpdateAnnotationsCmd{}), wrap(UpdateAnnotation)) annotationsRoute.Delete("/region/:regionId", wrap(DeleteAnnotationRegion)) - annotationsRoute.Post("/graphite", bind(dtos.PostGraphiteAnnotationsCmd{}), wrap(PostGraphiteAnnotation)) + annotationsRoute.Post("/graphite", bind(dtos.PostGraphiteAnnotationsCmd{}), PostGraphiteAnnotation) }, reqEditorRole) // error test diff --git a/pkg/services/sqlstore/annotation_test.go b/pkg/services/sqlstore/annotation_test.go index 3f7415a952b..e1902b63fa8 100644 --- a/pkg/services/sqlstore/annotation_test.go +++ b/pkg/services/sqlstore/annotation_test.go @@ -37,16 +37,18 @@ func TestAnnotations(t *testing.T) { repo := SqlAnnotationRepo{} Convey("Can save annotation", func() { - err := repo.Save(&annotations.Item{ + annotation := &annotations.Item{ OrgId: 1, UserId: 1, DashboardId: 1, Text: "hello", Epoch: 10, Tags: []string{"outage", "error", "type:outage", "server:server-1"}, - }) + } + err := repo.Save(annotation) So(err, ShouldBeNil) + So(annotation.Id, ShouldBeGreaterThan, 0) Convey("Can query for annotation", func() { items, err := repo.Find(&annotations.ItemQuery{ From 89b27b35ba894aea7084f6663fad98b2b0f58805 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Thu, 16 Nov 2017 16:18:03 +0100 Subject: [PATCH 2/3] fix: Use Response as return type --- pkg/api/annotations.go | 35 +++++++++++++---------------------- pkg/api/api.go | 4 ++-- 2 files changed, 15 insertions(+), 24 deletions(-) diff --git a/pkg/api/annotations.go b/pkg/api/annotations.go index db1a2f668ae..32a0a3035d3 100644 --- a/pkg/api/annotations.go +++ b/pkg/api/annotations.go @@ -49,13 +49,12 @@ func (e *CreateAnnotationError) Error() string { return e.message } -func PostAnnotation(c *middleware.Context, cmd dtos.PostAnnotationsCmd) { +func PostAnnotation(c *middleware.Context, cmd dtos.PostAnnotationsCmd) Response { repo := annotations.GetRepository() if cmd.Text == "" { err := &CreateAnnotationError{"text field should not be empty"} - c.JsonApiErr(500, "Failed to save annotation", err) - return + return ApiError(500, "Failed to save annotation", err) } item := annotations.Item{ @@ -74,8 +73,7 @@ func PostAnnotation(c *middleware.Context, cmd dtos.PostAnnotationsCmd) { } if err := repo.Save(&item); err != nil { - c.JsonApiErr(500, "Failed to save annotation", err) - return + return ApiError(500, "Failed to save annotation", err) } startID := item.Id @@ -89,27 +87,24 @@ func PostAnnotation(c *middleware.Context, cmd dtos.PostAnnotationsCmd) { } if err := repo.Update(&item); err != nil { - c.JsonApiErr(500, "Failed set regionId on annotation", err) - return + return ApiError(500, "Failed set regionId on annotation", err) } item.Id = 0 item.Epoch = cmd.TimeEnd / 1000 if err := repo.Save(&item); err != nil { - c.JsonApiErr(500, "Failed save annotation for region end time", err) - return + return ApiError(500, "Failed save annotation for region end time", err) } - c.JSON(200, util.DynMap{ + return Json(200, util.DynMap{ "message": "Annotation added", "id": startID, "endId": item.Id, }) - return } - c.JSON(200, util.DynMap{ + return Json(200, util.DynMap{ "message": "Annotation added", "id": startID, }) @@ -123,13 +118,12 @@ func formatGraphiteAnnotation(what string, data string) string { return text } -func PostGraphiteAnnotation(c *middleware.Context, cmd dtos.PostGraphiteAnnotationsCmd) { +func PostGraphiteAnnotation(c *middleware.Context, cmd dtos.PostGraphiteAnnotationsCmd) Response { repo := annotations.GetRepository() if cmd.What == "" { err := &CreateAnnotationError{"what field should not be empty"} - c.JsonApiErr(500, "Failed to save Graphite annotation", err) - return + return ApiError(500, "Failed to save Graphite annotation", err) } if cmd.When == 0 { @@ -152,14 +146,12 @@ func PostGraphiteAnnotation(c *middleware.Context, cmd dtos.PostGraphiteAnnotati tagsArray = append(tagsArray, tagStr) } else { err := &CreateAnnotationError{"tag should be a string"} - c.JsonApiErr(500, "Failed to save Graphite annotation", err) - return + return ApiError(500, "Failed to save Graphite annotation", err) } } default: err := &CreateAnnotationError{"unsupported tags format"} - c.JsonApiErr(500, "Failed to save Graphite annotation", err) - return + return ApiError(500, "Failed to save Graphite annotation", err) } item := annotations.Item{ @@ -171,11 +163,10 @@ func PostGraphiteAnnotation(c *middleware.Context, cmd dtos.PostGraphiteAnnotati } if err := repo.Save(&item); err != nil { - c.JsonApiErr(500, "Failed to save Graphite annotation", err) - return + return ApiError(500, "Failed to save Graphite annotation", err) } - c.JSON(200, util.DynMap{ + return Json(200, util.DynMap{ "message": "Graphite annotation added", "id": item.Id, }) diff --git a/pkg/api/api.go b/pkg/api/api.go index 75ad23fbb21..ba5e7ae4e10 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -288,11 +288,11 @@ func (hs *HttpServer) registerRoutes() { apiRoute.Post("/annotations/mass-delete", reqOrgAdmin, bind(dtos.DeleteAnnotationsCmd{}), wrap(DeleteAnnotations)) apiRoute.Group("/annotations", func(annotationsRoute RouteRegister) { - annotationsRoute.Post("/", bind(dtos.PostAnnotationsCmd{}), PostAnnotation) + annotationsRoute.Post("/", bind(dtos.PostAnnotationsCmd{}), wrap(PostAnnotation)) annotationsRoute.Delete("/:annotationId", wrap(DeleteAnnotationById)) annotationsRoute.Put("/:annotationId", bind(dtos.UpdateAnnotationsCmd{}), wrap(UpdateAnnotation)) annotationsRoute.Delete("/region/:regionId", wrap(DeleteAnnotationRegion)) - annotationsRoute.Post("/graphite", bind(dtos.PostGraphiteAnnotationsCmd{}), PostGraphiteAnnotation) + annotationsRoute.Post("/graphite", bind(dtos.PostGraphiteAnnotationsCmd{}), wrap(PostGraphiteAnnotation)) }, reqEditorRole) // error test From 909601d6abce1dd3d427181d922cc6fcf3a4bc2c Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Thu, 16 Nov 2017 16:29:05 +0100 Subject: [PATCH 3/3] api: fix so that datasources functions returns Response --- pkg/api/api.go | 6 +++--- pkg/api/datasources.go | 42 ++++++++++++++++-------------------------- 2 files changed, 19 insertions(+), 29 deletions(-) diff --git a/pkg/api/api.go b/pkg/api/api.go index ba5e7ae4e10..88d8a925826 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -199,10 +199,10 @@ func (hs *HttpServer) registerRoutes() { // Data sources apiRoute.Group("/datasources", func(datasourceRoute RouteRegister) { datasourceRoute.Get("/", wrap(GetDataSources)) - datasourceRoute.Post("/", quota("data_source"), bind(m.AddDataSourceCommand{}), AddDataSource) + datasourceRoute.Post("/", quota("data_source"), bind(m.AddDataSourceCommand{}), wrap(AddDataSource)) datasourceRoute.Put("/:id", bind(m.UpdateDataSourceCommand{}), wrap(UpdateDataSource)) - datasourceRoute.Delete("/:id", DeleteDataSourceById) - datasourceRoute.Delete("/name/:name", DeleteDataSourceByName) + datasourceRoute.Delete("/:id", wrap(DeleteDataSourceById)) + datasourceRoute.Delete("/name/:name", wrap(DeleteDataSourceByName)) datasourceRoute.Get("/:id", wrap(GetDataSourceById)) datasourceRoute.Get("/name/:name", wrap(GetDataSourceByName)) }, reqOrgAdmin) diff --git a/pkg/api/datasources.go b/pkg/api/datasources.go index 27b91bc2802..b5c5f9cb834 100644 --- a/pkg/api/datasources.go +++ b/pkg/api/datasources.go @@ -69,80 +69,70 @@ func GetDataSourceById(c *middleware.Context) Response { return Json(200, &dtos) } -func DeleteDataSourceById(c *middleware.Context) { +func DeleteDataSourceById(c *middleware.Context) Response { id := c.ParamsInt64(":id") if id <= 0 { - c.JsonApiErr(400, "Missing valid datasource id", nil) - return + return ApiError(400, "Missing valid datasource id", nil) } ds, err := getRawDataSourceById(id, c.OrgId) if err != nil { - c.JsonApiErr(400, "Failed to delete datasource", nil) - return + return ApiError(400, "Failed to delete datasource", nil) } if ds.ReadOnly { - c.JsonApiErr(403, "Cannot delete read-only data source", nil) - return + return ApiError(403, "Cannot delete read-only data source", nil) } cmd := &m.DeleteDataSourceByIdCommand{Id: id, OrgId: c.OrgId} err = bus.Dispatch(cmd) if err != nil { - c.JsonApiErr(500, "Failed to delete datasource", err) - return + return ApiError(500, "Failed to delete datasource", err) } - c.JsonOK("Data source deleted") + return ApiSuccess("Data source deleted") } -func DeleteDataSourceByName(c *middleware.Context) { +func DeleteDataSourceByName(c *middleware.Context) Response { name := c.Params(":name") if name == "" { - c.JsonApiErr(400, "Missing valid datasource name", nil) - return + return ApiError(400, "Missing valid datasource name", nil) } getCmd := &m.GetDataSourceByNameQuery{Name: name, OrgId: c.OrgId} if err := bus.Dispatch(getCmd); err != nil { - c.JsonApiErr(500, "Failed to delete datasource", err) - return + return ApiError(500, "Failed to delete datasource", err) } if getCmd.Result.ReadOnly { - c.JsonApiErr(403, "Cannot delete read-only data source", nil) - return + return ApiError(403, "Cannot delete read-only data source", nil) } cmd := &m.DeleteDataSourceByNameCommand{Name: name, OrgId: c.OrgId} err := bus.Dispatch(cmd) if err != nil { - c.JsonApiErr(500, "Failed to delete datasource", err) - return + return ApiError(500, "Failed to delete datasource", err) } - c.JsonOK("Data source deleted") + return ApiSuccess("Data source deleted") } -func AddDataSource(c *middleware.Context, cmd m.AddDataSourceCommand) { +func AddDataSource(c *middleware.Context, cmd m.AddDataSourceCommand) Response { cmd.OrgId = c.OrgId if err := bus.Dispatch(&cmd); err != nil { if err == m.ErrDataSourceNameExists { - c.JsonApiErr(409, err.Error(), err) - return + return ApiError(409, err.Error(), err) } - c.JsonApiErr(500, "Failed to add datasource", err) - return + return ApiError(500, "Failed to add datasource", err) } ds := convertModelToDtos(cmd.Result) - c.JSON(200, util.DynMap{ + return Json(200, util.DynMap{ "message": "Datasource added", "id": cmd.Result.Id, "name": cmd.Result.Name,