Skip to content

Commit

Permalink
CLOUDP-279336: Avoid that squashing overrides hideFromChangelog (#281)
Browse files Browse the repository at this point in the history
blva authored Nov 13, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent 37be35b commit 0b82932
Showing 2 changed files with 270 additions and 19 deletions.
52 changes: 41 additions & 11 deletions tools/cli/internal/changelog/outputfilter/squash.go
Original file line number Diff line number Diff line change
@@ -111,28 +111,41 @@ func newSquashHandlers() []handler {
}
}

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

for _, entry := range entries {
code := entry.ID
operationID := entry.OperationID
hidden := entry.HideFromChangelog

// Ensure the code map exists
if _, exists := result[code]; !exists {
result[code] = make(map[string][]*OasDiffEntry)
if hidden {
addToMap(hiddenResult, code, operationID, entry)
continue
}

// Append the entry to the appropriate operationID slice
result[code][operationID] = append(result[code][operationID], entry)
addToMap(result, code, operationID, entry)
}

return result, hiddenResult
}

func addToMap(m map[string]map[string][]*OasDiffEntry, code, operationID string, entry *OasDiffEntry) {
// Ensure the code map exists
if _, exists := m[code]; !exists {
m[code] = make(map[string][]*OasDiffEntry)
}

return result
// Append the entry to the appropriate operationID slice
m[code][operationID] = append(m[code][operationID], entry)
}

func squashEntries(entries []*OasDiffEntry) ([]*OasDiffEntry, error) {
entriesMap := newEntriesMapPerIDAndOperationID(entries)
entriesByIDandOperationID, hiddenEntriesByIDandOperationID := newEntriesMapPerIDAndOperationID(entries)

squashHandlers := newSquashHandlers()
squashedEntries := []*OasDiffEntry{}

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

squashedEntriesNotHidden, err := appplySquashHandlerToMap(squashHandlers, entriesByIDandOperationID)
if err != nil {
return nil, err
}

squashedEntriesHidden, err := appplySquashHandlerToMap(squashHandlers, hiddenEntriesByIDandOperationID)
if err != nil {
return nil, err
}

squashedEntries = append(squashedEntries, squashedEntriesNotHidden...)
squashedEntries = append(squashedEntries, squashedEntriesHidden...)

return squashedEntries, nil
}

func appplySquashHandlerToMap(squashHandlers []handler, entriesMap map[string]map[string][]*OasDiffEntry) ([]*OasDiffEntry, error) {
squashedEntries := []*OasDiffEntry{}
for _, handler := range squashHandlers {
entryMapPerOperationID, ok := entriesMap[handler.id]
if !ok {
@@ -158,7 +189,6 @@ func squashEntries(entries []*OasDiffEntry) ([]*OasDiffEntry, error) {

squashedEntries = append(squashedEntries, sortEntriesByDescription(entries)...)
}

return squashedEntries, nil
}

237 changes: 229 additions & 8 deletions tools/cli/internal/changelog/outputfilter/squash_test.go
Original file line number Diff line number Diff line change
@@ -2,21 +2,24 @@ package outputfilter

import (
"reflect"
"sort"
"testing"

"github.com/stretchr/testify/require"
)

func TestNewEntriesMapPerIDAndOperationID(t *testing.T) {
testCases := []struct {
name string
entries []*OasDiffEntry
want map[string]map[string][]*OasDiffEntry
name string
entries []*OasDiffEntry
want map[string]map[string][]*OasDiffEntry
wantHidden map[string]map[string][]*OasDiffEntry
}{
{
name: "Empty entries",
entries: []*OasDiffEntry{},
want: map[string]map[string][]*OasDiffEntry{},
name: "Empty entries",
entries: []*OasDiffEntry{},
want: map[string]map[string][]*OasDiffEntry{},
wantHidden: map[string]map[string][]*OasDiffEntry{},
},
{
name: "Single entry",
@@ -30,6 +33,7 @@ func TestNewEntriesMapPerIDAndOperationID(t *testing.T) {
},
},
},
wantHidden: map[string]map[string][]*OasDiffEntry{},
},
{
name: "Multiple entries with same ID",
@@ -47,8 +51,8 @@ func TestNewEntriesMapPerIDAndOperationID(t *testing.T) {
},
},
},
wantHidden: map[string]map[string][]*OasDiffEntry{},
},

