Skip to content

Commit 0b82932

Browse files
blvablva
authored
CLOUDP-279336: Avoid that squashing overrides hideFromChangelog (#281)
1 parent 37be35b commit 0b82932

File tree

2 files changed

+270
-19
lines changed

2 files changed

+270
-19
lines changed

‎tools/cli/internal/changelog/outputfilter/squash.go

+41-11
Original file line numberDiff line numberDiff line change
@@ -111,28 +111,41 @@ func newSquashHandlers() []handler {
111111
}
112112
}
113113

114-
// EntryMappings groups entries by ID and then by OperationID and returns a Map[changeCode, Map[operationId, List[oasdiffEntry]]]
115-
func newEntriesMapPerIDAndOperationID(entries []*OasDiffEntry) map[string]map[string][]*OasDiffEntry {
116-
result := make(map[string]map[string][]*OasDiffEntry)
114+
// EntryMappings groups entries by ID and then by OperationID and returns two maps
115+
// of type Map[changeCode, Map[operationId, List[oasdiffEntry]]], one for hidden squashed entries and one for visible squashed entries.
116+
func newEntriesMapPerIDAndOperationID(entries []*OasDiffEntry) (result, hiddenResult map[string]map[string][]*OasDiffEntry) {
117+
result = make(map[string]map[string][]*OasDiffEntry)
118+
hiddenResult = make(map[string]map[string][]*OasDiffEntry)
117119

118120
for _, entry := range entries {
119121
code := entry.ID
120122
operationID := entry.OperationID
123+
hidden := entry.HideFromChangelog
121124

122-
// Ensure the code map exists
123-
if _, exists := result[code]; !exists {
124-
result[code] = make(map[string][]*OasDiffEntry)
125+
if hidden {
126+
addToMap(hiddenResult, code, operationID, entry)
127+
continue
125128
}
126129

127-
// Append the entry to the appropriate operationID slice
128-
result[code][operationID] = append(result[code][operationID], entry)
130+
addToMap(result, code, operationID, entry)
131+
}
132+
133+
return result, hiddenResult
134+
}
135+
136+
func addToMap(m map[string]map[string][]*OasDiffEntry, code, operationID string, entry *OasDiffEntry) {
137+
// Ensure the code map exists
138+
if _, exists := m[code]; !exists {
139+
m[code] = make(map[string][]*OasDiffEntry)
129140
}
130141

131-
return result
142+
// Append the entry to the appropriate operationID slice
143+
m[code][operationID] = append(m[code][operationID], entry)
132144
}
133145

134146
func squashEntries(entries []*OasDiffEntry) ([]*OasDiffEntry, error) {
135-
entriesMap := newEntriesMapPerIDAndOperationID(entries)
147+
entriesByIDandOperationID, hiddenEntriesByIDandOperationID := newEntriesMapPerIDAndOperationID(entries)
148+
136149
squashHandlers := newSquashHandlers()
137150
squashedEntries := []*OasDiffEntry{}
138151

@@ -145,6 +158,24 @@ func squashEntries(entries []*OasDiffEntry) ([]*OasDiffEntry, error) {
145158
}
146159
}
147160

161+
squashedEntriesNotHidden, err := appplySquashHandlerToMap(squashHandlers, entriesByIDandOperationID)
162+
if err != nil {
163+
return nil, err
164+
}
165+
166+
squashedEntriesHidden, err := appplySquashHandlerToMap(squashHandlers, hiddenEntriesByIDandOperationID)
167+
if err != nil {
168+
return nil, err
169+
}
170+
171+
squashedEntries = append(squashedEntries, squashedEntriesNotHidden...)
172+
squashedEntries = append(squashedEntries, squashedEntriesHidden...)
173+
174+
return squashedEntries, nil
175+
}
176+
177+
func appplySquashHandlerToMap(squashHandlers []handler, entriesMap map[string]map[string][]*OasDiffEntry) ([]*OasDiffEntry, error) {
178+
squashedEntries := []*OasDiffEntry{}
148179
for _, handler := range squashHandlers {
149180
entryMapPerOperationID, ok := entriesMap[handler.id]
150181
if !ok {
@@ -158,7 +189,6 @@ func squashEntries(entries []*OasDiffEntry) ([]*OasDiffEntry, error) {
158189

159190
squashedEntries = append(squashedEntries, sortEntriesByDescription(entries)...)
160191
}
161-
162192
return squashedEntries, nil
163193
}
164194

‎tools/cli/internal/changelog/outputfilter/squash_test.go

+229-8
Original file line numberDiff line numberDiff line change
@@ -2,21 +2,24 @@ package outputfilter
22

33
import (
44
"reflect"
5+
"sort"
56
"testing"
67

78
"github.com/stretchr/testify/require"
89
)
910

1011
func TestNewEntriesMapPerIDAndOperationID(t *testing.T) {
1112
testCases := []struct {
12-
name string
13-
entries []*OasDiffEntry
14-
want map[string]map[string][]*OasDiffEntry
13+
name string
14+
entries []*OasDiffEntry
15+
want map[string]map[string][]*OasDiffEntry
16+
wantHidden map[string]map[string][]*OasDiffEntry
1517
}{
1618
{
17-
name: "Empty entries",
18-
entries: []*OasDiffEntry{},
19-
want: map[string]map[string][]*OasDiffEntry{},
19+
name: "Empty entries",
20+
entries: []*OasDiffEntry{},
21+
want: map[string]map[string][]*OasDiffEntry{},
22+
wantHidden: map[string]map[string][]*OasDiffEntry{},
2023
},
2124
{
2225
name: "Single entry",
@@ -30,6 +33,7 @@ func TestNewEntriesMapPerIDAndOperationID(t *testing.T) {
3033
},
3134
},
3235
},
36+
wantHidden: map[string]map[string][]*OasDiffEntry{},
3337
},
3438
{
3539
name: "Multiple entries with same ID",
@@ -47,8 +51,8 @@ func TestNewEntriesMapPerIDAndOperationID(t *testing.T) {
4751
},
4852
},
4953
},
54+
wantHidden: map[string]map[string][]*OasDiffEntry{},
5055
},
51-
5256
{
5357
name: "Multiple entries with same ID and OperationID",
5458
entries: []*OasDiffEntry{
@@ -63,6 +67,7 @@ func TestNewEntriesMapPerIDAndOperationID(t *testing.T) {
6367
},
6468
},
6569
},
70+
wantHidden: map[string]map[string][]*OasDiffEntry{},
6671
},
6772
{
6873
name: "Multiple entries with different IDs",
@@ -90,15 +95,66 @@ func TestNewEntriesMapPerIDAndOperationID(t *testing.T) {
9095
},
9196
},
9297
},
98+
wantHidden: map[string]map[string][]*OasDiffEntry{},
99+
},
100+
{
101+
name: "Hidden entries",
102+
entries: []*OasDiffEntry{
103+
{ID: "response-write-only-property-enum-value-added", OperationID: "op1", HideFromChangelog: true},
104+
{ID: "response-write-only-property-enum-value-added", OperationID: "op2", HideFromChangelog: true},
105+
},
106+
want: map[string]map[string][]*OasDiffEntry{},
107+
wantHidden: map[string]map[string][]*OasDiffEntry{
108+
"response-write-only-property-enum-value-added": {
109+
"op1": []*OasDiffEntry{
110+
{ID: "response-write-only-property-enum-value-added", OperationID: "op1", HideFromChangelog: true},
111+
},
112+
"op2": []*OasDiffEntry{
113+
{ID: "response-write-only-property-enum-value-added", OperationID: "op2", HideFromChangelog: true},
114+
},
115+
},
116+
},
117+
},
118+
{
119+
name: "Mixed hidden and non-hidden entries",
120+
entries: []*OasDiffEntry{
121+
{ID: "response-write-only-property-enum-value-added", OperationID: "op1"},
122+
{ID: "response-write-only-property-enum-value-added", OperationID: "op2", HideFromChangelog: true},
123+
{ID: "response-write-only-property-enum-value-added", OperationID: "op3"},
124+
{ID: "response-write-only-property-enum-value-added", OperationID: "op4", HideFromChangelog: true},
125+
},
126+
want: map[string]map[string][]*OasDiffEntry{
127+
"response-write-only-property-enum-value-added": {
128+
"op1": []*OasDiffEntry{
129+
{ID: "response-write-only-property-enum-value-added", OperationID: "op1"},
130+
},
131+
"op3": []*OasDiffEntry{
132+
{ID: "response-write-only-property-enum-value-added", OperationID: "op3"},
133+
},
134+
},
135+
},
136+
wantHidden: map[string]map[string][]*OasDiffEntry{
137+
"response-write-only-property-enum-value-added": {
138+
"op2": []*OasDiffEntry{
139+
{ID: "response-write-only-property-enum-value-added", OperationID: "op2", HideFromChangelog: true},
140+
},
141+
"op4": []*OasDiffEntry{
142+
{ID: "response-write-only-property-enum-value-added", OperationID: "op4", HideFromChangelog: true},
143+
},
144+
},
145+
},
93146
},
94147
}
95148

