Skip to content

Commit 5c59118

Browse files
tomazputomazpu
authored
refactor(idutils): rename GenerateExternalIDForDocument to GenerateExternalID and remove validation(#1713)
* refactor: rename GenerateExternalIDForDocument to GenerateExternalID * refactor: remove validation of externalID length and move it into unit test * refactor: update usage, and remove err form caller code * refactor: update slo delete after rebase
1 parent 7950487 commit 5c59118

File tree

13 files changed

+62
-71
lines changed

13 files changed

+62
-71
lines changed

‎cmd/monaco/integrationtest/v2/segments_integration_test.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,8 +183,7 @@ func parseSegmentsPayload(t *testing.T, resp api.Response) segmentsResponse {
183183
}
184184

185185
func assertSegmentIsInResponse(t *testing.T, present bool, responses []api.Response, coord coordinate.Coordinate) {
186-
externalId, err := idutils.GenerateExternalIDForDocument(coord)
187-
assert.NoError(t, err)
186+
externalId := idutils.GenerateExternalID(coord)
188187

189188
found := false
190189

‎internal/idutils/external_id.go

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package idutils
1919
import (
2020
"encoding/base64"
2121
"fmt"
22+
2223
"github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/config/coordinate"
2324
)
2425

@@ -51,22 +52,13 @@ func GenerateExternalIDForSettingsObject(c coordinate.Coordinate) (string, error
5152

5253
type ExternalIDGenerator func(coordinate.Coordinate) (string, error)
5354

54-
// GenerateExternalIDForDocument generates an external ID for a document configuration. It is under 50 characters long and uses at most only "a-z", "A-Z", "0-9" and "-".
55-
func GenerateExternalIDForDocument(c coordinate.Coordinate) (string, error) {
56-
// external ID must be at most 50 characters
57-
const maxLength = 50
58-
55+
// GenerateExternalID generates an external ID for a configuration. It is under 50 characters long and uses at most only "a-z", "A-Z", "0-9" and "-".
56+
func GenerateExternalID(c coordinate.Coordinate) string {
5957
// prefix should be 7 characters
6058
const prefix = "monaco-"
6159

6260
// uuid should be 8 + 1 + 4 + 1 + 4 + 1 + 4 + 1 + 12 = 36 characters
6361
uuid := GenerateUUIDFromCoordinate(c)
6462

65-
externalID := fmt.Sprintf("%s%s", prefix, uuid)
66-
67-
// this should not occur: 36 + 7 < 50
68-
if len(externalID) > maxLength {
69-
return "", fmt.Errorf("calculated external id '%s' is longer than the max length %d", externalID, maxLength)
70-
}
71-
return externalID, nil
63+
return fmt.Sprintf("%s%s", prefix, uuid)
7264
}

‎internal/idutils/external_id_test.go

Lines changed: 41 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -16,25 +16,27 @@
1616
* limitations under the License.
1717
*/
1818

19-
package idutils
19+
package idutils_test
2020

2121
import (
2222
"encoding/base64"
23-
"github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/config/coordinate"
24-
"github.com/stretchr/testify/assert"
2523
"strings"
2624
"testing"
25+
26+
"github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/idutils"
27+
"github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/config/coordinate"
28+
"github.com/stretchr/testify/assert"
2729
)
2830

2931
func TestGenerateExternalIdIsStable(t *testing.T) {
3032
schemaId, id := "a", "b"
3133

32-
output1, err := GenerateExternalIDForSettingsObject(coordinate.Coordinate{
34+
output1, err := idutils.GenerateExternalIDForSettingsObject(coordinate.Coordinate{
3335
Type: schemaId,
3436
ConfigId: id,
3537
})
3638
assert.NoError(t, err)
37-
output2, err := GenerateExternalIDForSettingsObject(coordinate.Coordinate{
39+
output2, err := idutils.GenerateExternalIDForSettingsObject(coordinate.Coordinate{
3840
Type: schemaId,
3941
ConfigId: id,
4042
})
@@ -43,11 +45,11 @@ func TestGenerateExternalIdIsStable(t *testing.T) {
4345
}
4446

4547
func TestGenerateExternalIdGeneratesDifferentValuesForDifferentInput(t *testing.T) {
46-
output1, err := GenerateExternalIDForSettingsObject(coordinate.Coordinate{Type: "a", ConfigId: "a"})
48+
output1, err := idutils.GenerateExternalIDForSettingsObject(coordinate.Coordinate{Type: "a", ConfigId: "a"})
4749
assert.NoError(t, err)
48-
output2, err := GenerateExternalIDForSettingsObject(coordinate.Coordinate{Type: "a", ConfigId: "b"})
50+
output2, err := idutils.GenerateExternalIDForSettingsObject(coordinate.Coordinate{Type: "a", ConfigId: "b"})
4951
assert.NoError(t, err)
50-
output3, err := GenerateExternalIDForSettingsObject(coordinate.Coordinate{Type: "b", ConfigId: "b"})
52+
output3, err := idutils.GenerateExternalIDForSettingsObject(coordinate.Coordinate{Type: "b", ConfigId: "b"})
5153
assert.NoError(t, err)
5254

5355
assert.NotEqual(t, output1, output2)
@@ -56,24 +58,24 @@ func TestGenerateExternalIdGeneratesDifferentValuesForDifferentInput(t *testing.
5658
}
5759

5860
func TestGenerateExternalIdWithOver500CharsCutsIt(t *testing.T) {
59-
output1, err := GenerateExternalIDForSettingsObject(coordinate.Coordinate{Type: strings.Repeat("a", 501)})
61+
output1, err := idutils.GenerateExternalIDForSettingsObject(coordinate.Coordinate{Type: strings.Repeat("a", 501)})
6062
assert.Zero(t, output1)
6163
assert.Error(t, err)
62-
output2, err := GenerateExternalIDForSettingsObject(coordinate.Coordinate{ConfigId: strings.Repeat("a", 501)})
64+
output2, err := idutils.GenerateExternalIDForSettingsObject(coordinate.Coordinate{ConfigId: strings.Repeat("a", 501)})
6365
assert.Zero(t, output2)
6466
assert.Error(t, err)
65-
output3, err := GenerateExternalIDForSettingsObject(coordinate.Coordinate{Type: strings.Repeat("a", 250), ConfigId: strings.Repeat("a", 251)})
67+
output3, err := idutils.GenerateExternalIDForSettingsObject(coordinate.Coordinate{Type: strings.Repeat("a", 250), ConfigId: strings.Repeat("a", 251)})
6668
assert.LessOrEqual(t, len(output3), 500)
6769
assert.NoError(t, err)
6870

6971
}
7072

7173
func TestGenerateExternalIdWithOver500CharactersProducesUniqueIDs(t *testing.T) {
72-
uniqueID1, err := GenerateExternalIDForSettingsObject(coordinate.Coordinate{Type: strings.Repeat("a", 250), ConfigId: strings.Repeat("a", 251)})
74+
uniqueID1, err := idutils.GenerateExternalIDForSettingsObject(coordinate.Coordinate{Type: strings.Repeat("a", 250), ConfigId: strings.Repeat("a", 251)})
7375
assert.NoError(t, err)
74-
uniqueID2, err := GenerateExternalIDForSettingsObject(coordinate.Coordinate{Type: strings.Repeat("a", 250), ConfigId: strings.Repeat("a", 251)})
76+
uniqueID2, err := idutils.GenerateExternalIDForSettingsObject(coordinate.Coordinate{Type: strings.Repeat("a", 250), ConfigId: strings.Repeat("a", 251)})
7577
assert.NoError(t, err)
76-
uniqueID3, err := GenerateExternalIDForSettingsObject(coordinate.Coordinate{Type: strings.Repeat("a", 250), ConfigId: strings.Repeat("a", 300)})
78+
uniqueID3, err := idutils.GenerateExternalIDForSettingsObject(coordinate.Coordinate{Type: strings.Repeat("a", 250), ConfigId: strings.Repeat("a", 300)})
7779
assert.NoError(t, err)
7880

7981
assert.Equal(t, uniqueID1, uniqueID2)
@@ -83,28 +85,28 @@ func TestGenerateExternalIdWithOver500CharactersProducesUniqueIDs(t *testing.T)
8385
func TestGenerateExternalIdStartsWithKnownPrefix(t *testing.T) {
8486
schemaId, id := "a", "b"
8587

86-
extId, err := GenerateExternalIDForSettingsObject(coordinate.Coordinate{Type: schemaId, ConfigId: id})
88+
extId, err := idutils.GenerateExternalIDForSettingsObject(coordinate.Coordinate{Type: schemaId, ConfigId: id})
8789
assert.NoError(t, err)
8890
assert.True(t, strings.HasPrefix(extId, "monaco:"))
8991
}
9092

9193
func TestGenerateExternalIdWithOther500CharsStartsWithKnownPrefix(t *testing.T) {
92-
extId, err := GenerateExternalIDForSettingsObject(coordinate.Coordinate{Type: strings.Repeat("a", 250), ConfigId: strings.Repeat("a", 251)})
94+
extId, err := idutils.GenerateExternalIDForSettingsObject(coordinate.Coordinate{Type: strings.Repeat("a", 250), ConfigId: strings.Repeat("a", 251)})
9395
assert.NoError(t, err)
9496
assert.True(t, strings.HasPrefix(extId, "monaco:"))
9597
}
9698

9799
func TestGenerateExternalIdConsidersProjectName(t *testing.T) {
98100
expectIDWithoutProjectName := "monaco:c2NoZW1hLWlkJGNvbmZpZy1pZA=="
99101
expectIDWithProjectName := "monaco:cHJvamVjdC1uYW1lJHNjaGVtYS1pZCRjb25maWctaWQ="
100-
id1, err := GenerateExternalIDForSettingsObject(coordinate.Coordinate{
102+
id1, err := idutils.GenerateExternalIDForSettingsObject(coordinate.Coordinate{
101103
Project: "",
102104
Type: "schema-id",
103105
ConfigId: "config-id",
104106
})
105107
assert.Equal(t, expectIDWithoutProjectName, id1)
106108
assert.NoError(t, err)
107-
id2, err := GenerateExternalIDForSettingsObject(coordinate.Coordinate{
109+
id2, err := idutils.GenerateExternalIDForSettingsObject(coordinate.Coordinate{
108110
Project: "project-name",
109111
Type: "schema-id",
110112
ConfigId: "config-id",
@@ -115,20 +117,38 @@ func TestGenerateExternalIdConsidersProjectName(t *testing.T) {
115117

116118
func TestGenerateExternalIdReturnsErrIfSchemaIDorConfigIDisMissing(t *testing.T) {
117119

118-
id, err := GenerateExternalIDForSettingsObject(coordinate.Coordinate{ConfigId: "config-id"})
120+
id, err := idutils.GenerateExternalIDForSettingsObject(coordinate.Coordinate{ConfigId: "config-id"})
119121
assert.Zero(t, id)
120122
assert.Error(t, err)
121123

122-
id, err = GenerateExternalIDForSettingsObject(coordinate.Coordinate{Type: "schema-id"})
124+
id, err = idutils.GenerateExternalIDForSettingsObject(coordinate.Coordinate{Type: "schema-id"})
123125
assert.Zero(t, id)
124126
assert.Error(t, err)
125127

126128
}
127129

128130
func TestGenerateExternalIdRawIdParts(t *testing.T) {
129-
id, _ := GenerateExternalIDForSettingsObject(coordinate.Coordinate{Project: "project-name", Type: "schema-id", ConfigId: "config-id"})
131+
id, _ := idutils.GenerateExternalIDForSettingsObject(coordinate.Coordinate{Project: "project-name", Type: "schema-id", ConfigId: "config-id"})
130132
decoded, _ := base64.StdEncoding.DecodeString(strings.TrimPrefix(id, "monaco:"))
131133
rawId := make([]byte, len(decoded))
132134
copy(rawId, decoded)
133135
assert.Equal(t, "project-name$schema-id$config-id", string(decoded))
134136
}
137+
138+
func TestGenerateExternalID(t *testing.T) {
139+
coordinate := coordinate.Coordinate{Project: "project-name", Type: "schema-id", ConfigId: "config-id"}
140+
id := idutils.GenerateExternalID(coordinate)
141+
t.Run("Len of id is under 50", func(t *testing.T) {
142+
t.Parallel()
143+
len := len(id)
144+
assert.True(t, len <= 50)
145+
})
146+
t.Run("externalID correctly encoded", func(t *testing.T) {
147+
t.Parallel()
148+
assert.Equal(t, id, "monaco-ae5f7e18-84b6-3c05-98b9-3d8aba2c708c")
149+
})
150+
t.Run("externalID correctly prefixed", func(t *testing.T) {
151+
t.Parallel()
152+
assert.True(t, strings.HasPrefix(id, "monaco-"))
153+
})
154+
}

‎pkg/delete/delete_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1056,7 +1056,7 @@ func TestDelete_Documents(t *testing.T) {
10561056
Project: "project",
10571057
}
10581058

1059-
externalID, _ := idutils.GenerateExternalIDForDocument(given.AsCoordinate())
1059+
externalID := idutils.GenerateExternalID(given.AsCoordinate())
10601060
c := client.NewMockDocumentClient(gomock.NewController(t))
10611061
c.EXPECT().List(gomock.Any(), gomock.Eq(fmt.Sprintf("externalId=='%s'", externalID))).
10621062
Times(1).
@@ -1075,7 +1075,7 @@ func TestDelete_Documents(t *testing.T) {
10751075
Project: "project",
10761076
}
10771077

1078-
externalID, _ := idutils.GenerateExternalIDForDocument(given.AsCoordinate())
1078+
externalID := idutils.GenerateExternalID(given.AsCoordinate())
10791079
c := client.NewMockDocumentClient(gomock.NewController(t))
10801080
c.EXPECT().List(gomock.Any(), gomock.Eq(fmt.Sprintf("externalId=='%s'", externalID))).
10811081
Times(1).
@@ -1093,7 +1093,7 @@ func TestDelete_Documents(t *testing.T) {
10931093
Project: "project",
10941094
}
10951095

1096-
externalID, _ := idutils.GenerateExternalIDForDocument(given.AsCoordinate())
1096+
externalID := idutils.GenerateExternalID(given.AsCoordinate())
10971097
c := client.NewMockDocumentClient(gomock.NewController(t))
10981098
c.EXPECT().List(gomock.Any(), gomock.Eq(fmt.Sprintf("externalId=='%s'", externalID))).
10991099
Times(1).

‎pkg/delete/internal/document/delete.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,8 @@ func deleteSingle(ctx context.Context, c client, dp pointer.DeletePointer) error
5858
id = dp.OriginObjectId
5959
}
6060
if id == "" {
61-
extID, err := idutils.GenerateExternalIDForDocument(dp.AsCoordinate())
62-
if err != nil {
63-
return fmt.Errorf("unable to generate externalID: %w", err)
64-
}
61+
var err error
62+
extID := idutils.GenerateExternalID(dp.AsCoordinate())
6563

6664
id, err = tryGetDocumentIDByExternalID(ctx, c, extID)
6765
if err != nil {

‎pkg/delete/internal/segment/delete.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,10 +84,7 @@ func findEntryWithExternalID(ctx context.Context, c client, dp pointer.DeletePoi
8484
return "", err
8585
}
8686

87-
extID, err := idutils.GenerateExternalIDForDocument(dp.AsCoordinate())
88-
if err != nil {
89-
return "", fmt.Errorf("unable to generate externalID: %w", err)
90-
}
87+
extID := idutils.GenerateExternalID(dp.AsCoordinate())
9188

9289
var foundUUIDs []string
9390
for _, i := range items {

‎pkg/delete/internal/segment/delete_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ func TestDeleteByCoordinate(t *testing.T) {
5656
Identifier: "monaco_identifier",
5757
Project: "project",
5858
}
59-
externalID, _ := idutils.GenerateExternalIDForDocument(given.AsCoordinate())
59+
externalID := idutils.GenerateExternalID(given.AsCoordinate())
6060

6161
c := stubClient{
6262
list: func() (libSegment.Response, error) {
@@ -97,7 +97,7 @@ func TestDeleteByCoordinate(t *testing.T) {
9797
Project: "project",
9898
}
9999

100-
externalID, _ := idutils.GenerateExternalIDForDocument(given.AsCoordinate())
100+
externalID := idutils.GenerateExternalID(given.AsCoordinate())
101101
c := stubClient{
102102
list: func() (libSegment.Response, error) {
103103
return libSegment.Response{Data: []byte(fmt.Sprintf(`[{"uid": "uid_1", "externalId":"%s"},{"uid": "uid_2", "externalId":"%s"}]`, externalID, externalID))}, nil

‎pkg/delete/internal/slo/delete.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,7 @@ func findEntryWithExternalID(ctx context.Context, c client, dp pointer.DeletePoi
8383
return "", err
8484
}
8585

86-
extID, err := idutils.GenerateExternalIDForDocument(dp.AsCoordinate())
87-
if err != nil {
88-
return "", fmt.Errorf("unable to generate externalID: %w", err)
89-
}
86+
extID := idutils.GenerateExternalID(dp.AsCoordinate())
9087

9188
var found []entry
9289
for _, i := range items.All() {

‎pkg/delete/internal/slo/delete_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ func TestDeleteWithCoordinate(t *testing.T) {
4141
Identifier: "monaco_identifier",
4242
Project: "project",
4343
}
44-
externalID, _ := idutils.GenerateExternalIDForDocument(given.AsCoordinate())
44+
externalID := idutils.GenerateExternalID(given.AsCoordinate())
4545

4646
c := stubClient{
4747
list: func() (libAPI.PagedListResponse, error) {
@@ -91,7 +91,7 @@ func TestDeleteWithCoordinate(t *testing.T) {
9191
Project: "project",
9292
}
9393

94-
externalID, _ := idutils.GenerateExternalIDForDocument(given.AsCoordinate())
94+
externalID := idutils.GenerateExternalID(given.AsCoordinate())
9595
c := stubClient{
9696
list: func() (libAPI.PagedListResponse, error) {
9797
return libAPI.PagedListResponse{

‎pkg/deploy/internal/document/document.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,7 @@ func Deploy(ctx context.Context, client Client, properties parameter.Properties,
7575
}
7676

7777
// strategy 2: find and update document via external id
78-
externalId, err := idutils.GenerateExternalIDForDocument(c.Coordinate)
79-
if err != nil {
80-
return entities.ResolvedEntity{}, deployErrors.NewConfigDeployErr(c, "error generating external id").WithError(err)
81-
}
78+
externalId := idutils.GenerateExternalID(c.Coordinate)
8279

8380
id, err := tryGetDocumentIDByExternalID(ctx, client, externalId)
8481
if err != nil {

0 commit comments

Comments
 (0)