Add RequestPathPattern method to Typhon Request #169
Conversation
| return "" | ||
| } | ||
|
|
||
| // RequestMethod returns the HTTP method of the request |
There was a problem hiding this comment.
I'm not particularly keen on the name of this so any suggestions would be appreciated 🙇 . I'm also wondering if RouterEndpointMethod (which could be a "*") would be more beneficial here...
RohanPadmanabhan
changed the title
request.go
Outdated
| @@ -213,6 +213,22 @@ func (r Request) ResponseWithCode(body interface{}, statusCode int) Response { | |||
| return rsp | |||
| } | |||
|
|
|||
| // RouterEndpointPattern finds the router pattern that matches the request. This is only callable while the request | |||
| // is being served. | |||
| func (r Request) RouterEndpointPattern() string { | |||
There was a problem hiding this comment.
RequestMethod() feels intuitive and simple given we can't use Method(). 👍
For this method, RouterEntryPattern() is probably the most correct, but would RequestPathPattern() or even RequestPattern() be simpler and consistent?
There was a problem hiding this comment.
Renamed to RequestPathPattern() in e9f9213. I suspect that RequestPattern by itself would require some background context to understand
request.go
Outdated
| if router := RouterForRequest(r); router != nil { | ||
| if pathPattern := router.Pattern(r); pathPattern != "" { | ||
| return pathPattern | ||
| } | ||
| } |
There was a problem hiding this comment.
An alternative implementation here would be to store the pattern used to serve the request in the context, as we do with the router currently. Then we don't need to look up the pattern separately (which could theoretically have changed if a new route is registered in the intervening time).
There was a problem hiding this comment.
I have changed it to store instead in e9f9213 👍
RohanPadmanabhan
force-pushed
the
add-router-endpoint-pattern
branch
from
bb1576e
to
e9f9213
Compare
router.go
Outdated
| routerContextKey = routerContextKeyType{} | ||
| routerComponentsRe = regexp.MustCompile(`(?:^|/)(\*\w*|:\w+)`) | ||
| routerContextKey = routerContextKeyType{} | ||
| routerPathContextKey = routerPathContextKeyType{} |
There was a problem hiding this comment.
| routerPathContextKey = routerPathContextKeyType{} | |
| routerRequestPatternContextKey = routerRequestPatternContextKeyType{} |
Optional: This might be clearer to distinguish between path and pattern?
There was a problem hiding this comment.
Renamed in 776b548 👍
router.go
Outdated
| @@ -44,6 +46,13 @@ func RouterForRequest(r Request) *Router { | |||
| return nil | |||
| } | |||
|
|
|||
| func routerEntryPathPatternForRequest(r Request) string { | |||
There was a problem hiding this comment.
| func routerEntryPathPatternForRequest(r Request) string { | |
| func routerPathPatternForRequest(r Request) string { |
There was a problem hiding this comment.
Renamed in fc0f840 👍
RohanPadmanabhan
changed the title
📚 Background
We have found several use cases such as metrics, logging, and authorization where knowing the router path (without any dynamic parameters) is useful to avoid unbounded cardinality or make searching for a particular value easier. At the moment, it is doable to get this but it's not easy.
🆕 What does this PR do?
RouterEndpointPatternmethod to get the router endpoint patternRequestMethodmethod to get the request method. This is needed to fulfil an interface between the two APIs. I couldn't name it justMethodas this clashes with the string inside the contained http.Request object🛡️ Testing
Added new unittests.