Object Model-related changes in Template generator

[Grzegorz Kossakowski <gr...@tuffmail.com>]

Hi,

It's been three times that I reverted my changes introducing ObjectModel interface and using it in Template generator. I must admit that I 
got stuck in unproductive state because I'm constantly unhappy with the design. Let me share my troubles in a hope that someone is much 
clever than me and will give some advise.

As discussed earlier[1][2], Object Model will be simple Map that can contain more than item for each key and preserves order in which items 
were added. I found (after several attempts to invent one myslef) MultiMap[3] interesting and close to what I want to achieve. It resulted 
in very simple ObjectModelImpl[4] that is only a prototype and will be replaced by Spring's, scoped bean that will utilize CallStack as 
Daniel proposed earlier.

I wanted to use ObjectModel interface in cocoon-template-impl and cocoon-expression-langauge-impl modules. After reading all the code I 
found that current implementation is little unsatisfying. I'll explain my concerns.

Let's examine how ExpressionContext[5] class looks like and what's purpose. Basically, it's a holder for data passed to expression 
evaluators. It can contain several named variables, context bean and namespace table. This class is really vague for me because:
   * it's not clear what's the difference between context bean and named variables (you can put everything everywhere)
   * namespace table is very mysterious thing when it comes to general data holder and seems to be very specific

To find out what's the purpose for variabales/context bean you must take a look at all implementations of Expression interface and find that 
it's rather JXPath-specific feature. If ExpressionContext holds something as variable ("variable1") you can access that data by using 
"$variable1/[...]" expression, if the same data is held as context bean it's available by "./[...]". Other expression languages 
implementations ignore variables completely so if you happened to rely on this feature you are in big trouble. How intuitive...

Namespace table is also very specific to JXPath (because only JXPath has concept of namespace implemented) and I'll not discuss in detail 
how it works. It only makes me wonder why it is put in rather general ExpressionContext class. Situation is even more worst because Template 
generator uses this namespace table for its very internal reasons (emitting right SAX events, cleaning up namespace declarations and so on).

I'll show you, as an example, execute() method of o.a.c.template.script.event.Event class:

     public Event execute(final XMLConsumer consumer,
                          ExpressionContext expressionContext,
                          ExecutionContext executionContext,
                          MacroContext macroContext, Event startEvent, Event endEvent)
         throws SAXException {
         return getNext();
     }

You can see that method's signature is already quite fat - there are three different contexts. What I'm going to propose is not much better 
because I want to introduce one more argument (NamespaceTable namespaces) and change existing ExpressionContext expressionContext to 
ObjectModel objectModel. I just want to move namespace handling out of general interface for data holders.

Before calling Expression#evaluate() I would like to add namespace table to Object Model with "namespace" (or similiar) key so it's 
available for JXPath implementation.

Ok, I'm not sure if such approach of slimming interface to absolute minimum and putting things in a Map is right but nothing better comes to 
my mind.


Could you give your advice? I'm really afraid that I'd made a commit only to realize that my changes are dumb and there is better design.


[1] http://article.gmane.org/gmane.text.xml.cocoon.devel/73700
[2] http://article.gmane.org/gmane.text.xml.cocoon.devel/73760
[3] http://jakarta.apache.org/commons/collections/apidocs/org/apache/commons/collections/MultiMap.html
[4] http://article.gmane.org/gmane.text.xml.cocoon.cvs/24747
[5] 
http://svn.apache.org/viewvc/cocoon/trunk/core/cocoon-expression-language/cocoon-expression-language-api/src/main/java/org/apache/cocoon/components/expression/ExpressionContext.java?view=markup

-- 
Grzegorz Kossakowski
http://reflectingonthevicissitudes.wordpress.com/

Re: Object Model-related changes in Template generator

[Daniel Fagerstrom <da...@nada.kth.se>]

Grzegorz Kossakowski skrev:
> Hi,
> 
> It's been three times that I reverted my changes introducing ObjectModel 
> interface and using it in Template generator. I must admit that I got 
> stuck in unproductive state because I'm constantly unhappy with the 
> design.

You are probably striving for perfection and think that you can achieve 
it by staying in the design phase just a little bit longer. But it 
doesn't work that way. Perfection takes for ever and we don't have that 
much time ;) Be happy with what is good enough.

Also you need to start implementing your design. Choose a design (NOW!), 
stick with it for a while and implement the important parts of it in the 
simplest possible way. By doing this you get *real* experience with the 
pros and cons of your design choices and can refactor and redesign if 
necessary until it becomes good enough. You will also get better 
feedback from the rest of us by committing code.

As a good rule of thumb: when the design phase stops to be stimulating 
and has become frustrating it is better to start to implement something ;)

Just as an example, the servlet service framework in its current shape 
is the fifth mayor reimplementation since we started to actually 
implement it. Before that the community did design for a couple of 
years, which in the end was really frustrating for everyone involved. 
Also we spent so much time implementing things that would have been 
certainly would have been nice to have but that is not part of the 
current implementation.

