Custom context for mux.Vars

[Southern]

Is there a particular reason that varsKey and routeKey are using the contextKey type? https://github.com/gorilla/mux/blob/master/mux.go#L322-L327

This causes problems when attempting to use request.WithContext(context.WithValue(request.Context(), 0, map[string]string{...})) for testing route params because mux.Vars is looking for contextKey(0) as the key rather than int(0).

[elithrar]

The custom, private types exist so that other packages won't clobber over it. If any other user was using int(0) as a key, it would override mux's vars.

[Southern]

Ah. In that case, could we expose ContextKey or something so that mux.Vars can be used without having to setup a route and use httptest?

[elithrar]

Would the solution in #167 not work for you? Directly mutating internal state should be avoidable.

[Southern]

Of course it would work, but I'm not trying to handle the response or even care about the route itself. I'm just pulling params from the *http.Request.

It would be so much easier for people to use something like:

request, _ := http.NewRequest("PATCH", "", nil)
ctx := context.WithValue(request.Context(), 0, map[string]string{
    "param1": param1,
    "param2": param2,
})
data := mapParams(request.WithContext(ctx))
// assertions to check data

Verses having all of this to achieve the same result:

router := mux.NewRouter()
router.HandleFunc("/{param1}/{param2}", func (response http.ResponseWriter, request *http.Request) {
    data := mapParams(request)
    // assertions to check data
})

ts := httptest.NewServer(router)
defer ts.Close()

url := fmt.Sprintf("%s/%s/%s", ts.URL, param1, param2)
request, _ := http.NewRequest("PATCH", url, nil)

c := &http.Client{}

response, e := c.Do(request)
if e != nil {
    t.Error(e)
}

[elithrar]

If you're testing multiple routes via table-driven testing, I would think
the extra 5-10 lines of boilerplate would be a wash?

The benefit to not just injecting into the context is that you also ensure
that mux's own route/var parsing is what your application expects over
time, rather than shortcutting it.
On Sat, Aug 20, 2016 at 7:36 PM Colton Baker [email protected]
wrote:

Of course it would work, but I'm not trying to handle the response or even
care about the route itself. I'm just pulling params from the
*http.Request.

It would be so much easier for people to use this:

request, _ := http.NewRequest("PATCH", "", nil)ctx := context.WithValue(request.Context(), 0, map[string]string{
"param1": param1,
"param2": param2,
})data := mapParams(request.WithContext(ctx))// assertions to check data

Verses having all of this to achieve the same result:

router := mux.NewRouter()
router.HandleFunc("/%s/%s", func (response http.ResponseWriter, request *http.Request) {
data := mapParams(request)
// assertions to check data
})
ts := httptest.NewServer(router)defer ts.Close()
url := fmt.Sprintf("/%s/%s", param1, param2)request, _ := http.NewRequests("PATCH", url, nil)
c := &http.Client{}
response, e := c.Do(request)if e != nil {
t.Error(e)
}

—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
#187 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AABIcPl2tvEpi5xITBoyThUgOEzwFY3Zks5qh7mhgaJpZM4JpFkr
.

[Southern]

I'm not testing routes at all. I would just like to be able to force a context with params during testing instead of setting up a router, handler, httptest server, http client, and then finally be able to send my request for testing.

I'm not sure that's a benefit because I highly doubt most people will ever want to modify the context during runtime in the first place.

[chr0n1x]

I'm not testing routes either. Instead, I have collections of func(ResponseWriter, *Request)s that I would just like to test without having to set up an http server. All I want to do is call http.NewRequest and throw the object directly into the handlers to test specific cases.

That being said, I do see my doing this as a code smell in my application. I guess that it would be preferable to have middleware that passes mux.Vars(r) as a separate argument to all of my handlers, however that's a fairly large refactor in my apps current state.

@elithrar - do you have any other suggestions on how to test functions that use mux.Vars(r)? I kind of want to avoid splitting mux.Vars(r) calls into a separate object that I can swap out with some sort of test dummy object.

[grim-fendango]

Hey we can close this now 🎉 since this PR has been merged.

To set context vars you just need to do the following:

request, _ := http.NewRequest("PATCH", "", nil)
request = mux.SetURLVars(request, map[string]string{
    "param1": param1,
    "param2": param2,
})