ShortURL: App platform migration support for dual write (#109221)
This commit is contained in:
@@ -12,19 +12,21 @@ var (
|
||||
ErrShortURLAbsolutePath = errutil.ValidationFailed("shorturl.absolute-path", errutil.WithPublicMessage("Path should be relative"))
|
||||
ErrShortURLInvalidPath = errutil.ValidationFailed("shorturl.invalid-path", errutil.WithPublicMessage("Invalid short URL path"))
|
||||
ErrShortURLInternal = errutil.Internal("shorturl.internal")
|
||||
ErrShortURLConflict = errutil.Conflict("shorturl.conflict")
|
||||
)
|
||||
|
||||
type ShortUrl struct {
|
||||
Id int64
|
||||
OrgId int64
|
||||
Uid string
|
||||
Path string
|
||||
CreatedBy int64
|
||||
CreatedAt int64
|
||||
LastSeenAt int64
|
||||
Id int64 `json:"-"`
|
||||
OrgId int64 `json:"-"`
|
||||
Uid string `json:"uid"`
|
||||
Path string `json:"path"`
|
||||
CreatedBy int64 `json:"-"`
|
||||
CreatedAt int64 `json:"-"`
|
||||
LastSeenAt int64 `json:"lastSeenAt"`
|
||||
}
|
||||
|
||||
type DeleteShortUrlCommand struct {
|
||||
Uid string
|
||||
OlderThan time.Time
|
||||
|
||||
NumDeleted int64
|
||||
|
||||
@@ -3,12 +3,14 @@ package shorturls
|
||||
import (
|
||||
"context"
|
||||
|
||||
"github.com/grafana/grafana/pkg/api/dtos"
|
||||
"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)
|
||||
CreateShortURL(ctx context.Context, user *user.SignedInUser, cmd *dtos.CreateShortURLCmd) (*ShortUrl, error)
|
||||
UpdateLastSeenAt(ctx context.Context, shortURL *ShortUrl) error
|
||||
DeleteStaleShortURLs(ctx context.Context, cmd *DeleteShortUrlCommand) error
|
||||
ConvertShortURLToDTO(shortURL *ShortUrl, appURL string) *dtos.ShortURL
|
||||
}
|
||||
|
||||
@@ -2,13 +2,16 @@ package shorturlimpl
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"path"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/grafana/grafana/pkg/api/dtos"
|
||||
"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"
|
||||
"github.com/teris-io/shortid"
|
||||
)
|
||||
|
||||
@@ -34,8 +37,8 @@ func (s ShortURLService) UpdateLastSeenAt(ctx context.Context, shortURL *shortur
|
||||
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)
|
||||
func (s ShortURLService) CreateShortURL(ctx context.Context, user *user.SignedInUser, cmd *dtos.CreateShortURLCmd) (*shorturls.ShortUrl, error) {
|
||||
relPath := strings.TrimSpace(cmd.Path)
|
||||
|
||||
if path.IsAbs(relPath) {
|
||||
return nil, shorturls.ErrShortURLAbsolutePath.Errorf("expected relative path: %s", relPath)
|
||||
@@ -44,9 +47,30 @@ func (s ShortURLService) CreateShortURL(ctx context.Context, user *user.SignedIn
|
||||
return nil, shorturls.ErrShortURLInvalidPath.Errorf("path cannot contain '../': %s", relPath)
|
||||
}
|
||||
|
||||
uid, err := shortid.Generate()
|
||||
if err != nil {
|
||||
return nil, shorturls.ErrShortURLInternal.Errorf("failed to generate uid: %w", err)
|
||||
uid := cmd.UID
|
||||
if uid == "" {
|
||||
var err error
|
||||
uid, err = shortid.Generate()
|
||||
if err != nil {
|
||||
return nil, shorturls.ErrShortURLInternal.Errorf("failed to generate uid: %w", err)
|
||||
}
|
||||
} else {
|
||||
// Ensure the UID is valid
|
||||
if !util.IsValidShortUID(uid) {
|
||||
return nil, shorturls.ErrShortURLBadRequest.Errorf("invalid UID: %s", uid)
|
||||
}
|
||||
|
||||
// Check if the UID already exists
|
||||
existingShortURL, err := s.SQLStore.Get(ctx, user, uid)
|
||||
if err != nil {
|
||||
if !shorturls.ErrShortURLNotFound.Is(err) {
|
||||
return nil, shorturls.ErrShortURLInternal.Errorf("failed to check existing short URL: %w", err)
|
||||
}
|
||||
}
|
||||
if existingShortURL != nil {
|
||||
// If the UID already exists, we return an error
|
||||
return nil, shorturls.ErrShortURLConflict.Errorf("short URL with UID '%s' already exists", uid)
|
||||
}
|
||||
}
|
||||
|
||||
now := time.Now().Unix()
|
||||
@@ -68,3 +92,12 @@ func (s ShortURLService) CreateShortURL(ctx context.Context, user *user.SignedIn
|
||||
func (s ShortURLService) DeleteStaleShortURLs(ctx context.Context, cmd *shorturls.DeleteShortUrlCommand) error {
|
||||
return s.SQLStore.Delete(ctx, cmd)
|
||||
}
|
||||
|
||||
func (s ShortURLService) ConvertShortURLToDTO(shortURL *shorturls.ShortUrl, appURL string) *dtos.ShortURL {
|
||||
url := fmt.Sprintf("%s/goto/%s?orgId=%d", strings.TrimSuffix(appURL, "/"), shortURL.Uid, shortURL.OrgId)
|
||||
|
||||
return &dtos.ShortURL{
|
||||
UID: shortURL.Uid,
|
||||
URL: url,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -7,6 +7,7 @@ import (
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/grafana/grafana/pkg/api/dtos"
|
||||
"github.com/grafana/grafana/pkg/infra/db"
|
||||
"github.com/grafana/grafana/pkg/services/shorturls"
|
||||
"github.com/grafana/grafana/pkg/services/user"
|
||||
@@ -25,11 +26,13 @@ func TestIntegrationShortURLService(t *testing.T) {
|
||||
store := db.InitTestDB(t)
|
||||
|
||||
t.Run("User can create and read short URLs", func(t *testing.T) {
|
||||
const refPath = "mock/path?test=true"
|
||||
cmd := &dtos.CreateShortURLCmd{
|
||||
Path: "mock/path?test=true",
|
||||
}
|
||||
|
||||
service := ShortURLService{SQLStore: &sqlStore{db: store}}
|
||||
|
||||
newShortURL, err := service.CreateShortURL(context.Background(), user, refPath)
|
||||
newShortURL, err := service.CreateShortURL(context.Background(), user, cmd)
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, newShortURL)
|
||||
require.NotEmpty(t, newShortURL.Uid)
|
||||
@@ -37,7 +40,7 @@ func TestIntegrationShortURLService(t *testing.T) {
|
||||
existingShortURL, err := service.GetShortURLByUID(context.Background(), user, newShortURL.Uid)
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, existingShortURL)
|
||||
require.Equal(t, refPath, existingShortURL.Path)
|
||||
require.Equal(t, cmd.Path, existingShortURL.Path)
|
||||
|
||||
t.Run("and update last seen at", func(t *testing.T) {
|
||||
origGetTime := getTime
|
||||
@@ -59,7 +62,7 @@ func TestIntegrationShortURLService(t *testing.T) {
|
||||
})
|
||||
|
||||
t.Run("and stale short urls can be deleted", func(t *testing.T) {
|
||||
staleShortURL, err := service.CreateShortURL(context.Background(), user, refPath)
|
||||
staleShortURL, err := service.CreateShortURL(context.Background(), user, cmd)
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, staleShortURL)
|
||||
require.NotEmpty(t, staleShortURL.Uid)
|
||||
@@ -100,18 +103,25 @@ func TestIntegrationShortURLService(t *testing.T) {
|
||||
|
||||
ctx := context.Background()
|
||||
|
||||
absolutePath := "/path?test=true"
|
||||
newShortURL, err := service.CreateShortURL(ctx, user, absolutePath)
|
||||
cmd := &dtos.CreateShortURLCmd{
|
||||
Path: "/path?test=true",
|
||||
}
|
||||
|
||||
newShortURL, err := service.CreateShortURL(ctx, user, cmd)
|
||||
require.ErrorIs(t, err, shorturls.ErrShortURLAbsolutePath)
|
||||
require.Nil(t, newShortURL)
|
||||
|
||||
relativePath := "path/../test?test=true"
|
||||
newShortURL, err = service.CreateShortURL(ctx, user, relativePath)
|
||||
cmd2 := &dtos.CreateShortURLCmd{
|
||||
Path: "path/../test?test=true",
|
||||
}
|
||||
newShortURL, err = service.CreateShortURL(ctx, user, cmd2)
|
||||
require.ErrorIs(t, err, shorturls.ErrShortURLInvalidPath)
|
||||
require.Nil(t, newShortURL)
|
||||
|
||||
relativePath = "../path/test?test=true"
|
||||
newShortURL, err = service.CreateShortURL(ctx, user, relativePath)
|
||||
cmd3 := &dtos.CreateShortURLCmd{
|
||||
Path: "../path/test?test=true",
|
||||
}
|
||||
newShortURL, err = service.CreateShortURL(ctx, user, cmd3)
|
||||
require.ErrorIs(t, err, shorturls.ErrShortURLInvalidPath)
|
||||
require.Nil(t, newShortURL)
|
||||
})
|
||||
@@ -121,14 +131,15 @@ func TestIntegrationShortURLService(t *testing.T) {
|
||||
|
||||
ctx := context.Background()
|
||||
|
||||
const refPath = "mock/path?test=true"
|
||||
|
||||
newShortURL1, err := service.CreateShortURL(ctx, user, refPath)
|
||||
cmd := &dtos.CreateShortURLCmd{
|
||||
Path: "mock/path?test=true",
|
||||
}
|
||||
newShortURL1, err := service.CreateShortURL(ctx, user, cmd)
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, newShortURL1)
|
||||
require.NotEmpty(t, newShortURL1.Uid)
|
||||
|
||||
newShortURL2, err := service.CreateShortURL(ctx, user, refPath)
|
||||
newShortURL2, err := service.CreateShortURL(ctx, user, cmd)
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, newShortURL2)
|
||||
require.NotEmpty(t, newShortURL2.Uid)
|
||||
@@ -136,4 +147,38 @@ func TestIntegrationShortURLService(t *testing.T) {
|
||||
require.NotEqual(t, newShortURL1.Uid, newShortURL2.Uid)
|
||||
require.Equal(t, newShortURL1.Path, newShortURL2.Path)
|
||||
})
|
||||
|
||||
t.Run("Create URL providing the UID", func(t *testing.T) {
|
||||
service := ShortURLService{SQLStore: &sqlStore{db: store}}
|
||||
|
||||
ctx := context.Background()
|
||||
|
||||
cmd := &dtos.CreateShortURLCmd{
|
||||
Path: "mock/path?test=true",
|
||||
UID: "custom-uid",
|
||||
}
|
||||
newShortURL1, err := service.CreateShortURL(ctx, user, cmd)
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, newShortURL1)
|
||||
require.Equal(t, cmd.UID, newShortURL1.Uid)
|
||||
})
|
||||
|
||||
t.Run("Create URL providing an existing UID should fail", func(t *testing.T) {
|
||||
service := ShortURLService{SQLStore: &sqlStore{db: store}}
|
||||
|
||||
ctx := context.Background()
|
||||
|
||||
cmd := &dtos.CreateShortURLCmd{
|
||||
Path: "mock/path?test=true",
|
||||
UID: "custom-uid-2",
|
||||
}
|
||||
newShortURL1, err := service.CreateShortURL(ctx, user, cmd)
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, newShortURL1)
|
||||
require.Equal(t, cmd.UID, newShortURL1.Uid)
|
||||
|
||||
newShortURL2, err := service.CreateShortURL(ctx, user, cmd)
|
||||
require.ErrorIs(t, err, shorturls.ErrShortURLConflict)
|
||||
require.Nil(t, newShortURL2)
|
||||
})
|
||||
}
|
||||
|
||||
@@ -58,6 +58,21 @@ func (s sqlStore) Insert(ctx context.Context, shortURL *shorturls.ShortUrl) erro
|
||||
}
|
||||
|
||||
func (s sqlStore) Delete(ctx context.Context, cmd *shorturls.DeleteShortUrlCommand) error {
|
||||
// If a UID is provided, delete that specific short URL
|
||||
if cmd.Uid != "" {
|
||||
return s.db.WithTransactionalDbSession(ctx, func(session *db.Session) error {
|
||||
var rawSql = "DELETE FROM short_url WHERE uid = ?"
|
||||
|
||||
if result, err := session.Exec(rawSql, cmd.Uid); err != nil {
|
||||
return err
|
||||
} else if cmd.NumDeleted, err = result.RowsAffected(); err != nil {
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
})
|
||||
}
|
||||
|
||||
// Otherwise, delete all stale short URLs older than the specified time
|
||||
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)"
|
||||
|
||||
|
||||
Reference in New Issue
Block a user