> Let me share my troubles in a hope that someone is much clever 
> than me and will give some advise.
> 
> As discussed earlier[1][2], Object Model will be simple Map that can 
> contain more than item for each key and preserves order in which items 
> were added. I found (after several attempts to invent one myslef) 
> MultiMap[3] interesting and close to what I want to achieve. It resulted 
> in very simple ObjectModelImpl[4]

I'm not convinced. I would prefer to just use an implementation of the 
ordinary Map that returns *one* item for each key (respecting the stack 
order in the underlying stack based model).

While the MultiMap idea solves the problem of "../1" patterns in the 
map: EL, it complicates all the rest of the ELs. Normally, there is no 
point in being able to be able to access values for a parameter name 
that are shadowed by newer values in a stack based context. The only 
reason that this is needed in the map language is that the parameter 
naming is automatic 1,2,3,... so that values that you might need are 
shadowed automatically. It would have been better if the map language 
would have had user named parameters instead, it would have been easier 
to understand and we wouldn't have the current problem.

I would propose that we create a new interface that extends Map and 
provide a method like:

   List getStack(Object key);

or something similar. Then the map language implementation could depend 
on this interface for handling its stack access, while the rest of the 
ELs only needs to do access on a Map.

> that is only a prototype and will be 
> replaced by Spring's, scoped bean that will utilize CallStack as Daniel 
> proposed earlier.

I've changed my mind ;) I still believe that using a custom Spring scope 
that is similar to the CallStack is a good idea. But I think it might be 
overly complicated to evolve the current CallStack implementation so 
that it covers all our stack based bean access needs. I think it is 
better to have an own stack for the object model. Then we can see at a 
later stage if we can find some common abstraction. Right now I'm afraid 
that it would just complicate things.

> I wanted to use ObjectModel interface in cocoon-template-impl and 
> cocoon-expression-langauge-impl modules. After reading all the code I 
> found that current implementation is little unsatisfying. I'll explain 
> my concerns.
> 
> Let's examine how ExpressionContext[5] class looks like and what's 
> purpose.

The design was based on Jex 
http://svn.apache.org/repos/asf/jakarta/commons/dormant/jex/ a 
whiteboard project in Jakarta commons. Compared to that it was 
simplified, Avalon based and adapted to Cocoon's needs. More about the 
design can be found here 
http://marc.info/?l=xml-cocoon-dev&m=110595826010124&w=2.

> Basically, it's a holder for data passed to expression 
> evaluators. It can contain several named variables, context bean and 
> namespace table. This class is really vague for me because:
>   * it's not clear what's the difference between context bean and named 
> variables (you can put everything everywhere)
>   * namespace table is very mysterious thing when it comes to general 
> data holder and seems to be very specific
> 
> To find out what's the purpose for variabales/context bean you must take 
> a look at all implementations of Expression interface and find that it's 
> rather JXPath-specific feature. If ExpressionContext holds something as 
> variable ("variable1") you can access that data by using 
> "$variable1/[...]" expression, if the same data is held as context bean 
> it's available by "./[...]".

It is a JXPath specific feature.

> Other expression languages implementations 
> ignore variables completely so if you happened to rely on this feature 
> you are in big trouble. How intuitive...

Might be. But it is not that easy, JXPath is supposed to be applied on a 
bean (or DOM) and optionally parameters. Jexl on a map. I felt that 
restricting to a common subset of input had been to limiting. You will 
be in big trouble if you try to access an XML DOM with Jexl, but it 
works fine with JXPath.

For our current use, my design was probably overly generic. We could 
remove the contextBean and replace it with a reserved variable name in 
the map in  the context.

> Namespace table is also very specific to JXPath (because only JXPath has 
> concept of namespace implemented) and I'll not discuss in detail how it 
> works. It only makes me wonder why it is put in rather general 
> ExpressionContext class.

No idea, I was not involved in implementing this part.

> Situation is even more worst because Template 
> generator uses this namespace table for its very internal reasons 
> (emitting right SAX events, cleaning up namespace declarations and so on).
> 
> I'll show you, as an example, execute() method of 
> o.a.c.template.script.event.Event class:
> 
>     public Event execute(final XMLConsumer consumer,
>                          ExpressionContext expressionContext,
>                          ExecutionContext executionContext,
>                          MacroContext macroContext, Event startEvent, 
> Event endEvent)
>         throws SAXException {
>         return getNext();
>     }
> 
> You can see that method's signature is already quite fat - there are 
> three different contexts. What I'm going to propose is not much better 
> because I want to introduce one more argument (NamespaceTable 
> namespaces) and change existing ExpressionContext expressionContext to 
> ObjectModel objectModel. I just want to move namespace handling out of 
> general interface for data holders.
> 
> Before calling Expression#evaluate() I would like to add namespace table 
> to Object Model with "namespace" (or similiar) key so it's available for 
> JXPath implementation.

Sounds reasonable.

> Ok, I'm not sure if such approach of slimming interface to absolute 
> minimum and putting things in a Map is right but nothing better comes to 
> my mind.
> 
> 
> Could you give your advice? I'm really afraid that I'd made a commit 
> only to realize that my changes are dumb and there is better design.

