Add monadic actions before, after and with multi-GET handling.

[hesselink]

I haven't thought about an alternative for this, but if we don't figure one out and we end up adding this, there's a few things I would improve:

• I'd add a generic Config like type to the run functions instead of the MultiGetHooks you're passing now. The multiget-hooks can then be one field of the config. That way we can add more settings without breaking clients if we're careful. That means not exposing the Config constructor, but instead providing a defaultConfig and getters and setters.
• Also regarding backwards compat, that is broken currently since you change the signature for route. It might be better to add another version, routeWith or something, and keep the original as well (using a default config).
• Additionally, I'd add the config to the reader part of Router. That way you don't have to explicitly pass it everywhere.
• Travis seems to say the tests are broken. That's probably due to the signature of route changing, now that I think about it, so perhaps that will be fixed by the second point above.

I'll think more about an alternative to this that is less ad-hoc, but I'm not super optimistic about finding it.

[hesselink]

I've been thinking about this a bit more, and came up with something I think you also briefly suggested: what if we allow passing in the whole of mkMultiGetHandler, and expose the default implementation and/or some building blocks? That way you could rebuild it with logging added, but it also allows people to give e.g. more efficient implementations, or turn it off by throwing NotFound.

[bergmark]

My opinion is: please no RecordWildCards :-)

[hesselink]

This looks a lot better! I've added some small remarks here and there. Two extra ones:

• You updated rest-happstack. Should we do similar updates to rest-snap and rest-wai, or is this not relevant for those packages?
• Could you squash both commits into one? It's one change, the previous attempt is really irrelevant now.

The tests are also failing, but I'm not sure if that's related to your changes. Do all the commands that are run on Travis succeed locally for you?

[lostbean]

Sure, I'll take a look on both rest-snap and rest-wai and re-organise the commits.

stack test rest-gen is fine but stack test rest-core fails. I've to update the test suite.

[hesselink]

Looks great! I'll think about the right form of the hooks one last time, then merge and do a release tomorrow, hopefully.

[hesselink]

@lostbean Could you push the relaxed bounds that fix the test cases, and perhaps the rename of Cfg to RouterData? Then I can merge this and make a release.

[hesselink]

Sorry for dragging this on, but I just remembered the scenario why I thought the override should be different: optimizing/specializing the multiget. We've been planning to allow users to give a custom, hopefully faster implementation of multi-PUT and multi-DELETE on a single endpoint. This would be relatively straightforward, doing a single update or delete call, for example, instead of many.

The scenario for multi-anything is less straightforward, but you could definitely imagine doing less authentication checks, combining common db calls, etc. You could even specialize just one known combination of calls.

But all this requires that you can combine separate urls that occur in the multi-call, which the current setup can't do.

If we were to implement it this way, we'd have to expose a little bit more of the internals. Looking quickly at it, at a minimum we'd need runResource and routeResource. The utils that are there (toRestInput, mkHeaders and mkBodyParts) would probably also be useful.

Since this is all pretty uncertain, I'd be happy to just expose the thing with the right signature, and add an Internal module exposing all the stuff you need right now. The only important thing is getting the signature right, since that way we don't have to break our code later if we want to do more.

[lostbean]

It seems an interesting feature. Do you have any type signature in mind?

Maybe something like multiblahblah :: [(Method, RunnableHandler m -> Resource -> m BodyPart)] as a field in the RouterData. And the default multi-get implementation would be [(GET, (\h r -> = fmap (uncurry mkBodyPart) $ runRestM (toRestInput r) h)] if we expose some of the internals.

[hesselink]

I don't quite understand that signature... I was thinking more of something like [Resource] -> ErrorT Reason_ m [BodyPart]. The Resource here is Rest.Types.Container.Resource, not Rest.Resource, which might be confusing, so perhaps we shouldn't unpack the newtype, and instead pass a Resources, or rename the type.

What do you think about this signature?

[lostbean]

Me neither :(

Running cabal install code-builder/ rest-stringmap/ rest-client/ rest-core/ rest-gen/ rest-happstack/ rest-snap/ rest-types/ rest-wai/ --max-backjumps=-1 --reorder-goals --enable-tests --constraint='network==2.5.*' --jobs=1 just works.

According to the Travis log, the error seems related to some network error:

cabal: Error: some packages failed to install:
cryptohash-0.11.6 failed while downloading the package. The exception was:
user error (Failed to download
http://hackage.haskell.org/packages/archive/cryptohash/0.11.6/cryptohash-0.11.6.tar.gz
: ErrorMisc "Error HTTP code: 502")

[lostbean]

Now it works. It was some network error on Travis or Hackage.