From 2898aa90dffd9aee366592a95e038c40a957510c Mon Sep 17 00:00:00 2001
From: Mustafa Saber <mustafa.abdelrahman@zalando.de>
Date: Thu, 26 Jan 2023 17:22:33 +0100
Subject: [PATCH] Sort all predicates (#2208)

* Sort predicates
Signed-off-by: Mustafa Abdelrahman <mustafa.abdelrahman@zalando.de>
---
 eskip/string.go      | 72 ++++++++++++++++++++++++++++++++++++--------
 eskip/string_test.go | 12 ++++++++
 2 files changed, 72 insertions(+), 12 deletions(-)

diff --git a/eskip/string.go b/eskip/string.go
index 4bb68a737e..935c6b5039 100644
--- a/eskip/string.go
+++ b/eskip/string.go
@@ -80,6 +80,22 @@ 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
 
@@ -87,11 +103,23 @@ func (r *Route) predicateString(sortPredicates bool) string {
 		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)
 	}
 
@@ -99,24 +127,44 @@ func (r *Route) predicateString(sortPredicates bool) string {
 		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
index 4dfcfd2404..29af8b97b1 100644
--- 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)
 	}