Skip to content

Commit

Permalink
do not initilize empty labelset
Browse files Browse the repository at this point in the history
  • Loading branch information
eutopian committed Feb 3, 2025
1 parent 63092e4 commit 3cf2764
Show file tree
Hide file tree
Showing 4 changed files with 434 additions and 53 deletions.
81 changes: 64 additions & 17 deletions deployment/address_book.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/Masterminds/semver/v3"
"github.com/ethereum/go-ethereum/common"
"github.com/pkg/errors"

chainsel "github.com/smartcontractkit/chain-selectors"
)

Expand Down Expand Up @@ -39,16 +40,10 @@ type TypeAndVersion struct {

func (tv TypeAndVersion) String() string {
if len(tv.Labels) == 0 {
return fmt.Sprintf("%s %s", tv.Type, tv.Version.String())
return fmt.Sprintf("%s %s", tv.Type, tv.Version)
}

// Use the LabelSet's String method for sorted labels
sortedLabels := tv.Labels.String()
return fmt.Sprintf("%s %s %s",
tv.Type,
tv.Version.String(),
sortedLabels,
)
return fmt.Sprintf("%s %s %s", tv.Type, tv.Version, tv.Labels)
}

func (tv TypeAndVersion) Equal(other TypeAndVersion) bool {
Expand Down Expand Up @@ -83,7 +78,7 @@ func TypeAndVersionFromString(s string) (TypeAndVersion, error) {
if err != nil {
return TypeAndVersion{}, err
}
labels := make(LabelSet)
var labels LabelSet
if len(parts) > 2 {
labels = NewLabelSet(parts[2:]...)
}
Expand All @@ -98,8 +93,21 @@ func NewTypeAndVersion(t ContractType, v semver.Version) TypeAndVersion {
return TypeAndVersion{
Type: t,
Version: v,
Labels: make(LabelSet), // empty set,
Labels: nil,
}
}

// DeepClone returns a copy of the TypeAndVersion struct with its Labels cloned.
func (tv TypeAndVersion) DeepClone() TypeAndVersion {
// Make a shallow copy first
out := tv

// Now deep-copy the Labels map
if tv.Labels != nil {
out.Labels = tv.Labels.DeepClone()
}

return out
}

// AddressBook is a simple interface for storing and retrieving contract addresses across
Expand Down Expand Up @@ -172,7 +180,7 @@ func (m *AddressBookMap) Addresses() (map[uint64]map[string]TypeAndVersion, erro
// maps are mutable and pass via a pointer
// creating a copy of the map to prevent concurrency
// read and changes outside object-bound
return m.cloneAddresses(m.addressesByChain), nil
return m.deepCloneAddresses(m.addressesByChain), nil
}

func (m *AddressBookMap) AddressesForChain(chainSelector uint64) (map[string]TypeAndVersion, error) {
Expand Down Expand Up @@ -245,7 +253,7 @@ func (m *AddressBookMap) Remove(ab AddressBook) error {
return nil
}

// cloneAddresses creates a deep copy of map[uint64]map[string]TypeAndVersion object
// cloneAddresses creates a shallow copy of map[uint64]map[string]TypeAndVersion object
func (m *AddressBookMap) cloneAddresses(input map[uint64]map[string]TypeAndVersion) map[uint64]map[string]TypeAndVersion {
result := make(map[uint64]map[string]TypeAndVersion)
for chainSelector, chainAddresses := range input {
Expand All @@ -254,6 +262,23 @@ func (m *AddressBookMap) cloneAddresses(input map[uint64]map[string]TypeAndVersi
return result
}

// deepCloneAddresses creates a deep copy of map[uint64]map[string]TypeAndVersion object
func (m *AddressBookMap) deepCloneAddresses(
input map[uint64]map[string]TypeAndVersion,
) map[uint64]map[string]TypeAndVersion {
result := make(map[uint64]map[string]TypeAndVersion, len(input))
for chainSelector, chainAddresses := range input {
// Make a new map for the nested addresses
newChainMap := make(map[string]TypeAndVersion, len(chainAddresses))
for addr, tv := range chainAddresses {
// Use the DeepClone method on the TypeAndVersion
newChainMap[addr] = tv.DeepClone()
}
result[chainSelector] = newChainMap
}
return result
}

// TODO: Maybe could add an environment argument
// which would ensure only mainnet/testnet chain selectors are used
// for further safety?
Expand Down Expand Up @@ -307,11 +332,15 @@ type typeVersionKey struct {
}

func tvKey(tv TypeAndVersion) typeVersionKey {
sortedLabels := tv.Labels.String()
var labels string
if tv.Labels != nil {
labels = tv.Labels.String()
}

return typeVersionKey{
Type: tv.Type,
Version: tv.Version.String(),
Labels: sortedLabels,
Labels: labels,
}
}

Expand All @@ -328,8 +357,12 @@ func AddressesContainBundle(addrs map[string]TypeAndVersion, wantTypes []TypeAnd
// They match exactly (Type, Version, Labels)
counts[wantKey]++
if counts[wantKey] > 1 {
var labels string
if wantTV.Labels != nil {
labels = wantTV.Labels.String()
}
return false, fmt.Errorf("found more than one instance of contract %s %s (labels=%s)",
wantTV.Type, wantTV.Version.String(), wantTV.Labels.String())
wantTV.Type, wantTV.Version, labels)
}
}
}
Expand All @@ -340,9 +373,23 @@ func AddressesContainBundle(addrs map[string]TypeAndVersion, wantTypes []TypeAnd
}

// AddLabel adds a string to the LabelSet in the TypeAndVersion.
func (tv *TypeAndVersion) AddLabel(label string) {
func (tv *TypeAndVersion) AddLabel(label ...string) {
if tv.Labels == nil {
tv.Labels = make(LabelSet)
}
tv.Labels.Add(label)
tv.Labels.Add(label...)
}

func (tv *TypeAndVersion) RemoveLabel(label ...string) {
if tv.Labels == nil {
return
}
tv.Labels.Remove(label...)
}

func (tv *TypeAndVersion) LabelsString() string {
if tv.Labels == nil {
return ""
}
return tv.Labels.String()
}
24 changes: 20 additions & 4 deletions deployment/address_book_labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,17 @@ func NewLabelSet(labels ...string) LabelSet {
}

// Add inserts a labels into the set.
func (ls LabelSet) Add(labels string) {
ls[labels] = struct{}{}
func (ls LabelSet) Add(labels ...string) {
for _, label := range labels {
ls[label] = struct{}{}
}
}

// Remove deletes a labels from the set, if it exists.
func (ls LabelSet) Remove(labels string) {
delete(ls, labels)
func (ls LabelSet) Remove(labels ...string) {
for _, label := range labels {
delete(ls, label)
}
}

// Contains checks if the set contains the given labels.
Expand Down Expand Up @@ -65,3 +69,15 @@ func (ls LabelSet) Equal(other LabelSet) bool {
}
return true
}

// DeepClone returns a copy of the LabelSet.
func (ls LabelSet) DeepClone() LabelSet {
if ls == nil {
return nil
}
out := make(LabelSet, len(ls))
for label := range ls {
out[label] = struct{}{}
}
return out
}
70 changes: 45 additions & 25 deletions deployment/address_book_labels_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,51 +8,71 @@ import (

func TestNewLabelSet(t *testing.T) {
t.Run("no labels", func(t *testing.T) {
ms := NewLabelSet()
assert.Empty(t, ms, "expected empty set")
ls := NewLabelSet()
assert.Empty(t, ls, "expected empty set")
})

t.Run("some labels", func(t *testing.T) {
ms := NewLabelSet("foo", "bar")
assert.Len(t, ms, 2)
assert.True(t, ms.Contains("foo"))
assert.True(t, ms.Contains("bar"))
assert.False(t, ms.Contains("baz"))
ls := NewLabelSet("foo", "bar")
assert.Len(t, ls, 2)
assert.True(t, ls.Contains("foo"))
assert.True(t, ls.Contains("bar"))
assert.False(t, ls.Contains("baz"))
})
}

func TestLabelSet_Add(t *testing.T) {
ms := NewLabelSet("initial")
ms.Add("new")
ls := NewLabelSet("initial")
ls.Add("new")

assert.True(t, ms.Contains("initial"), "expected 'initial' in set")
assert.True(t, ms.Contains("new"), "expected 'new' in set")
assert.Len(t, ms, 2, "expected 2 distinct labels in set")
assert.True(t, ls.Contains("initial"), "expected 'initial' in set")
assert.True(t, ls.Contains("new"), "expected 'new' in set")
assert.Len(t, ls, 2, "expected 2 distinct labels in set")

// Add duplicate "new" again; size should remain 2
ms.Add("new")
assert.Len(t, ms, 2, "expected size to remain 2 after adding a duplicate")
ls.Add("new")
assert.Len(t, ls, 2, "expected size to remain 2 after adding a duplicate")

// Add multiple labels at once
ls.Add("label1", "label2", "label3")
assert.Len(t, ls, 5, "expected 5 distinct labels in set") // 2 previous + 3 new
assert.True(t, ls.Contains("label1"))
assert.True(t, ls.Contains("label2"))
assert.True(t, ls.Contains("label3"))
}

func TestLabelSet_Remove(t *testing.T) {
ms := NewLabelSet("remove_me", "keep")
ms.Remove("remove_me")
ls := NewLabelSet("remove_me", "keep", "label1", "label2", "label3")
ls.Remove("remove_me")

assert.False(t, ms.Contains("remove_me"), "expected 'remove_me' to be removed")
assert.True(t, ms.Contains("keep"), "expected 'keep' to remain")
assert.Len(t, ms, 1, "expected set size to be 1 after removal")
assert.False(t, ls.Contains("remove_me"), "expected 'remove_me' to be removed")
assert.True(t, ls.Contains("keep"), "expected 'keep' to remain")
assert.True(t, ls.Contains("label1"), "expected 'label1' to remain")
assert.True(t, ls.Contains("label2"), "expected 'label2' to remain")
assert.True(t, ls.Contains("label3"), "expected 'label3' to remain")
assert.Len(t, ls, 4, "expected set size to be 4 after removal")

// Removing a non-existent item shouldn't change the size
ms.Remove("non_existent")
assert.Len(t, ms, 1, "expected size to remain 1 after removing a non-existent item")
ls.Remove("non_existent")
assert.Len(t, ls, 4, "expected size to remain 4 after removing a non-existent item")

// Remove multiple labels at once
ls.Remove("label2", "label4")

assert.Len(t, ls, 3, "expected 3 distinct labels in set after removal") // keep, label1, label3
assert.True(t, ls.Contains("keep"))
assert.True(t, ls.Contains("label1"))
assert.False(t, ls.Contains("label2"))
assert.True(t, ls.Contains("label3"))
assert.False(t, ls.Contains("label4"))
}

func TestLabelSet_Contains(t *testing.T) {
ms := NewLabelSet("foo", "bar")
ls := NewLabelSet("foo", "bar")

assert.True(t, ms.Contains("foo"))
assert.True(t, ms.Contains("bar"))
assert.False(t, ms.Contains("baz"))
assert.True(t, ls.Contains("foo"))
assert.True(t, ls.Contains("bar"))
assert.False(t, ls.Contains("baz"))
}

// TestLabelSet_String tests the String() method of the LabelSet type.
Expand Down
Loading

0 comments on commit 3cf2764

Please sign in to comment.