96149
for _, tc := range testCases {
97150
t.Run(tc.name, func(t *testing.T) {
98-
got := newEntriesMapPerIDAndOperationID(tc.entries)
151+
got, gotHidden := newEntriesMapPerIDAndOperationID(tc.entries)
99152
if !reflect.DeepEqual(got, tc.want) {
100153
t.Errorf("got %v, want %v", got, tc.want)
101154
}
155+
if !reflect.DeepEqual(gotHidden, tc.wantHidden) {
156+
t.Errorf("gotHidden %v, wantHidden %v", gotHidden, tc.wantHidden)
157+
}
102158
})
103159
}
104160
}
@@ -238,3 +294,168 @@ func TestNewSquashMap(t *testing.T) {
238294
})
239295
}
240296
}
297+
298+
func TestSquashEntries(t *testing.T) {
299+
testCases := []struct {
300+
name string
301+
entries []*OasDiffEntry
302+
want []*OasDiffEntry
303+
}{
304+
{
305+
name: "Empty entries",
306+
entries: []*OasDiffEntry{},
307+
want: []*OasDiffEntry{},
308+
},
309+
{
310+
name: "Single entry",
311+
entries: []*OasDiffEntry{
312+
{
313+
ID: "response-write-only-property-enum-value-added",
314+
OperationID: "op1",
315+
Text: "added the new 'ENUM1' enum value to the 'region' response write-only property",
316+
},
317+
},
318+
want: []*OasDiffEntry{
319+
{
320+
ID: "response-write-only-property-enum-value-added",
321+
OperationID: "op1",
322+
Text: "added the new 'ENUM1' enum value to the 'region' response write-only property",
323+
},
324+
},
325+
},
326+
{
327+
name: "Multiple entries with same ID and OperationID",
328+
entries: []*OasDiffEntry{
329+
{
330+
ID: "response-write-only-property-enum-value-added",
331+
OperationID: "op1",
332+
Text: "added the new 'ENUM1' enum value to the 'region' response write-only property",
333+
},
334+
{
335+
ID: "response-write-only-property-enum-value-added",
336+
OperationID: "op1",
337+
Text: "added the new 'ENUM2' enum value to the 'region' response write-only property",
338+
},
339+
},
340+
want: []*OasDiffEntry{
341+
{
342+
ID: "response-write-only-property-enum-value-added",
343+
OperationID: "op1",
344+
Text: "added the new 'ENUM1, ENUM2' enum values to the 'region' response write-only property",
345+
},
346+
},
347+
},
348+
{
349+
name: "Multiple entries with different IDs",
350+
entries: []*OasDiffEntry{
351+
{
352+
ID: "response-write-only-property-enum-value-added",
353+
OperationID: "op1",
354+
Text: "added the new 'ENUM1' enum value to the 'region' response write-only property",
355+
},
356+
{
357+
ID: "request-write-only-property-enum-value-added",
358+
OperationID: "op2",
359+
Text: "added the new 'ENUM2' enum value to the 'region' request write-only property",
360+
},
361+
},
362+
want: []*OasDiffEntry{
363+
{
364+
ID: "response-write-only-property-enum-value-added",
365+
OperationID: "op1",
366+
Text: "added the new 'ENUM1' enum value to the 'region' response write-only property",
367+
},
368+
{
369+
ID: "request-write-only-property-enum-value-added",
370+
OperationID: "op2",
371+
Text: "added the new 'ENUM2' enum value to the 'region' request write-only property",
372+
},
373+
},
374+
},
375+
{
376+
name: "Hidden entries",
377+
entries: []*OasDiffEntry{
378+
{
379+
ID: "response-write-only-property-enum-value-added",
380+
OperationID: "op1",
381+
Text: "added the new 'ENUM1' enum value to the 'region' response write-only property",
382+
HideFromChangelog: true,
383+
},
384+
{
385+
ID: "response-write-only-property-enum-value-added",
386+
OperationID: "op1",
387+
Text: "added the new 'ENUM2' enum value to the 'region' response write-only property",
388+
HideFromChangelog: true,
389+
},
390+
},
391+
want: []*OasDiffEntry{
392+
{
393+
ID: "response-write-only-property-enum-value-added",
394+
OperationID: "op1",
395+
Text: "added the new 'ENUM1, ENUM2' enum values to the 'region' response write-only property",
396+
HideFromChangelog: true,
397+
},
398+
},
399+
},
400+
{
401+
name: "Mixed hidden and non-hidden entries",
402+
entries: []*OasDiffEntry{
403+
{
404+
ID: "response-write-only-property-enum-value-added",
405+
OperationID: "op1",
406+
Text: "added the new 'ENUM1' enum value to the 'region' response write-only property",
407+
},
408+
{
409+
ID: "response-write-only-property-enum-value-added",
410+
OperationID: "op1",
411+
Text: "added the new 'ENUM2' enum value to the 'region' response write-only property",
412+
HideFromChangelog: true,
413+
},
414+
{
415+
ID: "response-write-only-property-enum-value-added",
416+
OperationID: "op1",
417+
Text: "added the new 'ENUM3' enum value to the 'region' response write-only property",
418+
},
419+
{
420+
ID: "response-write-only-property-enum-value-added",
421+
OperationID: "op1",
422+
Text: "added the new 'ENUM4' enum value to the 'region' response write-only property",
423+
HideFromChangelog: true,
424+
},
425+
},
426+
want: []*OasDiffEntry{
427+
{
428+
ID: "response-write-only-property-enum-value-added",
429+
OperationID: "op1",
430+
Text: "added the new 'ENUM1, ENUM3' enum values to the 'region' response write-only property",
431+
},
432+
{
433+
ID: "response-write-only-property-enum-value-added",
434+
OperationID: "op1",
435+
Text: "added the new 'ENUM2, ENUM4' enum values to the 'region' response write-only property",
436+
HideFromChangelog: true,
437+
},
438+
},
439+
},
440+
}
441+
442+
for _, tc := range testCases {
443+
t.Run(tc.name, func(t *testing.T) {
444+
got, err := squashEntries(tc.entries)
445+
require.NoError(t, err)
446+
sortEntries(got)
447+
sortEntries(tc.want)
448+
require.Equal(t, tc.want, got)
449+
})
450+
}
451+
}
452+
453+
// sortEntries sorts the entries by their ID and OperationID.
454+
func sortEntries(entries []*OasDiffEntry) {
455+
sort.SliceStable(entries, func(i, j int) bool {
456+
if entries[i].ID != entries[j].ID {
457+
return entries[i].ID < entries[j].ID
458+
}
459+
return entries[i].OperationID < entries[j].OperationID
460+
})
461+
}

0 commit comments

Comments
 (0)