diff --git a/pkg/utils/overriding/merging.go b/pkg/utils/overriding/merging.go index eb1aec164..9b784a248 100644 --- a/pkg/utils/overriding/merging.go +++ b/pkg/utils/overriding/merging.go @@ -108,19 +108,19 @@ func MergeDevWorkspaceTemplateSpec( } } - preStartCommands := sets.String{} - postStartCommands := sets.String{} - preStopCommands := sets.String{} - postStopCommands := sets.String{} + var preStartCommands []string + var postStartCommands []string + var preStopCommands []string + var postStopCommands []string for _, content := range allContents { if content.Events != nil { if result.Events == nil { result.Events = &dw.Events{} } - preStartCommands = preStartCommands.Union(sets.NewString(content.Events.PreStart...)) - postStartCommands = postStartCommands.Union(sets.NewString(content.Events.PostStart...)) - preStopCommands = preStopCommands.Union(sets.NewString(content.Events.PreStop...)) - postStopCommands = postStopCommands.Union(sets.NewString(content.Events.PostStop...)) + preStartCommands = UnionStrings(preStartCommands, content.Events.PreStart) + postStartCommands = UnionStrings(postStartCommands, content.Events.PostStart) + preStopCommands = UnionStrings(preStopCommands, content.Events.PreStop) + postStopCommands = UnionStrings(postStopCommands, content.Events.PostStop) } if len(content.Variables) > 0 { @@ -147,10 +147,10 @@ func MergeDevWorkspaceTemplateSpec( } if result.Events != nil { - result.Events.PreStart = preStartCommands.List() - result.Events.PostStart = postStartCommands.List() - result.Events.PreStop = preStopCommands.List() - result.Events.PostStop = postStopCommands.List() + result.Events.PreStart = preStartCommands + result.Events.PostStart = postStartCommands + result.Events.PreStop = preStopCommands + result.Events.PostStop = postStopCommands } return &result, nil diff --git a/pkg/utils/overriding/merging_test.go b/pkg/utils/overriding/merging_test.go index 5430b8577..71c8f4ea4 100644 --- a/pkg/utils/overriding/merging_test.go +++ b/pkg/utils/overriding/merging_test.go @@ -134,3 +134,24 @@ func TestMergingOnlyParent(t *testing.T) { assert.Equal(t, &expectedDWT, gotDWT) } } + +func TestMergePostStartInOrder(t *testing.T) { + // Given + baseFile := "test-fixtures/merges/multiple-post-start/main.yaml" + parentFile := "test-fixtures/merges/multiple-post-start/plugin.yaml" + resultFile := "test-fixtures/merges/multiple-post-start/result.yaml" + + baseDWT := dw.DevWorkspaceTemplateSpecContent{} + parentDWT := dw.DevWorkspaceTemplateSpecContent{} + expectedDWT := dw.DevWorkspaceTemplateSpecContent{} + + readFileToStruct(t, baseFile, &baseDWT) + readFileToStruct(t, parentFile, &parentDWT) + readFileToStruct(t, resultFile, &expectedDWT) + + // When + Then + gotDWT, err := MergeDevWorkspaceTemplateSpec(&baseDWT, &parentDWT) + if assert.NoError(t, err) { + assert.Equal(t, &expectedDWT, gotDWT) + } +} diff --git a/pkg/utils/overriding/test-fixtures/merges/events-merge/result.yaml b/pkg/utils/overriding/test-fixtures/merges/events-merge/result.yaml index 17052f8f3..e20764c4d 100644 --- a/pkg/utils/overriding/test-fixtures/merges/events-merge/result.yaml +++ b/pkg/utils/overriding/test-fixtures/merges/events-merge/result.yaml @@ -1,32 +1,31 @@ events: preStart: - - "preStartFromMainContent" - "preStartFromParent" - "preStartFromPlugin" + - "preStartFromMainContent" preStop: - - "preStopFromMainContent" - "preStopFromParent" - "preStopFromPlugin" + - "preStopFromMainContent" postStart: - - "postStartFromMainContent" - "postStartFromParent" - "postStartFromPlugin" + - "postStartFromMainContent" postStop: - - "postStopFromMainContent" - "postStopFromParent" - "postStopFromPlugin" + - "postStopFromMainContent" # Note: # # The command Id are merged *per-event type* # from the commands of the corresponding event type -# in parent, plugins and devfile main content. +# in parent, plugins and devfile main content. # -# The command Ids in each event type are unordered sets. -# Only event types are ordered; no guarantee is provided that -# Commands inside an event type will be executed in the order -# they appear in the list. +# The command Ids in each event type are string slices. +# While merging original insertion order of events would +# be preserved. # # However in the test results, as it will happen usually, -# command ids of a given event type will simply be ordered -# in alphabetical order. +# command ids of a given event type will be merged preserving +# insertion order in source devfiles. diff --git a/pkg/utils/overriding/test-fixtures/merges/multiple-post-start/main.yaml b/pkg/utils/overriding/test-fixtures/merges/multiple-post-start/main.yaml new file mode 100644 index 000000000..e86859a86 --- /dev/null +++ b/pkg/utils/overriding/test-fixtures/merges/multiple-post-start/main.yaml @@ -0,0 +1,9 @@ +parent: + uri: "anyParent" +components: + - plugin: + uri: "aCustomLocation" + name: "the-only-plugin" +events: + postStart: + - "init-che-code-command" diff --git a/pkg/utils/overriding/test-fixtures/merges/multiple-post-start/plugin.yaml b/pkg/utils/overriding/test-fixtures/merges/multiple-post-start/plugin.yaml new file mode 100644 index 000000000..68bb26486 --- /dev/null +++ b/pkg/utils/overriding/test-fixtures/merges/multiple-post-start/plugin.yaml @@ -0,0 +1,5 @@ +events: + postStart: + - "event-one" + - "event-two" + - "event-three" diff --git a/pkg/utils/overriding/test-fixtures/merges/multiple-post-start/result.yaml b/pkg/utils/overriding/test-fixtures/merges/multiple-post-start/result.yaml new file mode 100644 index 000000000..dcb4bd2ad --- /dev/null +++ b/pkg/utils/overriding/test-fixtures/merges/multiple-post-start/result.yaml @@ -0,0 +1,9 @@ +events: + preStart: [] + preStop: [] + postStop: [] + postStart: + - "event-one" + - "event-two" + - "event-three" + - "init-che-code-command" diff --git a/pkg/utils/overriding/utils.go b/pkg/utils/overriding/utils.go index 9fd1e7700..5a9e9fdb3 100644 --- a/pkg/utils/overriding/utils.go +++ b/pkg/utils/overriding/utils.go @@ -32,3 +32,34 @@ func handleUnmarshal(j []byte) (map[string]interface{}, error) { } return m, nil } + +// UnionStrings returns the union of two string slices, preserving the order +// of first occurrence and removing duplicates. +// +// Elements from the first slice `a` are added in order. Elements from the second +// slice `b` are appended only if they do not already exist in `a`. +// +// The returned slice is guaranteed to be non-nil, even if both input slices are nil. +// +// Example: +// +// UnionStrings([]string{"a", "b"}, []string{"b", "c"}) +// // Returns: []string{"a", "b", "c"} +func UnionStrings(a, b []string) []string { + alreadyExistsInList := make(map[string]bool) + result := make([]string, 0) + + for _, s := range a { + if !alreadyExistsInList[s] { + alreadyExistsInList[s] = true + result = append(result, s) + } + } + for _, s := range b { + if !alreadyExistsInList[s] { + alreadyExistsInList[s] = true + result = append(result, s) + } + } + return result +} diff --git a/pkg/utils/overriding/utils_test.go b/pkg/utils/overriding/utils_test.go new file mode 100644 index 000000000..b35774ec4 --- /dev/null +++ b/pkg/utils/overriding/utils_test.go @@ -0,0 +1,100 @@ +// +// +// Copyright Red Hat +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package overriding + +import ( + "reflect" + "testing" +) + +func TestUnionStrings(t *testing.T) { + tests := []struct { + name string + a []string + b []string + expected []string + }{ + { + name: "both empty", + a: []string{}, + b: []string{}, + expected: []string{}, + }, + { + name: "first is nil, second is empty", + a: nil, + b: []string{}, + expected: []string{}, + }, + { + name: "first is empty, second is nil", + a: []string{}, + b: nil, + expected: []string{}, + }, + { + name: "both are nil", + a: nil, + b: nil, + expected: []string{}, + }, + { + name: "no overlap", + a: []string{"x", "y"}, + b: []string{"a", "b"}, + expected: []string{"x", "y", "a", "b"}, + }, + { + name: "partial overlap", + a: []string{"a", "b"}, + b: []string{"b", "c"}, + expected: []string{"a", "b", "c"}, + }, + { + name: "duplicate in same slice", + a: []string{"a", "a", "b"}, + b: []string{"b", "c"}, + expected: []string{"a", "b", "c"}, + }, + { + name: "only duplicates", + a: []string{"a", "b"}, + b: []string{"a", "b"}, + expected: []string{"a", "b"}, + }, + { + name: "preserves order", + a: []string{"c", "a"}, + b: []string{"b", "a", "d"}, + expected: []string{"c", "a", "b", "d"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + actual := UnionStrings(tt.a, tt.b) + + if actual == nil { + t.Errorf("expected non-nil slice, got nil") + } + + if !reflect.DeepEqual(actual, tt.expected) { + t.Errorf("UnionStrings(%v, %v) = %v; want %v", tt.a, tt.b, actual, tt.expected) + } + }) + } +}