diff --git a/pkg/api/short_url.go b/pkg/api/short_url.go index 8e7b8cd171a..040818547ae 100644 --- a/pkg/api/short_url.go +++ b/pkg/api/short_url.go @@ -8,6 +8,7 @@ import ( "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/shorturls" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" "github.com/grafana/grafana/pkg/web" @@ -17,7 +18,7 @@ import ( func (hs *HTTPServer) createShortURL(c *models.ReqContext) response.Response { cmd := dtos.CreateShortURLCmd{} if err := web.Bind(c.Req, &cmd); err != nil { - return response.Err(models.ErrShortURLBadRequest.Errorf("bad request data: %w", err)) + return response.Err(shorturls.ErrShortURLBadRequest.Errorf("bad request data: %w", err)) } hs.log.Debug("Received request to create short URL", "path", cmd.Path) shortURL, err := hs.ShortURLService.CreateShortURL(c.Req.Context(), c.SignedInUser, cmd.Path) @@ -45,7 +46,7 @@ func (hs *HTTPServer) redirectFromShortURL(c *models.ReqContext) { shortURL, err := hs.ShortURLService.GetShortURLByUID(c.Req.Context(), c.SignedInUser, shortURLUID) if err != nil { - if models.ErrShortURLNotFound.Is(err) { + if shorturls.ErrShortURLNotFound.Is(err) { hs.log.Debug("Not redirecting short URL since not found") return } diff --git a/pkg/api/short_url_test.go b/pkg/api/short_url_test.go index 9008fe5a540..4a25feec1a6 100644 --- a/pkg/api/short_url_test.go +++ b/pkg/api/short_url_test.go @@ -23,14 +23,14 @@ func TestShortURLAPIEndpoint(t *testing.T) { Path: "d/TxKARsmGz/new-dashboard?orgId=1&from=1599389322894&to=1599410922894", } - createResp := &models.ShortUrl{ + createResp := &shorturls.ShortUrl{ Id: 1, OrgId: testOrgID, Uid: "N1u6L4eGz", Path: cmd.Path, } service := &fakeShortURLService{ - createShortURLFunc: func(ctx context.Context, user *user.SignedInUser, path string) (*models.ShortUrl, error) { + createShortURLFunc: func(ctx context.Context, user *user.SignedInUser, path string) (*shorturls.ShortUrl, error) { return createResp, nil }, } @@ -77,14 +77,14 @@ func createShortURLScenario(t *testing.T, desc string, url string, routePattern } type fakeShortURLService struct { - createShortURLFunc func(ctx context.Context, user *user.SignedInUser, path string) (*models.ShortUrl, error) + createShortURLFunc func(ctx context.Context, user *user.SignedInUser, path string) (*shorturls.ShortUrl, error) } -func (s *fakeShortURLService) GetShortURLByUID(ctx context.Context, user *user.SignedInUser, uid string) (*models.ShortUrl, error) { +func (s *fakeShortURLService) GetShortURLByUID(ctx context.Context, user *user.SignedInUser, uid string) (*shorturls.ShortUrl, error) { return nil, nil } -func (s *fakeShortURLService) CreateShortURL(ctx context.Context, user *user.SignedInUser, path string) (*models.ShortUrl, error) { +func (s *fakeShortURLService) CreateShortURL(ctx context.Context, user *user.SignedInUser, path string) (*shorturls.ShortUrl, error) { if s.createShortURLFunc != nil { return s.createShortURLFunc(ctx, user, path) } @@ -92,10 +92,10 @@ func (s *fakeShortURLService) CreateShortURL(ctx context.Context, user *user.Sig return nil, nil } -func (s *fakeShortURLService) UpdateLastSeenAt(ctx context.Context, shortURL *models.ShortUrl) error { +func (s *fakeShortURLService) UpdateLastSeenAt(ctx context.Context, shortURL *shorturls.ShortUrl) error { return nil } -func (s *fakeShortURLService) DeleteStaleShortURLs(ctx context.Context, cmd *models.DeleteShortUrlCommand) error { +func (s *fakeShortURLService) DeleteStaleShortURLs(ctx context.Context, cmd *shorturls.DeleteShortUrlCommand) error { return nil } diff --git a/pkg/cmd/grafana-cli/runner/wire.go b/pkg/cmd/grafana-cli/runner/wire.go index f4493320527..cfd97956c16 100644 --- a/pkg/cmd/grafana-cli/runner/wire.go +++ b/pkg/cmd/grafana-cli/runner/wire.go @@ -103,6 +103,7 @@ import ( "github.com/grafana/grafana/pkg/services/serviceaccounts" serviceaccountsmanager "github.com/grafana/grafana/pkg/services/serviceaccounts/manager" "github.com/grafana/grafana/pkg/services/shorturls" + "github.com/grafana/grafana/pkg/services/shorturls/shorturlimpl" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/star/starimpl" "github.com/grafana/grafana/pkg/services/store" @@ -184,8 +185,8 @@ var wireSet = wire.NewSet( wire.Bind(new(httpclient.Provider), new(*sdkhttpclient.Provider)), serverlock.ProvideService, cleanup.ProvideService, - shorturls.ProvideService, - wire.Bind(new(shorturls.Service), new(*shorturls.ShortURLService)), + shorturlimpl.ProvideService, + wire.Bind(new(shorturls.Service), new(*shorturlimpl.ShortURLService)), queryhistory.ProvideService, wire.Bind(new(queryhistory.Service), new(*queryhistory.QueryHistoryService)), quotaimpl.ProvideService, diff --git a/pkg/server/wire.go b/pkg/server/wire.go index 7944477350d..c4f266566dc 100644 --- a/pkg/server/wire.go +++ b/pkg/server/wire.go @@ -114,6 +114,7 @@ import ( serviceaccountsmanager "github.com/grafana/grafana/pkg/services/serviceaccounts/manager" serviceaccountsretriever "github.com/grafana/grafana/pkg/services/serviceaccounts/retriever" "github.com/grafana/grafana/pkg/services/shorturls" + "github.com/grafana/grafana/pkg/services/shorturls/shorturlimpl" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/sqlstore/mockstore" "github.com/grafana/grafana/pkg/services/star/starimpl" @@ -204,8 +205,8 @@ var wireBasicSet = wire.NewSet( annotationsimpl.ProvideCleanupService, wire.Bind(new(annotations.Cleaner), new(*annotationsimpl.CleanupServiceImpl)), cleanup.ProvideService, - shorturls.ProvideService, - wire.Bind(new(shorturls.Service), new(*shorturls.ShortURLService)), + shorturlimpl.ProvideService, + wire.Bind(new(shorturls.Service), new(*shorturlimpl.ShortURLService)), queryhistory.ProvideService, wire.Bind(new(queryhistory.Service), new(*queryhistory.QueryHistoryService)), correlations.ProvideService, diff --git a/pkg/services/cleanup/cleanup.go b/pkg/services/cleanup/cleanup.go index 8d6840a90f2..ecfe5fc0e4d 100644 --- a/pkg/services/cleanup/cleanup.go +++ b/pkg/services/cleanup/cleanup.go @@ -16,7 +16,6 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/serverlock" "github.com/grafana/grafana/pkg/infra/tracing" - "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/annotations" "github.com/grafana/grafana/pkg/services/dashboardsnapshots" dashver "github.com/grafana/grafana/pkg/services/dashboardversion" @@ -240,7 +239,7 @@ func (srv *CleanUpService) expireOldUserInvites(ctx context.Context) { func (srv *CleanUpService) deleteStaleShortURLs(ctx context.Context) { logger := srv.log.FromContext(ctx) - cmd := models.DeleteShortUrlCommand{ + cmd := shorturls.DeleteShortUrlCommand{ OlderThan: time.Now().Add(-time.Hour * 24 * 7), } if err := srv.ShortURLService.DeleteStaleShortURLs(ctx, &cmd); err != nil { diff --git a/pkg/models/shorturl.go b/pkg/services/shorturls/models.go similarity index 98% rename from pkg/models/shorturl.go rename to pkg/services/shorturls/models.go index 1d008bb5b35..c6cb840c40c 100644 --- a/pkg/models/shorturl.go +++ b/pkg/services/shorturls/models.go @@ -1,4 +1,4 @@ -package models +package shorturls import ( "time" diff --git a/pkg/services/shorturls/short_url_service.go b/pkg/services/shorturls/short_url_service.go deleted file mode 100644 index c1051803aa9..00000000000 --- a/pkg/services/shorturls/short_url_service.go +++ /dev/null @@ -1,109 +0,0 @@ -package shorturls - -import ( - "context" - "path" - "strings" - "time" - - "github.com/grafana/grafana/pkg/infra/db" - "github.com/grafana/grafana/pkg/models" - "github.com/grafana/grafana/pkg/services/user" - "github.com/grafana/grafana/pkg/util" -) - -var getTime = time.Now - -func ProvideService(sqlStore db.DB) *ShortURLService { - return &ShortURLService{ - SQLStore: sqlStore, - } -} - -type Service interface { - GetShortURLByUID(ctx context.Context, user *user.SignedInUser, uid string) (*models.ShortUrl, error) - CreateShortURL(ctx context.Context, user *user.SignedInUser, path string) (*models.ShortUrl, error) - UpdateLastSeenAt(ctx context.Context, shortURL *models.ShortUrl) error - DeleteStaleShortURLs(ctx context.Context, cmd *models.DeleteShortUrlCommand) error -} - -type ShortURLService struct { - SQLStore db.DB -} - -func (s ShortURLService) GetShortURLByUID(ctx context.Context, user *user.SignedInUser, uid string) (*models.ShortUrl, error) { - var shortURL models.ShortUrl - err := s.SQLStore.WithDbSession(ctx, func(dbSession *db.Session) error { - exists, err := dbSession.Where("org_id=? AND uid=?", user.OrgID, uid).Get(&shortURL) - if err != nil { - return err - } - if !exists { - return models.ErrShortURLNotFound.Errorf("short URL not found") - } - - return nil - }) - if err != nil { - return nil, err - } - - return &shortURL, nil -} - -func (s ShortURLService) UpdateLastSeenAt(ctx context.Context, shortURL *models.ShortUrl) error { - shortURL.LastSeenAt = getTime().Unix() - return s.SQLStore.WithTransactionalDbSession(ctx, func(dbSession *db.Session) error { - _, err := dbSession.ID(shortURL.Id).Update(shortURL) - if err != nil { - return err - } - - return nil - }) -} - -func (s ShortURLService) CreateShortURL(ctx context.Context, user *user.SignedInUser, relPath string) (*models.ShortUrl, error) { - relPath = strings.TrimSpace(relPath) - - if path.IsAbs(relPath) { - return nil, models.ErrShortURLAbsolutePath.Errorf("expected relative path: %s", relPath) - } - if strings.Contains(relPath, "../") { - return nil, models.ErrShortURLInvalidPath.Errorf("path cannot contain '../': %s", relPath) - } - - now := time.Now().Unix() - shortURL := models.ShortUrl{ - OrgId: user.OrgID, - Uid: util.GenerateShortUID(), - Path: relPath, - CreatedBy: user.UserID, - CreatedAt: now, - } - - err := s.SQLStore.WithDbSession(ctx, func(session *db.Session) error { - _, err := session.Insert(&shortURL) - return err - }) - if err != nil { - return nil, models.ErrShortURLInternal.Errorf("failed to insert shorturl: %w", err) - } - - return &shortURL, nil -} - -func (s ShortURLService) DeleteStaleShortURLs(ctx context.Context, cmd *models.DeleteShortUrlCommand) error { - return s.SQLStore.WithTransactionalDbSession(ctx, func(session *db.Session) error { - var rawSql = "DELETE FROM short_url WHERE created_at <= ? AND (last_seen_at IS NULL OR last_seen_at = 0)" - - if result, err := session.Exec(rawSql, cmd.OlderThan.Unix()); err != nil { - return err - } else if cmd.NumDeleted, err = result.RowsAffected(); err != nil { - return err - } - return nil - }) -} - -var _ Service = &ShortURLService{} diff --git a/pkg/services/shorturls/shorturl.go b/pkg/services/shorturls/shorturl.go new file mode 100644 index 00000000000..5472b8e6e57 --- /dev/null +++ b/pkg/services/shorturls/shorturl.go @@ -0,0 +1,14 @@ +package shorturls + +import ( + "context" + + "github.com/grafana/grafana/pkg/services/user" +) + +type Service interface { + GetShortURLByUID(ctx context.Context, user *user.SignedInUser, uid string) (*ShortUrl, error) + CreateShortURL(ctx context.Context, user *user.SignedInUser, path string) (*ShortUrl, error) + UpdateLastSeenAt(ctx context.Context, shortURL *ShortUrl) error + DeleteStaleShortURLs(ctx context.Context, cmd *DeleteShortUrlCommand) error +} diff --git a/pkg/services/shorturls/shorturlimpl/shorturl.go b/pkg/services/shorturls/shorturlimpl/shorturl.go new file mode 100644 index 00000000000..95e295233c4 --- /dev/null +++ b/pkg/services/shorturls/shorturlimpl/shorturl.go @@ -0,0 +1,65 @@ +package shorturlimpl + +import ( + "context" + "path" + "strings" + "time" + + "github.com/grafana/grafana/pkg/infra/db" + "github.com/grafana/grafana/pkg/services/shorturls" + "github.com/grafana/grafana/pkg/services/user" + "github.com/grafana/grafana/pkg/util" +) + +var getTime = time.Now + +type ShortURLService struct { + SQLStore store +} + +func ProvideService(db db.DB) *ShortURLService { + return &ShortURLService{ + SQLStore: &sqlStore{ + db: db, + }, + } +} + +func (s ShortURLService) GetShortURLByUID(ctx context.Context, user *user.SignedInUser, uid string) (*shorturls.ShortUrl, error) { + return s.SQLStore.Get(ctx, user, uid) +} + +func (s ShortURLService) UpdateLastSeenAt(ctx context.Context, shortURL *shorturls.ShortUrl) error { + return s.SQLStore.Update(ctx, shortURL) +} + +func (s ShortURLService) CreateShortURL(ctx context.Context, user *user.SignedInUser, relPath string) (*shorturls.ShortUrl, error) { + relPath = strings.TrimSpace(relPath) + + if path.IsAbs(relPath) { + return nil, shorturls.ErrShortURLAbsolutePath.Errorf("expected relative path: %s", relPath) + } + if strings.Contains(relPath, "../") { + return nil, shorturls.ErrShortURLInvalidPath.Errorf("path cannot contain '../': %s", relPath) + } + + now := time.Now().Unix() + shortURL := shorturls.ShortUrl{ + OrgId: user.OrgID, + Uid: util.GenerateShortUID(), + Path: relPath, + CreatedBy: user.UserID, + CreatedAt: now, + } + + if err := s.SQLStore.Insert(ctx, &shortURL); err != nil { + return nil, shorturls.ErrShortURLInternal.Errorf("failed to insert shorturl: %w", err) + } + + return &shortURL, nil +} + +func (s ShortURLService) DeleteStaleShortURLs(ctx context.Context, cmd *shorturls.DeleteShortUrlCommand) error { + return s.SQLStore.Delete(ctx, cmd) +} diff --git a/pkg/services/shorturls/short_url_service_test.go b/pkg/services/shorturls/shorturlimpl/shorturl_test.go similarity index 83% rename from pkg/services/shorturls/short_url_service_test.go rename to pkg/services/shorturls/shorturlimpl/shorturl_test.go index bb3588c9bda..37913f1833e 100644 --- a/pkg/services/shorturls/short_url_service_test.go +++ b/pkg/services/shorturls/shorturlimpl/shorturl_test.go @@ -1,4 +1,4 @@ -package shorturls +package shorturlimpl import ( "context" @@ -8,18 +8,18 @@ import ( "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/infra/db" - "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/shorturls" "github.com/grafana/grafana/pkg/services/user" ) func TestShortURLService(t *testing.T) { user := &user.SignedInUser{UserID: 1} - sqlStore := db.InitTestDB(t) + store := db.InitTestDB(t) t.Run("User can create and read short URLs", func(t *testing.T) { const refPath = "mock/path?test=true" - service := ShortURLService{SQLStore: sqlStore} + service := ShortURLService{SQLStore: &sqlStore{db: store}} newShortURL, err := service.CreateShortURL(context.Background(), user, refPath) require.NoError(t, err) @@ -57,7 +57,7 @@ func TestShortURLService(t *testing.T) { require.NotEmpty(t, staleShortURL.Uid) require.Equal(t, int64(0), staleShortURL.LastSeenAt) - cmd := models.DeleteShortUrlCommand{OlderThan: time.Unix(staleShortURL.CreatedAt, 0)} + cmd := shorturls.DeleteShortUrlCommand{OlderThan: time.Unix(staleShortURL.CreatedAt, 0)} err = service.DeleteStaleShortURLs(context.Background(), &cmd) require.NoError(t, err) require.Equal(t, int64(1), cmd.NumDeleted) @@ -69,7 +69,7 @@ func TestShortURLService(t *testing.T) { }) t.Run("and no action when no stale short urls exist", func(t *testing.T) { - cmd := models.DeleteShortUrlCommand{OlderThan: time.Unix(existingShortURL.CreatedAt, 0)} + cmd := shorturls.DeleteShortUrlCommand{OlderThan: time.Unix(existingShortURL.CreatedAt, 0)} require.NoError(t, err) require.Equal(t, int64(0), cmd.NumDeleted) }) @@ -77,11 +77,11 @@ func TestShortURLService(t *testing.T) { }) t.Run("User cannot look up nonexistent short URLs", func(t *testing.T) { - service := ShortURLService{SQLStore: sqlStore} + service := ShortURLService{SQLStore: &sqlStore{db: store}} shortURL, err := service.GetShortURLByUID(context.Background(), user, "testnotfounduid") require.Error(t, err) - require.True(t, models.ErrShortURLNotFound.Is(err)) + require.True(t, shorturls.ErrShortURLNotFound.Is(err)) require.Nil(t, shortURL) }) } diff --git a/pkg/services/shorturls/shorturlimpl/store.go b/pkg/services/shorturls/shorturlimpl/store.go new file mode 100644 index 00000000000..31600394512 --- /dev/null +++ b/pkg/services/shorturls/shorturlimpl/store.go @@ -0,0 +1,71 @@ +package shorturlimpl + +import ( + "context" + + "github.com/grafana/grafana/pkg/infra/db" + "github.com/grafana/grafana/pkg/services/shorturls" + "github.com/grafana/grafana/pkg/services/user" +) + +type store interface { + Get(ctx context.Context, user *user.SignedInUser, uid string) (*shorturls.ShortUrl, error) + Update(ctx context.Context, shortURL *shorturls.ShortUrl) error + Insert(ctx context.Context, shortURL *shorturls.ShortUrl) error + Delete(ctx context.Context, cmd *shorturls.DeleteShortUrlCommand) error +} + +type sqlStore struct { + db db.DB +} + +func (s sqlStore) Get(ctx context.Context, user *user.SignedInUser, uid string) (*shorturls.ShortUrl, error) { + var shortURL shorturls.ShortUrl + err := s.db.WithDbSession(ctx, func(dbSession *db.Session) error { + exists, err := dbSession.Where("org_id=? AND uid=?", user.OrgID, uid).Get(&shortURL) + if err != nil { + return err + } + if !exists { + return shorturls.ErrShortURLNotFound.Errorf("short URL not found") + } + + return nil + }) + if err != nil { + return nil, err + } + + return &shortURL, nil +} + +func (s sqlStore) Update(ctx context.Context, shortURL *shorturls.ShortUrl) error { + shortURL.LastSeenAt = getTime().Unix() + return s.db.WithTransactionalDbSession(ctx, func(dbSession *db.Session) error { + _, err := dbSession.ID(shortURL.Id).Update(shortURL) + if err != nil { + return err + } + return nil + }) +} + +func (s sqlStore) Insert(ctx context.Context, shortURL *shorturls.ShortUrl) error { + return s.db.WithDbSession(ctx, func(session *db.Session) error { + _, err := session.Insert(shortURL) + return err + }) +} + +func (s sqlStore) Delete(ctx context.Context, cmd *shorturls.DeleteShortUrlCommand) error { + return s.db.WithTransactionalDbSession(ctx, func(session *db.Session) error { + var rawSql = "DELETE FROM short_url WHERE created_at <= ? AND (last_seen_at IS NULL OR last_seen_at = 0)" + + if result, err := session.Exec(rawSql, cmd.OlderThan.Unix()); err != nil { + return err + } else if cmd.NumDeleted, err = result.RowsAffected(); err != nil { + return err + } + return nil + }) +}