fix (utils) : [CRW-8603] Use slice of string instead of apimachinery Set to merge events

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

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)
+	}
+}

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.

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"

pkg/utils/overriding/test-fixtures/merges/multiple-post-start/plugin.yaml

@@ -0,0 +1,5 @@
+events:
+  postStart:
+    - "event-one"
+    - "event-two"
+    - "event-three"

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"

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
+}

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)
+			}
+		})
+	}
+}