cocoon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daniel Fagerstrom <dani...@nada.kth.se>
Subject Re: HttpServletRequest vs o.a.c.e.Request saga continues
Date Mon, 30 Jul 2007 11:00:05 GMT
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


Mime
View raw message