Grzegorz Kossakowski skrev:
Hi guys,
============ Background ============
My recent commit r559394[1] broke Cocoon again. More specifically, you
will get
java.lang.NoClassDefFoundError: org/apache/commons/beanutils/ConvertUtils
or JXPath's error that it can't find
org.apache.cocoon.forms.generation.JXMacrosHelper.createHelper function.
This error has been mentioned[2] by Reinhard long time ago and Daniel
gave response[3]:
You are only supposed to have direct access to java.*, IIUC, packages
from FOM for other packages you must have a "Package." prefix, i.e.
value="#{Package.org.apache.cocoon.forms.generation.JXMacrosHelper.createHelper($cocoon/consumer,$cocoon/request)}
This is the behaviour that is implemented in
o.a.c.environment.TemplateObjectModelHelper.addJavaPackages.
So I thought that I messed some initialization and wasted handful amount
of time to track down the problem. I was wrong. JXPath uses it's default
set of functions if none is registered (and it *is* the case, so
Daniel's response was completely misleading) that gives access to all
Java classes. The error I'm getting is because of createHelper signature:
public static JXMacrosHelper createHelper(XMLConsumer consumer,
Request request, String locale)
My response wasn't misleading when I wrote it, but the code has changed
since then:
http://svn.apache.org/viewvc/cocoon/trunk/blocks/cocoon-forms/cocoon-forms-impl/src/main/java/org/apache/cocoon/forms/generation/JXMacrosHelper.java?r1=307119&r2=321148,
http://svn.apache.org/viewvc?view=rev&revision=416391.
It expects o.a.c.e.Request but after applying[1] object model contains
HttpServletRequest thus JXPath's evaluation fails.
I'm not certain that it is such a good idea to use HttpServletRequest
instead of o.a.c.e.Request in your new object model. By doing that you,
except for breaking some code, miss info about the path of the current
sub sitemap which might be useful. Request contains the following
methods that not is part of HttpServletRequest:
String getSitemapURI();
String getSitemapURIPrefix();
String getSitemapPath();
============ Possible solutions ============
The half of the work is already done - we know why it is broken, now I
would like to discuss with you possible fixes. We have three distinct
options:
Four options with the one I described above.
1. Store o.a.c.e.Request implementation in Object Model and provide
wrappers that would be manually created in places where needed.
HttpRequest (which is only implementation of Request interface that we
can consider) wraps HttpServletRequest. It means we could have several
wrapping levels. *Messy* to my taste.
We already has these wrapping levels in current Cocoon so you don't add
any "messiness", AFAICS. But some way to differ between the Request and
the HttpServletRequest would be needed while accessing the object model.
One could use e.g. "cocoon.request" and "cocoon.httpservletrequest". But
that does not exactly simplifying the object model, which is your task,
so I don't like this option.
2. Make a o.a.c.e.Request interface extend HttpServletRequest interface.
That would be a solution, because we could store such implementation in
Object Model as HttpServletRequest and cast where its needed to
o.a.c.e.Request (JXPath would do it automatically for us). Such solution
was mentioned several times before, like here[4] by Carsten. I haven't
evaluated this option much. Does it still makes sense, what about
concerns raised by Carsten? Are there any other?
This is the preferred option IMO. It is simple, easy to understand and
essentially back compatible. As I discussed in
http://marc.info/?l=xml-cocoon-dev&m=118578978307510&w=2 I think the
minor incompatibilities that we would get is well worth the advantages.
3. Forget about back compatibility and clean up the mess, start
switching to HttpServletRequest *right away*. In most cases the changes
would be trivial (changing imports in various classes). Of course I
don't want to get rid of Cocoon's Request interface everywhere at once
because it's not in scope of my GSoC work. I only want to get rid of it
where it's needed.
"Cleaning up the mess" sounds like a rather vague plan to me ;) I would
prefer some suggestions about how it could be done and what consequences
it would have. You still need to handle the sub sitemap path stuff that
is part of Request but not HttpServletRequest.
My proposal would be to combine my option in the beginning with option
2, and follow option 3 in the sense that Request can be switched to
HttpServletRequest everywhere in the code where the sub sitemap path
methods not are used.
Ok, you guessed it I prefer third option. I've performed already
incompatible changes like Ajax and Forms migration to
servlet-service-fw[5] and provided quite good migration guides like[6].
People were happy with it... I think that the key was they clearly knew
what to do with their applications in order to get back them to usable
state. Isn't it a case, Felix?
I could provide such guide for incompatible changes if they occurred.
After playing with Cocoon's internals for some time I think that there
is no way that I can improve something significantly and stay 100%
back-compliant because current functionality relies on hacks, plumbing,
bungs and other weird things. Undoubtedly, we are approaching to my
sermonizing.
============ Sermonizing ============
I must confess that I'm fed up with Cocoon's internals.
Cocoon's internal are fucking crazy.
I agree. As you see there is some reason that only a few dare to touch
it. But it is nothing unique for Cocoon, most projects that has been
around for many years suffers from the same problems. Continuous
refactoring and rearchetecturing is needed to keep the code manageable.
While it might be hard to believe, it has been even worse, we have
considerably simplified the code in several areas (the setup sequence
i.e. was complicated beyond belief before). And now you are working to
simplify another significant area. Still it is from time to time a
deeply frustrating experience to clean up the mess.
Sorry for strong words but I must give vent to my emotions. Now more
matter-of-fact arguments:
* We have two different request objects original servlet's one and
our own one. The problems that this situation causes has been discussed
above.
Option 2, solves the involved problems. The sub sitemap paths still
require us to have some custom interfaces.
* We have at least four different object models!
a) objectModel map that is passed to components
I think we should deprecate (or at least discourage) the use of the
object model. With a sitemap scoped beans for the environment objects
(as described in
http://marc.info/?l=xml-cocoon-dev&m=118578691800176&w=2) we have a
mechanism for getting environment objects with DI, which is much
preferable IMO.
b) FlowscriptObjectModel
c) TemplateObjectModel (if template was called by flowscript)
d) TemplateObjectModel (if template wasn't called by flowscript)
That is part of the scope of your GSoC to clean up isn't it?
Items c) and d) do not differ only in data user puts while calling
sendPage, it would be too obvious. It differs in some internal, obscure
areas so some functionality working in template called from flowscript
(like constructing new objects) will not work if you just call it out of
flow execution scope.
Is the differences intentional or are they bugs?
/Daniel
