java-war/dellroad-stuff

support for Portlet API

Closed this issue · 11 comments

Support for Portlet API would be a great feature.

Original issue reported on code.google.com by r.nagrod...@gmail.com on 13 Jan 2012 at 10:34

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

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

Original comment by archie.c...@gmail.com on 13 Jan 2012 at 3:06

  • Changed state: Accepted

Original comment by archie.c...@gmail.com on 13 Jan 2012 at 3:06

  • Added labels: Type-Enhancement
  • Removed labels: Type-Defect
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.

Original comment by jani.lus...@gmail.com on 4 Mar 2014 at 8:40

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?


Original comment by archie.c...@gmail.com on 4 Mar 2014 at 4:09

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.

Original comment by jani.lus...@gmail.com on 11 Mar 2014 at 3:27

Attachments:

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.

Original comment by archie.c...@gmail.com on 17 Mar 2014 at 10:32

[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.

Original comment by jani.lus...@gmail.com on 18 Mar 2014 at 4:44

[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!

Original comment by jani.lus...@gmail.com on 25 Mar 2014 at 7:05

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.

Original comment by archie.c...@gmail.com on 25 Mar 2014 at 2:47

  • Changed state: Fixed