From a7a964ec19cec8f0848d14eee6109d3656f61701 Mon Sep 17 00:00:00 2001 From: SamuelToh Date: Sun, 27 Jan 2019 21:49:22 +1000 Subject: [PATCH 1/3] Added PATCH verb end point for annotation op Added new PATCH verb annotation endpoint Removed unwanted fmt Added test cases for PATCH verb annotation endpoint Fixed formatting issue Check arr len before proceeding Updated doc to include PATCH verb annotation endpt --- docs/sources/http_api/annotations.md | 27 +++++++++++- pkg/api/annotations.go | 59 ++++++++++++++++++++++++++ pkg/api/annotations_test.go | 62 ++++++++++++++++++++++++++++ pkg/api/api.go | 1 + pkg/api/dtos/annotations.go | 8 ++++ 5 files changed, 156 insertions(+), 1 deletion(-) diff --git a/docs/sources/http_api/annotations.md b/docs/sources/http_api/annotations.md index 6633714d77b..dee4ede0777 100644 --- a/docs/sources/http_api/annotations.md +++ b/docs/sources/http_api/annotations.md @@ -160,15 +160,18 @@ Content-Type: application/json } ``` -## Update Annotation +## Replace Annotation `PUT /api/annotations/:id` +Replaces the annotation that matches the specified id. + **Example Request**: ```json PUT /api/annotations/1141 HTTP/1.1 Accept: application/json +Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk Content-Type: application/json { @@ -180,6 +183,28 @@ Content-Type: application/json } ``` +## Update Annotation + +`PATCH /api/annotations/:id` + +Updates one or more properties of an annotation that matches the specified id. + +**Example Request**: + +```json +PATCH /api/annotations/1145 HTTP/1.1 +Accept: application/json +Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk +Content-Type: application/json + +{ + "time":1507037197000, + "timeEnd":1507180807095, + "text":"New Annotation Description", + "tags":["tag6","tag7","tag8"] +} +``` + ## Delete Annotation By Id `DELETE /api/annotations/:id` diff --git a/pkg/api/annotations.go b/pkg/api/annotations.go index 242b5531f51..da9b55a1c16 100644 --- a/pkg/api/annotations.go +++ b/pkg/api/annotations.go @@ -210,6 +210,65 @@ func UpdateAnnotation(c *m.ReqContext, cmd dtos.UpdateAnnotationsCmd) Response { return Success("Annotation updated") } +func PatchAnnotation(c *m.ReqContext, cmd dtos.PatchAnnotationsCmd) Response { + annotationID := c.ParamsInt64(":annotationId") + + repo := annotations.GetRepository() + + if resp := canSave(c, repo, annotationID); resp != nil { + return resp + } + + items, err := repo.Find(&annotations.ItemQuery{AnnotationId: annotationID, OrgId: c.OrgId}) + + if err != nil || len(items) == 0 { + return Error(500, "Could not find annotation to update", err) + } + + existing := annotations.Item{ + OrgId: c.OrgId, + UserId: c.UserId, + Id: annotationID, + Epoch: items[0].Time, + Text: items[0].Text, + Tags: items[0].Tags, + RegionId: items[0].RegionId, + } + + if cmd.Tags != nil { + existing.Tags = cmd.Tags + } + + if cmd.Text != "" && cmd.Text != existing.Text { + existing.Text = cmd.Text + } + + if cmd.Time > 0 && cmd.Time != existing.Epoch { + existing.Epoch = cmd.Time + } + + if err := repo.Update(&existing); err != nil { + return Error(500, "Failed to update annotation", err) + } + + // Update region end time if provided + if existing.RegionId != 0 && cmd.TimeEnd > 0 { + itemRight := existing + itemRight.RegionId = existing.Id + itemRight.Epoch = cmd.TimeEnd + + // We don't know id of region right event, so set it to 0 and find then using query like + // ... WHERE region_id = AND id != ... + itemRight.Id = 0 + + if err := repo.Update(&itemRight); err != nil { + return Error(500, "Failed to update annotation for region end time", err) + } + } + + return Success("Annotation patched") +} + func DeleteAnnotations(c *m.ReqContext, cmd dtos.DeleteAnnotationsCmd) Response { repo := annotations.GetRepository() diff --git a/pkg/api/annotations_test.go b/pkg/api/annotations_test.go index 08f3018c694..ebdd867a031 100644 --- a/pkg/api/annotations_test.go +++ b/pkg/api/annotations_test.go @@ -27,6 +27,12 @@ func TestAnnotationsApiEndpoint(t *testing.T) { IsRegion: false, } + patchCmd := dtos.PatchAnnotationsCmd{ + Time: 1000, + Text: "annotation text", + Tags: []string{"tag1", "tag2"}, + } + Convey("When user is an Org Viewer", func() { role := m.ROLE_VIEWER Convey("Should not be allowed to save an annotation", func() { @@ -40,6 +46,11 @@ func TestAnnotationsApiEndpoint(t *testing.T) { So(sc.resp.Code, ShouldEqual, 403) }) + patchAnnotationScenario("When calling PATCH on", "/api/annotations/1", "/api/annotations/:annotationId", role, patchCmd, func(sc *scenarioContext) { + sc.fakeReqWithParams("PATCH", sc.url, map[string]string{}).exec() + So(sc.resp.Code, ShouldEqual, 403) + }) + loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/annotations/1", "/api/annotations/:annotationId", role, func(sc *scenarioContext) { sc.handlerFunc = DeleteAnnotationByID sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec() @@ -67,6 +78,11 @@ func TestAnnotationsApiEndpoint(t *testing.T) { So(sc.resp.Code, ShouldEqual, 200) }) + patchAnnotationScenario("When calling PATCH on", "/api/annotations/1", "/api/annotations/:annotationId", role, patchCmd, func(sc *scenarioContext) { + sc.fakeReqWithParams("PATCH", sc.url, map[string]string{}).exec() + So(sc.resp.Code, ShouldEqual, 200) + }) + loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/annotations/1", "/api/annotations/:annotationId", role, func(sc *scenarioContext) { sc.handlerFunc = DeleteAnnotationByID sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec() @@ -100,6 +116,13 @@ func TestAnnotationsApiEndpoint(t *testing.T) { Id: 1, } + patchCmd := dtos.PatchAnnotationsCmd{ + Time: 8000, + Text: "annotation text 50", + Tags: []string{"foo", "bar"}, + Id: 1, + } + deleteCmd := dtos.DeleteAnnotationsCmd{ DashboardId: 1, PanelId: 1, @@ -136,6 +159,11 @@ func TestAnnotationsApiEndpoint(t *testing.T) { So(sc.resp.Code, ShouldEqual, 403) }) + patchAnnotationScenario("When calling PATCH on", "/api/annotations/1", "/api/annotations/:annotationId", role, patchCmd, func(sc *scenarioContext) { + sc.fakeReqWithParams("PATCH", sc.url, map[string]string{}).exec() + So(sc.resp.Code, ShouldEqual, 403) + }) + loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/annotations/1", "/api/annotations/:annotationId", role, func(sc *scenarioContext) { sc.handlerFunc = DeleteAnnotationByID sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec() @@ -163,6 +191,11 @@ func TestAnnotationsApiEndpoint(t *testing.T) { So(sc.resp.Code, ShouldEqual, 200) }) + patchAnnotationScenario("When calling PATCH on", "/api/annotations/1", "/api/annotations/:annotationId", role, patchCmd, func(sc *scenarioContext) { + sc.fakeReqWithParams("PATCH", sc.url, map[string]string{}).exec() + So(sc.resp.Code, ShouldEqual, 200) + }) + loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/annotations/1", "/api/annotations/:annotationId", role, func(sc *scenarioContext) { sc.handlerFunc = DeleteAnnotationByID sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec() @@ -189,6 +222,12 @@ func TestAnnotationsApiEndpoint(t *testing.T) { sc.fakeReqWithParams("PUT", sc.url, map[string]string{}).exec() So(sc.resp.Code, ShouldEqual, 200) }) + + patchAnnotationScenario("When calling PATCH on", "/api/annotations/1", "/api/annotations/:annotationId", role, patchCmd, func(sc *scenarioContext) { + sc.fakeReqWithParams("PATCH", sc.url, map[string]string{}).exec() + So(sc.resp.Code, ShouldEqual, 200) + }) + deleteAnnotationsScenario("When calling POST on", "/api/annotations/mass-delete", "/api/annotations/mass-delete", role, deleteCmd, func(sc *scenarioContext) { sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec() So(sc.resp.Code, ShouldEqual, 200) @@ -264,6 +303,29 @@ func putAnnotationScenario(desc string, url string, routePattern string, role m. }) } +func patchAnnotationScenario(desc string, url string, routePattern string, role m.RoleType, cmd dtos.PatchAnnotationsCmd, fn scenarioFunc) { + Convey(desc+" "+url, func() { + defer bus.ClearBusHandlers() + + sc := setupScenarioContext(url) + sc.defaultHandler = Wrap(func(c *m.ReqContext) Response { + sc.context = c + sc.context.UserId = TestUserID + sc.context.OrgId = TestOrgID + sc.context.OrgRole = role + + return PatchAnnotation(c, cmd) + }) + + fakeAnnoRepo = &fakeAnnotationsRepo{} + annotations.SetRepository(fakeAnnoRepo) + + sc.m.Patch(routePattern, sc.defaultHandler) + + fn(sc) + }) +} + func deleteAnnotationsScenario(desc string, url string, routePattern string, role m.RoleType, cmd dtos.DeleteAnnotationsCmd, fn scenarioFunc) { Convey(desc+" "+url, func() { defer bus.ClearBusHandlers() diff --git a/pkg/api/api.go b/pkg/api/api.go index 980706d8355..0685ef3814d 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -354,6 +354,7 @@ func (hs *HTTPServer) registerRoutes() { annotationsRoute.Post("/", bind(dtos.PostAnnotationsCmd{}), Wrap(PostAnnotation)) annotationsRoute.Delete("/:annotationId", Wrap(DeleteAnnotationByID)) annotationsRoute.Put("/:annotationId", bind(dtos.UpdateAnnotationsCmd{}), Wrap(UpdateAnnotation)) + annotationsRoute.Patch("/:annotationId", bind(dtos.PatchAnnotationsCmd{}), Wrap(PatchAnnotation)) annotationsRoute.Delete("/region/:regionId", Wrap(DeleteAnnotationRegion)) annotationsRoute.Post("/graphite", reqEditorRole, bind(dtos.PostGraphiteAnnotationsCmd{}), Wrap(PostGraphiteAnnotation)) }) diff --git a/pkg/api/dtos/annotations.go b/pkg/api/dtos/annotations.go index c917b0d9feb..b64329e56d1 100644 --- a/pkg/api/dtos/annotations.go +++ b/pkg/api/dtos/annotations.go @@ -22,6 +22,14 @@ type UpdateAnnotationsCmd struct { TimeEnd int64 `json:"timeEnd"` } +type PatchAnnotationsCmd struct { + Id int64 `json:"id"` + Time int64 `json:"time"` + Text string `json:"text"` + Tags []string `json:"tags"` + TimeEnd int64 `json:"timeEnd"` +} + type DeleteAnnotationsCmd struct { AlertId int64 `json:"alertId"` DashboardId int64 `json:"dashboardId"` From 4de9e3598b8bb35b6f700719a6e626e77e36c197 Mon Sep 17 00:00:00 2001 From: SamuelToh Date: Wed, 6 Feb 2019 06:41:39 +1000 Subject: [PATCH 2/3] Address review comments --- docs/sources/http_api/annotations.md | 4 ++-- pkg/api/annotations.go | 2 +- pkg/api/dtos/annotations.go | 10 +++++----- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/sources/http_api/annotations.md b/docs/sources/http_api/annotations.md index dee4ede0777..ca589581f96 100644 --- a/docs/sources/http_api/annotations.md +++ b/docs/sources/http_api/annotations.md @@ -189,6 +189,8 @@ Content-Type: application/json Updates one or more properties of an annotation that matches the specified id. +The `PATCH` operation currently supports updating of the `text`, `tags`, `time` and `timeEnd` properties. It does not handle updating of the `isRegion` and `regionId` properties. To make an annotation regional or vice versa, consider using the `PUT` operation. + **Example Request**: ```json @@ -198,8 +200,6 @@ Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk Content-Type: application/json { - "time":1507037197000, - "timeEnd":1507180807095, "text":"New Annotation Description", "tags":["tag6","tag7","tag8"] } diff --git a/pkg/api/annotations.go b/pkg/api/annotations.go index da9b55a1c16..de9d2517caa 100644 --- a/pkg/api/annotations.go +++ b/pkg/api/annotations.go @@ -222,7 +222,7 @@ func PatchAnnotation(c *m.ReqContext, cmd dtos.PatchAnnotationsCmd) Response { items, err := repo.Find(&annotations.ItemQuery{AnnotationId: annotationID, OrgId: c.OrgId}) if err != nil || len(items) == 0 { - return Error(500, "Could not find annotation to update", err) + return Error(404, "Could not find annotation to update", err) } existing := annotations.Item{ diff --git a/pkg/api/dtos/annotations.go b/pkg/api/dtos/annotations.go index b64329e56d1..bdee8599fea 100644 --- a/pkg/api/dtos/annotations.go +++ b/pkg/api/dtos/annotations.go @@ -23,11 +23,11 @@ type UpdateAnnotationsCmd struct { } type PatchAnnotationsCmd struct { - Id int64 `json:"id"` - Time int64 `json:"time"` - Text string `json:"text"` - Tags []string `json:"tags"` - TimeEnd int64 `json:"timeEnd"` + Id int64 `json:"id"` + Time int64 `json:"time"` + Text string `json:"text"` + Tags []string `json:"tags"` + TimeEnd int64 `json:"timeEnd"` } type DeleteAnnotationsCmd struct { From 6848fe0edf0480eda09efa300f075ac4f560b3ee Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Wed, 6 Feb 2019 14:26:43 +0100 Subject: [PATCH 3/3] docs: update annotaions http api --- docs/sources/http_api/annotations.md | 50 +++++++++++++++++++++------- 1 file changed, 38 insertions(+), 12 deletions(-) diff --git a/docs/sources/http_api/annotations.md b/docs/sources/http_api/annotations.md index ca589581f96..e1d2876f48a 100644 --- a/docs/sources/http_api/annotations.md +++ b/docs/sources/http_api/annotations.md @@ -97,7 +97,7 @@ Creates an annotation in the Grafana database. The `dashboardId` and `panelId` f **Example Request**: -```json +```http POST /api/annotations HTTP/1.1 Accept: application/json Content-Type: application/json @@ -115,7 +115,7 @@ Content-Type: application/json **Example Response**: -```json +```http HTTP/1.1 200 Content-Type: application/json @@ -135,7 +135,7 @@ format (string with multiple tags being separated by a space). **Example Request**: -```json +```http POST /api/annotations/graphite HTTP/1.1 Accept: application/json Content-Type: application/json @@ -150,7 +150,7 @@ Content-Type: application/json **Example Response**: -```json +```http HTTP/1.1 200 Content-Type: application/json @@ -160,15 +160,15 @@ Content-Type: application/json } ``` -## Replace Annotation +## Update Annotation `PUT /api/annotations/:id` -Replaces the annotation that matches the specified id. +Updates all properties of an annotation that matches the specified id. To only update certain property, consider using the [Patch Annotation](#patch-annotation) operation. **Example Request**: -```json +```http PUT /api/annotations/1141 HTTP/1.1 Accept: application/json Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk @@ -183,17 +183,28 @@ Content-Type: application/json } ``` -## Update Annotation +**Example Response**: + +```http +HTTP/1.1 200 +Content-Type: application/json + +{ + "message":"Annotation updated" +} +``` + +## Patch Annotation `PATCH /api/annotations/:id` Updates one or more properties of an annotation that matches the specified id. -The `PATCH` operation currently supports updating of the `text`, `tags`, `time` and `timeEnd` properties. It does not handle updating of the `isRegion` and `regionId` properties. To make an annotation regional or vice versa, consider using the `PUT` operation. +This operation currently supports updating of the `text`, `tags`, `time` and `timeEnd` properties. It does not handle updating of the `isRegion` and `regionId` properties. To make an annotation regional or vice versa, consider using the [Update Annotation](#update-annotation) operation. **Example Request**: -```json +```http PATCH /api/annotations/1145 HTTP/1.1 Accept: application/json Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk @@ -205,6 +216,17 @@ Content-Type: application/json } ``` +**Example Response**: + +```http +HTTP/1.1 200 +Content-Type: application/json + +{ + "message":"Annotation patched" +} +``` + ## Delete Annotation By Id `DELETE /api/annotations/:id` @@ -226,7 +248,9 @@ Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk HTTP/1.1 200 Content-Type: application/json -{"message":"Annotation deleted"} +{ + "message":"Annotation deleted" +} ``` ## Delete Annotation By RegionId @@ -250,5 +274,7 @@ Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk HTTP/1.1 200 Content-Type: application/json -{"message":"Annotation region deleted"} +{ + "message":"Annotation region deleted" +} ```