Sort all predicates (#2208)

* Sort predicates Signed-off-by: Mustafa Abdelrahman <[email protected]>
zalando · Jan 26, 2023 · 2898aa9 · 2898aa9
1 parent 7851d88
commit 2898aa9
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 12 deletions.
diff --git a/eskip/string.go b/eskip/string.go
@@ -80,43 +80,91 @@ func argsString(args []interface{}) string {
 	return strings.Join(sargs, ", ")
 }
 
+func sortedKeys[V interface{}](m map[string]V) []string {
+	keys := make([]string, 0, len(m))
+	for k := range m {
+		keys = append(keys, k)
+	}
+	sort.Strings(keys)
+	return keys
+}
+
+func sortedCopy(values []string) []string {
+	toSortValues := make([]string, len(values))
+	copy(toSortValues, values)
+	sort.Strings(toSortValues)
+	return toSortValues
+}
+
 func (r *Route) predicateString(sortPredicates bool) string {
 	var predicates []string
 
 	if r.Path != "" {
 		predicates = appendFmtEscape(predicates, `Path("%s")`, `"`, r.Path)
 	}
 
-	for _, h := range r.HostRegexps {
+	hostRegexps := r.HostRegexps
+
+	if sortPredicates {
+		hostRegexps = sortedCopy(r.HostRegexps)
+	}
+
+	for _, h := range hostRegexps {
 		predicates = appendFmtEscape(predicates, "Host(/%s/)", "/", h)
 	}
 
-	for _, p := range r.PathRegexps {
+	pathRegexps := r.PathRegexps
+
+	if sortPredicates {
+		pathRegexps = sortedCopy(r.PathRegexps)
+	}
+
+	for _, p := range pathRegexps {
 		predicates = appendFmtEscape(predicates, "PathRegexp(/%s/)", "/", p)
 	}
 
 	if r.Method != "" {
 		predicates = appendFmtEscape(predicates, `Method("%s")`, `"`, r.Method)
 	}
 
-	for k, v := range r.Headers {
-		predicates = appendFmtEscape(predicates, `Header("%s", "%s")`, `"`, k, v)
+	if sortPredicates {
+		headerKeys := sortedKeys(r.Headers)
+		for _, key := range headerKeys {
+			predicates = appendFmtEscape(predicates, `Header("%s", "%s")`, `"`, key, r.Headers[key])
+		}
+
+	} else {
+		for k, v := range r.Headers {
+			predicates = appendFmtEscape(predicates, `Header("%s", "%s")`, `"`, k, v)
+		}
 	}
 
-	for k, rxs := range r.HeaderRegexps {
-		for _, rx := range rxs {
-			predicates = appendFmt(predicates, `HeaderRegexp("%s", /%s/)`, escape(k, `"`), escape(rx, "/"))
+	if sortPredicates {
+		headerKeys := sortedKeys(r.HeaderRegexps)
+		for _, k := range headerKeys {
+			for _, rx := range r.HeaderRegexps[k] {
+				predicates = appendFmt(predicates, `HeaderRegexp("%s", /%s/)`, escape(k, `"`), escape(rx, "/"))
+			}
+		}
+	} else {
+		for k, rxs := range r.HeaderRegexps {
+			for _, rx := range rxs {
+				predicates = appendFmt(predicates, `HeaderRegexp("%s", /%s/)`, escape(k, `"`), escape(rx, "/"))
+			}
 		}
 	}
 
-	rp := r.Predicates
+	routePredicates := r.Predicates
+
 	if sortPredicates {
-		rp = make([]*Predicate, len(r.Predicates))
-		copy(rp, r.Predicates)
-		sort.Slice(rp, func(i, j int) bool { return rp[i].String() < rp[j].String() })
+		routePredicates = make([]*Predicate, len(r.Predicates))
+		copy(routePredicates, r.Predicates)
+		sort.SliceStable(routePredicates, func(i, j int) bool {
+			return routePredicates[i].String() < routePredicates[j].String()
+		})
 	}
 
-	for _, p := range rp {
+	for _, p := range routePredicates {
 		if p.Name != "Any" {
 			predicates = appendFmt(predicates, "%s(%s)", p.Name, argsString(p.Args))
 		}

diff --git a/eskip/string_test.go b/eskip/string_test.go
@@ -195,6 +195,18 @@ func TestPrintSortedPredicates(t *testing.T) {
 			`routeWithDefaultPredicatesOnly: Header("Accept", "application/json") && Method("GET") -> "https://www.example.org"`,
 			`Method("GET") && Header("Accept", "application/json") -> "https://www.example.org"`,
 		},
+		{
+			`routeWithMultipleHeaders: Header("x-frontend-type", "mobile-app") && Header("X-Forwarded-Proto", "http") -> "https://www.example.org"`,
+			`Header("X-Forwarded-Proto", "http") && Header("x-frontend-type", "mobile-app") -> "https://www.example.org"`,
+		},
+		{
+			`routeWithMultipleHeadersRegex: HeaderRegexp("User-Agent", /Zelt-(.*)/) && HeaderRegexp("age", /\\d/) -> "https://www.example.org"`,
+			`HeaderRegexp("User-Agent", /Zelt-(.*)/) && HeaderRegexp("age", /\\d/) -> "https://www.example.org"`,
+		},
+		{
+			`routeComplex: True() && Cookie("alpha", "/^enabled$/") && Method("GET") && Header("x-frontend-type", "mobile-app") && Header("X-Forwarded-Proto", "http") && HeaderRegexp("User-Agent", /Zelt-(.*)/) && HeaderRegexp("age", /\\d/) -> "https://www.example.org"`,
+			`Method("GET") && Header("X-Forwarded-Proto", "http") && Header("x-frontend-type", "mobile-app") && HeaderRegexp("User-Agent", /Zelt-(.*)/) && HeaderRegexp("age", /\\d/) && Cookie("alpha", "/^enabled$/") && True() -> "https://www.example.org"`,
+		},
 	} {
 		testPrinting(item.route, item.expected, t, i, PrettyPrintInfo{Pretty: false, IndentStr: "", SortPredicates: true}, false)
 	}