Skip to content

Commit fc73bc1

Browse files
hugohaggmarkhugohaggmark
authored
LibraryElements: Enables creating library elements with specific UID (grafana#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
1 parent 580b03c commit fc73bc1

File tree

9 files changed

+224
-6
lines changed

9 files changed

+224
-6
lines changed

‎pkg/services/dashboards/dashboard_service.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ func (dr *dashboardServiceImpl) buildSaveDashboardCommand(dto *SaveDashboardDTO,
103103

104104
if !util.IsValidShortUID(dash.Uid) {
105105
return nil, models.ErrDashboardInvalidUid
106-
} else if len(dash.Uid) > 40 {
106+
} else if util.IsShortUIDTooLong(dash.Uid) {
107107
return nil, models.ErrDashboardUidTooLong
108108
}
109109

‎pkg/services/libraryelements/api.go

+6
Original file line numberDiff line numberDiff line change
@@ -125,5 +125,11 @@ func toLibraryElementError(err error, message string) response.Response {
125125
if errors.Is(err, errLibraryElementHasConnections) {
126126
return response.Error(403, errLibraryElementHasConnections.Error(), err)
127127
}
128+
if errors.Is(err, errLibraryElementInvalidUID) {
129+
return response.Error(400, errLibraryElementInvalidUID.Error(), err)
130+
}
131+
if errors.Is(err, errLibraryElementUIDTooLong) {
132+
return response.Error(400, errLibraryElementUIDTooLong.Error(), err)
133+
}
128134
return response.Error(500, message, err)
129135
}

‎pkg/services/libraryelements/database.go

+28-2
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package libraryelements
22

33
import (
44
"encoding/json"
5+
"errors"
56
"fmt"
67
"strings"
78
"time"
@@ -92,10 +93,20 @@ func (l *LibraryElementService) createLibraryElement(c *models.ReqContext, cmd C
9293
if err := l.requireSupportedElementKind(cmd.Kind); err != nil {
9394
return LibraryElementDTO{}, err
9495
}
96+
createUID := cmd.UID
97+
if len(createUID) == 0 {
98+
createUID = util.GenerateShortUID()
99+
} else {
100+
if !util.IsValidShortUID(createUID) {
101+
return LibraryElementDTO{}, errLibraryElementInvalidUID
102+
} else if util.IsShortUIDTooLong(createUID) {
103+
return LibraryElementDTO{}, errLibraryElementUIDTooLong
104+
}
105+
}
95106
element := LibraryElement{
96107
OrgID: c.SignedInUser.OrgId,
97108
FolderID: cmd.FolderID,
98-
UID: util.GenerateShortUID(),
109+
UID: createUID,
99110
Name: cmd.Name,
100111
Model: cmd.Model,
101112
Version: 1,
@@ -434,12 +445,27 @@ func (l *LibraryElementService) patchLibraryElement(c *models.ReqContext, cmd pa
434445
if elementInDB.Version != cmd.Version {
435446
return errLibraryElementVersionMismatch
436447
}
448+
updateUID := cmd.UID
449+
if len(updateUID) == 0 {
450+
updateUID = uid
451+
} else if updateUID != uid {
452+
if !util.IsValidShortUID(updateUID) {
453+
return errLibraryElementInvalidUID
454+
} else if util.IsShortUIDTooLong(updateUID) {
455+
return errLibraryElementUIDTooLong
456+
}
457+
458+
_, err := getLibraryElement(l.SQLStore.Dialect, session, updateUID, c.SignedInUser.OrgId)
459+
if !errors.Is(err, errLibraryElementNotFound) {
460+
return errLibraryElementAlreadyExists
461+
}
462+
}
437463

438464
var libraryElement = LibraryElement{
439465
ID: elementInDB.ID,
440466
OrgID: c.SignedInUser.OrgId,
441467
FolderID: cmd.FolderID,
442-
UID: uid,
468+
UID: updateUID,
443469
Name: cmd.Name,
444470
Kind: elementInDB.Kind,
445471
Type: elementInDB.Type,

‎pkg/services/libraryelements/libraryelements_create_test.go

+71
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"github.com/stretchr/testify/require"
88

99
"github.com/grafana/grafana/pkg/models"
10+
"github.com/grafana/grafana/pkg/util"
1011
)
1112

1213
func TestCreateLibraryElement(t *testing.T) {
@@ -59,6 +60,76 @@ func TestCreateLibraryElement(t *testing.T) {
5960
}
6061
})
6162

63+
testScenario(t, "When an admin tries to create a library panel that does not exists using an nonexistent UID, it should succeed",
64+
func(t *testing.T, sc scenarioContext) {
65+
command := getCreatePanelCommand(sc.folder.Id, "Nonexistent UID")
66+
command.UID = util.GenerateShortUID()
67+
resp := sc.service.createHandler(sc.reqContext, command)
68+
var result = validateAndUnMarshalResponse(t, resp)
69+
var expected = libraryElementResult{
70+
Result: libraryElement{
71+
ID: 1,
72+
OrgID: 1,
73+
FolderID: 1,
74+
UID: command.UID,
75+
Name: "Nonexistent UID",
76+
Kind: int64(models.PanelElement),
77+
Type: "text",
78+
Description: "A description",
79+
Model: map[string]interface{}{
80+
"datasource": "${DS_GDEV-TESTDATA}",
81+
"description": "A description",
82+
"id": float64(1),
83+
"title": "Text - Library Panel",
84+
"type": "text",
85+
},
86+
Version: 1,
87+
Meta: LibraryElementDTOMeta{
88+
ConnectedDashboards: 0,
89+
Created: result.Result.Meta.Created,
90+
Updated: result.Result.Meta.Updated,
91+
CreatedBy: LibraryElementDTOMetaUser{
92+
ID: 1,
93+
Name: "signed_in_user",
94+
AvatarURL: "/avatar/37524e1eb8b3e32850b57db0a19af93b",
95+
},
96+
UpdatedBy: LibraryElementDTOMetaUser{
97+
ID: 1,
98+
Name: "signed_in_user",
99+
AvatarURL: "/avatar/37524e1eb8b3e32850b57db0a19af93b",
100+
},
101+
},
102+
},
103+
}
104+
if diff := cmp.Diff(expected, result, getCompareOptions()...); diff != "" {
105+
t.Fatalf("Result mismatch (-want +got):\n%s", diff)
106+
}
107+
})
108+
109+
scenarioWithPanel(t, "When an admin tries to create a library panel that does not exists using an existent UID, it should fail",
110+
func(t *testing.T, sc scenarioContext) {
111+
command := getCreatePanelCommand(sc.folder.Id, "Existing UID")
112+
command.UID = sc.initialResult.Result.UID
113+
resp := sc.service.createHandler(sc.reqContext, command)
114+
require.Equal(t, 400, resp.Status())
115+
})
116+
117+
scenarioWithPanel(t, "When an admin tries to create a library panel that does not exists using an invalid UID, it should fail",
118+
func(t *testing.T, sc scenarioContext) {
119+
command := getCreatePanelCommand(sc.folder.Id, "Invalid UID")
120+
command.UID = "Testing an invalid UID"
121+
resp := sc.service.createHandler(sc.reqContext, command)
122+
require.Equal(t, 400, resp.Status())
123+
})
124+
125+
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",
126+
func(t *testing.T, sc scenarioContext) {
127+
command := getCreatePanelCommand(sc.folder.Id, "Invalid UID")
128+
command.UID = "j6T00KRZzj6T00KRZzj6T00KRZzj6T00KRZzj6T00K"
129+
resp := sc.service.createHandler(sc.reqContext, command)
130+
require.Equal(t, 400, resp.Status())
131+
})
132+
62133
testScenario(t, "When an admin tries to create a library panel where name and panel title differ, it should not update panel title",
63134
func(t *testing.T, sc scenarioContext) {
64135
command := getCreatePanelCommand(1, "Library Panel Name")

‎pkg/services/libraryelements/libraryelements_patch_test.go

+66
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ package libraryelements
33
import (
44
"testing"
55

6+
"github.com/grafana/grafana/pkg/util"
7+
68
"github.com/google/go-cmp/cmp"
79
"github.com/stretchr/testify/require"
810

@@ -122,6 +124,70 @@ func TestPatchLibraryElement(t *testing.T) {
122124
}
123125
})
124126

127+
scenarioWithPanel(t, "When an admin tries to patch a library panel with a nonexistent UID, it should change UID successfully and return correct result",
128+
func(t *testing.T, sc scenarioContext) {
129+
cmd := patchLibraryElementCommand{
130+
FolderID: -1,
131+
UID: util.GenerateShortUID(),
132+
Kind: int64(models.PanelElement),
133+
Version: 1,
134+
}
135+
sc.reqContext.ReplaceAllParams(map[string]string{":uid": sc.initialResult.Result.UID})
136+
resp := sc.service.patchHandler(sc.reqContext, cmd)
137+
var result = validateAndUnMarshalResponse(t, resp)
138+
sc.initialResult.Result.UID = cmd.UID
139+
sc.initialResult.Result.Meta.CreatedBy.Name = userInDbName
140+
sc.initialResult.Result.Meta.CreatedBy.AvatarURL = userInDbAvatar
141+
sc.initialResult.Result.Model["title"] = "Text - Library Panel"
142+
sc.initialResult.Result.Version = 2
143+
if diff := cmp.Diff(sc.initialResult.Result, result.Result, getCompareOptions()...); diff != "" {
144+
t.Fatalf("Result mismatch (-want +got):\n%s", diff)
145+
}
146+
})
147+
148+
scenarioWithPanel(t, "When an admin tries to patch a library panel with an invalid UID, it should fail",
149+
func(t *testing.T, sc scenarioContext) {
150+
cmd := patchLibraryElementCommand{
151+
FolderID: -1,
152+
UID: "Testing an invalid UID",
153+
Kind: int64(models.PanelElement),
154+
Version: 1,
155+
}
156+
sc.reqContext.ReplaceAllParams(map[string]string{":uid": sc.initialResult.Result.UID})
157+
resp := sc.service.patchHandler(sc.reqContext, cmd)
158+
require.Equal(t, 400, resp.Status())
159+
})
160+
161+
scenarioWithPanel(t, "When an admin tries to patch a library panel with an UID that is too long, it should fail",
162+
func(t *testing.T, sc scenarioContext) {
163+
cmd := patchLibraryElementCommand{
164+
FolderID: -1,
165+
UID: "j6T00KRZzj6T00KRZzj6T00KRZzj6T00KRZzj6T00K",
166+
Kind: int64(models.PanelElement),
167+
Version: 1,
168+
}
169+
sc.reqContext.ReplaceAllParams(map[string]string{":uid": sc.initialResult.Result.UID})
170+
resp := sc.service.patchHandler(sc.reqContext, cmd)
171+
require.Equal(t, 400, resp.Status())
172+
})
173+
174+
scenarioWithPanel(t, "When an admin tries to patch a library panel with an existing UID, it should fail",
175+
func(t *testing.T, sc scenarioContext) {
176+
command := getCreatePanelCommand(sc.folder.Id, "Existing UID")
177+
command.UID = util.GenerateShortUID()
178+
resp := sc.service.createHandler(sc.reqContext, command)
179+
require.Equal(t, 200, resp.Status())
180+
cmd := patchLibraryElementCommand{
181+
FolderID: -1,
182+
UID: command.UID,
183+
Kind: int64(models.PanelElement),
184+
Version: 1,
185+
}
186+
sc.reqContext.ReplaceAllParams(map[string]string{":uid": sc.initialResult.Result.UID})
187+
resp = sc.service.patchHandler(sc.reqContext, cmd)
188+
require.Equal(t, 400, resp.Status())
189+
})
190+
125191
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",
126192
func(t *testing.T, sc scenarioContext) {
127193
cmd := patchLibraryElementCommand{

‎pkg/services/libraryelements/models.go

+8-2
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ type LibraryElementConnectionDTO struct {
136136

137137
var (
138138
// errLibraryElementAlreadyExists is an error for when the user tries to add a library element that already exists.
139-
errLibraryElementAlreadyExists = errors.New("library element with that name already exists")
139+
errLibraryElementAlreadyExists = errors.New("library element with that name or UID already exists")
140140
// errLibraryElementNotFound is an error for when a library element can't be found.
141141
errLibraryElementNotFound = errors.New("library element could not be found")
142142
// errLibraryElementDashboardNotFound is an error for when a library element connection can't be found.
@@ -147,8 +147,12 @@ var (
147147
errLibraryElementVersionMismatch = errors.New("the library element has been changed by someone else")
148148
// errLibraryElementUnSupportedElementKind is an error for when the kind is unsupported.
149149
errLibraryElementUnSupportedElementKind = errors.New("the element kind is not supported")
150-
// ErrFolderHasConnectedLibraryElements is an error for when an user deletes a folder that contains connected library elements.
150+
// ErrFolderHasConnectedLibraryElements is an error for when a user deletes a folder that contains connected library elements.
151151
ErrFolderHasConnectedLibraryElements = errors.New("folder contains library elements that are linked in use")
152+
// errLibraryElementInvalidUID is an error for when the uid of a library element is invalid
153+
errLibraryElementInvalidUID = errors.New("uid contains illegal characters")
154+
// errLibraryElementUIDTooLong is an error for when the uid of a library element is invalid
155+
errLibraryElementUIDTooLong = errors.New("uid too long, max 40 characters")
152156
)
153157

154158
// Commands
@@ -159,6 +163,7 @@ type CreateLibraryElementCommand struct {
159163
Name string `json:"name"`
160164
Model json.RawMessage `json:"model"`
161165
Kind int64 `json:"kind" binding:"Required"`
166+
UID string `json:"uid"`
162167
}
163168

164169
// patchLibraryElementCommand is the command for patching a LibraryElement
@@ -168,6 +173,7 @@ type patchLibraryElementCommand struct {
168173
Model json.RawMessage `json:"model"`
169174
Kind int64 `json:"kind" binding:"Required"`
170175
Version int64 `json:"version" binding:"Required"`
176+
UID string `json:"uid"`
171177
}
172178

173179
// searchLibraryElementsQuery is the query used for searching for Elements

‎pkg/services/sqlstore/migrations/libraryelements.go

+4
Original file line numberDiff line numberDiff line change
@@ -50,4 +50,8 @@ func addLibraryElementsMigrations(mg *migrator.Migrator) {
5050

5151
mg.AddMigration("create "+models.LibraryElementConnectionTableName+" table v1", migrator.NewAddTableMigration(libraryElementConnectionV1))
5252
mg.AddMigration("add index "+models.LibraryElementConnectionTableName+" element_id-kind-connection_id", migrator.NewAddIndexMigration(libraryElementConnectionV1, libraryElementConnectionV1.Indices[0]))
53+
54+
mg.AddMigration("add unique index library_element org_id_uid", migrator.NewAddIndexMigration(libraryElementsV1, &migrator.Index{
55+
Cols: []string{"org_id", "uid"}, Type: migrator.UniqueIndex,
56+
}))
5357
}

‎pkg/util/shortid_generator.go

+5
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,11 @@ func IsValidShortUID(uid string) bool {
2020
return validUIDPattern(uid)
2121
}
2222

23+
// IsShortUIDTooLong checks if short unique identifier is too long
24+
func IsShortUIDTooLong(uid string) bool {
25+
return len(uid) > 40
26+
}
27+
2328
// GenerateShortUID generates a short unique identifier.
2429
func GenerateShortUID() string {
2530
return shortid.MustGenerate()

‎pkg/util/shortid_generator_test.go

+35-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
package util
22

3-
import "testing"
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/require"
7+
)
48

59
func TestAllowedCharMatchesUidPattern(t *testing.T) {
610
for _, c := range allowedChars {
@@ -9,3 +13,33 @@ func TestAllowedCharMatchesUidPattern(t *testing.T) {
913
}
1014
}
1115
}
16+
17+
func TestIsShortUIDTooLong(t *testing.T) {
18+
var tests = []struct {
19+
name string
20+
uid string
21+
expected bool
22+
}{
23+
{
24+
name: "when the length of uid is longer than 40 chars then IsShortUIDTooLong should return true",
25+
uid: allowedChars,
26+
expected: true,
27+
},
28+
{
29+
name: "when the length of uid is equal too 40 chars then IsShortUIDTooLong should return false",
30+
uid: "0123456789012345678901234567890123456789",
31+
expected: false,
32+
},
33+
{
34+
name: "when the length of uid is shorter than 40 chars then IsShortUIDTooLong should return false",
35+
uid: "012345678901234567890123456789012345678",
36+
expected: false,
37+
},
38+
}
39+
40+
for _, tt := range tests {
41+
t.Run(tt.name, func(t *testing.T) {
42+
require.Equal(t, tt.expected, IsShortUIDTooLong(tt.uid))
43+
})
44+
}
45+
}

0 commit comments

Comments
 (0)