Review Board 1.5.4

ContextPath clean-up

Updated 2 years, 1 month ago

Marius Danciu Reviewers
269 dchenbecker, dpp, marius, charles, joni, atsuhiko, jorge, naftoli, kris, indrajit, alexb
None LiftWeb-archive
First of all I took the liberty of doing some changes ... some of them being breaking changes. Conceptually I see the LiftRules.calculateContextPath as a way for applications to specify the context path from their own config files or some other logic such as system variables. The value of the context path is typically an environment setting rather then a quantity that should change per request/session. Therefore LiftRules.calculateContextPath doesn't take the request parameter any more. But there is another reason for this change: ... for instance the default implementation was looking for X-Lift-ContextPath HTTP header attribute. However this lead to inconsistencies such that if a comet actor is generating some markup containing links etc. Lift could not fix those urls to add the contextpath since at this point lift doesn't have the request in order to extract the header information. Thus those links were "corrupted". 

Users could still implement LiftRules.calculateContextPath and inside use S but on their own risks as not always S is available when LiftRules.calculateContextPath is invoked. Therefore in order to be safe with contextpath, the contextpath should not be determined based on the request information and should be encouraged to be invariant or a global setting per application deployment. And I think this is the very point of the contextpath.

If you feel different, please elaborate and let's see what the use-cases are.

Br's,
Marius
Yes by setting:

LiftRules.calculateContextPath = () => {
  Full("/myapp")
}

and having JEE container contextPath as "/"
Posted 2 years, 1 month ago (January 1st, 2010, 10:45 a.m.)
Ok, so I just applied the diff and tried a whole bunch of things. The app works now (which is great), however, none of the CSS, JS etc load because they are not being served by the container as it does know about the lift context override. This they gives us a problem around resources that developers put in the webapp directory... IMHO, its counter intuitive for this stuff to "just stop working" so we should provide some means that these resources keep working as one would expect. Thoughts?
  1. Oh, one other thing. I had to change my rewrite proxy to be: 
    
    RewriteProxy /demoserver/proposals/(.*) http://127.0.0.1:8081/demoserver/proposals/$1 [H]
    
    Is this correct, or would we prefer it to be:
    
    RewriteProxy /demoserver/proposals/(.*) http://127.0.0.1:8081/$1 [H]
    
    That is, when lift automatically changes the context, but resources and assets continue to be served by the container. I think the latter example was Ross' setup also. 
  2. Right since the context set by the calculateContextPath is unknown to the container the resource would not be found. For instance:
    
    Assume calculateCotnextPath = /mycontext
    
    a request to /mycontext/js/jquery.js will fail as the container cannot find /mycontext/js/jquery.js
    
    To work around this you can try:
    
    1. Put your js folder under /mycontext folder so that container can see the path /mycontext/js/jquery.js. The big downside is that this is standing in the way of flexibility since if calculateContextPath would return some other contextpath
    
    2. Put your js/css resources in toserve folder and reference them using /classpath/... This is the standard Lift way of referencing resources.
    
    3. Try to think of something else ...
  3. Yeah - good thinking batman. I moved the resources to toserve, and added the resource server stuff... all works now. Super! 
Review request changed
Updated 2 years, 1 month ago (January 2nd, 2010, 1:15 a.m.)
added more safeguards
Ship it!
Posted 2 years, 1 month ago (January 4th, 2010, 7:31 a.m.)
Nice cleanup.  I agree contextPath shouldn't be computed dynamically from HTTPRequest.
Ship it!
Posted 2 years, 1 month ago (January 4th, 2010, 10:02 a.m.)
Looks good