Skip to content

Commit 52932c5

Browse files
blvablva
committed
Separate squashing of hidden and non-hidden entries
1 parent 1415585 commit 52932c5

File tree

2 files changed

+271
-19
lines changed

2 files changed

+271
-19
lines changed

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

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -112,44 +112,70 @@ func newSquashHandlers() []handler {
112112
}
113113

114114
// 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)
115+
func newEntriesMapPerIDAndOperationID(entries []*OasDiffEntry) (result, hiddenResult map[string]map[string][]*OasDiffEntry) {
116+
result = make(map[string]map[string][]*OasDiffEntry)
117+
hiddenResult = make(map[string]map[string][]*OasDiffEntry)
117118

118119
for _, entry := range entries {
119120
code := entry.ID
120121
operationID := entry.OperationID
122+
hidden := entry.HideFromChangelog
121123

122124
// Ensure the code map exists
123125
if _, exists := result[code]; !exists {
124126
result[code] = make(map[string][]*OasDiffEntry)
125127
}
126128

129+
if _, exists := hiddenResult[code]; !exists {
130+
hiddenResult[code] = make(map[string][]*OasDiffEntry)
131+
}
132+
133+
if hidden {
134+
// Append the entry to the appropriate operationID slice
135+
hiddenResult[code][operationID] = append(hiddenResult[code][operationID], entry)
136+
continue
137+
}
138+
127139
// Append the entry to the appropriate operationID slice
128140
result[code][operationID] = append(result[code][operationID], entry)
129141
}
130142

131-
return result
143+
return result, hiddenResult
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{}
138-
hidddenSquashedEntries := []*OasDiffEntry{}
139151

140152
for _, entry := range entries {
141153
// if no squash handlers implemented for entry's code,
142154
// just append the entry to the result
143155
if _, ok := findHandler(entry.ID); !ok {
144-
if entry.HideFromChangelog {
145-
hidddenSquashedEntries = append(hidddenSquashedEntries, entry)
146-
continue
147-
}
148156
squashedEntries = append(squashedEntries, entry)
149157
continue
150158
}
151159
}
152160

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{}
153179
for _, handler := range squashHandlers {
154180
entryMapPerOperationID, ok := entriesMap[handler.id]
155181
if !ok {
@@ -161,10 +187,8 @@ func squashEntries(entries []*OasDiffEntry) ([]*OasDiffEntry, error) {
161187
return nil, err
162188
}
163189

164-
squashedEntries = append(squashedEntries, hidddenSquashedEntries...)
165190
squashedEntries = append(squashedEntries, sortEntriesByDescription(entries)...)
166191
}
167-
168192
return squashedEntries, nil
169193
}
170194

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

Lines changed: 236 additions & 8 deletions
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,8 @@ func TestNewEntriesMapPerIDAndOperationID(t *testing.T) {
3033
},
3134
},
3235
},
36+
wantHidden: map[string]map[string][]*OasDiffEntry{
37+
"response-write-only-property-enum-value-added": {}},
3338
},
3439
{
3540
name: "Multiple entries with same ID",
@@ -47,8 +52,9 @@ func TestNewEntriesMapPerIDAndOperationID(t *testing.T) {
4752
},
4853
},
4954
},
55+
wantHidden: map[string]map[string][]*OasDiffEntry{
56+
"response-write-only-property-enum-value-added": {}},
5057
},
51-
5258
{
5359
name: "Multiple entries with same ID and OperationID",
5460
entries: []*OasDiffEntry{
@@ -63,6 +69,8 @@ func TestNewEntriesMapPerIDAndOperationID(t *testing.T) {
6369
},
6470
},
6571
},
72+
wantHidden: map[string]map[string][]*OasDiffEntry{
73+
"response-write-only-property-enum-value-added": {}},
6674
},
6775
{
6876
name: "Multiple entries with different IDs",
@@ -90,15 +98,70 @@ func TestNewEntriesMapPerIDAndOperationID(t *testing.T) {
9098
},
9199
},
92100
},
101+
wantHidden: map[string]map[string][]*OasDiffEntry{
102+
"response-write-only-property-enum-value-added": {},
103+
"request-write-only-property-enum-value-added": {},
104+
},
105+
},
106+
{
107+
name: "Hidden entries",
108+
entries: []*OasDiffEntry{
109+
{ID: "response-write-only-property-enum-value-added", OperationID: "op1", HideFromChangelog: true},
110+
{ID: "response-write-only-property-enum-value-added", OperationID: "op2", HideFromChangelog: true},
111+
},
112+
want: map[string]map[string][]*OasDiffEntry{
113+
"response-write-only-property-enum-value-added": {}},
114+
wantHidden: map[string]map[string][]*OasDiffEntry{
115+
"response-write-only-property-enum-value-added": {
116+
"op1": []*OasDiffEntry{
117+
{ID: "response-write-only-property-enum-value-added", OperationID: "op1", HideFromChangelog: true},
118+
},
119+
"op2": []*OasDiffEntry{
120+
{ID: "response-write-only-property-enum-value-added", OperationID: "op2", HideFromChangelog: true},
121+
},
122+
},
123+
},
124+
},
125+
{
126+
name: "Mixed hidden and non-hidden entries",
127+
entries: []*OasDiffEntry{
128+
{ID: "response-write-only-property-enum-value-added", OperationID: "op1"},
129+
{ID: "response-write-only-property-enum-value-added", OperationID: "op2", HideFromChangelog: true},
130+
{ID: "response-write-only-property-enum-value-added", OperationID: "op3"},
131+
{ID: "response-write-only-property-enum-value-added", OperationID: "op4", HideFromChangelog: true},
132+
},
133+
want: map[string]map[string][]*OasDiffEntry{
134+
"response-write-only-property-enum-value-added": {
135+
"op1": []*OasDiffEntry{
136+
{ID: "response-write-only-property-enum-value-added", OperationID: "op1"},
137+
},
138+
"op3": []*OasDiffEntry{
139+
{ID: "response-write-only-property-enum-value-added", OperationID: "op3"},
140+
},
141+
},
142+
},
143+
wantHidden: map[string]map[string][]*OasDiffEntry{
144+
"response-write-only-property-enum-value-added": {
145+
"op2": []*OasDiffEntry{
146+
{ID: "response-write-only-property-enum-value-added", OperationID: "op2", HideFromChangelog: true},
147+
},
148+
"op4": []*OasDiffEntry{
149+
{ID: "response-write-only-property-enum-value-added", OperationID: "op4", HideFromChangelog: true},
150+
},
151+
},
152+
},
93153
},
94154
}
95155

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

0 commit comments

Comments
 (0)