fn: remove redundant httprouter code (#532)

*) tree from https://github.com/julienschmidt/httprouter
is already in Gin and this only seems to be parsing
parameters from URI.

api/agent/agent_test.go

@@ -68,9 +68,14 @@ func TestCallConfigurationRequest(t *testing.T) {
 	req.Header.Add("Content-Length", contentLength)
 	req.Header.Add("FN_PATH", "thewrongroute") // ensures that this doesn't leak out, should be overwritten
 
+	// let's assume we got there params from the URL
+	params := make(Params, 0, 2)
+	params = append(params, Param{Key: "YOGURT", Value: "garlic"})
+	params = append(params, Param{Key: "LEGUME", Value: "garbanzo"})
+
 	call, err := a.GetCall(
 		WithWriter(w), // XXX (reed): order matters [for now]
-		FromRequest(appName, path, req),
+		FromRequest(appName, path, req, params),
 	)
 	if err != nil {
 		t.Fatal(err)
@@ -146,6 +151,11 @@ func TestCallConfigurationRequest(t *testing.T) {
 	expectedEnv["FN_METHOD"] = method
 	expectedEnv["FN_REQUEST_URL"] = url
 
+	// add expected parameters from URL
+	for _, val := range params {
+		expectedEnv[fmt.Sprintf("FN_PARAM_%s", val.Key)] = val.Value
+	}
+
 	// do this before the "real" headers get sucked in cuz they are formatted differently
 	expectedHeaders := make(http.Header)
 	for k, v := range expectedEnv {

api/agent/call.go

@@ -41,7 +41,13 @@ type Call interface {
 // TODO build w/o closures... lazy
 type CallOpt func(a *agent, c *call) error
 
-func FromRequest(appName, path string, req *http.Request) CallOpt {
+type Param struct {
+	Key   string
+	Value string
+}
+type Params []Param
+
+func FromRequest(appName, path string, req *http.Request, params Params) CallOpt {
 	return func(a *agent, c *call) error {
 		app, err := a.ds.GetApp(req.Context(), appName)
 		if err != nil {
@@ -53,11 +59,6 @@ func FromRequest(appName, path string, req *http.Request) CallOpt {
 			return err
 		}
 
-		params, match := matchRoute(route.Path, path)
-		if !match {
-			return errors.New("route does not match") // TODO wtf, can we ignore match?
-		}
-
 		if route.Format == "" {
 			route.Format = "default"
 		}
@@ -368,20 +369,6 @@ func (c *call) End(ctx context.Context, errIn error, t callTrigger) error {
 	return errIn
 }
 
-func fakeHandler(http.ResponseWriter, *http.Request, Params) {}
-
-// TODO what is this stuff anyway?
-func matchRoute(baseRoute, route string) (Params, bool) {
-	tree := &node{}
-	tree.addRoute(baseRoute, fakeHandler)
-	handler, p, _ := tree.getValue(route)
-	if handler == nil {
-		return nil, false
-	}
-
-	return p, true
-}
-
 func toEnvName(envtype, name string) string {
 	name = strings.Replace(name, "-", "_", -1)
 	if envtype == "" {

api/agent/tree.go

@@ -1,660 +0,0 @@
-// Copyright 2013 Julien Schmidt. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be found
-// in the LICENSE file.
-
-// TODO what is this, why do we have it, and where can we put it if we need it
-// TODO move to common
-
-package agent
-
-import (
-	"net/http"
-	"strings"
-	"unicode"
-	"unicode/utf8"
-)
-
-type Handle func(http.ResponseWriter, *http.Request, Params)
-type Param struct {
-	Key   string
-	Value string
-}
-type Params []Param
-
-func min(a, b int) int {
-	if a <= b {
-		return a
-	}
-	return b
-}
-
-func countParams(path string) uint8 {
-	var n uint
-	for i := 0; i < len(path); i++ {
-		if path[i] != ':' && path[i] != '*' {
-			continue
-		}
-		n++
-	}
-	if n >= 255 {
-		return 255
-	}
-	return uint8(n)
-}
-
-type nodeType uint8
-
-const (
-	static nodeType = iota // default
-	root
-	param
-	catchAll
-)
-
-type node struct {
-	path      string
-	wildChild bool
-	nType     nodeType
-	maxParams uint8
-	indices   string
-	children  []*node
-	handle    Handle
-	priority  uint32
-}
-
-// increments priority of the given child and reorders if necessary
-func (n *node) incrementChildPrio(pos int) int {
-	n.children[pos].priority++
-	prio := n.children[pos].priority
-
-	// adjust position (move to front)
-	newPos := pos
-	for newPos > 0 && n.children[newPos-1].priority < prio {
-		// swap node positions
-		tmpN := n.children[newPos-1]
-		n.children[newPos-1] = n.children[newPos]
-		n.children[newPos] = tmpN
-
-		newPos--
-	}
-
-	// build new index char string
-	if newPos != pos {
-		n.indices = n.indices[:newPos] + // unchanged prefix, might be empty
-			n.indices[pos:pos+1] + // the index char we move
-			n.indices[newPos:pos] + n.indices[pos+1:] // rest without char at 'pos'
-	}
-
-	return newPos
-}
-
-// addRoute adds a node with the given handle to the path.
-// Not concurrency-safe!
-func (n *node) addRoute(path string, handle Handle) {
-	fullPath := path
-	n.priority++
-	numParams := countParams(path)
-
-	// non-empty tree
-	if len(n.path) > 0 || len(n.children) > 0 {
-	walk:
-		for {
-			// Update maxParams of the current node
-			if numParams > n.maxParams {
-				n.maxParams = numParams
-			}
-
-			// Find the longest common prefix.
-			// This also implies that the common prefix contains no ':' or '*'
-			// since the existing key can't contain those chars.
-			i := 0
-			max := min(len(path), len(n.path))
-			for i < max && path[i] == n.path[i] {
-				i++
-			}
-
-			// Split edge
-			if i < len(n.path) {
-				child := node{
-					path:      n.path[i:],
-					wildChild: n.wildChild,
-					nType:     static,
-					indices:   n.indices,
-					children:  n.children,
-					handle:    n.handle,
-					priority:  n.priority - 1,
-				}
-
-				// Update maxParams (max of all children)
-				for i := range child.children {
-					if child.children[i].maxParams > child.maxParams {
-						child.maxParams = child.children[i].maxParams
-					}
-				}
-
-				n.children = []*node{&child}
-				// []byte for proper unicode char conversion, see #65
-				n.indices = string([]byte{n.path[i]})
-				n.path = path[:i]
-				n.handle = nil
-				n.wildChild = false
-			}
-
-			// Make new node a child of this node
-			if i < len(path) {
-				path = path[i:]
-
-				if n.wildChild {
-					n = n.children[0]
-					n.priority++
-
-					// Update maxParams of the child node
-					if numParams > n.maxParams {
-						n.maxParams = numParams
-					}
-					numParams--
-
-					// Check if the wildcard matches
-					if len(path) >= len(n.path) && n.path == path[:len(n.path)] {
-						// check for longer wildcard, e.g. :name and :names
-						if len(n.path) >= len(path) || path[len(n.path)] == '/' {
-							continue walk
-						}
-					}
-
-					panic("path segment '" + path +
-						"' conflicts with existing wildcard '" + n.path +
-						"' in path '" + fullPath + "'")
-				}
-
-				c := path[0]
-
-				// slash after param
-				if n.nType == param && c == '/' && len(n.children) == 1 {
-					n = n.children[0]
-					n.priority++
-					continue walk
-				}
-
-				// Check if a child with the next path byte exists
-				for i := 0; i < len(n.indices); i++ {
-					if c == n.indices[i] {
-						i = n.incrementChildPrio(i)
-						n = n.children[i]
-						continue walk
-					}
-				}
-
-				// Otherwise insert it
-				if c != ':' && c != '*' {
-					// []byte for proper unicode char conversion, see #65
-					n.indices += string([]byte{c})
-					child := &node{
-						maxParams: numParams,
-					}
-					n.children = append(n.children, child)
-					n.incrementChildPrio(len(n.indices) - 1)
-					n = child
-				}
-				n.insertChild(numParams, path, fullPath, handle)
-				return
-
-			} else if i == len(path) { // Make node a (in-path) leaf
-				if n.handle != nil {
-					panic("a handle is already registered for path '" + fullPath + "'")
-				}
-				n.handle = handle
-			}
-			return
-		}
-	} else { // Empty tree
-		n.insertChild(numParams, path, fullPath, handle)
-		n.nType = root
-	}
-}
-
-func (n *node) insertChild(numParams uint8, path, fullPath string, handle Handle) {
-	var offset int // already handled bytes of the path
-
-	// find prefix until first wildcard (beginning with ':'' or '*'')
-	for i, max := 0, len(path); numParams > 0; i++ {
-		c := path[i]
-		if c != ':' && c != '*' {
-			continue
-		}
-
-		// find wildcard end (either '/' or path end)
-		end := i + 1
-		for end < max && path[end] != '/' {
-			switch path[end] {
-			// the wildcard name must not contain ':' and '*'
-			case ':', '*':
-				panic("only one wildcard per path segment is allowed, has: '" +
-					path[i:] + "' in path '" + fullPath + "'")
-			default:
-				end++
-			}
-		}
-
-		// check if this Node existing children which would be
-		// unreachable if we insert the wildcard here
-		if len(n.children) > 0 {
-			panic("wildcard route '" + path[i:end] +
-				"' conflicts with existing children in path '" + fullPath + "'")
-		}
-
-		// check if the wildcard has a name
-		if end-i < 2 {
-			panic("wildcards must be named with a non-empty name in path '" + fullPath + "'")
-		}
-
-		if c == ':' { // param
-			// split path at the beginning of the wildcard
-			if i > 0 {
-				n.path = path[offset:i]
-				offset = i
-			}
-
-			child := &node{
-				nType:     param,
-				maxParams: numParams,
-			}
-			n.children = []*node{child}
-			n.wildChild = true
-			n = child
-			n.priority++
-			numParams--
-
-			// if the path doesn't end with the wildcard, then there
-			// will be another non-wildcard subpath starting with '/'
-			if end < max {
-				n.path = path[offset:end]
-				offset = end
-
-				child := &node{
-					maxParams: numParams,
-					priority:  1,
-				}
-				n.children = []*node{child}
-				n = child
-			}
-
-		} else { // catchAll
-			if end != max || numParams > 1 {
-				panic("catch-all routes are only allowed at the end of the path in path '" + fullPath + "'")
-			}
-
-			if len(n.path) > 0 && n.path[len(n.path)-1] == '/' {
-				panic("catch-all conflicts with existing handle for the path segment root in path '" + fullPath + "'")
-			}
-
-			// currently fixed width 1 for '/'
-			i--
-			if path[i] != '/' {
-				panic("no / before catch-all in path '" + fullPath + "'")
-			}
-
-			n.path = path[offset:i]
-
-			// first node: catchAll node with empty path
-			child := &node{
-				wildChild: true,
-				nType:     catchAll,
-				maxParams: 1,
-			}
-			n.children = []*node{child}
-			n.indices = string(path[i])
-			n = child
-			n.priority++
-
-			// second node: node holding the variable
-			child = &node{
-				path:      path[i:],
-				nType:     catchAll,
-				maxParams: 1,
-				handle:    handle,
-				priority:  1,
-			}
-			n.children = []*node{child}
-
-			return
-		}
-	}
-
-	// insert remaining path part and handle to the leaf
-	n.path = path[offset:]
-	n.handle = handle
-}
-
-// Returns the handle registered with the given path (key). The values of
-// wildcards are saved to a map.
-// If no handle can be found, a TSR (trailing slash redirect) recommendation is
-// made if a handle exists with an extra (without the) trailing slash for the
-// given path.
-func (n *node) getValue(path string) (handle Handle, p Params, tsr bool) {
-walk: // outer loop for walking the tree
-	for {
-		if len(path) > len(n.path) {
-			if path[:len(n.path)] == n.path {
-				path = path[len(n.path):]
-				// If this node does not have a wildcard (param or catchAll)
-				// child,  we can just look up the next child node and continue
-				// to walk down the tree
-				if !n.wildChild {
-					c := path[0]
-					for i := 0; i < len(n.indices); i++ {
-						if c == n.indices[i] {
-							n = n.children[i]
-							continue walk
-						}
-					}
-
-					// Nothing found.
-					// We can recommend to redirect to the same URL without a
-					// trailing slash if a leaf exists for that path.
-					tsr = (path == "/" && n.handle != nil)
-					return
-
-				}
-
-				// handle wildcard child
-				n = n.children[0]
-				switch n.nType {
-				case param:
-					// find param end (either '/' or path end)
-					end := 0
-					for end < len(path) && path[end] != '/' {
-						end++
-					}
-
-					// save param value
-					if p == nil {
-						// lazy allocation
-						p = make(Params, 0, n.maxParams)
-					}
-					i := len(p)
-					p = p[:i+1] // expand slice within preallocated capacity
-					p[i].Key = n.path[1:]
-					p[i].Value = path[:end]
-
-					// we need to go deeper!
-					if end < len(path) {
-						if len(n.children) > 0 {
-							path = path[end:]
-							n = n.children[0]
-							continue walk
-						}
-
-						// ... but we can't
-						tsr = (len(path) == end+1)
-						return
-					}
-
-					if handle = n.handle; handle != nil {
-						return
-					} else if len(n.children) == 1 {
-						// No handle found. Check if a handle for this path + a
-						// trailing slash exists for TSR recommendation
-						n = n.children[0]
-						tsr = (n.path == "/" && n.handle != nil)
-					}
-
-					return
-
-				case catchAll:
-					// save param value
-					if p == nil {
-						// lazy allocation
-						p = make(Params, 0, n.maxParams)
-					}
-					i := len(p)
-					p = p[:i+1] // expand slice within preallocated capacity
-					p[i].Key = n.path[2:]
-					p[i].Value = path
-
-					handle = n.handle
-					return
-
-				default:
-					panic("invalid node type")
-				}
-			}
-		} else if path == n.path {
-			// We should have reached the node containing the handle.
-			// Check if this node has a handle registered.
-			if handle = n.handle; handle != nil {
-				return
-			}
-
-			if path == "/" && n.wildChild && n.nType != root {
-				tsr = true
-				return
-			}
-
-			// No handle found. Check if a handle for this path + a
-			// trailing slash exists for trailing slash recommendation
-			for i := 0; i < len(n.indices); i++ {
-				if n.indices[i] == '/' {
-					n = n.children[i]
-					tsr = (len(n.path) == 1 && n.handle != nil) ||
-						(n.nType == catchAll && n.children[0].handle != nil)
-					return
-				}
-			}
-
-			return
-		}
-
-		// Nothing found. We can recommend to redirect to the same URL with an
-		// extra trailing slash if a leaf exists for that path
-		tsr = (path == "/") ||
-			(len(n.path) == len(path)+1 && n.path[len(path)] == '/' &&
-				path == n.path[:len(n.path)-1] && n.handle != nil)
-		return
-	}
-}
-
-// Makes a case-insensitive lookup of the given path and tries to find a handler.
-// It can optionally also fix trailing slashes.
-// It returns the case-corrected path and a bool indicating whether the lookup
-// was successful.
-func (n *node) findCaseInsensitivePath(path string, fixTrailingSlash bool) (ciPath []byte, found bool) {
-	return n.findCaseInsensitivePathRec(
-		path,
-		strings.ToLower(path),
-		make([]byte, 0, len(path)+1), // preallocate enough memory for new path
-		[4]byte{},                    // empty rune buffer
-		fixTrailingSlash,
-	)
-}
-
-// shift bytes in array by n bytes left
-func shiftNRuneBytes(rb [4]byte, n int) [4]byte {
-	switch n {
-	case 0:
-		return rb
-	case 1:
-		return [4]byte{rb[1], rb[2], rb[3], 0}
-	case 2:
-		return [4]byte{rb[2], rb[3]}
-	case 3:
-		return [4]byte{rb[3]}
-	default:
-		return [4]byte{}
-	}
-}
-
-// recursive case-insensitive lookup function used by n.findCaseInsensitivePath
-func (n *node) findCaseInsensitivePathRec(path, loPath string, ciPath []byte, rb [4]byte, fixTrailingSlash bool) ([]byte, bool) {
-	loNPath := strings.ToLower(n.path)
-
-walk: // outer loop for walking the tree
-	for len(loPath) >= len(loNPath) && (len(loNPath) == 0 || loPath[1:len(loNPath)] == loNPath[1:]) {
-		// add common path to result
-		ciPath = append(ciPath, n.path...)
-
-		if path = path[len(n.path):]; len(path) > 0 {
-			loOld := loPath
-			loPath = loPath[len(loNPath):]
-
-			// If this node does not have a wildcard (param or catchAll) child,
-			// we can just look up the next child node and continue to walk down
-			// the tree
-			if !n.wildChild {
-				// skip rune bytes already processed
-				rb = shiftNRuneBytes(rb, len(loNPath))
-
-				if rb[0] != 0 {
-					// old rune not finished
-					for i := 0; i < len(n.indices); i++ {
-						if n.indices[i] == rb[0] {
-							// continue with child node
-							n = n.children[i]
-							loNPath = strings.ToLower(n.path)
-							continue walk
-						}
-					}
-				} else {
-					// process a new rune
-					var rv rune
-
-					// find rune start
-					// runes are up to 4 byte long,
-					// -4 would definitely be another rune
-					var off int
-					for max := min(len(loNPath), 3); off < max; off++ {
-						if i := len(loNPath) - off; utf8.RuneStart(loOld[i]) {
-							// read rune from cached lowercase path
-							rv, _ = utf8.DecodeRuneInString(loOld[i:])
-							break
-						}
-					}
-
-					// calculate lowercase bytes of current rune
-					utf8.EncodeRune(rb[:], rv)
-					// skipp already processed bytes
-					rb = shiftNRuneBytes(rb, off)
-
-					for i := 0; i < len(n.indices); i++ {
-						// lowercase matches
-						if n.indices[i] == rb[0] {
-							// must use a recursive approach since both the
-							// uppercase byte and the lowercase byte might exist
-							// as an index
-							if out, found := n.children[i].findCaseInsensitivePathRec(
-								path, loPath, ciPath, rb, fixTrailingSlash,
-							); found {
-								return out, true
-							}
-							break
-						}
-					}
-
-					// same for uppercase rune, if it differs
-					if up := unicode.ToUpper(rv); up != rv {
-						utf8.EncodeRune(rb[:], up)
-						rb = shiftNRuneBytes(rb, off)
-
-						for i := 0; i < len(n.indices); i++ {
-							// uppercase matches
-							if n.indices[i] == rb[0] {
-								// continue with child node
-								n = n.children[i]
-								loNPath = strings.ToLower(n.path)
-								continue walk
-							}
-						}
-					}
-				}
-
-				// Nothing found. We can recommend to redirect to the same URL
-				// without a trailing slash if a leaf exists for that path
-				return ciPath, (fixTrailingSlash && path == "/" && n.handle != nil)
-			}
-
-			n = n.children[0]
-			switch n.nType {
-			case param:
-				// find param end (either '/' or path end)
-				k := 0
-				for k < len(path) && path[k] != '/' {
-					k++
-				}
-
-				// add param value to case insensitive path
-				ciPath = append(ciPath, path[:k]...)
-
-				// we need to go deeper!
-				if k < len(path) {
-					if len(n.children) > 0 {
-						// continue with child node
-						n = n.children[0]
-						loNPath = strings.ToLower(n.path)
-						loPath = loPath[k:]
-						path = path[k:]
-						continue
-					}
-
-					// ... but we can't
-					if fixTrailingSlash && len(path) == k+1 {
-						return ciPath, true
-					}
-					return ciPath, false
-				}
-
-				if n.handle != nil {
-					return ciPath, true
-				} else if fixTrailingSlash && len(n.children) == 1 {
-					// No handle found. Check if a handle for this path + a
-					// trailing slash exists
-					n = n.children[0]
-					if n.path == "/" && n.handle != nil {
-						return append(ciPath, '/'), true
-					}
-				}
-				return ciPath, false
-
-			case catchAll:
-				return append(ciPath, path...), true
-
-			default:
-				panic("invalid node type")
-			}
-		} else {
-			// We should have reached the node containing the handle.
-			// Check if this node has a handle registered.
-			if n.handle != nil {
-				return ciPath, true
-			}
-
-			// No handle found.
-			// Try to fix the path by adding a trailing slash
-			if fixTrailingSlash {
-				for i := 0; i < len(n.indices); i++ {
-					if n.indices[i] == '/' {
-						n = n.children[i]
-						if (len(n.path) == 1 && n.handle != nil) ||
-							(n.nType == catchAll && n.children[0].handle != nil) {
-							return append(ciPath, '/'), true
-						}
-						return ciPath, false
-					}
-				}
-			}
-			return ciPath, false
-		}
-	}
-
-	// Nothing found.
-	// Try to fix the path by adding / removing a trailing slash
-	if fixTrailingSlash {
-		if path == "/" {
-			return ciPath, true
-		}
-		if len(loPath)+1 == len(loNPath) && loNPath[len(loPath)] == '/' &&
-			loPath[1:] == loNPath[1:len(loPath)] && n.handle != nil {
-			return append(ciPath, n.path...), true
-		}
-	}
-	return ciPath, false
-}

api/server/runner.go

@@ -40,6 +40,16 @@ func (s *Server) handleRequest(c *gin.Context) {
 
 }
 
+// convert gin.Params to agent.Params to avoid introducing gin
+// dependency to agent
+func parseParams(params gin.Params) agent.Params {
+	out := make(agent.Params, 0, len(params))
+	for _, val := range params {
+		out = append(out, agent.Param{Key: val.Key, Value: val.Value})
+	}
+	return out
+}
+
 // TODO it would be nice if we could make this have nothing to do with the gin.Context but meh
 // TODO make async store an *http.Request? would be sexy until we have different api format...
 func (s *Server) serve(c *gin.Context, appName, path string) {
@@ -47,7 +57,7 @@ func (s *Server) serve(c *gin.Context, appName, path string) {
 	// strip params, etc.
 	call, err := s.Agent.GetCall(
 		agent.WithWriter(c.Writer), // XXX (reed): order matters [for now]
-		agent.FromRequest(appName, path, c.Request),
+		agent.FromRequest(appName, path, c.Request, parseParams(c.Params)),
 	)
 	if err != nil {
 		handleErrorResponse(c, err)