Browse Source

refactor routeRegexp, particularily newRouteRegexp. (#328)

The existing options matchPrefix, matchHost, and matchQueries are
mutually exclusive so there's no point in having a separate boolean
argument for each one. It's clearer if there's a single type variable.

strictSlash and useEncodedPath were also being passed as naked bools so
I've wrapped these in a struct called routeRegexpOptions for more clarity
at the call site.
pull/329/head
Kamil Kisiel 8 years ago committed by GitHub
parent
commit
512169e5d7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 2
      mux_test.go
  2. 2
      old_test.go
  3. 64
      regexp.go
  4. 21
      route.go

2
mux_test.go

@ -25,7 +25,7 @@ func (r *Route) GoString() string {
} }
func (r *routeRegexp) GoString() string { func (r *routeRegexp) GoString() string {
return fmt.Sprintf("&routeRegexp{template: %q, matchHost: %t, matchQuery: %t, strictSlash: %t, regexp: regexp.MustCompile(%q), reverse: %q, varsN: %v, varsR: %v", r.template, r.matchHost, r.matchQuery, r.strictSlash, r.regexp.String(), r.reverse, r.varsN, r.varsR) return fmt.Sprintf("&routeRegexp{template: %q, regexpType: %v, options: %v, regexp: regexp.MustCompile(%q), reverse: %q, varsN: %v, varsR: %v", r.template, r.regexpType, r.options, r.regexp.String(), r.reverse, r.varsN, r.varsR)
} }
type routeTest struct { type routeTest struct {

2
old_test.go

@ -681,7 +681,7 @@ func TestNewRegexp(t *testing.T) {
} }
for pattern, paths := range tests { for pattern, paths := range tests {
p, _ = newRouteRegexp(pattern, false, false, false, false, false) p, _ = newRouteRegexp(pattern, regexpTypePath, routeRegexpOptions{})
for path, result := range paths { for path, result := range paths {
matches = p.regexp.FindStringSubmatch(path) matches = p.regexp.FindStringSubmatch(path)
if result == nil { if result == nil {

64
regexp.go

@ -14,6 +14,20 @@ import (
"strings" "strings"
) )
type routeRegexpOptions struct {
strictSlash bool
useEncodedPath bool
}
type regexpType int
const (
regexpTypePath regexpType = 0
regexpTypeHost regexpType = 1
regexpTypePrefix regexpType = 2
regexpTypeQuery regexpType = 3
)
// newRouteRegexp parses a route template and returns a routeRegexp, // newRouteRegexp parses a route template and returns a routeRegexp,
// used to match a host, a path or a query string. // used to match a host, a path or a query string.
// //
@ -24,7 +38,7 @@ import (
// Previously we accepted only Python-like identifiers for variable // Previously we accepted only Python-like identifiers for variable
// names ([a-zA-Z_][a-zA-Z0-9_]*), but currently the only restriction is that // names ([a-zA-Z_][a-zA-Z0-9_]*), but currently the only restriction is that
// name and pattern can't be empty, and names can't contain a colon. // name and pattern can't be empty, and names can't contain a colon.
func newRouteRegexp(tpl string, matchHost, matchPrefix, matchQuery, strictSlash, useEncodedPath bool) (*routeRegexp, error) { func newRouteRegexp(tpl string, typ regexpType, options routeRegexpOptions) (*routeRegexp, error) {
// Check if it is well-formed. // Check if it is well-formed.
idxs, errBraces := braceIndices(tpl) idxs, errBraces := braceIndices(tpl)
if errBraces != nil { if errBraces != nil {
@ -34,19 +48,18 @@ func newRouteRegexp(tpl string, matchHost, matchPrefix, matchQuery, strictSlash,
template := tpl template := tpl
// Now let's parse it. // Now let's parse it.
defaultPattern := "[^/]+" defaultPattern := "[^/]+"
if matchQuery { if typ == regexpTypeQuery {
defaultPattern = ".*" defaultPattern = ".*"
} else if matchHost { } else if typ == regexpTypeHost {
defaultPattern = "[^.]+" defaultPattern = "[^.]+"
matchPrefix = false
} }
// Only match strict slash if not matching // Only match strict slash if not matching
if matchPrefix || matchHost || matchQuery { if typ != regexpTypePath {
strictSlash = false options.strictSlash = false
} }
// Set a flag for strictSlash. // Set a flag for strictSlash.
endSlash := false endSlash := false
if strictSlash && strings.HasSuffix(tpl, "/") { if options.strictSlash && strings.HasSuffix(tpl, "/") {
tpl = tpl[:len(tpl)-1] tpl = tpl[:len(tpl)-1]
endSlash = true endSlash = true
} }
@ -88,16 +101,16 @@ func newRouteRegexp(tpl string, matchHost, matchPrefix, matchQuery, strictSlash,
// Add the remaining. // Add the remaining.
raw := tpl[end:] raw := tpl[end:]
pattern.WriteString(regexp.QuoteMeta(raw)) pattern.WriteString(regexp.QuoteMeta(raw))
if strictSlash { if options.strictSlash {
pattern.WriteString("[/]?") pattern.WriteString("[/]?")
} }
if matchQuery { if typ == regexpTypeQuery {
// Add the default pattern if the query value is empty // Add the default pattern if the query value is empty
if queryVal := strings.SplitN(template, "=", 2)[1]; queryVal == "" { if queryVal := strings.SplitN(template, "=", 2)[1]; queryVal == "" {
pattern.WriteString(defaultPattern) pattern.WriteString(defaultPattern)
} }
} }
if !matchPrefix { if typ != regexpTypePrefix {
pattern.WriteByte('$') pattern.WriteByte('$')
} }
reverse.WriteString(raw) reverse.WriteString(raw)
@ -119,10 +132,8 @@ func newRouteRegexp(tpl string, matchHost, matchPrefix, matchQuery, strictSlash,
// Done! // Done!
return &routeRegexp{ return &routeRegexp{
template: template, template: template,
matchHost: matchHost, regexpType: typ,
matchQuery: matchQuery, options: options,
strictSlash: strictSlash,
useEncodedPath: useEncodedPath,
regexp: reg, regexp: reg,
reverse: reverse.String(), reverse: reverse.String(),
varsN: varsN, varsN: varsN,
@ -135,15 +146,10 @@ func newRouteRegexp(tpl string, matchHost, matchPrefix, matchQuery, strictSlash,
type routeRegexp struct { type routeRegexp struct {
// The unmodified template. // The unmodified template.
template string template string
// True for host match, false for path or query string match. // The type of match
matchHost bool regexpType regexpType
// True for query string match, false for path and host match. // Options for matching
matchQuery bool options routeRegexpOptions
// The strictSlash value defined on the route, but disabled if PathPrefix was used.
strictSlash bool
// Determines whether to use encoded req.URL.EnscapedPath() or unencoded
// req.URL.Path for path matching
useEncodedPath bool
// Expanded regexp. // Expanded regexp.
regexp *regexp.Regexp regexp *regexp.Regexp
// Reverse template. // Reverse template.
@ -156,12 +162,12 @@ type routeRegexp struct {
// Match matches the regexp against the URL host or path. // Match matches the regexp against the URL host or path.
func (r *routeRegexp) Match(req *http.Request, match *RouteMatch) bool { func (r *routeRegexp) Match(req *http.Request, match *RouteMatch) bool {
if !r.matchHost { if r.regexpType != regexpTypeHost {
if r.matchQuery { if r.regexpType == regexpTypeQuery {
return r.matchQueryString(req) return r.matchQueryString(req)
} }
path := req.URL.Path path := req.URL.Path
if r.useEncodedPath { if r.options.useEncodedPath {
path = req.URL.EscapedPath() path = req.URL.EscapedPath()
} }
return r.regexp.MatchString(path) return r.regexp.MatchString(path)
@ -178,7 +184,7 @@ func (r *routeRegexp) url(values map[string]string) (string, error) {
if !ok { if !ok {
return "", fmt.Errorf("mux: missing route variable %q", v) return "", fmt.Errorf("mux: missing route variable %q", v)
} }
if r.matchQuery { if r.regexpType == regexpTypeQuery {
value = url.QueryEscape(value) value = url.QueryEscape(value)
} }
urlValues[k] = value urlValues[k] = value
@ -203,7 +209,7 @@ func (r *routeRegexp) url(values map[string]string) (string, error) {
// For a URL with foo=bar&baz=ding, we return only the relevant key // For a URL with foo=bar&baz=ding, we return only the relevant key
// value pair for the routeRegexp. // value pair for the routeRegexp.
func (r *routeRegexp) getURLQuery(req *http.Request) string { func (r *routeRegexp) getURLQuery(req *http.Request) string {
if !r.matchQuery { if r.regexpType != regexpTypeQuery {
return "" return ""
} }
templateKey := strings.SplitN(r.template, "=", 2)[0] templateKey := strings.SplitN(r.template, "=", 2)[0]
@ -280,7 +286,7 @@ func (v *routeRegexpGroup) setMatch(req *http.Request, m *RouteMatch, r *Route)
if len(matches) > 0 { if len(matches) > 0 {
extractVars(path, matches, v.path.varsN, m.Vars) extractVars(path, matches, v.path.varsN, m.Vars)
// Check if we should redirect. // Check if we should redirect.
if v.path.strictSlash { if v.path.options.strictSlash {
p1 := strings.HasSuffix(path, "/") p1 := strings.HasSuffix(path, "/")
p2 := strings.HasSuffix(v.path.template, "/") p2 := strings.HasSuffix(v.path.template, "/")
if p1 != p2 { if p1 != p2 {

21
route.go

@ -171,12 +171,12 @@ func (r *Route) addMatcher(m matcher) *Route {
} }
// addRegexpMatcher adds a host or path matcher and builder to a route. // addRegexpMatcher adds a host or path matcher and builder to a route.
func (r *Route) addRegexpMatcher(tpl string, matchHost, matchPrefix, matchQuery bool) error { func (r *Route) addRegexpMatcher(tpl string, typ regexpType) error {
if r.err != nil { if r.err != nil {
return r.err return r.err
} }
r.regexp = r.getRegexpGroup() r.regexp = r.getRegexpGroup()
if !matchHost && !matchQuery { if typ == regexpTypePath || typ == regexpTypePrefix {
if len(tpl) > 0 && tpl[0] != '/' { if len(tpl) > 0 && tpl[0] != '/' {
return fmt.Errorf("mux: path must start with a slash, got %q", tpl) return fmt.Errorf("mux: path must start with a slash, got %q", tpl)
} }
@ -184,7 +184,10 @@ func (r *Route) addRegexpMatcher(tpl string, matchHost, matchPrefix, matchQuery
tpl = strings.TrimRight(r.regexp.path.template, "/") + tpl tpl = strings.TrimRight(r.regexp.path.template, "/") + tpl
} }
} }
rr, err := newRouteRegexp(tpl, matchHost, matchPrefix, matchQuery, r.strictSlash, r.useEncodedPath) rr, err := newRouteRegexp(tpl, typ, routeRegexpOptions{
strictSlash: r.strictSlash,
useEncodedPath: r.useEncodedPath,
})
if err != nil { if err != nil {
return err return err
} }
@ -193,7 +196,7 @@ func (r *Route) addRegexpMatcher(tpl string, matchHost, matchPrefix, matchQuery
return err return err
} }
} }
if matchHost { if typ == regexpTypeHost {
if r.regexp.path != nil { if r.regexp.path != nil {
if err = uniqueVars(rr.varsN, r.regexp.path.varsN); err != nil { if err = uniqueVars(rr.varsN, r.regexp.path.varsN); err != nil {
return err return err
@ -206,7 +209,7 @@ func (r *Route) addRegexpMatcher(tpl string, matchHost, matchPrefix, matchQuery
return err return err
} }
} }
if matchQuery { if typ == regexpTypeQuery {
r.regexp.queries = append(r.regexp.queries, rr) r.regexp.queries = append(r.regexp.queries, rr)
} else { } else {
r.regexp.path = rr r.regexp.path = rr
@ -289,7 +292,7 @@ func (r *Route) HeadersRegexp(pairs ...string) *Route {
// Variable names must be unique in a given route. They can be retrieved // Variable names must be unique in a given route. They can be retrieved
// calling mux.Vars(request). // calling mux.Vars(request).
func (r *Route) Host(tpl string) *Route { func (r *Route) Host(tpl string) *Route {
r.err = r.addRegexpMatcher(tpl, true, false, false) r.err = r.addRegexpMatcher(tpl, regexpTypeHost)
return r return r
} }
@ -349,7 +352,7 @@ func (r *Route) Methods(methods ...string) *Route {
// Variable names must be unique in a given route. They can be retrieved // Variable names must be unique in a given route. They can be retrieved
// calling mux.Vars(request). // calling mux.Vars(request).
func (r *Route) Path(tpl string) *Route { func (r *Route) Path(tpl string) *Route {
r.err = r.addRegexpMatcher(tpl, false, false, false) r.err = r.addRegexpMatcher(tpl, regexpTypePath)
return r return r
} }
@ -365,7 +368,7 @@ func (r *Route) Path(tpl string) *Route {
// Also note that the setting of Router.StrictSlash() has no effect on routes // Also note that the setting of Router.StrictSlash() has no effect on routes
// with a PathPrefix matcher. // with a PathPrefix matcher.
func (r *Route) PathPrefix(tpl string) *Route { func (r *Route) PathPrefix(tpl string) *Route {
r.err = r.addRegexpMatcher(tpl, false, true, false) r.err = r.addRegexpMatcher(tpl, regexpTypePrefix)
return r return r
} }
@ -396,7 +399,7 @@ func (r *Route) Queries(pairs ...string) *Route {
return nil return nil
} }
for i := 0; i < length; i += 2 { for i := 0; i < length; i += 2 {
if r.err = r.addRegexpMatcher(pairs[i]+"="+pairs[i+1], false, false, true); r.err != nil { if r.err = r.addRegexpMatcher(pairs[i]+"="+pairs[i+1], regexpTypeQuery); r.err != nil {
return r return r
} }
} }

Loading…
Cancel
Save