{
name: "Multiple entries with same ID and OperationID",
entries: []*OasDiffEntry{
@@ -63,6 +67,7 @@ func TestNewEntriesMapPerIDAndOperationID(t *testing.T) {
},
},
},
wantHidden: map[string]map[string][]*OasDiffEntry{},
},
{
name: "Multiple entries with different IDs",
@@ -90,15 +95,66 @@ func TestNewEntriesMapPerIDAndOperationID(t *testing.T) {
},
},
},
wantHidden: map[string]map[string][]*OasDiffEntry{},
},
{
name: "Hidden entries",
entries: []*OasDiffEntry{
{ID: "response-write-only-property-enum-value-added", OperationID: "op1", HideFromChangelog: true},
{ID: "response-write-only-property-enum-value-added", OperationID: "op2", HideFromChangelog: true},
},
want: map[string]map[string][]*OasDiffEntry{},
wantHidden: map[string]map[string][]*OasDiffEntry{
"response-write-only-property-enum-value-added": {
"op1": []*OasDiffEntry{
{ID: "response-write-only-property-enum-value-added", OperationID: "op1", HideFromChangelog: true},
},
"op2": []*OasDiffEntry{
{ID: "response-write-only-property-enum-value-added", OperationID: "op2", HideFromChangelog: true},
},
},
},
},
{
name: "Mixed hidden and non-hidden entries",
entries: []*OasDiffEntry{
{ID: "response-write-only-property-enum-value-added", OperationID: "op1"},
{ID: "response-write-only-property-enum-value-added", OperationID: "op2", HideFromChangelog: true},
{ID: "response-write-only-property-enum-value-added", OperationID: "op3"},
{ID: "response-write-only-property-enum-value-added", OperationID: "op4", HideFromChangelog: true},
},
want: map[string]map[string][]*OasDiffEntry{
"response-write-only-property-enum-value-added": {
"op1": []*OasDiffEntry{
{ID: "response-write-only-property-enum-value-added", OperationID: "op1"},
},
"op3": []*OasDiffEntry{
{ID: "response-write-only-property-enum-value-added", OperationID: "op3"},
},
},
},
wantHidden: map[string]map[string][]*OasDiffEntry{
"response-write-only-property-enum-value-added": {
"op2": []*OasDiffEntry{
{ID: "response-write-only-property-enum-value-added", OperationID: "op2", HideFromChangelog: true},
},
"op4": []*OasDiffEntry{
{ID: "response-write-only-property-enum-value-added", OperationID: "op4", HideFromChangelog: true},
},
},
},
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
got := newEntriesMapPerIDAndOperationID(tc.entries)
got, gotHidden := newEntriesMapPerIDAndOperationID(tc.entries)
if !reflect.DeepEqual(got, tc.want) {
t.Errorf("got %v, want %v", got, tc.want)
}
if !reflect.DeepEqual(gotHidden, tc.wantHidden) {
t.Errorf("gotHidden %v, wantHidden %v", gotHidden, tc.wantHidden)
}
})
}
}
@@ -238,3 +294,168 @@ func TestNewSquashMap(t *testing.T) {
})
}
}

