Revert "Revert the /graph changes."

This reverts commit aa43d34. This brings back the /graph changes so that @grandbora can continue to work on the redirect for backwards compatibility. And other changes can already take the new /graph parameters into account.
zhaosijun · Sep 3, 2016 · 094a098 · 094a098
1 parent a60378c
commit 094a098
Show file tree

Hide file tree

Showing 5 changed files with 267 additions and 136 deletions.
diff --git a/template/template_test.go b/template/template_test.go
@@ -182,12 +182,12 @@ func TestTemplateExpansion(t *testing.T) {
 		{
 			// graphLink.
 			text:   "{{ graphLink \"up\" }}",
-			output: "/graph#%5B%7B%22expr%22%3A%22up%22%2C%22tab%22%3A0%7D%5D",
+			output: "/graph?g0.expr=up&g0.tab=0",
 		},
 		{
 			// tableLink.
 			text:   "{{ tableLink \"up\" }}",
-			output: "/graph#%5B%7B%22expr%22%3A%22up%22%2C%22tab%22%3A1%7D%5D",
+			output: "/graph?g0.expr=up&g0.tab=1",
 		},
 		{
 			// tmpl.

diff --git a/util/strutil/strconv.go b/util/strutil/strconv.go
@@ -17,7 +17,6 @@ import (
 	"fmt"
 	"net/url"
 	"regexp"
-	"strings"
 )
 
 var (
@@ -27,22 +26,15 @@ var (
 // TableLinkForExpression creates an escaped relative link to the table view of
 // the provided expression.
 func TableLinkForExpression(expr string) string {
-	// url.QueryEscape percent-escapes everything except spaces, for which it
-	// uses "+". However, in the non-query part of a URI, only percent-escaped
-	// spaces are legal, so we need to manually replace "+" with "%20" after
-	// query-escaping the string.
-	//
-	// See also:
-	// http://stackoverflow.com/questions/1634271/url-encoding-the-space-character-or-20.
-	urlData := url.QueryEscape(fmt.Sprintf(`[{"expr":%q,"tab":1}]`, expr))
-	return fmt.Sprintf("/graph#%s", strings.Replace(urlData, "+", "%20", -1))
+	escapedExpression := url.QueryEscape(expr)
+	return fmt.Sprintf("/graph?g0.expr=%s&g0.tab=1", escapedExpression)
 }
 
 // GraphLinkForExpression creates an escaped relative link to the graph view of
 // the provided expression.
 func GraphLinkForExpression(expr string) string {
-	urlData := url.QueryEscape(fmt.Sprintf(`[{"expr":%q,"tab":0}]`, expr))
-	return fmt.Sprintf("/graph#%s", strings.Replace(urlData, "+", "%20", -1))
+	escapedExpression := url.QueryEscape(expr)
+	return fmt.Sprintf("/graph?g0.expr=%s&g0.tab=0", escapedExpression)
 }
 
 // SanitizeLabelName replaces anything that doesn't match

diff --git a/util/strutil/strconv_test.go b/util/strutil/strconv_test.go
@@ -0,0 +1,49 @@
+// Copyright 2016 The Prometheus Authors
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package strutil
+
+import (
+	"testing"
+)
+
+type linkTest struct {
+	expression        string
+	expectedGraphLink string
+	expectedTableLink string
+}
+
+var linkTests = []linkTest{
+	{
+		"sum(incoming_http_requests_total) by (system)",
+		"/graph?g0.expr=sum%28incoming_http_requests_total%29+by+%28system%29&g0.tab=0",
+		"/graph?g0.expr=sum%28incoming_http_requests_total%29+by+%28system%29&g0.tab=1",
+	},
+	{
+		"sum(incoming_http_requests_total{system=\"trackmetadata\"})",
+		"/graph?g0.expr=sum%28incoming_http_requests_total%7Bsystem%3D%22trackmetadata%22%7D%29&g0.tab=0",
+		"/graph?g0.expr=sum%28incoming_http_requests_total%7Bsystem%3D%22trackmetadata%22%7D%29&g0.tab=1",
+	},
+}
+
+func TestLink(t *testing.T) {
+	for _, tt := range linkTests {
+		if graphLink := GraphLinkForExpression(tt.expression); graphLink != tt.expectedGraphLink {
+			t.Errorf("GraphLinkForExpression failed for expression (%#q), want %q got %q", tt.expression, tt.expectedGraphLink, graphLink)
+		}
+
+		if tableLink := TableLinkForExpression(tt.expression); tableLink != tt.expectedTableLink {
+			t.Errorf("TableLinkForExpression failed for expression (%#q), want %q got %q", tt.expression, tt.expectedTableLink, tableLink)
+		}
+	}
+}