From 0b82932908503d13c22490696174886ea59d931d Mon Sep 17 00:00:00 2001 From: Bianca Lisle <40155621+blva@users.noreply.github.com> Date: Wed, 13 Nov 2024 10:35:13 +0000 Subject: [PATCH] CLOUDP-279336: Avoid that squashing overrides hideFromChangelog (#281) --- .../internal/changelog/outputfilter/squash.go | 52 +++- .../changelog/outputfilter/squash_test.go | 237 +++++++++++++++++- 2 files changed, 270 insertions(+), 19 deletions(-) diff --git a/tools/cli/internal/changelog/outputfilter/squash.go b/tools/cli/internal/changelog/outputfilter/squash.go index 75c0c6b63..92bbba0ce 100644 --- a/tools/cli/internal/changelog/outputfilter/squash.go +++ b/tools/cli/internal/changelog/outputfilter/squash.go @@ -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 } diff --git a/tools/cli/internal/changelog/outputfilter/squash_test.go b/tools/cli/internal/changelog/outputfilter/squash_test.go index 147ab6c45..b3ccb7bbc 100644 --- a/tools/cli/internal/changelog/outputfilter/squash_test.go +++ b/tools/cli/internal/changelog/outputfilter/squash_test.go @@ -2,6 +2,7 @@ package outputfilter import ( "reflect" + "sort" "testing" "github.com/stretchr/testify/require" @@ -9,14 +10,16 @@ import ( 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 + }) +}