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 RequestPathPattern method to Typhon Request #169

Merged
merged 7 commits into from
Dec 4, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
18 changes: 17 additions & 1 deletion request.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ func (r *Request) BodyBytes(consume bool) ([]byte, error) {
// Send round-trips the request via the default Client. It does not block, instead returning a ResponseFuture
// representing the asynchronous operation to produce the response. It is equivalent to:
//
// r.SendVia(Client)
// r.SendVia(Client)
func (r Request) Send() *ResponseFuture {
return Send(r)
}
Expand Down Expand Up @@ -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 {
Copy link
Member

@priyeshpatel priyeshpatel Dec 1, 2023

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed to RequestPathPattern() in e9f9213. I suspect that RequestPattern by itself would require some background context to understand

if router := RouterForRequest(r); router != nil {
if pathPattern := router.Pattern(r); pathPattern != "" {
return pathPattern
}
}
Copy link
Member

Choose a reason for hiding this comment

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

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).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have changed it to store instead in e9f9213 👍

return ""
}

// RequestMethod returns the HTTP method of the request
Copy link
Contributor Author

@RohanPadmanabhan RohanPadmanabhan Dec 1, 2023

Choose a reason for hiding this comment

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

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...

func (r Request) RequestMethod() string {
return r.Method
}

func (r Request) String() string {
if r.URL == nil {
return "Request(Unknown)"
Expand Down
22 changes: 22 additions & 0 deletions request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"encoding/json"
"io/ioutil"
"math"
"net/http"
"strings"
"testing"

Expand Down Expand Up @@ -223,6 +224,27 @@ func TestRequestSetMetadata(t *testing.T) {
assert.Equal(t, []string{"data"}, req.Request.Header["meta"])
}

func TestRouterEndpointPattern(t *testing.T) {
req := NewRequest(context.Background(), http.MethodGet, "/foo/some-url-identifier", nil)
assert.Equal(t, "", req.RouterEndpointPattern()) // should be empty if request has not been served by a router

router := Router{}
routerEndpointPattern := "/foo/:id"
router.GET(routerEndpointPattern, func(req Request) Response {
// as we are currently serving the request, we should be able to get the router endpoint pattern
assert.Equal(t, routerEndpointPattern, req.RouterEndpointPattern())
return req.Response(nil)
})

rsp := req.SendVia(router.Serve()).Response()
require.NoError(t, rsp.Error) // check we didn't get a "route not found" error
}

func TestRequestMethod(t *testing.T) {
req := NewRequest(context.Background(), http.MethodGet, "", nil)
assert.Equal(t, http.MethodGet, req.RequestMethod())
}

func jsonStreamMarshal(v interface{}) ([]byte, error) {
var buffer bytes.Buffer
writer := bufio.NewWriter(&buffer)
Expand Down
Loading