func TestSquashEntries(t *testing.T) {
testCases := []struct {
name string
entries []*OasDiffEntry
want []*OasDiffEntry
}{
{
name: "Empty entries",
entries: []*OasDiffEntry{},
want: []*OasDiffEntry{},
},
{
name: "Single entry",
entries: []*OasDiffEntry{
{
ID: "response-write-only-property-enum-value-added",
OperationID: "op1",
Text: "added the new 'ENUM1' enum value to the 'region' response write-only property",
},
},
want: []*OasDiffEntry{
{
ID: "response-write-only-property-enum-value-added",
OperationID: "op1",
Text: "added the new 'ENUM1' enum value to the 'region' response write-only property",
},
},
},
{
name: "Multiple entries with same ID and OperationID",
entries: []*OasDiffEntry{
{
ID: "response-write-only-property-enum-value-added",
OperationID: "op1",
Text: "added the new 'ENUM1' enum value to the 'region' response write-only property",
},
{
ID: "response-write-only-property-enum-value-added",
OperationID: "op1",
Text: "added the new 'ENUM2' enum value to the 'region' response write-only property",
},
},
want: []*OasDiffEntry{
{
ID: "response-write-only-property-enum-value-added",
OperationID: "op1",
Text: "added the new 'ENUM1, ENUM2' enum values to the 'region' response write-only property",
},
},
},
{
name: "Multiple entries with different IDs",
entries: []*OasDiffEntry{
{
ID: "response-write-only-property-enum-value-added",
OperationID: "op1",
Text: "added the new 'ENUM1' enum value to the 'region' response write-only property",
},
{
ID: "request-write-only-property-enum-value-added",
OperationID: "op2",
Text: "added the new 'ENUM2' enum value to the 'region' request write-only property",
},
},
want: []*OasDiffEntry{
{
ID: "response-write-only-property-enum-value-added",
OperationID: "op1",
Text: "added the new 'ENUM1' enum value to the 'region' response write-only property",
},
{
ID: "request-write-only-property-enum-value-added",
OperationID: "op2",
Text: "added the new 'ENUM2' enum value to the 'region' request write-only property",
},
},
},
{
name: "Hidden entries",
entries: []*OasDiffEntry{
{
ID: "response-write-only-property-enum-value-added",
OperationID: "op1",
Text: "added the new 'ENUM1' enum value to the 'region' response write-only property",
HideFromChangelog: true,
},
{
ID: "response-write-only-property-enum-value-added",
OperationID: "op1",
Text: "added the new 'ENUM2' enum value to the 'region' response write-only property",
HideFromChangelog: true,
},
},
want: []*OasDiffEntry{
{
ID: "response-write-only-property-enum-value-added",
OperationID: "op1",
Text: "added the new 'ENUM1, ENUM2' enum values to the 'region' response write-only property",
HideFromChangelog: true,
},
},
},
{
name: "Mixed hidden and non-hidden entries",
entries: []*OasDiffEntry{
{
ID: "response-write-only-property-enum-value-added",
OperationID: "op1",
Text: "added the new 'ENUM1' enum value to the 'region' response write-only property",
},
{
ID: "response-write-only-property-enum-value-added",
OperationID: "op1",
Text: "added the new 'ENUM2' enum value to the 'region' response write-only property",
HideFromChangelog: true,
},
{
ID: "response-write-only-property-enum-value-added",
OperationID: "op1",
Text: "added the new 'ENUM3' enum value to the 'region' response write-only property",
},
{
ID: "response-write-only-property-enum-value-added",
OperationID: "op1",
Text: "added the new 'ENUM4' enum value to the 'region' response write-only property",
HideFromChangelog: true,
},
},
want: []*OasDiffEntry{
{
ID: "response-write-only-property-enum-value-added",
OperationID: "op1",
Text: "added the new 'ENUM1, ENUM3' enum values to the 'region' response write-only property",
},
{
ID: "response-write-only-property-enum-value-added",
OperationID: "op1",
Text: "added the new 'ENUM2, ENUM4' enum values to the 'region' response write-only property",
HideFromChangelog: true,
},
},
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
got, err := squashEntries(tc.entries)
require.NoError(t, err)
sortEntries(got)
sortEntries(tc.want)
require.Equal(t, tc.want, got)
})
}
}

// sortEntries sorts the entries by their ID and OperationID.
func sortEntries(entries []*OasDiffEntry) {
sort.SliceStable(entries, func(i, j int) bool {
if entries[i].ID != entries[j].ID {
return entries[i].ID < entries[j].ID
}
return entries[i].OperationID < entries[j].OperationID
})
}

0 comments on commit 0b82932

Please sign in to comment.