support for Portlet API
Closed this issue · 11 comments
GoogleCodeExporter commented
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
GoogleCodeExporter commented
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
GoogleCodeExporter commented
Original comment by archie.c...@gmail.com
on 13 Jan 2012 at 3:06
- Added labels: Type-Enhancement
- Removed labels: Type-Defect
GoogleCodeExporter commented
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
GoogleCodeExporter commented
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
GoogleCodeExporter commented
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:
GoogleCodeExporter commented
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
GoogleCodeExporter commented
[deleted comment]
GoogleCodeExporter commented
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
GoogleCodeExporter commented
[deleted comment]
GoogleCodeExporter commented
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
GoogleCodeExporter commented
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