LibraryElements: Enables creating library elements with specific UID (#39019)

* LibraryPanels: Enables create/update library panels with specific UID

* Chore: added check for uid length after PR comments

* Refactor: creates IsShortUIDTooLong function

* Refactor: adds UID to PATCH endpoint

* Refactor: clarifies the patch code

* Refactor: changes  after PR comments
This commit is contained in:
Hugo Häggmark 2021-09-10 11:22:13 +02:00 committed by GitHub
parent 580b03c952
commit fc73bc1161
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 224 additions and 6 deletions

View File

@ -103,7 +103,7 @@ func (dr *dashboardServiceImpl) buildSaveDashboardCommand(dto *SaveDashboardDTO,
if !util.IsValidShortUID(dash.Uid) {
return nil, models.ErrDashboardInvalidUid
} else if len(dash.Uid) > 40 {
} else if util.IsShortUIDTooLong(dash.Uid) {
return nil, models.ErrDashboardUidTooLong
}

View File

@ -125,5 +125,11 @@ func toLibraryElementError(err error, message string) response.Response {
if errors.Is(err, errLibraryElementHasConnections) {
return response.Error(403, errLibraryElementHasConnections.Error(), err)
}
if errors.Is(err, errLibraryElementInvalidUID) {
return response.Error(400, errLibraryElementInvalidUID.Error(), err)
}
if errors.Is(err, errLibraryElementUIDTooLong) {
return response.Error(400, errLibraryElementUIDTooLong.Error(), err)
}
return response.Error(500, message, err)
}

View File

@ -2,6 +2,7 @@ package libraryelements
import (
"encoding/json"
"errors"
"fmt"
"strings"
"time"
@ -92,10 +93,20 @@ func (l *LibraryElementService) createLibraryElement(c *models.ReqContext, cmd C
if err := l.requireSupportedElementKind(cmd.Kind); err != nil {
return LibraryElementDTO{}, err
}
createUID := cmd.UID
if len(createUID) == 0 {
createUID = util.GenerateShortUID()
} else {
if !util.IsValidShortUID(createUID) {
return LibraryElementDTO{}, errLibraryElementInvalidUID
} else if util.IsShortUIDTooLong(createUID) {
return LibraryElementDTO{}, errLibraryElementUIDTooLong
}
}
element := LibraryElement{
OrgID: c.SignedInUser.OrgId,
FolderID: cmd.FolderID,
UID: util.GenerateShortUID(),
UID: createUID,
Name: cmd.Name,
Model: cmd.Model,
Version: 1,
@ -434,12 +445,27 @@ func (l *LibraryElementService) patchLibraryElement(c *models.ReqContext, cmd pa
if elementInDB.Version != cmd.Version {
return errLibraryElementVersionMismatch
}
updateUID := cmd.UID
if len(updateUID) == 0 {
updateUID = uid
} else if updateUID != uid {
if !util.IsValidShortUID(updateUID) {
return errLibraryElementInvalidUID
} else if util.IsShortUIDTooLong(updateUID) {
return errLibraryElementUIDTooLong
}
_, err := getLibraryElement(l.SQLStore.Dialect, session, updateUID, c.SignedInUser.OrgId)
if !errors.Is(err, errLibraryElementNotFound) {
return errLibraryElementAlreadyExists
}
}
var libraryElement = LibraryElement{
ID: elementInDB.ID,
OrgID: c.SignedInUser.OrgId,
FolderID: cmd.FolderID,
UID: uid,
UID: updateUID,
Name: cmd.Name,
Kind: elementInDB.Kind,
Type: elementInDB.Type,

View File

@ -7,6 +7,7 @@ import (
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/util"
)
func TestCreateLibraryElement(t *testing.T) {
@ -59,6 +60,76 @@ func TestCreateLibraryElement(t *testing.T) {
}
})
testScenario(t, "When an admin tries to create a library panel that does not exists using an nonexistent UID, it should succeed",
func(t *testing.T, sc scenarioContext) {
command := getCreatePanelCommand(sc.folder.Id, "Nonexistent UID")
command.UID = util.GenerateShortUID()
resp := sc.service.createHandler(sc.reqContext, command)
var result = validateAndUnMarshalResponse(t, resp)
var expected = libraryElementResult{
Result: libraryElement{
ID: 1,
OrgID: 1,
FolderID: 1,
UID: command.UID,
Name: "Nonexistent UID",
Kind: int64(models.PanelElement),
Type: "text",
Description: "A description",
Model: map[string]interface{}{
"datasource": "${DS_GDEV-TESTDATA}",
"description": "A description",
"id": float64(1),
"title": "Text - Library Panel",
"type": "text",
},
Version: 1,
Meta: LibraryElementDTOMeta{
ConnectedDashboards: 0,
Created: result.Result.Meta.Created,
Updated: result.Result.Meta.Updated,
CreatedBy: LibraryElementDTOMetaUser{
ID: 1,
Name: "signed_in_user",
AvatarURL: "/avatar/37524e1eb8b3e32850b57db0a19af93b",
},
UpdatedBy: LibraryElementDTOMetaUser{
ID: 1,
Name: "signed_in_user",
AvatarURL: "/avatar/37524e1eb8b3e32850b57db0a19af93b",
},
},
},
}
if diff := cmp.Diff(expected, result, getCompareOptions()...); diff != "" {
t.Fatalf("Result mismatch (-want +got):\n%s", diff)
}
})
scenarioWithPanel(t, "When an admin tries to create a library panel that does not exists using an existent UID, it should fail",
func(t *testing.T, sc scenarioContext) {
command := getCreatePanelCommand(sc.folder.Id, "Existing UID")
command.UID = sc.initialResult.Result.UID
resp := sc.service.createHandler(sc.reqContext, command)
require.Equal(t, 400, resp.Status())
})
scenarioWithPanel(t, "When an admin tries to create a library panel that does not exists using an invalid UID, it should fail",
func(t *testing.T, sc scenarioContext) {
command := getCreatePanelCommand(sc.folder.Id, "Invalid UID")
command.UID = "Testing an invalid UID"
resp := sc.service.createHandler(sc.reqContext, command)
require.Equal(t, 400, resp.Status())
})
scenarioWithPanel(t, "When an admin tries to create a library panel that does not exists using an UID that is too long, it should fail",
func(t *testing.T, sc scenarioContext) {
command := getCreatePanelCommand(sc.folder.Id, "Invalid UID")
command.UID = "j6T00KRZzj6T00KRZzj6T00KRZzj6T00KRZzj6T00K"
resp := sc.service.createHandler(sc.reqContext, command)
require.Equal(t, 400, resp.Status())
})
testScenario(t, "When an admin tries to create a library panel where name and panel title differ, it should not update panel title",
func(t *testing.T, sc scenarioContext) {
command := getCreatePanelCommand(1, "Library Panel Name")

View File

@ -3,6 +3,8 @@ package libraryelements
import (
"testing"
"github.com/grafana/grafana/pkg/util"
"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/require"
@ -122,6 +124,70 @@ func TestPatchLibraryElement(t *testing.T) {
}
})
scenarioWithPanel(t, "When an admin tries to patch a library panel with a nonexistent UID, it should change UID successfully and return correct result",
func(t *testing.T, sc scenarioContext) {
cmd := patchLibraryElementCommand{
FolderID: -1,
UID: util.GenerateShortUID(),
Kind: int64(models.PanelElement),
Version: 1,
}
sc.reqContext.ReplaceAllParams(map[string]string{":uid": sc.initialResult.Result.UID})
resp := sc.service.patchHandler(sc.reqContext, cmd)
var result = validateAndUnMarshalResponse(t, resp)
sc.initialResult.Result.UID = cmd.UID
sc.initialResult.Result.Meta.CreatedBy.Name = userInDbName
sc.initialResult.Result.Meta.CreatedBy.AvatarURL = userInDbAvatar
sc.initialResult.Result.Model["title"] = "Text - Library Panel"
sc.initialResult.Result.Version = 2
if diff := cmp.Diff(sc.initialResult.Result, result.Result, getCompareOptions()...); diff != "" {
t.Fatalf("Result mismatch (-want +got):\n%s", diff)
}
})
scenarioWithPanel(t, "When an admin tries to patch a library panel with an invalid UID, it should fail",
func(t *testing.T, sc scenarioContext) {
cmd := patchLibraryElementCommand{
FolderID: -1,
UID: "Testing an invalid UID",
Kind: int64(models.PanelElement),
Version: 1,
}
sc.reqContext.ReplaceAllParams(map[string]string{":uid": sc.initialResult.Result.UID})
resp := sc.service.patchHandler(sc.reqContext, cmd)
require.Equal(t, 400, resp.Status())
})
scenarioWithPanel(t, "When an admin tries to patch a library panel with an UID that is too long, it should fail",
func(t *testing.T, sc scenarioContext) {
cmd := patchLibraryElementCommand{
FolderID: -1,
UID: "j6T00KRZzj6T00KRZzj6T00KRZzj6T00KRZzj6T00K",
Kind: int64(models.PanelElement),
Version: 1,
}
sc.reqContext.ReplaceAllParams(map[string]string{":uid": sc.initialResult.Result.UID})
resp := sc.service.patchHandler(sc.reqContext, cmd)
require.Equal(t, 400, resp.Status())
})
scenarioWithPanel(t, "When an admin tries to patch a library panel with an existing UID, it should fail",
func(t *testing.T, sc scenarioContext) {
command := getCreatePanelCommand(sc.folder.Id, "Existing UID")
command.UID = util.GenerateShortUID()
resp := sc.service.createHandler(sc.reqContext, command)
require.Equal(t, 200, resp.Status())
cmd := patchLibraryElementCommand{
FolderID: -1,
UID: command.UID,
Kind: int64(models.PanelElement),
Version: 1,
}
sc.reqContext.ReplaceAllParams(map[string]string{":uid": sc.initialResult.Result.UID})
resp = sc.service.patchHandler(sc.reqContext, cmd)
require.Equal(t, 400, resp.Status())
})
scenarioWithPanel(t, "When an admin tries to patch a library panel with model only, it should change model successfully, sync type and description fields and return correct result",
func(t *testing.T, sc scenarioContext) {
cmd := patchLibraryElementCommand{

View File

@ -136,7 +136,7 @@ type LibraryElementConnectionDTO struct {
var (
// errLibraryElementAlreadyExists is an error for when the user tries to add a library element that already exists.
errLibraryElementAlreadyExists = errors.New("library element with that name already exists")
errLibraryElementAlreadyExists = errors.New("library element with that name or UID already exists")
// errLibraryElementNotFound is an error for when a library element can't be found.
errLibraryElementNotFound = errors.New("library element could not be found")
// errLibraryElementDashboardNotFound is an error for when a library element connection can't be found.
@ -147,8 +147,12 @@ var (
errLibraryElementVersionMismatch = errors.New("the library element has been changed by someone else")
// errLibraryElementUnSupportedElementKind is an error for when the kind is unsupported.
errLibraryElementUnSupportedElementKind = errors.New("the element kind is not supported")
// ErrFolderHasConnectedLibraryElements is an error for when an user deletes a folder that contains connected library elements.
// ErrFolderHasConnectedLibraryElements is an error for when a user deletes a folder that contains connected library elements.
ErrFolderHasConnectedLibraryElements = errors.New("folder contains library elements that are linked in use")
// errLibraryElementInvalidUID is an error for when the uid of a library element is invalid
errLibraryElementInvalidUID = errors.New("uid contains illegal characters")
// errLibraryElementUIDTooLong is an error for when the uid of a library element is invalid
errLibraryElementUIDTooLong = errors.New("uid too long, max 40 characters")
)
// Commands
@ -159,6 +163,7 @@ type CreateLibraryElementCommand struct {
Name string `json:"name"`
Model json.RawMessage `json:"model"`
Kind int64 `json:"kind" binding:"Required"`
UID string `json:"uid"`
}
// patchLibraryElementCommand is the command for patching a LibraryElement
@ -168,6 +173,7 @@ type patchLibraryElementCommand struct {
Model json.RawMessage `json:"model"`
Kind int64 `json:"kind" binding:"Required"`
Version int64 `json:"version" binding:"Required"`
UID string `json:"uid"`
}
// searchLibraryElementsQuery is the query used for searching for Elements

View File

@ -50,4 +50,8 @@ func addLibraryElementsMigrations(mg *migrator.Migrator) {
mg.AddMigration("create "+models.LibraryElementConnectionTableName+" table v1", migrator.NewAddTableMigration(libraryElementConnectionV1))
mg.AddMigration("add index "+models.LibraryElementConnectionTableName+" element_id-kind-connection_id", migrator.NewAddIndexMigration(libraryElementConnectionV1, libraryElementConnectionV1.Indices[0]))
mg.AddMigration("add unique index library_element org_id_uid", migrator.NewAddIndexMigration(libraryElementsV1, &migrator.Index{
Cols: []string{"org_id", "uid"}, Type: migrator.UniqueIndex,
}))
}

View File

@ -20,6 +20,11 @@ func IsValidShortUID(uid string) bool {
return validUIDPattern(uid)
}
// IsShortUIDTooLong checks if short unique identifier is too long
func IsShortUIDTooLong(uid string) bool {
return len(uid) > 40
}
// GenerateShortUID generates a short unique identifier.
func GenerateShortUID() string {
return shortid.MustGenerate()

View File

@ -1,6 +1,10 @@
package util
import "testing"
import (
"testing"
"github.com/stretchr/testify/require"
)
func TestAllowedCharMatchesUidPattern(t *testing.T) {
for _, c := range allowedChars {
@ -9,3 +13,33 @@ func TestAllowedCharMatchesUidPattern(t *testing.T) {
}
}
}
func TestIsShortUIDTooLong(t *testing.T) {
var tests = []struct {
name string
uid string
expected bool
}{
{
name: "when the length of uid is longer than 40 chars then IsShortUIDTooLong should return true",
uid: allowedChars,
expected: true,
},
{
name: "when the length of uid is equal too 40 chars then IsShortUIDTooLong should return false",
uid: "0123456789012345678901234567890123456789",
expected: false,
},
{
name: "when the length of uid is shorter than 40 chars then IsShortUIDTooLong should return false",
uid: "012345678901234567890123456789012345678",
expected: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
require.Equal(t, tt.expected, IsShortUIDTooLong(tt.uid))
})
}
}