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

add regular expression matching to --output-http-rewrite-url #127

Merged
merged 3 commits into from
Oct 29, 2014
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
13 changes: 9 additions & 4 deletions settings_url_map.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@ package main
import (
"errors"
"fmt"
"regexp"
"strings"
)

type urlRewrite struct {
src string
src *regexp.Regexp
target string
}

Expand All @@ -22,14 +23,18 @@ func (r *UrlRewriteMap) Set(value string) error {
if len(valArr) < 2 {
return errors.New("need both src and target, colon-delimited (ex. /a:/b).")
}
*r = append(*r, urlRewrite{src: valArr[0], target: valArr[1]})
regexp, err := regexp.Compile(valArr[0])
if err != nil {
return err
}
*r = append(*r, urlRewrite{src: regexp, target: valArr[1]})
return nil
}

func (r *UrlRewriteMap) Rewrite(path string) string {
for _, f := range *r {
if f.src == path {
return f.target
if f.src.MatchString(path) {
return f.src.ReplaceAllString(path, f.target)
Copy link
Owner

Choose a reason for hiding this comment

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

Did not know about ReplaceAllString function, love go standard lib! :)

}
}
return path
Expand Down
29 changes: 27 additions & 2 deletions settings_url_map_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"testing"
)

func TestUrlRewriteMap(t *testing.T) {
func TestUrlRewriteMap_1(t *testing.T) {
var url string

rewrites := UrlRewriteMap{}
Expand All @@ -15,7 +15,6 @@ func TestUrlRewriteMap(t *testing.T) {
}

url = "/abc"

if rewrites.Rewrite(url) == url {
t.Error("Request url should have been rewritten, wasn't")
}
Expand All @@ -25,3 +24,29 @@ func TestUrlRewriteMap(t *testing.T) {
t.Error("Request url should not have been rewritten, was")
}
}

func TestUrlRewriteMap_2(t *testing.T) {
var url string

rewrites := UrlRewriteMap{}

err := rewrites.Set("/v1/user/([^\\/]+)/ping:/v2/user/$1/ping")
if err != nil {
t.Error("Should not error on /v1/user/([^\\/]+)/ping:/v2/user/$1/ping")
}

url = "/v1/user/joe/ping"
Copy link
Owner

Choose a reason for hiding this comment

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

Pls also add tests that explicitly checks if rewrites.Rewrite(url) == /v2/user/joe/ping

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pushed. does that work for you?

2014-10-28 18:24 GMT+01:00 Leonid Bugaev notifications@github.com:

In settings_url_map_test.go:

@@ -25,3 +24,24 @@ func TestUrlRewriteMap(t *testing.T) {
t.Error("Request url should not have been rewritten, was")
}
}
+
+func TestUrlRewriteMap_2(t *testing.T) {

  • var url string
  • rewrites := UrlRewriteMap{}
  • err := rewrites.Set("/v1/user/([^\/]+)/ping:/v2/user/$1/ping")
  • if err != nil {
  •   t.Error("Should not error on /v1/user/([^\/]+)/ping:/v2/user/$1/ping")
    
  • }
  • url = "/v1/user/joe/ping"

Pls also add tests that explicitly checks if rewrites.Rewrite(url) ==
/v2/user/joe/ping


Reply to this email directly or view it on GitHub
https://github.com/buger/gor/pull/127/files#r19487611.

if rewrites.Rewrite(url) == url {
t.Error("Request url should have been rewritten, wasn't")
}

url = "/v1/user/joe/ping"
if rewrites.Rewrite(url) != "/v2/user/joe/ping" {
t.Error("Request url should have been rewritten, wasn't")
}

url = "/v1/user/ping"
if rewrites.Rewrite(url) != url {
t.Error("Request url should not have been rewritten, was")
}
}