Possible bug with S.contextPath and CometActors

[Diego Medina]

Hi,

S.contextPath is defined as:

def contextPath: String = (request.map(_.contextPath) or session.map(_.contextPath)) openOr ""

Turns out that in the case of a CometActor, while  we don't have access to the Request, the value S.request is this:

Full(Req(List(), Map(), ParsePath(List(),,true,false), , GetRequest, Empty))

instead of Empty.

This means that def contextPath is doing:

Full() or Full("/validContext") //because the cometactor has access to the session info and we can get the context from the session object

which in turn results in S.contextPath being "", because the right side of the or isn't called.

If you all agree this is a bug, I can see at least a couple of ways to fix it:

1- change contextPath to simply look at the session value, assuming there isn't a container out there that can change the context path of a running app and keep the same session alive[1].

2- check if request.map(_.contextPath) and session.map(_.contextPath) are both the same, if so, use the value from the request, if the request is empty, and the session has a value, use the session value //this sounds like magic and hard to guess from a user point of view), and for this, we may as well do "option 1" imho)

3- Change the definition of Req.contextPath from String to Box[String], and change all the places that use contextPath and expect a String vs a Box[String]

4- Change the fact that a comet actor sees a Full(empty Req), and instead have it be an Empty Box

5- Some other awesome solution I haven't thought of

[1] Some google fu showed that tomcat and jetty only set the contextPath at boot time, so at least on those two we should be safe.

Thanks

  Diego

--
Diego Medina
Lift/Scala Developer
di...@fmpwizard.com
http://fmpwizard.telegr.am

[Andreas Joseph Krogh]

I stumbled into this once, https://groups.google.com/forum/#!msg/liftweb/By_bC0hmywU/vomn-492Fx8J, and I definitely think S.contextPath should return the same as theSession.contextPath in the context of a comet-actor.

 

--
Andreas Joseph Krogh <and...@officenet.no>      mob: +47 909 56 963
Senior Software Developer / CTO - OfficeNet AS - http://www.officenet.no
Public key: http://home.officenet.no/~andreak/public_key.asc

 

[Diego Medina]

In that case, to still support servicing the same app using netty and a regular container at the same time. we could use one of these:

3- Change the definition of Req.contextPath from String to Box[String], and change all the places that use contextPath and expect a String vs a Box[String]

4- Change the fact that a comet actor sees a Full(empty Req), and instead have it be an Empty Box

and I think option 4 may be the least invasive change.

Thanks

  Diego

 

 

--
--
Lift, the simply functional web framework: http://liftweb.net
Code: http://github.com/lift
Discussion: http://groups.google.com/group/liftweb
Stuck? Help us help you: https://www.assembla.com/wiki/show/liftweb/Posting_example_code
 
---
You received this message because you are subscribed to the Google Groups "Lift" group.
To unsubscribe from this group and stop receiving emails from it, send an email to liftweb+u...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

[Andreas Joseph Krogh]

På fredag 02. august 2013 kl. 18:50:18, skrev Diego Medina <di...@fmpwizard.com>:

In that case, to still support servicing the same app using netty and a regular container at the same time. we could use one of these:

 

3- Change the definition of Req.contextPath from String to Box[String], and change all the places that use contextPath and expect a String vs a Box[String]

4- Change the fact that a comet actor sees a Full(empty Req), and instead have it be an Empty Box

 

and I think option 4 may be the least invasive change.

 

I still don't understand why S.contextPath should be any different than S.locale, that is - using a "calculator" for calculating the value analogous to S.locale.

[Diego Medina]

3- Change the definition of Req.contextPath from String to Box[String], and change all the places that use contextPath and expect a String vs a Box[String]

4- Change the fact that a comet actor sees a Full(empty Req), and instead have it be an Empty Box

 

and I think option 4 may be the least invasive change.

 

I still don't understand why S.contextPath should be any different than S.locale, that is - using a "calculator" for calculating the value analogous to S.locale.

 

I guess that could be option 5 and then we can have the default as it is now and for those who run into issues when using cometactors, we let them just rely on the session information. But I still think that making S.request return an Empty box in comet actors would be a "good thing"™

Thanks

  Diego

 

--
Andreas Joseph Krogh <and...@officenet.no>      mob: +47 909 56 963
Senior Software Developer / CTO - OfficeNet AS - http://www.officenet.no
Public key: http://home.officenet.no/~andreak/public_key.asc

 

--

--
Lift, the simply functional web framework: http://liftweb.net
Code: http://github.com/lift
Discussion: http://groups.google.com/group/liftweb
Stuck? Help us help you: https://www.assembla.com/wiki/show/liftweb/Posting_example_code
 
---
You received this message because you are subscribed to the Google Groups "Lift" group.
To unsubscribe from this group and stop receiving emails from it, send an email to liftweb+u...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

[Andreas Joseph Krogh]

På fredag 02. august 2013 kl. 20:55:06, skrev Diego Medina <di...@fmpwizard.com>:

 

3- Change the definition of Req.contextPath from String to Box[String], and change all the places that use contextPath and expect a String vs a Box[String]

4- Change the fact that a comet actor sees a Full(empty Req), and instead have it be an Empty Box

 

and I think option 4 may be the least invasive change.

 

I still don't understand why S.contextPath should be any different than S.locale, that is - using a "calculator" for calculating the value analogous to S.locale.

 

 

 

I guess that could be option 5 and then we can have the default as it is now and for those who run into issues when using cometactors, we let them just rely on the session information. But I still think that making S.request return an Empty box in comet actors would be a "good thing"™

 

+1, and also +1 for making S.request return Empty in comet-actors.

[Diego Medina]

cool, if I don't get any negative votes over the weekend, I'll enter a ticket and find some time to send a pull request.

Thanks

  Diego

--

--
Lift, the simply functional web framework: http://liftweb.net
Code: http://github.com/lift
Discussion: http://groups.google.com/group/liftweb
Stuck? Help us help you: https://www.assembla.com/wiki/show/liftweb/Posting_example_code
 
---
You received this message because you are subscribed to the Google Groups "Lift" group.
To unsubscribe from this group and stop receiving emails from it, send an email to liftweb+u...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

[Diego Medina]

ticket created

https://github.com/lift/framework/issues/1495

and I hope to fins some time this week.

--

Diego Medina
Lift/Scala consultant
di...@fmpwizard.com
http://fmpwizard.telegr.am

[Antonio Salazar Cardozo]

I must have missed this when it first came up.

I think we'll want to make very sure before setting S.request to Empty, since that's a relatively large change in behavior vs expecting a Full there (even if it is a mock request). Is there a way to instead provide a proper Req instance associated with the session? I know I've seen that come up a couple of times but I don't remember exactly what David's concerns were with it. I haven't looked into it deeply enough to be sure, either, though reflexively it seems like it should be doable.

Thanks,

Antonio

[Diego Medina]

The reason for not using the session to fill in the Req of a comet was that if an app was being served from something other than a container (netty as one example), then we don't have the same rules as when running under a container.

Another reason for not constructing a Req for a comet is that by design we keep the cometActor not tied to the Request, but instead to the Session, so, even though it would be a breaking change, right now you don't get any useful information from a CometActor when accessing S.request.

The way I see it is that we have come this far with something that wasn't optimal and this could be our chance to fix it. And of course we would add this to the breaking changes section when announcing the next 2.6-M1 build (if this makes it there).

Thanks

Diego Medina
Lift/Scala consultant
di...@fmpwizard.com
http://fmpwizard.telegr.am