Skip to content
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.

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

refactor: add comparisons pkg for reused helpers #682

Merged
merged 1 commit into from
Jul 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 12 additions & 0 deletions equinix/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,10 @@ func configureProvider(ctx context.Context, d *schema.ResourceData, p *schema.Pr
return &config, nil
}

// stringsFound returns true if all strings in source are found in target
// Deprecated: stringsFound is shared between provider, tests, and resources
// and is being relocated for package refactoring.
// Use github.com/equinix/terraform-provider-equinix/internal/comparisons.Subsets
func stringsFound(source []string, target []string) bool {
for i := range source {
if !slices.Contains(target, source[i]) {
Expand All @@ -202,6 +206,10 @@ func stringsFound(source []string, target []string) bool {
return true
}

// isEmpty returns true if the value is nil or zero
// Deprecated: isEmpty is shared between provider, tests, and resources
// and is being relocated for package refactoring.
// Use github.com/equinix/terraform-provider-equinix/internal/comparisons.IsEmpty
func isEmpty(v interface{}) bool {
switch v := v.(type) {
case int:
Expand All @@ -219,6 +227,10 @@ func isEmpty(v interface{}) bool {
}
}

// slicesMatch returns true if all strings in s1 are found in s2
// Deprecated: slicesMatch is shared between provider, tests, and resources
// and is being relocated for package refactoring.
// Use github.com/equinix/terraform-provider-equinix/internal/comparisons.SlicesMatch
func slicesMatch(s1, s2 []string) bool {
if len(s1) != len(s2) {
return false
Expand Down
4 changes: 4 additions & 0 deletions equinix/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ func TestProvider(t *testing.T) {
}
}

// Deprecated test moved to internal/comparissons/comparisons_test.go
func TestProvider_stringsFound(t *testing.T) {
// given
needles := []string{"key1", "key5"}
Expand All @@ -81,6 +82,7 @@ func TestProvider_stringsFound(t *testing.T) {
assert.True(t, result, "Given strings were found")
}

// Deprecated test moved to internal/comparissons/comparisons_test.go
func TestProvider_stringsFound_negative(t *testing.T) {
// given
needles := []string{"key1", "key6"}
Expand All @@ -91,6 +93,7 @@ func TestProvider_stringsFound_negative(t *testing.T) {
assert.False(t, result, "Given strings were found")
}

// Deprecated test moved to internal/comparissons/comparisons_test.go
func TestProvider_isEmpty(t *testing.T) {
// given
input := []interface{}{
Expand Down Expand Up @@ -134,6 +137,7 @@ func TestProvider_setSchemaValueIfNotEmpty(t *testing.T) {
assert.False(t, ok, "Key was not set")
}

// Deprecated test moved to internal/comparissons/comparisons_test.go
func TestProvider_slicesMatch(t *testing.T) {
// given
input := [][][]string{
Expand Down
67 changes: 67 additions & 0 deletions internal/comparisons/comparisons.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package comparisons

import (
"cmp"
"slices"
"strings"
)

// isEmpty returns true if the given value is empty
displague marked this conversation as resolved.
Show resolved Hide resolved
func IsEmpty(v interface{}) bool {
switch v := v.(type) {
case int:
return v == 0
case *int:
return v == nil || *v == 0
case string:
return v == ""
case *string:
return v == nil || *v == ""
case nil:
return true
default:
return false
}
}

// Subsets returns true if the first slice is a subset of the second slice
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compared to the original, this function uses generics:

func stringsFound(source []string, target []string) bool {
for i := range source {
if !slices.Contains(target, source[i]) {
return false
}
}
return true
}

func Subsets[T cmp.Ordered](s1, s2 []T) bool {
// Iterate over the first slice
for _, e := range s1 {
// If the element is not in the second slice, return false
if !slices.Contains(s2, e) {
return false
}
}

return true
}

// comparisons.SlicesMatch returns true if the two slices contain the same elements, regardless of order
Copy link
Member Author

@displague displague May 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

compared to the original, this function uses slices, is more efficient (in ways we won't hit in this project), and is easier to read in my opinion.

func slicesMatch(s1, s2 []string) bool {
if len(s1) != len(s2) {
return false
}
visited := make([]bool, len(s1))
for i := 0; i < len(s1); i++ {
found := false
for j := 0; j < len(s2); j++ {
if visited[j] {
continue
}
if s1[i] == s2[j] {
visited[j] = true
found = true
break
}
}
if !found {
return false
}
}
return true
}

func SlicesMatch[T cmp.Ordered](s1, s2 []T) bool {
if len(s1) != len(s2) {
return false
}

// Create copies of the slices to avoid mutating the input slices
s1Copy := append([]T(nil), s1...)
s2Copy := append([]T(nil), s2...)

// Sort the slices
slices.Sort(s1Copy)
slices.Sort(s2Copy)

return slices.Equal(s1Copy, s2Copy)
}

// caseInsensitiveLess is a comparison function for sorting strings case-insensitively
func caseInsensitiveLess(s1, s2 string) int {
switch {
case strings.ToLower(s1) == strings.ToLower(s2):
return 0
case strings.ToLower(s1) < strings.ToLower(s2):
return -1
default:
return 1
}
}
89 changes: 89 additions & 0 deletions internal/comparisons/comparisons_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
package comparisons_test
Copy link
Member Author

@displague displague Jun 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests were taken from https://github.com/equinix/terraform-provider-equinix/blob/comparisons/equinix/provider_test.go#L147-L254

Those tests can be removed with the deprecated functions they test.


import (
"testing"

"github.com/equinix/terraform-provider-equinix/internal/comparisons"
"github.com/stretchr/testify/assert"
)

func TestSubsets(t *testing.T) {
// given
needles := []string{"key1", "key5"}
hay := []string{"key1", "key2", "Key3", "key4", "key5"}
// when
result := comparisons.Subsets(needles, hay)
// then
assert.True(t, result, "Given strings were found")
}

func TestSubsets_negative(t *testing.T) {
// given
needles := []string{"key1", "key6"}
hay := []string{"key1", "key2", "Key3", "key4", "key5"}
// when
result := comparisons.Subsets(hay, needles)
// then
assert.False(t, result, "Given strings were found")
}

func TestIsEmpty(t *testing.T) {
// given
input := []interface{}{
"test",
"",
nil,
123,
0,
43.43,
}
expected := []bool{
false,
true,
true,
false,
true,
false,
true,
}
// when then
for i := range input {
assert.Equal(t, expected[i], comparisons.IsEmpty(input[i]), "Input %v produces expected result %v", input[i], expected[i])
}
}

func TestSlicesMatch(t *testing.T) {
// given
input := [][][]string{
{
{"DC", "SV", "FR"},
{"FR", "SV", "DC"},
},
{
{"SV"},
{},
},
{
{"DC", "DC", "DC"},
{"DC", "SV", "DC"},
},
{
{}, {},
},
}
expected := []bool{
true,
false,
false,
true,
}
// when
results := make([]bool, len(expected))
for i := range input {
results[i] = comparisons.SlicesMatch(input[i][0], input[i][1])
}
// then
for i := range expected {
assert.Equal(t, expected[i], results[i])
}
}
Loading