Controller definition with 22+ parameters in routes file

[Clara Allende]

Hi all!

In our application we manage URLs with lots of filters, which we get form the URL's query string. Once we hit the 22-parameter limit, we decide to move from scala 2.10 to scala 2.11, thus moving to Play! 2.3.7.
And then we stepped into a new problem: Play! 2.3 does not support controller definitions in routes file with more than 22 parameters. This is:

GET   /myUrl       myController(p1, p2, .... p23)

is not possible right now.

Putting aside the discussion of how do we handle the increasing amount of filters we have in the URLs (which is a matter of design of our application, and given our business constraints we cannot change immediately)... I have touched a little Play's routes compiler and router so that they take a list of parameters (thus making the controller definitions independent of the number of parameters). It's a minimal change, but it means a lot for us while we decide which approach to take regarding our long URLs.

Does it make sense for you? We know have a custom version of Play that we use to run our application, and which behaves correctly with those controller definitions. (Both for 2.3.x and for 2.4)
I am aware that ours is a very particular case, and that normally you don't have controllers with that many parameters... So I understand if this is not a priority for you.
I also know that there are some changes under the hood for routing in the upcoming Play 2.4, but I don't know if this one is feasible or not...

Maybe if I ask nicely you guys could provide us a solution (I am sure you know the glitches of routing better than I do). :)
But I didn't want to open an issue before asking here what do you guys think.

Thanks in advance!

[Husrev Ozayman]

Hi,

I wouldn't go the path of having a custom version of anything. 

You can have your custom router with custom handling of some routes and fallback to the default one. 

https://jazzy.id.au/2013/05/08/advanced_routing_in_play_framework.html

Something like this might work:

override def onRouteRequest(req: RequestHeader): Option[Handler] = {
  if (req.path == "/myUrl") {
    val v1 = req.queryString("p1")
    ...
    val v23 = req.queryString("p23")
    Some(myController(v1, .., v23))
  } else {
    super.onRouteRequest(req)
  }
}

--
You received this message because you are subscribed to the Google Groups "play-framework" group.
To unsubscribe from this group and stop receiving emails from it, send an email to play-framewor...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Clara Allende]

Yep, I didn't to have my own version of anything... but I'd like much more not being coupled to the number of parameters.

Besides, given that scala has already lifted this limitation, why don't do the same for controllers?

[alex s]

пятница, 27 марта 2015 г., 23:19:33 UTC+3 пользователь Clara Allende написал:

But I didn't want to open an issue before asking here what do you guys think.

понедельник, 30 марта 2015 г., 16:58:45 UTC+3 пользователь Clara Allende написал:

Besides, given that scala has already lifted this limitation, why don't do the same for controllers?

Do you want to miss a chance for inclusion in 2.4.x? :) Send a pull request already.

[James Roper]

Hi Clara,

My advice would be to not extract parameters out in the router, but rather extract them out directly from the request.  Are you using Java or Scala?  Both the Java and Scala request objects provide the query parameters as a map, so you can pull them out of that.

Regards,

James

--

You received this message because you are subscribed to the Google Groups "play-framework" group.
To unsubscribe from this group and stop receiving emails from it, send an email to play-framewor...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

James Roper
Software Engineer

Typesafe – Build reactive apps!
Twitter: @jroper

[Clara Allende]

Hi James,

We're using Scala. I am aware of that, and we have considered extracting the paramaters directly from the request, but we abandoned that path because of some conflicts with the framework we use to autogenerate our documentation (and of course we could change it, but that would imply a major change for us, and we need a faster solution).
Of course I understand our case is a *super* particular one, but I wanted to put this here for your consideration, given maybe someone finds the change useful...

[Clara Allende]

And besides, I don't want to lose the type checking that play does on the parameters in the query string :|

[alex s]

вторник, 31 марта 2015 г., 17:28:54 UTC+3 пользователь Clara Allende написал:

we have considered extracting the parameters directly from the request, but we abandoned that path because of some conflicts with the framework we use to autogenerate our documentation

 What about QueryStringBindable?

[James Roper]

On 1 April 2015 at 01:28, Clara Allende <clari....@gmail.com> wrote:

Hi James,

We're using Scala. I am aware of that, and we have considered extracting the paramaters directly from the request, but we abandoned that path because of some conflicts with the framework we use to autogenerate our documentation (and of course we could change it, but that would imply a major change for us, and we need a faster solution).
Of course I understand our case is a *super* particular one, but I wanted to put this here for your consideration, given maybe someone finds the change useful...

Fair enough.

So, the 22 parameter limit comes from the way Play puts the parameters into an Either of an error or a tuple of all the parameters, and then folds that Either using a Function, and the arity of Function and tuple only go up to 22 in Scala.

We can probably avoid the 22 parameter limit, here are two options I can think of:

* Rather than using tuples, put the parameters in an HList, calculate the errors from the HList, and then rather than using a FunctionN, generate code that takes a HList of parameters and applies it to your arbitrary length action method.