You are taking things to seriously. Don't worry, just commit things and 
refactor and change them later if you or anyone else get a better idea.

By not committing, you can be absolutely sure that your work will be of 
no use.

/Daniel

Re: Object Model-related changes in Template generator

[Grzegorz Kossakowski <gk...@apache.org>]

Joerg Heinicke pisze:
> On 12.07.2007 16:34, Grzegorz Kossakowski wrote:
> 
>> it's rather JXPath-specific feature.
> 
>> Namespace table is also very specific to JXPath
> 
> I understand your concerns. Unfortunately, it's hard to give any advice 
> without knowing the internals.

I know and it makes me feel even more grateful for your attempt. :-)

>>     public Event execute(final XMLConsumer consumer,
>>                          ExpressionContext expressionContext,
>>                          ExecutionContext executionContext,
>>                          MacroContext macroContext, Event startEvent, 
> 
> What are all these contexts about? Couldn't they somehow be merged? What 
> are their life cycles? Are they all different?

They have different life cycles:
   * executionContext is created only once, at the beginning of template processing
   * macroContext is overrided every time macro is executed
   * expressionContext has the same life cycle as executionContext

It's a good idea to consider merging some of them.

> Isn't for example the 
> namespace context quite similar to a variable context? Or asked in 
> different words: Isn't a namespace prefix something like a variable? 
>  From a short look on the code (only via ViewVC, no Eclipse) similar 
> stuff seems to happen with Java packages for Rhino 
> (FlowObjectModelHelper.addJavaPackages()). I wonder if this makes sense 
> for all cases.

You are right, but since namespaces are also used for generators purpose it makes sense to have a separate parameter for it.

> Decoupled from the names and concepts used in the current 
> implementation: What is actually needed to evaluate an expression? The 
> expression itself and the context it is evaluated in. This expression 
> context stores all information in it the expression can or needs to 
> access like for example the namespaces.
> 
>  From what I see there is only one expression context during the 
> rendering of a template at the moment. It might be necessary to create 
> one expression-specific per expression. They can then also be expression 
> language specific and Expression.evaluate() could retrieve expression 
> language specific information like namespaces from it. The current 
> ExpressionContext would get a very simple interface not aware of 
> concepts like namespaces. Since those "things" need to get injected on 
> instantiation of the ExpressionContext there also needs to be somthing 
> that keeps track of those "things". These track-keepers have different 
> information needs per expression language, so they need to be registered 
> somewhere.
> 
> Hmm, this sounds like a major change - maybe not even reasonable without 
> knowing how it is really supposed to work. Does it make sense? Maybe you 
> should really go with the simple and "fat" solution for the moment since 
> those expression language specifics are "in" at the moment anyway.

Yes, I'll go with fat solution for now and decide later what is really needed. Also, I think that ExpressionContext should be passed to 
Expression by Spring and not by caller.

-- 
Grzegorz Kossakowski
http://reflectingonthevicissitudes.wordpress.com/

Re: Object Model-related changes in Template generator

[Joerg Heinicke <jo...@gmx.de>]

On 12.07.2007 16:34, Grzegorz Kossakowski wrote:

> it's rather JXPath-specific feature.

> Namespace table is also very specific to JXPath

I understand your concerns. Unfortunately, it's hard to give any advice 
without knowing the internals.

>     public Event execute(final XMLConsumer consumer,
>                          ExpressionContext expressionContext,
>                          ExecutionContext executionContext,
>                          MacroContext macroContext, Event startEvent, 

What are all these contexts about? Couldn't they somehow be merged? What 
are their life cycles? Are they all different? Isn't for example the 
namespace context quite similar to a variable context? Or asked in 
different words: Isn't a namespace prefix something like a variable? 
 From a short look on the code (only via ViewVC, no Eclipse) similar 
stuff seems to happen with Java packages for Rhino 
(FlowObjectModelHelper.addJavaPackages()). I wonder if this makes sense 
for all cases.

Decoupled from the names and concepts used in the current 
implementation: What is actually needed to evaluate an expression? The 
expression itself and the context it is evaluated in. This expression 
context stores all information in it the expression can or needs to 
access like for example the namespaces.

 From what I see there is only one expression context during the 
rendering of a template at the moment. It might be necessary to create 
one expression-specific per expression. They can then also be expression 
language specific and Expression.evaluate() could retrieve expression 
language specific information like namespaces from it. The current 
ExpressionContext would get a very simple interface not aware of 
concepts like namespaces. Since those "things" need to get injected on 
instantiation of the ExpressionContext there also needs to be somthing 
that keeps track of those "things". These track-keepers have different 
information needs per expression language, so they need to be registered 
somewhere.

Hmm, this sounds like a major change - maybe not even reasonable without 
knowing how it is really supposed to work. Does it make sense? Maybe you 
should really go with the simple and "fat" solution for the moment since 
those expression language specifics are "in" at the moment anyway.

Joerg