Sai/efficient union (#561)

* efficient union

* update

* nil check on union

* comment code and add fallback in case argument set in Union is not sorted

* add more tests for union

* update to ensure sets are kept in sorted order

* update changelog location

* update changelog location

* change issortedby method

* remove issortedby

* go fmt

changelog/v0.39.1/efficient-union.yaml

@@ -0,0 +1,5 @@
+changelog:
+  - type: NON_USER_FACING
+    description: >
+      "Makes the `Union` method in v2 sets more efficient. If the input set is sorted by resource ID, we can take advantage of this to 
+      do a more efficient Union while keeping the Unioned set in sorted order. 

contrib/pkg/sets/v2/sets.go

@@ -229,13 +229,48 @@ func (s *resourceSet[T]) Delete(resource ezkube.ResourceId) {
 }
 
 func (s *resourceSet[T]) Union(set ResourceSet[T]) ResourceSet[T] {
-	list := []T{}
-	for _, resource := range s.Generic().Union(set.Generic()).List() {
-		list = append(list, resource.(T))
+
+	if s == nil && set == nil {
+		return NewResourceSet[T]()
 	}
-	return NewResourceSet[T](
-		list...,
-	)
+	if s == nil {
+		return set.ShallowCopy()
+	}
+	if set == nil {
+		return s.ShallowCopy()
+	}
+
+	return s.unionSortedSet(set)
+}
+
+// Assuming that the argument set is sorted by resource id,
+// this method will efficiently union the two sets together and return the unioned set.
+func (s *resourceSet[T]) unionSortedSet(set ResourceSet[T]) *resourceSet[T] {
+	merged := make([]T, 0, len(s.set)+set.Len())
+	idx := 0
+
+	// Iterate through the second set
+	set.Iter(func(_ int, resource T) bool {
+		// Ensure all elements from s.set are added in sorted order
+		for idx < len(s.set) && ezkube.ResourceIdsCompare(s.set[idx], resource) < 0 {
+			merged = append(merged, s.set[idx])
+			idx++
+		}
+		// If elements are equal, skip the element from s.set and use the element from the argument set
+		if idx < len(s.set) && ezkube.ResourceIdsCompare(s.set[idx], resource) == 0 {
+			idx++ // Increment to skip the element in s.set since it's equal and we use the one from set
+		}
+		// Add the current element from the second set
+		merged = append(merged, resource)
+		return true
+	})
+
+	// Append any remaining elements from s.set that were not added
+	if idx < len(s.set) {
+		merged = append(merged, s.set[idx:]...)
+	}
+
+	return &resourceSet[T]{set: merged}
 }
 
 func (s *resourceSet[T]) Difference(set ResourceSet[T]) ResourceSet[T] {

contrib/tests/set_v2_test.go

@@ -301,4 +301,123 @@ var _ = Describe("PaintSetV2", func() {
 			Expect(paintList2[i]).To(Equal(paint))
 		}
 	})
+	Context("Union", func() {
+		var (
+			setA, setB, setC sets_v2.ResourceSet[*v1.Paint]
+			paintA           *v1.Paint
+			paintB           *v1.Paint
+		)
+		BeforeEach(func() {
+			setA = sets_v2.NewResourceSet[*v1.Paint]()
+			setB = sets_v2.NewResourceSet[*v1.Paint]()
+			setC = sets_v2.NewResourceSet[*v1.Paint]()
+			paintA = &v1.Paint{
+				ObjectMeta: metav1.ObjectMeta{Name: "nameA", Namespace: "nsA"},
+			}
+			paintB = &v1.Paint{
+				ObjectMeta: metav1.ObjectMeta{Name: "nameB", Namespace: "nsB"},
+			}
+		})
+		It("should correctly perform union of a set with itself", func() {
+			setA.Insert(paintA)
+			unionSet := setA.Union(setA)
+			Expect(unionSet.Len()).To(Equal(1))
+			Expect(unionSet.Has(paintA)).To(BeTrue())
+		})
+
+		It("should correctly perform union of two distinct sets", func() {
+			setA.Insert(paintA)
+			setB.Insert(paintB)
+			unionSet := setA.Union(setB)
+			Expect(unionSet.Len()).To(Equal(2))
+			Expect(unionSet.Has(paintA)).To(BeTrue())
+			Expect(unionSet.Has(paintB)).To(BeTrue())
+		})
+
+		It("should handle union with an empty set", func() {
+			setA.Insert(paintA)
+			unionSet := setA.Union(setC) // setC is empty
+			Expect(unionSet.Len()).To(Equal(1))
+			Expect(unionSet.Has(paintA)).To(BeTrue())
+		})
+
+		It("should return an empty set when unioning two empty sets", func() {
+			unionSet := setC.Union(setB) // both setC and setB are empty
+			Expect(unionSet.Len()).To(Equal(0))
+		})
+
+		It("should maintain distinct elements when unioning sets with overlap", func() {
+			setA.Insert(paintA)
+			setB.Insert(paintA, paintB)
+			unionSet := setA.Union(setB)
+			Expect(unionSet.Len()).To(Equal(2))
+			Expect(unionSet.Has(paintA)).To(BeTrue())
+			Expect(unionSet.Has(paintB)).To(BeTrue())
+		})
+
+		It("should be commutative (A union B = B union A)", func() {
+			setA.Insert(paintA)
+			setB.Insert(paintB)
+			unionSetAB := setA.Union(setB)
+			unionSetBA := setB.Union(setA)
+			Expect(unionSetAB.Len()).To(Equal(unionSetBA.Len()))
+			Expect(unionSetAB.Has(paintA) && unionSetAB.Has(paintB)).To(BeTrue())
+			Expect(unionSetBA.Has(paintA) && unionSetBA.Has(paintB)).To(BeTrue())
+		})
+
+		Context("Sorted Order Preservation after Union", func() {
+			var (
+				setA, setB, unionSet   sets_v2.ResourceSet[*v1.Paint]
+				paint1, paint2, paint3 *v1.Paint
+			)
+
+			BeforeEach(func() {
+				setA = sets_v2.NewResourceSet[*v1.Paint]()
+				setB = sets_v2.NewResourceSet[*v1.Paint]()
+				paint1 = &v1.Paint{
+					ObjectMeta: metav1.ObjectMeta{Name: "C", Namespace: "1"},
+				}
+				paint2 = &v1.Paint{
+					ObjectMeta: metav1.ObjectMeta{Name: "A", Namespace: "3"},
+				}
+				paint3 = &v1.Paint{
+					ObjectMeta: metav1.ObjectMeta{Name: "B", Namespace: "2"},
+				}
+				setA.Insert(paint1)
+				setB.Insert(paint2, paint3)
+				unionSet = setA.Union(setB)
+			})
+
+			It("should maintain sorted order in Iter after union", func() {
+				expectedOrder := []*v1.Paint{paint2, paint3, paint1} // Expected sorted by namespace
+				var actualOrder []*v1.Paint
+				unionSet.Iter(func(_ int, p *v1.Paint) bool {
+					actualOrder = append(actualOrder, p)
+					return true
+				})
+				Expect(actualOrder).To(Equal(expectedOrder))
+			})
+
+			It("should maintain sorted order in Filter after union", func() {
+				var filteredOrder []*v1.Paint
+				filterFunc := func(p *v1.Paint) bool {
+					return true // Select all
+				}
+				unionSet.Filter(filterFunc)(func(_ int, p *v1.Paint) bool {
+					filteredOrder = append(filteredOrder, p)
+					return true
+				})
+				Expect(filteredOrder).To(Equal([]*v1.Paint{paint2, paint3, paint1})) // Should match expected sorted order
+			})
+
+			It("should maintain sorted order in FilterOutAndCreateList after union", func() {
+				filterOutFunc := func(p *v1.Paint) bool {
+					return false // Do not filter out any items
+				}
+				filteredList := unionSet.FilterOutAndCreateList(filterOutFunc)
+				Expect(filteredList).To(Equal([]*v1.Paint{paint2, paint3, paint1})) // Should match expected sorted order
+			})
+		})
+
+	})
 })

pkg/ezkube/resource_id.go

@@ -174,9 +174,10 @@ func ResourceIdFromKeyWithSeparator(key string, separator string) (ResourceId, e
 	}
 }
 
-// ResourceIdsCompare returns an integer comparing two ResourceIds lexicographically.
-// The comparison is based on name, then namespace, then cluster name.
-// The result will be 0 if a == b, -1 if a < b, and +1 if a > b.
+// ResourceIdsCompare compares two ResourceId instances, first by name, then by namespace, and finally by cluster name.
+// If the names or namespaces differ, the comparison returns the result of strings.Compare on those values.
+// If the names and namespaces are the same, it attempts to cast the ResourceId instances to ClusterResourceId
+// and compares the cluster names. If the cast fails, it falls back to using the deprecated cluster name retrieval.
 func ResourceIdsCompare(a, b ResourceId) int {
 	// compare names
 	if cmp := strings.Compare(a.GetName(), b.GetName()); cmp != 0 {