remove global variable in annotation (#46746)

* remove global varaible in annotation

* remove todo

* replace intransaction with withdbtransaction

* fix typo
This commit is contained in:
ying-jeanne 2022-03-22 12:20:57 +01:00 committed by GitHub
parent 3d43535bc3
commit adc0cbf176
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 157 additions and 152 deletions

View File

@ -35,7 +35,7 @@ func (hs *HTTPServer) GetAnnotations(c *models.ReqContext) response.Response {
repo := annotations.GetRepository()
items, err := repo.Find(query)
items, err := repo.Find(c.Req.Context(), query)
if err != nil {
return response.Error(500, "Failed to get annotations", err)
}
@ -192,7 +192,7 @@ func (hs *HTTPServer) UpdateAnnotation(c *models.ReqContext) response.Response {
repo := annotations.GetRepository()
annotation, resp := findAnnotationByID(repo, annotationID, c.OrgId)
annotation, resp := findAnnotationByID(c.Req.Context(), repo, annotationID, c.OrgId)
if resp != nil {
return resp
}
@ -220,7 +220,7 @@ func (hs *HTTPServer) UpdateAnnotation(c *models.ReqContext) response.Response {
Tags: cmd.Tags,
}
if err := repo.Update(&item); err != nil {
if err := repo.Update(c.Req.Context(), &item); err != nil {
return response.Error(500, "Failed to update annotation", err)
}
@ -239,7 +239,7 @@ func (hs *HTTPServer) PatchAnnotation(c *models.ReqContext) response.Response {
repo := annotations.GetRepository()
annotation, resp := findAnnotationByID(repo, annotationID, c.OrgId)
annotation, resp := findAnnotationByID(c.Req.Context(), repo, annotationID, c.OrgId)
if resp != nil {
return resp
}
@ -282,7 +282,7 @@ func (hs *HTTPServer) PatchAnnotation(c *models.ReqContext) response.Response {
existing.EpochEnd = cmd.TimeEnd
}
if err := repo.Update(&existing); err != nil {
if err := repo.Update(c.Req.Context(), &existing); err != nil {
return response.Error(500, "Failed to update annotation", err)
}
@ -318,7 +318,7 @@ func (hs *HTTPServer) DeleteAnnotationByID(c *models.ReqContext) response.Respon
repo := annotations.GetRepository()
annotation, resp := findAnnotationByID(repo, annotationID, c.OrgId)
annotation, resp := findAnnotationByID(c.Req.Context(), repo, annotationID, c.OrgId)
if resp != nil {
return resp
}
@ -360,8 +360,8 @@ func canSaveOrganizationAnnotation(c *models.ReqContext) bool {
return c.SignedInUser.HasRole(models.ROLE_EDITOR)
}
func findAnnotationByID(repo annotations.Repository, annotationID int64, orgID int64) (*annotations.ItemDTO, response.Response) {
items, err := repo.Find(&annotations.ItemQuery{AnnotationId: annotationID, OrgId: orgID})
func findAnnotationByID(ctx context.Context, repo annotations.Repository, annotationID int64, orgID int64) (*annotations.ItemDTO, response.Response) {
items, err := repo.Find(ctx, &annotations.ItemQuery{AnnotationId: annotationID, OrgId: orgID})
if err != nil {
return nil, response.Error(500, "Failed to find annotation", err)
@ -406,7 +406,7 @@ func AnnotationTypeScopeResolver() (string, accesscontrol.AttributeScopeResolveF
return "", accesscontrol.ErrInvalidScope
}
annotation, resp := findAnnotationByID(annotations.GetRepository(), int64(annotationId), orgID)
annotation, resp := findAnnotationByID(ctx, annotations.GetRepository(), int64(annotationId), orgID)
if resp != nil {
return "", err
}

View File

@ -248,10 +248,10 @@ func (repo *fakeAnnotationsRepo) Save(item *annotations.Item) error {
item.Id = 1
return nil
}
func (repo *fakeAnnotationsRepo) Update(item *annotations.Item) error {
func (repo *fakeAnnotationsRepo) Update(_ context.Context, item *annotations.Item) error {
return nil
}
func (repo *fakeAnnotationsRepo) Find(query *annotations.ItemQuery) ([]*annotations.ItemDTO, error) {
func (repo *fakeAnnotationsRepo) Find(_ context.Context, query *annotations.ItemQuery) ([]*annotations.ItemDTO, error) {
if annotation, has := repo.annotations[query.AnnotationId]; has {
return []*annotations.ItemDTO{&annotation}, nil
}

View File

@ -14,8 +14,8 @@ var (
type Repository interface {
Save(item *Item) error
Update(item *Item) error
Find(query *ItemQuery) ([]*ItemDTO, error)
Update(ctx context.Context, item *Item) error
Find(ctx context.Context, query *ItemQuery) ([]*ItemDTO, error)
Delete(params *DeleteParams) error
FindTags(query *TagsQuery) (FindTagsResult, error)
}

View File

@ -62,7 +62,7 @@ func (c *PermissionChecker) CheckReadPermissions(ctx context.Context, orgId int6
if err != nil {
return false, nil
}
items, err := repo.Find(&annotations.ItemQuery{AnnotationId: annotationID, OrgId: orgId})
items, err := repo.Find(ctx, &annotations.ItemQuery{AnnotationId: annotationID, OrgId: orgId})
if err != nil || len(items) != 1 {
return false, nil
}
@ -109,7 +109,7 @@ func (c *PermissionChecker) CheckWritePermissions(ctx context.Context, orgId int
if err != nil {
return false, nil
}
items, err := repo.Find(&annotations.ItemQuery{AnnotationId: annotationID, OrgId: orgId})
items, err := repo.Find(ctx, &annotations.ItemQuery{AnnotationId: annotationID, OrgId: orgId})
if err != nil || len(items) != 1 {
return false, nil
}

View File

@ -505,11 +505,11 @@ func (repo *FakeAnnotationsRepo) Save(item *annotations.Item) error {
return nil
}
func (repo *FakeAnnotationsRepo) Update(item *annotations.Item) error {
func (repo *FakeAnnotationsRepo) Update(_ context.Context, item *annotations.Item) error {
return nil
}
func (repo *FakeAnnotationsRepo) Find(query *annotations.ItemQuery) ([]*annotations.ItemDTO, error) {
func (repo *FakeAnnotationsRepo) Find(_ context.Context, query *annotations.ItemQuery) ([]*annotations.ItemDTO, error) {
annotations := []*annotations.ItemDTO{{Id: 1}}
return annotations, nil
}

View File

@ -2,6 +2,7 @@ package sqlstore
import (
"bytes"
"context"
"errors"
"fmt"
"strings"
@ -29,6 +30,7 @@ func validateTimeRange(item *annotations.Item) error {
}
type SQLAnnotationRepo struct {
sql *SQLStore
}
func (r *SQLAnnotationRepo) Save(item *annotations.Item) error {
@ -64,8 +66,8 @@ func (r *SQLAnnotationRepo) Save(item *annotations.Item) error {
})
}
func (r *SQLAnnotationRepo) Update(item *annotations.Item) error {
return inTransaction(func(sess *DBSession) error {
func (r *SQLAnnotationRepo) Update(ctx context.Context, item *annotations.Item) error {
return r.sql.WithTransactionalDbSession(ctx, func(sess *DBSession) error {
var (
isExist bool
err error
@ -117,10 +119,11 @@ func (r *SQLAnnotationRepo) Update(item *annotations.Item) error {
})
}
func (r *SQLAnnotationRepo) Find(query *annotations.ItemQuery) ([]*annotations.ItemDTO, error) {
func (r *SQLAnnotationRepo) Find(ctx context.Context, query *annotations.ItemQuery) ([]*annotations.ItemDTO, error) {
var sql bytes.Buffer
params := make([]interface{}, 0)
items := make([]*annotations.ItemDTO, 0)
err := r.sql.WithDbSession(ctx, func(sess *DBSession) error {
sql.WriteString(`
SELECT
annotation.id,
@ -225,13 +228,15 @@ func (r *SQLAnnotationRepo) Find(query *annotations.ItemQuery) ([]*annotations.I
// order of ORDER BY arguments match the order of a sql index for performance
sql.WriteString(" ORDER BY a.org_id, a.epoch_end DESC, a.epoch DESC" + dialect.Limit(query.Limit) + " ) dt on dt.id = annotation.id")
items := make([]*annotations.ItemDTO, 0)
if err := x.SQL(sql.String(), params...).Find(&items); err != nil {
return nil, err
if err := sess.SQL(sql.String(), params...).Find(&items); err != nil {
items = nil
return err
}
return nil
},
)
return items, nil
return items, err
}
func (r *SQLAnnotationRepo) Delete(params *annotations.DeleteParams) error {

View File

@ -4,10 +4,10 @@
package sqlstore
import (
"testing"
"context"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"testing"
"github.com/grafana/grafana/pkg/services/annotations"
)
@ -15,8 +15,9 @@ import (
func TestAnnotations(t *testing.T) {
mockTimeNow()
defer resetTimeNow()
InitTestDB(t)
repo := SQLAnnotationRepo{}
repo.sql = InitTestDB(t)
t.Run("Testing annotation create, read, update and delete", func(t *testing.T) {
t.Cleanup(func() {
@ -79,9 +80,8 @@ func TestAnnotations(t *testing.T) {
err = repo.Save(globalAnnotation2)
require.NoError(t, err)
assert.Greater(t, globalAnnotation2.Id, int64(0))
t.Run("Can query for annotation by dashboard id", func(t *testing.T) {
items, err := repo.Find(&annotations.ItemQuery{
items, err := repo.Find(context.Background(), &annotations.ItemQuery{
OrgId: 1,
DashboardId: 1,
From: 0,
@ -99,7 +99,7 @@ func TestAnnotations(t *testing.T) {
})
t.Run("Can query for annotation by id", func(t *testing.T) {
items, err := repo.Find(&annotations.ItemQuery{
items, err := repo.Find(context.Background(), &annotations.ItemQuery{
OrgId: 1,
AnnotationId: annotation2.Id,
})
@ -109,7 +109,7 @@ func TestAnnotations(t *testing.T) {
})
t.Run("Should not find any when item is outside time range", func(t *testing.T) {
items, err := repo.Find(&annotations.ItemQuery{
items, err := repo.Find(context.Background(), &annotations.ItemQuery{
OrgId: 1,
DashboardId: 1,
From: 12,
@ -120,7 +120,7 @@ func TestAnnotations(t *testing.T) {
})
t.Run("Should not find one when tag filter does not match", func(t *testing.T) {
items, err := repo.Find(&annotations.ItemQuery{
items, err := repo.Find(context.Background(), &annotations.ItemQuery{
OrgId: 1,
DashboardId: 1,
From: 1,
@ -132,7 +132,7 @@ func TestAnnotations(t *testing.T) {
})
t.Run("Should not find one when type filter does not match", func(t *testing.T) {
items, err := repo.Find(&annotations.ItemQuery{
items, err := repo.Find(context.Background(), &annotations.ItemQuery{
OrgId: 1,
DashboardId: 1,
From: 1,
@ -144,7 +144,7 @@ func TestAnnotations(t *testing.T) {
})
t.Run("Should find one when all tag filters does match", func(t *testing.T) {
items, err := repo.Find(&annotations.ItemQuery{
items, err := repo.Find(context.Background(), &annotations.ItemQuery{
OrgId: 1,
DashboardId: 1,
From: 1,
@ -156,7 +156,7 @@ func TestAnnotations(t *testing.T) {
})
t.Run("Should find two annotations using partial match", func(t *testing.T) {
items, err := repo.Find(&annotations.ItemQuery{
items, err := repo.Find(context.Background(), &annotations.ItemQuery{
OrgId: 1,
From: 1,
To: 25,
@ -168,7 +168,7 @@ func TestAnnotations(t *testing.T) {
})
t.Run("Should find one when all key value tag filters does match", func(t *testing.T) {
items, err := repo.Find(&annotations.ItemQuery{
items, err := repo.Find(context.Background(), &annotations.ItemQuery{
OrgId: 1,
DashboardId: 1,
From: 1,
@ -186,11 +186,11 @@ func TestAnnotations(t *testing.T) {
From: 0,
To: 15,
}
items, err := repo.Find(query)
items, err := repo.Find(context.Background(), query)
require.NoError(t, err)
annotationId := items[0].Id
err = repo.Update(&annotations.Item{
err = repo.Update(context.Background(), &annotations.Item{
Id: annotationId,
OrgId: 1,
Text: "something new",
@ -198,7 +198,7 @@ func TestAnnotations(t *testing.T) {
})
require.NoError(t, err)
items, err = repo.Find(query)
items, err = repo.Find(context.Background(), query)
require.NoError(t, err)
assert.Equal(t, annotationId, items[0].Id)
@ -213,11 +213,11 @@ func TestAnnotations(t *testing.T) {
From: 0,
To: 15,
}
items, err := repo.Find(query)
items, err := repo.Find(context.Background(), query)
require.NoError(t, err)
annotationId := items[0].Id
err = repo.Update(&annotations.Item{
err = repo.Update(context.Background(), &annotations.Item{
Id: annotationId,
OrgId: 1,
Text: "something new",
@ -225,7 +225,7 @@ func TestAnnotations(t *testing.T) {
})
require.NoError(t, err)
items, err = repo.Find(query)
items, err = repo.Find(context.Background(), query)
require.NoError(t, err)
assert.Equal(t, annotationId, items[0].Id)
@ -241,14 +241,14 @@ func TestAnnotations(t *testing.T) {
From: 0,
To: 15,
}
items, err := repo.Find(query)
items, err := repo.Find(context.Background(), query)
require.NoError(t, err)
annotationId := items[0].Id
err = repo.Delete(&annotations.DeleteParams{Id: annotationId, OrgId: 1})
require.NoError(t, err)
items, err = repo.Find(query)
items, err = repo.Find(context.Background(), query)
require.NoError(t, err)
assert.Empty(t, items)
})
@ -270,7 +270,7 @@ func TestAnnotations(t *testing.T) {
OrgId: 1,
AnnotationId: annotation3.Id,
}
items, err := repo.Find(query)
items, err := repo.Find(context.Background(), query)
require.NoError(t, err)
dashboardId := items[0].DashboardId
@ -278,7 +278,7 @@ func TestAnnotations(t *testing.T) {
err = repo.Delete(&annotations.DeleteParams{DashboardId: dashboardId, PanelId: panelId, OrgId: 1})
require.NoError(t, err)
items, err = repo.Find(query)
items, err = repo.Find(context.Background(), query)
require.NoError(t, err)
assert.Empty(t, items)
})

View File

@ -109,7 +109,7 @@ func newSQLStore(cfg *setting.Cfg, cacheService *localcache.CacheService, b bus.
dialect = ss.Dialect
// Init repo instances
annotations.SetRepository(&SQLAnnotationRepo{})
annotations.SetRepository(&SQLAnnotationRepo{sql: ss})
annotations.SetAnnotationCleaner(&AnnotationCleanupService{batchSize: ss.Cfg.AnnotationCleanupJobBatchSize, log: log.New("annotationcleaner")})
ss.Bus.SetTransactionManager(ss)