Support HttpSession proxying to allow portlet bridges to provide "scoped" session view to dispatched servlets

The Jetspeed-2 - Jetty support I'm working on can be tracked here: http://issues.apache.org/jira/browse/JS2-736

I am certainly open to change jetty to make this work.

But I think we can do this without the special interface.

instead of calling through an interface, could you try some code like this

Session s=(session instanceof Session)?(Session)session:getHttpSession(session.getId());

if that works... it will be a little slower... and we may need to do a fiddle for node/cluster id's

Ok, I will give that a try and let you know if it works.

But as you said: it's (a lot) slower, more complex and more risks for errors.
Why would you prefer that above my proposed interface (which is only seen/used internally)?

Yes, your solution works too, patch2.txt attached.
And I can see the benefit of doing it this way as it allows for more use-cases, like wrapping HttpSession instead of using a proxy.

I'm fine with either solution

Thanks for the quick response.

Actually, I have thought of a third way that is fast.
I will use the getAttribute(String) method and do an == check on a name of "org.mortbay.jetty.servlet.Session"
and if it matches, I will pass back the session.

So the line is now:

Session s = (Session)((session instanceof Session)?session:session.getAttribute(SESSION_ATTR));

because it is an == check, it is fast
because the string is private static, no other session user can get it.

So it is a perfect way to tunnel through any wrappers/proxies.

I have implemented this and will check in after a bit more testing.

cheers

Although through session.getAttributeNames() someone could get to this "private" attribute and thus theoretically even remove it, they're self to blame then I guess.
So for me its a fine solution too.

Ate,
this is not a real attribute and is not listed in the names, can't be replaced and does not use memory/cycles in the attribute map.

It is a common technique used by most container implementations to expose private APIs
via attributes.

This is checked in now and it would be good to know if this is working for you.
I am pushing a maven snapshot at the moment.

Ok, that's cool.

I'll try to test it out today and let you know.

Sorry for the delay, I'm planning to test it out tomorrow.

Sorry Greg for getting back on this so late (we've been very busy preparing a new Jetspeed 2.1.2 release), but I'm sorry to say your solution is not working.
And in fact (and I should have recognized this earlier), using a hardcoded attribute name is exactly what is not possible for this purpose.

This issue specifically concerns scoping session attributes to allow isolated views by multiple portlet instances.
The way we provide this scoped view is intercepting the get/set attribute methods with a Proxy class and prefixing the attribute names with a scope name (e.g. portletWindowId).

The problem with the solution you've committed is that a session.getAttribute("org.mortbay.jetty.servlet.Session") call made on our Proxy instance is at the end delegated to the real Jetty Session as getAttribute("<portletWindowId>_org.mortbay.jetty.servlet.Session"), with as result an NPE in the subsequent usage of the (not) retrieved real session instance:

java.lang.NullPointerException
at org.mortbay.jetty.servlet.AbstractSessionManager.complete(AbstractSessionManager.java:146)
at org.mortbay.jetty.servlet.SessionHandler.handle(SessionHandler.java:194)
at org.mortbay.jetty.handler.ContextHandler.handle(ContextHandler.java:712)
at org.mortbay.jetty.webapp.WebAppContext.handle(WebAppContext.java:405)
at org.mortbay.jetty.servlet.Dispatcher.include(Dispatcher.java:192)
at org.apache.portals.bridges.struts.StrutsPortlet.processRequest(StrutsPortlet.java:427)
at org.apache.portals.bridges.struts.StrutsPortlet.doView(StrutsPortlet.java:301)
at javax.portlet.GenericPortlet.doDispatch(GenericPortlet.java:247)
at javax.portlet.GenericPortlet.render(GenericPortlet.java:175)
at org.apache.jetspeed.factory.JetspeedPortletInstance.render(JetspeedPortletInstance.java:103)

So could you please review (again) my original proposal of adding an JettySession interface to AbstractSessionManager.Session?
It is a solution which is 100% save (and tested already) and I think not very expensive either.

OK... I hate making others implement jetty specific interfaces, but if you are happy to do so...

I have checked in the change, but the interface is nested :

AbstractSessionManager.SessionIf

this will be in jetty 6.1.5rc0 this week.

thanks for persisting.

Great, I will try to test it tomorrow.

FYI, this interface is not needed to be implemented or used (directly) by anyone outside the AbstractSessionManager itself.
We use a dynamic proxy which just proxies any interface the session object has.
That way, calling the interface method to get back to the real instance simply passes through the proxy, but the "client" code never really needs to know, nor wants to care

Confirmed this now works fine.
Thanks a lot Greg for accepting the patch.

This allows us now to declare Jetspeed-2 to work 100% on Jetty 6.1.5+.
After our Jetspeed 2.1.2 release (planned this weekend), I will add Jetty based installation support out-of-the-box!

Regards,

Ate