MapController, MapStaticFile, MapStatic don't accept MatcherFuncs

[nelsam]

It seems slightly odd that Map, MapBefore, and MapAfter all accept MatcherFuncs as additional parameters, while MapController, MapStaticFile, and MapStatic don't.

Use Case:

I am writing a RESTful API. I want a different controller to be called for different types of Accept headers. When the Accept header includes 'text/html', I want to display my generated godoc documentation (using MapStatic or MapStaticFile). When the Accept header includes 'application/-+', I want to run the an API controller that runs the correct version.

I want to do this by essentially passing a MatcherFunc to the MapController, MapStatic, etc methods which checks the Accept header against a list of acceptable values. But currently, they cannot accept MatcherFuncs.

I'm forking the repo and tossing that in myself (because it's easy), and ... will probably make a pull request after I get it working, because ... that's the right thing to do? I think? Goodness, I'm too used to bugzilla and attaching patches. I feel old.

[edit] If there's a better way to do this, please let me know. I haven't been doing this very long, but from what I've seen, that seems to be the best route. [/edit]

[matryer]

You're not old - you using Go :)

You are of course correct that static mapping methods should also accept marcher funcs and if you could write your changes in a TDD manner, with supporting unit tests, then we'd love to accept a pull request from you.

Thanks for making Goweb better.

Mat

On 27 Jun 2013, at 12:04, nelsam notifications@github.com wrote:

It seems slightly odd that Map, MapBefore, and MapAfter all accept MatcherFuncs as additional parameters, while MapController, MapStaticFile, and MapStatic don't.

Use Case:

I am writing a RESTful API. I want a different controller to be called for different types of Accept headers. When the Accept header includes 'text/html', I want to display my generated godoc documentation (using MapStatic or MapStaticFile). When the Accept header includes 'application/-+', I want to run the an API controller that runs the correct version.

I want to do this by essentially passing a MatcherFunc to the MapController, MapStatic, etc methods which checks the Accept header against a list of acceptable values. But currently, they cannot accept MatcherFuncs.

I'm forking the repo and tossing that in myself (because it's easy), and ... will probably make a pull request after I get it working, because ... that's the right thing to do? I think? Goodness, I'm too used to bugzilla and attaching patches. I feel old.

— Reply to this email directly or view it on GitHub.

[matryer]

Although I'll say that serving different content based on accept header (and not just a different representation) might be a little confusing for users. Your call of course.

Mat

Sent from my iPhone

On 27 Jun 2013, at 12:04, nelsam notifications@github.com wrote:

It seems slightly odd that Map, MapBefore, and MapAfter all accept MatcherFuncs as additional parameters, while MapController, MapStaticFile, and MapStatic don't.

Use Case:

I am writing a RESTful API. I want a different controller to be called for different types of Accept headers. When the Accept header includes 'text/html', I want to display my generated godoc documentation (using MapStatic or MapStaticFile). When the Accept header includes 'application/-+', I want to run the an API controller that runs the correct version.

I want to do this by essentially passing a MatcherFunc to the MapController, MapStatic, etc methods which checks the Accept header against a list of acceptable values. But currently, they cannot accept MatcherFuncs.

I'm forking the repo and tossing that in myself (because it's easy), and ... will probably make a pull request after I get it working, because ... that's the right thing to do? I think? Goodness, I'm too used to bugzilla and attaching patches. I feel old.

— Reply to this email directly or view it on GitHub.

[matryer]

Personally I'd separate docs and functionality like this:

/docs/...
/api/...

And avoid the Accept header altogether.

Mat

On 27 Jun 2013, at 12:04, nelsam notifications@github.com wrote:

It seems slightly odd that Map, MapBefore, and MapAfter all accept MatcherFuncs as additional parameters, while MapController, MapStaticFile, and MapStatic don't.

Use Case:

