Guys,
I'd like to respond here on the concerns raised by Al, Gwyn and Kent on the "Re:
[jira] Resolved: (WICKET-647) New Wicket Portlet support" thread.
As I created a dedicated JIRA issue, WICKET-983, for this as well as this
specific mail subject, I think we should try to centralize the discussion about
it here.
First of all, thanks a lot for taking the time to look into this and review the
changes I propose for adding portlet-support.
I do understand the concerns raised but I'd like to point out that, at least in my view, the impact for *normal* wicket runtime (e.g. servlet, not portlet) the
changes really are very, very minimal.
Al, Gwyn and Kent: you indicate that you see major and/or fundamental changes
to wicket, but please make explicit which changes you are referring to.
Now I only have something like "the portlet changes /do/ touch some fundamental bits of Wicket", "making major changes to the codebase" and "It seems that
those changes are quite fundamental" to respond to.
Anyway, I'll try to make clear why in my view the portlet changes aren't going to cause
"major" or "fundamental" changes in the core wicket.
If you evaluate the issues I listed in WICKET-983 and how I resolved those, you'll see that the changes which *might* have a small to zero effect on the
standard wicket behavior are the following:
1) WICKET-649: fix appending query parameters
a) replacing blindly appending "&" with a check if it really should be a
"?"
In my view harmless and actually really improving wicket quality itself
b) moving AbstractAjaxBehavior.getCallbackUrl() appending
"wicket:ignoreIfNotActive=true" to WebRequestCodingStrategy.encode().
This is a quite simple reordering of internal processing with exactly the
same functionality and result as outcome, quite harmless I think
c) wicket-ajax Wicket.Ajax.Request.get(path) checking on query parameters
and in that case replacing it with a Request.post(body) call
This one I think can/should be discussed as it really does result in a
*technically* different behavior.
I do think in the case of Ajax requests, this really doesn't matter as it
won't effect the browser state at all, and my solution really is
most transparent one I could think of.
But, as it also changes the behavior in a plain servlet environment, and as
such it I could understand people would feel a bit uncomfortable with it.
I already suggest in the description for this change I'm open to other
solutions, like only doing this when running in a portlet environment.
Then, this issue would become moot for the servlet environment, we just need
to find some way to indicate the current environment (servlet|portlet)
in javascript to the browser.
d) IOnChangeListener components with
wantOnSelectionChangedNotifications()==true
If you review my changes you'll find out there are *zero* changes when
running in a servlet environment, as is with almost all other changes.
Based on the boolean evaluation of RequestContext.isPortletRequest(), which
is always false in a servlet environment as you can imagine, only the original
code is executed. Only the portlet specific behavior is "wrapped" in a
simple if (RequestContext.isPortletRequest())... nothing more.
2) WICKET-650: namespacing component markupId
This change does nothing when not running in a portlet environment
3) WICKET-651: extending IHeaderResponse
This change only adds a close() and isClosed() method to IHeaderResponse and
delegates getResponse() to a getRealResponse() implementation.
These changes also have no side-effect, all current behavior is maintained. It
just allows capturing the HeaderResponse when running in a portlet context.
It also allows you now to do the same in a servlet environment if you would want that, so it is an expansion of the api but it is not used other than from
portlet specific code.
4) WICKET-657: upgrading wicket-examples to require servlet api 2.4
I don't consider this a "major" change. For one, its just an example and not touching wicket core at all. Furthermore, I doubt there will be many users still
stuck on a servlet 2.3 container, especially as wicket-examples already requires Java 1.5 anyway.
Anyway, this change isn't critical for merging the portlet support in the trunk
but for demonstration purposes it would be nice to be able to use it.
5) WICKET-924: non-relative urls in Ajax.Request.redirect callback handling
Actually, I would consider proposing this change regardless of the discussion
of portlet-support. The current hard coded expectancy an Ajax callback redirect
can and may only be a Wicket relative url doesn't seem very flexible. For the
portlet-support this change is needed though.
And, if the redirect url *is* relative as expected in the current
implementation, my changes won't do anything anyway.
But if there are major objections to it, we could discuss solving it in a similar
way as we could do with the Ajax.Request.get(path) -> .post(body) change.
If we can push the current runtime environment (servlet|portlet) to the browser
the ajax script could maintain the current behavior when running in a servlet.
6) WICKET-926: recognizing popup/detached pages urlFor calls
This change also does nothing when not running in a portlet environment. But I
did listed in WICKET-983 because the solution I chose didn't exactly result in
"beautiful" code... I couldn't come up with something else though which would
be just as transparent and without *any* side-effects.
So, I'm actually hoping someone calls me stupid and comes up with a more "feel
good" solution :)
That's it. All other changes to existing wicket classes will only come into effect when run in a portlet environment. When running in a servlet environment
Wicket behaves exactly as before!
Concluding: in my view the only real "issue" we might want to discuss is the
Ajax.Request.get(path) -> .post(body) change.
I personally don't see it as an issue, but if that change would be viewed as unacceptable/dangerous for the current trunk, it should not be difficult to come up
with another solution which will keep the current behavior in a servlet environment (although that then will have to be a little bit less transparent I'm afraid).
So, if this assessment is valid, I really don't see why portlet support
couldn't be integrated into the 1.3.0 trunk.
Again, I do understand the concerns raised, but don't forget quite a few major changes (like for bugfixes, but probably not even only for that) have been
committed, and still are committed to the trunk too. Some of those might have (much) more side-effects and consequences then adding portlet-support...
Delaying portlet-support to after the 1.3 release really would be a shame in
this light in my view.
For those of us who don't need/care about portlet it won't matter, but I know quite a few developers and project teams are eager to be able to use Wicket for
portlet development. Waiting until the 1.4 branch stabilizes really means cutting them out.
Causing a delay of the 1.3 release isn't good for the community (and I don't think should be needed) but delaying portlet-support until after the 1.3 release
isn't good for the community either.
I do hope my above explanations make it a bit clearer what the real issues are
to discuss.
If anyone of you still have reservations and/or major objections of merging portlet-support in the current trunk, please give it head and toe and indicate which
changes you think are questionable so we can properly discuss them.
Regards,
Ate
Ate Douma wrote:
I've created https://issues.apache.org/jira/browse/WICKET-983 to help
out reviewing the portlet-support changes.
Although I initially planned to create separate "sub" patches for
reviewing the important wicket changes individually, this turned out to
be a
rather impossible task. The complete size of the full patch is almost
200k and several different issues I needed to solve concerned the same
classes/files. Breaking all that up in digestible peaces would cost just
to much time to be realistic.
I did however upload a full patch file and provide a short summary of
the most important changes with links to the specific subtask issues of
WICKET-647.
These subtasks provide more detailed descriptions of the issues at hand
as well as the commit logs of the changes I made.
All committers or otherwise interested: please take some time to at
least read the description of the problems I needed to solve.
If you see anything you don't like or would have chosen a different
solution, please bring it up here so we can discuss it.
I would like to merge the portlet support branch into the trunk before
the -beta4 cutoff, so your input on this is very much appreciated.
Thanks,
Ate Douma