* Same as before using HLists, but generate code that curries the action function, which should make it straight forward to invoke by folding it with the hlist.

Some concerns that spring to mind:

* Performance.  It's probably only a problem when you have many parameters, in my experience most actions tend to have at most 5 or 6 parameters, so performance probably isn't a problem.

* Compile time performance.  I have no idea how well big hlists work in compilation.

Anyway, if you wanted to have a go at implementing this, I think we'd happily accept either of the solutions above, though the routing code in Play is quite convoluted in places, and the routes generator code is even more convoluted, if it were me implementing it, and I'm very familiar with the code, it'd probably take me up to a week.

Regards,

James

[Michael Slinn]

HLists are fine for many, possibly most circumstances. However, even HLists have limits; see Slow, exponential compile times for HLists > 200 elements

Personally, I'd also like the 18 field limit in Play forms removed, or at least greatly increased.

Mike

[Clara Allende]

Hi again :)

Well, I've already given it a go, and I got working without using HLists... indeed, the code of the router is quite convoluted, and I took me like a month to get to a solution that changes as little as possible (most of that time went on understanding what is going on in the router and figuring out where exactly should I change it)... you can check out what I did at:

https://github.com/ClaraAllende/playframework/commit/5c0259623063bd57435557707c9ac6af7a9a5182

and a little refactor to use fold instead of find-

https://github.com/ClaraAllende/playframework/commit/c94d94e8d3b9199a7a3289a15a74b2036327209d

I think it's the change is as minimal as I could possibly think of. I do not use HLists (I thought it introduced more fuzz that what I wanted to), and I don't couple the code to FunctionN. I keep on updating this fork wiht your mainstream...
Nevertheless, I am not aware of the performance impact this change may imply... but I've tried this custom build agains our application, and it works just fine.

I would really like to read your opinions/considerations about it :)
Thanks a lot for your patience!

[James Roper]

Hi Clara,

That looks quite straight forward, great to see the simplicity.

Performance wise, it introduces a new instanceof check for each parameter, since the type information is lost and then regained in the extractor.  I don't think that's anything to worry about, our performance tests should pick up any regressions here anyway.  If it were a problem, we could keep the callN methods for, say, up to 6 parameters, and switch to using a list for anything over that.  But I don't think it's a big deal.

I'd love to include this in Play, the only issue is you've made the change against Play 2.3, and Play 2.4 has completely refactored the routes compiler (note, that doesn't necessarily mean that it's any less convoluted..... well hopefully it is, but I think it's a problem that has to be gradually chipped away at).  I think you'll need to update the twirl templates, and one of the methods in the templates package object.

There's a number of scripted tests in the sbt plugin project for the routes compiler, you could add a 23 parameter method/route to that to test the code.

Cheers,

James

[Clara Allende]

Glad you like it :)

I've ported it to 2.4 as well :) So I have both versions working. And yes, I need to add a test ^^

On Monday, 6 April 2015 23:02:57 UTC-3, James Roper wrote:

Hi Clara,

That looks quite straight forward, great to see the simplicity.

Performance wise, it introduces a new instanceof check for each parameter, since the type information is lost and then regained in the extractor.  I don't think that's anything to worry about, our performance tests should pick up any regressions here anyway.  If it were a problem, we could keep the callN methods for, say, up to 6 parameters, and switch to using a list for anything over that.  But I don't think it's a big deal.

I'd love to include this in Play, the only issue is you've made the change against Play 2.3, and Play 2.4 has completely refactored the routes compiler (note, that doesn't necessarily mean that it's any less convoluted..... well hopefully it is, but I think it's a problem that has to be gradually chipped away at). 

Actually it is :) The fact that the code is now split in smaller classes makes it simpler to follow :)
 

[Clara Allende]

Ok, so now I've updated my master with the latest changes of Play's master (2.4). I also added a test for a 22+ parameter route.
https://github.com/ClaraAllende/playframework/tree/master
In addition, the branch for 2.3.x is up-to-date as well.
Should I create a pull request?

Thanks a lot for your help and patience :)

[James Roper]

Hi Clara,

Great, we're about to release Play 2.4.0-RC1, if you could submit a PR on master in the next 24 hours, that'd be great.  Make sure you squash your commits (or at very least, rebase to remove the merge commits, instructions here).

For 2.3.x, the change you've done breaks binary compatibility, so you may need to modify it a bit (run sbt mimaReportBinaryIssues locally to see what they are), the problems will be with the methods on Routes that you deleted, if you could leave them there but unused, that will solve the problem, and also adding the new call method to the trait, if you make this a static method on an object somewhere (will need to pass a badRequest thunk to it as well), that should solve that problem.

Regards,

James

[Clara Allende]

Done for 2.4, I'm signing the license agreement.
Will do for 2.3.x ASAP (tomorrow probably)

Thanks again!
Best regards