I am writing a RESTful API. I want a different controller to be called for different types of Accept headers. When the Accept header includes 'text/html', I want to display my generated godoc documentation (using MapStatic or MapStaticFile). When the Accept header includes 'application/-+', I want to run the an API controller that runs the correct version.

I want to do this by essentially passing a MatcherFunc to the MapController, MapStatic, etc methods which checks the Accept header against a list of acceptable values. But currently, they cannot accept MatcherFuncs.

I'm forking the repo and tossing that in myself (because it's easy), and ... will probably make a pull request after I get it working, because ... that's the right thing to do? I think? Goodness, I'm too used to bugzilla and attaching patches. I feel old.

— Reply to this email directly or view it on GitHub.

[nelsam]

Well, to be a bit clearer, the biggest reason I want to map based on Accept header is for versioning the API. I just thought it would be kinda nice to display documentation about the request at the request address, as a kind of side effect of mapping based on the accept header.

But still, I would at least like to map versioning based on the accept header.

I'm gonna start working on my changes. I don't think it'll take me too long, but then, I'm somewhat new to Go, somewhat new to TDD, and quite new to goweb. So we'll see.

-Sam

[matryer]

Well good luck - feel free to chat to us here: http://www.hipchat.com/gXWgwTtX2

On 27 Jun 2013, at 13:59, nelsam notifications@github.com wrote:

Well, to be a bit clearer, the biggest reason I want to map based on Accept header is for versioning the API. I just thought it would be kinda nice to display documentation about the request at the request address, as a kind of side effect of mapping based on the accept header.

But still, I would at least like to map versioning based on the accept header.

I'm gonna start working on my changes. I don't think it'll take me too long, but then, I'm somewhat new to Go, somewhat new to TDD, and quite new to goweb. So we'll see.

-Sam

— Reply to this email directly or view it on GitHub.

[nelsam]

Hrm. Well, my first instinct (which was to set handlerMethods := options[handlerMethodsStart:] and then pass an extra parameter of handlerMethods... to the various calls to map) didn't work because I didn't understand varargs and variadic functions in Go properly. I was trying to use them like python's *args, but I should have known that a lower level language wouldn't be so quick to do all sorts of expensive conversion for me.

My second instinct was to make a slight modification to handlerForOptions to make it accept arguments of either MatcherFunc or []MatcherFunc and create a single slice containing all of the MatcherFunc values passed in. So I did that and kept toying with it until the tests completed successfully. The diff is here: https://github.com/nelsam/goweb/compare/map-matcher-funcs

After thinking about it a little, I thought it might work out better to make []MatcherFunc implement MatcherFunc. But then I found the definition of the MatcherFunc type, and I'm pretty sure that a []MatcherFunc can't act as a function, so that idea's out.

I've thought of one alternate method: create an interface (say, Matcher) that has a Match method (which takes the same arguments and returns the same values as MatcherFunc does), make a type of []MatcherFunc that implements the Matcher interface by having its Match method loop through the []MatcherFunc values and calling each one until it gets a response, and allow arguments of type Matcher instead of []MatcherFunc in handlerForOptions. But that sounds more complicated and not really any cleaner than just allowing arguments of type []MatcherFunc.

With my limited knowledge of Go and goweb, I don't think I know of a way to implement this fix that would be significantly better than what I finished last night. And with my limited knowledge of TDD and goweb, I don't know if my tests are comprehensive or clear enough (I tried to follow the style of other tests that check HandlerFuncs).

Any chance that someone could check out the link I posted to the diff between my branch and master, and give me some feedback? Any ideas about better ways to do it, what I might be doing wrong, feedback on whether my tests are okay or not, etc.

[matryer]

This work was done here: https://github.com/stretchr/goweb/commit/e78ac4fed0910182454df16779cbeb7d3e0a9cbe

TODO: Update docs, and we can close this issue.

[matryer]

@nelsam are the docs updated now?

[nelsam]

Oh, right. I am good at this game.

Closing the ticket.