support for Portlet API about dellroad-stuff HOT 11 CLOSED

OK. I haven't worked with Portlets at all.

If you have some suggested patches, I'll take a look.

Portlets used to work fine as long as you implemented PortletRequestListener 
yourself, dig up the HttpServletRequest and HttpServletResponse from your 
PortletRequest and PortletResponse and passed them as 
onRequestStart(servletRequest, servletResponse) and 
onRequestEnd(servletRequest, servletResponse) to the SpringContextApplication. 
I just tried to upgrade, but I'm getting an exception now.

The problem lies here: 
https://code.google.com/p/dellroad-stuff/source/browse/trunk/src/java/org/dellro
ad/stuff/vaadin/SpringContextApplication.java#267 . The applicationContext 
variable is not a WebApplicationContext, but a PortletApplicationContext2 in 
case of portlets. Why not to use currentRequest().getSession() as the request 
is already set there?

Also, it would be nice if ContextApplication implemented PortletRequestListener 
also, so it would be easier to use portlets, but then you'd need to rework on 
abstraction as there's no standard way of getting HttpServletRequest out of 
PortletRequest. (For Liferay it's 
PortalUtil.getHttpServletRequest(portletRequest). I guess that's too much work.

I don't have any way to test portlet stuff, so if you will send me a patch of 
what you think should be done I'll test it still works OK with servlets and add 
it in.

By the way, why haven't you switched to Vaadin 7? And does the Vaadin7 version 
of this code need the same patch?

Sorry for late reply..

I've attached a hotfix, which remedies the situation for now at least.

I also took a look at a proper portlet implementation. It seems to be 
straightforward, but the thing is that currently ContextApplication directly 
references HttpServletRequest and HttpServletResponse interfaces and that would 
need some work.

Basically the implementation is almost just replacing HttpServletRequest with 
PortletRequest, HttpServletResponse with PortletResponse and 
WebApplicationContextUtils with PortletApplicationContextUtils. But then, how 
to do that..

The quickest way to implement that would be a eg. CURRENT_PORTLET_REQUEST 
ThreadLocal in ContextApplication, which is null when Servlet being used and 
vice-versa. But I'm not sure if I like that, do you? If you're happy with it, 
I'm up for implementing it. This is kinda what I'm doing currently, but I don't 
have a public API to manage :)

A better approach would be to move those into a separate class and behind an 
interface, but I'm not sure how to properly do this. Do you have any interest 
to refactor the servlet part? I'd say composition would be favourable as 
abstraction doesn't really sound good either: AbstractContextApplication, 
ServletContextApplication, PortletContextApplication, and thus 
SpringServletContextApplication, SpringPortletContextApplication, or something 
like that..

I'm using Vaadin 6 as the application I'm working on, is an internal one and 
without any interest to spend any additional time to move to Vaadin 7. Not my 
call.

I can't really comment if Vaadin 7 requires the same patch. They've done much 
refactoring and I have zero experience with Vaadin 7. A hastly look in the 
vaadin7 folder tells me it needs totally a different implementation.

I've applied your 'hotfix' patch in r844... thanks.

It's probably not worth doing substantially more work on the Vaadin 6 version, 
if this fix is working for now.

FWIW, I think Vaadin 7 has more 'wrapper' classes that make this kind of thing 
easier.

[deleted comment]

Thank you too, I'm happy with the fix! I'm waiting for it to land to Maven to 
test it.

It's totally understandable that there's really no point working on the Vaadin 
6 version, just wanted to share my thoughts. The CURRENT_PORTLET_REQUEST 
ThreadLocal "side-by-side" hack is easily done, though, if you are interested 
in it. I don't really need it, but I don't mind implementing it either. The 
hardest part is that I'm on Windows and I can't really build the jar with your 
scripts.

Yeah, I've read that they've done a lot of refactoring, and thus they've much 
better abstraction. Though, I doubt I'll be working with Vaadin 7 any time soon.

[deleted comment]

Hi again. I finally upgraded the package. Turns out you've switched to Java 7 
compiler between 794 and 850 (as per r840). This finally gave us a reason 
forcing us upgrade our project to Java 7. Anyway, I wanted to let you know that 
the fix is working!

Great! Glad to hear it.

Marking this issue fixed. If/when you get to Vaadin 7.x and have any issues you 
can open a new bug.