Skip to content

chore: refactor containers to use generics #1535

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+42 −195
Open

chore: refactor containers to use generics #1535

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 20 additions & 9 deletions internal/legacy/container/set.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,25 @@ limitations under the License.
package container

// Set is a basic set-like data structure.
type Set map[Element]interface{}
type Set[K comparable] map[K]struct{}

// Element is a singleton item that goes with Set.
type Element interface{}
// NewSet contructs a Set with the specified items.
func NewSet[K comparable](items ...K) Set[K] {
s := make(Set[K])
for _, item := range items {
s[item] = struct{}{}
}
return s
}

// Insert inserts an item into the set.
func (s Set[K]) Insert(item K) {
s[item] = struct{}{}
}

// Minus returns a new set, by subtracting everything in b from a.
func (a Set) Minus(b Set) Set {
c := make(Set)
func (a Set[K]) Minus(b Set[K]) Set[K] {
c := make(Set[K])
for k, v := range a {
c[k] = v
}
Expand All @@ -35,8 +46,8 @@ func (a Set) Minus(b Set) Set {
}

// Union takes two sets and returns their union in a new set.
func (a Set) Union(b Set) Set {
c := make(Set)
func (a Set[K]) Union(b Set[K]) Set[K] {
c := make(Set[K])
for k, v := range a {
c[k] = v
}
Expand All @@ -48,8 +59,8 @@ func (a Set) Union(b Set) Set {

// Intersection takes two sets and returns elements common to both. Note that we
// throw away information about the values of the elements in b.
func (a Set) Intersection(b Set) Set {
c := make(Set)
func (a Set[K]) Intersection(b Set[K]) Set[K] {
c := make(Set[K])
for k, v := range a {
if _, ok := b[k]; ok {
c[k] = v
Expand Down
102 changes: 17 additions & 85 deletions internal/legacy/dockerregistry/inventory_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
cr "github.com/google/go-containerregistry/pkg/v1/types"
"github.com/stretchr/testify/require"

"sigs.k8s.io/promo-tools/v4/internal/legacy/container"
reg "sigs.k8s.io/promo-tools/v4/internal/legacy/dockerregistry"
"sigs.k8s.io/promo-tools/v4/internal/legacy/dockerregistry/registry"
"sigs.k8s.io/promo-tools/v4/internal/legacy/dockerregistry/schema"
Expand Down Expand Up @@ -1238,161 +1239,92 @@ func TestGetTokenKeyDomainRepoPath(t *testing.T) {
}
}

func TestSetManipulationsRegistryInventories(t *testing.T) {
tests := []struct {
name string
input1 registry.RegInvImage
input2 registry.RegInvImage
op func(a, b registry.RegInvImage) registry.RegInvImage
expectedOutput registry.RegInvImage
}{
{
"Set Minus",
registry.RegInvImage{
"foo": {
"sha256:abc": {"1.0", "latest"},
},
"bar": {
"sha256:def": {"0.9"},
},
},
registry.RegInvImage{
"foo": {
"sha256:abc": {"1.0", "latest"},
},
"bar": {
"sha256:def": {"0.9"},
},
},
registry.RegInvImage.Minus,
registry.RegInvImage{},
},
{
"Set Union",
registry.RegInvImage{
"foo": {
"sha256:abc": {"1.0", "latest"},
},
"bar": {
"sha256:def": {"0.9"},
},
},
registry.RegInvImage{
"apple": {
"sha256:abc": {"1.0", "latest"},
},
"banana": {
"sha256:def": {"0.9"},
},
},
registry.RegInvImage.Union,
registry.RegInvImage{
"foo": {
"sha256:abc": {"1.0", "latest"},
},
"bar": {
"sha256:def": {"0.9"},
},
"apple": {
"sha256:abc": {"1.0", "latest"},
},
"banana": {
"sha256:def": {"0.9"},
},
},
},
}

for _, test := range tests {
got := test.op(test.input1, test.input2)
expected := test.expectedOutput
require.Equal(t, expected, got)
}
}

func TestSetManipulationsTags(t *testing.T) {
tests := []struct {
name string
input1 registry.TagSlice
input2 registry.TagSlice
op func(a, b registry.TagSlice) registry.TagSet
expectedOutput registry.TagSet
op func(a, b registry.TagSlice) container.Set[image.Tag]
expectedOutput []string
}{
{
"Set Minus (both blank)",
registry.TagSlice{},
registry.TagSlice{},
registry.TagSlice.Minus,
registry.TagSet{},
[]string{},
},
{
"Set Minus (first blank)",
registry.TagSlice{},
registry.TagSlice{"a"},
registry.TagSlice.Minus,
registry.TagSet{},
[]string{},
},
{
"Set Minus (second blank)",
registry.TagSlice{"a", "b"},
registry.TagSlice{},
registry.TagSlice.Minus,
registry.TagSet{"a": nil, "b": nil},
[]string{"a", "b"},
},
{
"Set Minus",
registry.TagSlice{"a", "b"},
registry.TagSlice{"b"},
registry.TagSlice.Minus,
registry.TagSet{"a": nil},
[]string{"a"},
},
{
"Set Union (both blank)",
registry.TagSlice{},
registry.TagSlice{},
registry.TagSlice.Union,
registry.TagSet{},
[]string{},
},
{
"Set Union (first blank)",
registry.TagSlice{},
registry.TagSlice{"a"},
registry.TagSlice.Union,
registry.TagSet{"a": nil},
[]string{"a"},
},
{
"Set Union (second blank)",
registry.TagSlice{"a"},
registry.TagSlice{},
registry.TagSlice.Union,
registry.TagSet{"a": nil},
[]string{"a"},
},
{
"Set Union",
registry.TagSlice{"a", "c"},
registry.TagSlice{"b", "d"},
registry.TagSlice.Union,
registry.TagSet{"a": nil, "b": nil, "c": nil, "d": nil},
[]string{"a", "b", "c", "d"},
},
{
"Set Intersection (no intersection)",
registry.TagSlice{"a"},
registry.TagSlice{"b"},
registry.TagSlice.Intersection,
registry.TagSet{},
[]string{},
},
{
"Set Intersection (some intersection)",
registry.TagSlice{"a", "b"},
registry.TagSlice{"b", "c"},
registry.TagSlice.Intersection,
registry.TagSet{"b": nil},
[]string{"b"},
},
}

for _, test := range tests {
got := test.op(test.input1, test.input2)
expected := test.expectedOutput
expected := container.NewSet[image.Tag]()
for _, tag := range test.expectedOutput {
expected.Insert(image.Tag(tag))
}
require.Equal(t, expected, got)
}
}
Expand Down
3 changes: 0 additions & 3 deletions internal/legacy/dockerregistry/registry/registry.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,6 @@ type DigestTags map[image.Digest]TagSlice
// TagSlice is a slice of Tags.
type TagSlice []image.Tag

// TagSet is a set of Tags.
type TagSet map[image.Tag]interface{}

// ToYAML displays a RegInvImage as YAML, but with the map items sorted
// alphabetically.
func (a *RegInvImage) ToYAML(o YamlMarshalingOpts) string {
Expand Down
103 changes: 5 additions & 98 deletions internal/legacy/dockerregistry/registry/set.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,59 +24,13 @@ import (
// Various set manipulation operations. Some set operations are missing,
// because, we don't use them.

// ToSet converts a RegInvImage to a Set.
func (a RegInvImage) ToSet() container.Set {
b := make(container.Set)
for k, v := range a {
b[k] = v
}

return b
}

func toRegistryInventory(a container.Set) RegInvImage {
b := make(RegInvImage)
for k, v := range a {
// TODO: Why are we not checking errors here?
//nolint:errcheck
b[k.(image.Name)] = v.(DigestTags)
}

return b
}

// Minus is a set operation.
// TODO: ST1016: methods on the same type should have the same receiver name.
func (a RegInvImage) Minus(b RegInvImage) RegInvImage {
aSet := a.ToSet()
bSet := b.ToSet()
cSet := aSet.Minus(bSet)

return toRegistryInventory(cSet)
}

// Union is a set operation.
func (a RegInvImage) Union(b RegInvImage) RegInvImage {
aSet := a.ToSet()
bSet := b.ToSet()
cSet := aSet.Union(bSet)

return toRegistryInventory(cSet)
}

// ToTagSet converts a TagSlice to a TagSet.
func (a TagSlice) ToTagSet() TagSet {
b := make(TagSet)
for _, t := range a {
// The value doesn't matter.
b[t] = nil
}

return b
func (a TagSlice) ToTagSet() container.Set[image.Tag] {
return container.NewSet(a...)
}

// Minus is a set operation.
func (a TagSlice) Minus(b TagSlice) TagSet {
func (a TagSlice) Minus(b TagSlice) container.Set[image.Tag] {
aSet := a.ToTagSet()
bSet := b.ToTagSet()
cSet := aSet.Minus(bSet)
Expand All @@ -85,7 +39,7 @@ func (a TagSlice) Minus(b TagSlice) TagSet {
}

// Union is a set operation.
func (a TagSlice) Union(b TagSlice) TagSet {
func (a TagSlice) Union(b TagSlice) container.Set[image.Tag] {
aSet := a.ToTagSet()
bSet := b.ToTagSet()
cSet := aSet.Union(bSet)
Expand All @@ -94,57 +48,10 @@ func (a TagSlice) Union(b TagSlice) TagSet {
}

// Intersection is a set operation.
func (a TagSlice) Intersection(b TagSlice) TagSet {
func (a TagSlice) Intersection(b TagSlice) container.Set[image.Tag] {
aSet := a.ToTagSet()
bSet := b.ToTagSet()
cSet := aSet.Intersection(bSet)

return cSet
}

// ToSet converts a TagSet to a Set.
func (a TagSet) ToSet() container.Set {
b := make(container.Set)
for t := range a {
// The value doesn't matter.
b[t] = nil
}

return b
}

func setToTagSet(a container.Set) TagSet {
b := make(TagSet)
for k := range a {
b[k.(image.Tag)] = nil //nolint: errcheck
}

return b
}

// Minus is a set operation.
func (a TagSet) Minus(b TagSet) TagSet {
aSet := a.ToSet()
bSet := b.ToSet()
cSet := aSet.Minus(bSet)

return setToTagSet(cSet)
}

// Union is a set operation.
func (a TagSet) Union(b TagSet) TagSet {
aSet := a.ToSet()
bSet := b.ToSet()
cSet := aSet.Union(bSet)

return setToTagSet(cSet)
}

// Intersection is a set operation.
func (a TagSet) Intersection(b TagSet) TagSet {
aSet := a.ToSet()
bSet := b.ToSet()
cSet := aSet.Intersection(bSet)

return setToTagSet(cSet)
}