From 3437662a5ff93dd635379f00d212c5ae24cbdb78 Mon Sep 17 00:00:00 2001 From: Keerthan Ekbote Date: Fri, 19 Apr 2024 09:36:32 -0400 Subject: [PATCH 1/6] shallow copy on sets --- changelog/v0.38.4/sets-shallowcopy.yaml | 6 ++++++ contrib/pkg/sets/v2/sets.go | 10 ++++++++++ 2 files changed, 16 insertions(+) create mode 100644 changelog/v0.38.4/sets-shallowcopy.yaml diff --git a/changelog/v0.38.4/sets-shallowcopy.yaml b/changelog/v0.38.4/sets-shallowcopy.yaml new file mode 100644 index 000000000..f9b5f08d4 --- /dev/null +++ b/changelog/v0.38.4/sets-shallowcopy.yaml @@ -0,0 +1,6 @@ +changelog: + - type: NON_USER_FACING + issueLink: + description: > + "" + skipCI: "false" diff --git a/contrib/pkg/sets/v2/sets.go b/contrib/pkg/sets/v2/sets.go index b86ad6319..1c8702586 100644 --- a/contrib/pkg/sets/v2/sets.go +++ b/contrib/pkg/sets/v2/sets.go @@ -67,6 +67,8 @@ type ResourceSet[T client.Object] interface { Generic() sk_sets.ResourceSet // Clone returns a deep copy of the set Clone() ResourceSet[T] + // ShallowCopy returns a shallow copy of the set + ShallowCopy() ResourceSet[T] } // ResourceDelta represents the set of changes between two ResourceSets. @@ -335,6 +337,14 @@ func (oldSet *resourceSet[T]) Clone() ResourceSet[T] { return new } +func (oldSet *resourceSet[T]) ShallowCopy() ResourceSet[T] { + newSet := make([]T, len(oldSet.set)) + copy(newSet, oldSet.set) + return &resourceSet[T]{ + set: newSet, + } +} + func (s *resourceSet[T]) Generic() sk_sets.ResourceSet { set := sk_sets.NewResourceSet(nil) s.Iter(func(_ int, v T) bool { From 99a595220d0daef5e7041c37178bfb69b9267202 Mon Sep 17 00:00:00 2001 From: Keerthan Ekbote Date: Fri, 19 Apr 2024 11:26:33 -0400 Subject: [PATCH 2/6] add unit tests --- contrib/tests/set_v2_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/contrib/tests/set_v2_test.go b/contrib/tests/set_v2_test.go index 7c700e555..60b1284ad 100644 --- a/contrib/tests/set_v2_test.go +++ b/contrib/tests/set_v2_test.go @@ -110,6 +110,22 @@ var _ = Describe("PaintSetV2", func() { }) }) + It("should shallow copy", func() { + newPaint := &v1.Paint{ + ObjectMeta: metav1.ObjectMeta{Name: "newPaint", Namespace: "newPaint"}, + } + setA.Insert(paintA, paintBCluster2, paintC, newPaint) + Expect(setA.Has(newPaint)).To(BeTrue()) + Expect(setA.Len()).To(Equal(4)) + + setB = setA.ShallowCopy() + Expect(setB.Has(newPaint)).To(BeTrue()) + np := setA.Get(newPaint) + np.Name = "newPaintWithNewName" + Expect(setB.Len()).To(Equal(4)) + Expect(setB.Get(newPaint).Name).To(Equal("newPaintWithNewName")) + }) + It("should double filter List", func() { setA.Insert(paintA, paintBCluster2, paintC) Expect(setA.Has(paintA)).To(BeTrue()) From 5b9a01d0abc5a5b78895f581191f07934718407a Mon Sep 17 00:00:00 2001 From: Keerthan Ekbote Date: Fri, 19 Apr 2024 11:38:19 -0400 Subject: [PATCH 3/6] check for same pointers in shallow copy --- contrib/tests/set_v2_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/contrib/tests/set_v2_test.go b/contrib/tests/set_v2_test.go index 60b1284ad..0e61d40f5 100644 --- a/contrib/tests/set_v2_test.go +++ b/contrib/tests/set_v2_test.go @@ -123,7 +123,9 @@ var _ = Describe("PaintSetV2", func() { np := setA.Get(newPaint) np.Name = "newPaintWithNewName" Expect(setB.Len()).To(Equal(4)) - Expect(setB.Get(newPaint).Name).To(Equal("newPaintWithNewName")) + npShouldBeExactSame := setB.Get(newPaint) + Expect(npShouldBeExactSame.Name).To(Equal("newPaintWithNewName")) + Expect(npShouldBeExactSame == np).To(BeTrue()) }) It("should double filter List", func() { From 03b8b9e1d2bcd430a816be67b36c94cc9743dbf2 Mon Sep 17 00:00:00 2001 From: Keerthan Ekbote Date: Fri, 19 Apr 2024 11:42:04 -0400 Subject: [PATCH 4/6] update --- contrib/tests/set_v2_test.go | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/contrib/tests/set_v2_test.go b/contrib/tests/set_v2_test.go index 0e61d40f5..a4d0d4131 100644 --- a/contrib/tests/set_v2_test.go +++ b/contrib/tests/set_v2_test.go @@ -120,12 +120,22 @@ var _ = Describe("PaintSetV2", func() { setB = setA.ShallowCopy() Expect(setB.Has(newPaint)).To(BeTrue()) - np := setA.Get(newPaint) - np.Name = "newPaintWithNewName" - Expect(setB.Len()).To(Equal(4)) - npShouldBeExactSame := setB.Get(newPaint) - Expect(npShouldBeExactSame.Name).To(Equal("newPaintWithNewName")) - Expect(npShouldBeExactSame == np).To(BeTrue()) + // so sorry for this n^2 comparison, + // want to make sure that the pointers are the same in both sets but have no better way to do it + setB.Iter(func(i int, p *v1.Paint) bool { + setA.Iter(func(j int, p2 *v1.Paint) bool { + if i == j { + Expect(p == p2).To(BeTrue()) + } + return true + }) + }) + // np := setA.Get(newPaint) + // np.Name = "newPaintWithNewName" + // Expect(setB.Len()).To(Equal(4)) + // npShouldBeExactSame := setB.Get(newPaint) + // Expect(npShouldBeExactSame.Name).To(Equal("newPaintWithNewName")) + // Expect(npShouldBeExactSame == np).To(BeTrue()) }) It("should double filter List", func() { From 4700dbd7ed345548c765882dfd34902ed302c983 Mon Sep 17 00:00:00 2001 From: Keerthan Ekbote Date: Fri, 19 Apr 2024 11:42:43 -0400 Subject: [PATCH 5/6] update --- contrib/tests/set_v2_test.go | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/contrib/tests/set_v2_test.go b/contrib/tests/set_v2_test.go index a4d0d4131..68e96b238 100644 --- a/contrib/tests/set_v2_test.go +++ b/contrib/tests/set_v2_test.go @@ -1,6 +1,8 @@ package tests_test import ( + "fmt" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" v1 "github.com/solo-io/skv2/codegen/test/api/things.test.io/v1" @@ -120,8 +122,8 @@ var _ = Describe("PaintSetV2", func() { setB = setA.ShallowCopy() Expect(setB.Has(newPaint)).To(BeTrue()) - // so sorry for this n^2 comparison, - // want to make sure that the pointers are the same in both sets but have no better way to do it + // want to make sure that the pointers are the same in both sets + // without having to construct a new list, so we just iterate setB.Iter(func(i int, p *v1.Paint) bool { setA.Iter(func(j int, p2 *v1.Paint) bool { if i == j { @@ -129,13 +131,8 @@ var _ = Describe("PaintSetV2", func() { } return true }) + return true }) - // np := setA.Get(newPaint) - // np.Name = "newPaintWithNewName" - // Expect(setB.Len()).To(Equal(4)) - // npShouldBeExactSame := setB.Get(newPaint) - // Expect(npShouldBeExactSame.Name).To(Equal("newPaintWithNewName")) - // Expect(npShouldBeExactSame == np).To(BeTrue()) }) It("should double filter List", func() { From fcc46bac6edbb56df7c89de87586753ac3532ccf Mon Sep 17 00:00:00 2001 From: Keerthan Ekbote Date: Fri, 19 Apr 2024 11:42:55 -0400 Subject: [PATCH 6/6] goimports --- contrib/tests/set_v2_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/contrib/tests/set_v2_test.go b/contrib/tests/set_v2_test.go index 68e96b238..801519aef 100644 --- a/contrib/tests/set_v2_test.go +++ b/contrib/tests/set_v2_test.go @@ -1,8 +1,6 @@ package tests_test import ( - "fmt" - . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" v1 "github.com/solo-io/skv2/codegen/test/api/things.test.io/v1"