Просмотр исходного кода

Use recursive calls to Equal() for AND and OR =

This avoids an over-reliance on the string representation of the filters.
While the sort still requires the string representation, it could be
changed in the future to sort with some other function and this new
concept of equality would be resilient to that.

Signed-off-by: Michael Dresser <michaelmdresser@gmail.com>
Michael Dresser 3 лет назад
Родитель
Сommit
66d96a8e21
2 измененных файлов с 119 добавлено и 7 удалено
  1. 23 7
      pkg/kubecost/allocationfilter.go
  2. 96 0
      pkg/kubecost/allocationfilter_test.go

+ 23 - 7
pkg/kubecost/allocationfilter.go

@@ -208,7 +208,7 @@ func (filter AllocationFilterOr) sort() {
 	}
 
 	// While a slight hack, we can rely on the string serialization of the
-	// inner filters.
+	// inner filters to get a sortable representation.
 	sort.SliceStable(filter.Filters, func(i, j int) bool {
 		return filter.Filters[i].String() < filter.Filters[j].String()
 	})
@@ -221,11 +221,19 @@ func (left AllocationFilterOr) Equals(right AllocationFilter) bool {
 		return false
 	}
 
-	// Once sorted, the string representations should be equal. We can sort
-	// because ordering of logical OR statements does not matter.
+	if len(left.Filters) != len(rightOr.Filters) {
+		return false
+	}
+
 	left.sort()
 	rightOr.sort()
-	return left.String() == rightOr.String()
+
+	for i := range left.Filters {
+		if !left.Filters[i].Equals(rightOr.Filters[i]) {
+			return false
+		}
+	}
+	return true
 }
 
 // AllocationFilterOr is a set of filters that should be evaluated as a logical
@@ -287,11 +295,19 @@ func (left AllocationFilterAnd) Equals(right AllocationFilter) bool {
 		return false
 	}
 
-	// Once sorted, the string representations should be equal. We can sort
-	// because ordering of logical AND statements does not matter.
+	if len(left.Filters) != len(rightAnd.Filters) {
+		return false
+	}
+
 	left.sort()
 	rightAnd.sort()
-	return left.String() == rightAnd.String()
+
+	for i := range left.Filters {
+		if !left.Filters[i].Equals(rightAnd.Filters[i]) {
+			return false
+		}
+	}
+	return true
 }
 
 func (filter AllocationFilterCondition) Matches(a *Allocation) bool {

+ 96 - 0
pkg/kubecost/allocationfilter_test.go

@@ -1444,6 +1444,54 @@ func Test_AllocationFilter_Equals(t *testing.T) {
 			}},
 			expected: false,
 		},
+		{
+			left: AllocationFilterOr{Filters: []AllocationFilter{
+				AllocationFilterNone{},
+				AllocationFilterCondition{
+					Field: FilterLabel,
+					Op:    FilterStartsWith,
+					Key:   "xyz",
+					Value: "kubecost",
+				},
+				AllocationFilterAnd{
+					Filters: []AllocationFilter{
+						AllocationFilterCondition{
+							Field: FilterNamespace,
+							Op:    FilterEquals,
+							Value: "ns1",
+						},
+						AllocationFilterCondition{
+							Field: FilterNamespace,
+							Op:    FilterEquals,
+							Value: "ns2",
+						},
+					},
+				},
+			}},
+			right: AllocationFilterOr{Filters: []AllocationFilter{
+				AllocationFilterCondition{
+					Field: FilterLabel,
+					Op:    FilterStartsWith,
+					Key:   "xyz",
+					Value: "kubecost",
+				},
+				AllocationFilterAnd{
+					Filters: []AllocationFilter{
+						AllocationFilterCondition{
+							Field: FilterNamespace,
+							Op:    FilterEquals,
+							Value: "ns1",
+						},
+						AllocationFilterCondition{
+							Field: FilterNamespace,
+							Op:    FilterEquals,
+							Value: "ns2",
+						},
+					},
+				},
+			}},
+			expected: false,
+		},
 		// AND
 		// EMPTY
 		{
@@ -1675,6 +1723,54 @@ func Test_AllocationFilter_Equals(t *testing.T) {
 			}},
 			expected: false,
 		},
+		{
+			left: AllocationFilterAnd{Filters: []AllocationFilter{
+				AllocationFilterNone{},
+				AllocationFilterCondition{
+					Field: FilterLabel,
+					Op:    FilterStartsWith,
+					Key:   "xyz",
+					Value: "kubecost",
+				},
+				AllocationFilterOr{
+					Filters: []AllocationFilter{
+						AllocationFilterCondition{
+							Field: FilterNamespace,
+							Op:    FilterEquals,
+							Value: "ns1",
+						},
+						AllocationFilterCondition{
+							Field: FilterNamespace,
+							Op:    FilterEquals,
+							Value: "ns2",
+						},
+					},
+				},
+			}},
+			right: AllocationFilterAnd{Filters: []AllocationFilter{
+				AllocationFilterCondition{
+					Field: FilterLabel,
+					Op:    FilterStartsWith,
+					Key:   "xyz",
+					Value: "kubecost",
+				},
+				AllocationFilterOr{
+					Filters: []AllocationFilter{
+						AllocationFilterCondition{
+							Field: FilterNamespace,
+							Op:    FilterEquals,
+							Value: "ns1",
+						},
+						AllocationFilterCondition{
+							Field: FilterNamespace,
+							Op:    FilterEquals,
+							Value: "ns2",
+						},
+					},
+				},
+			}},
+			expected: false,
+		},
 	}
 
 	for _, c := range cases {