You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cocoon.apache.org by Daniel Fagerstrom <da...@nada.kth.se> on 2004/12/09 11:51:09 UTC
[RFC] JXTG Refactoring (was: JXTG: invoke macro by name from expression)
Leszek and I have started refactoring JXTG, by breaking it up in its
subclasses. Later we will work on creating more detailed interfaces
between the different parts and all the other stuff that has been
discussed on the list.
Now the question is: where should this work take place?
For the refactoring aspect its better to work on JXTG in core (BTW what
is the correct terminology, before we refered to code not being in a
block as core but now ECM++ is placed in a directory named core).
But considering that we are going to add stuff, and make it a framework
for further template experiments, it makes more sense to place it in a
block. Also our long time plan is to remove as much as possible from
core, AFAIU.
So what I propose is that we do the refactoring in the template block.
And that we call the refactored JXTG something else to avoid collisions
with the original one, e.g. JXTemplateGenerator2 or
o.a.c.template.generator.JXTemplateGenerator.
While doing this I suggest that we freeze development of the original
JXTG, except for bug fixes. And after a while when the refactored
version start to become stable, concerning functionality and contracts,
we can discuss replacing the original JXTG with the refactored and
improved one.
Is this a good plan or do you have other suggestions?
/Daniel
Some comments to Leszek's mail below.
Leszek Gawron wrote:
> Antonio Gallardo wrote:
>
>> On Jue, 9 de Diciembre de 2004, 3:01, Leszek Gawron dijo:
>>
>>>> If you already have something I should not commit - maybe I would send
>>>> you the source via email so you could use it if you wanted?
>>>
>>> http://www.apache.org/~lgawron/jxtg-20041209.zip
>>>
>>> Wasn't that hard with eclipse. Still took 4 hours so I'd appreciate if
>>> you looked at it :)
>>
>> I think you can commit the changes, if it is working. This way all of us
>> can review it.
>>
>> WDYT?
>
> The problem is that it will make all Daniel's changes uncommitable as
> my changes are massive. Daniel - how many changes have you done already?
Quite a lot of them, but mainly in other areas then the ones you have
worked in. And I'm quite busy with other things the next few days so I
think it is better that you commit your stuff than that I block the process.
I would prefer to divede all the classes in some different directories
like environment for the code that connects to the "cocoon object",
expression for epression related functionality, tag for executable tags,
script for parsing, execution, basic xml event classes and interface for
executable tags. But there is no hurry with that we can do such things
later, you can commit it as is.
I set up some basic testing stuff that I can commit as soon as you have
commited your stuff. I think creating a testing set for JXTG is an
important part of our futire work.
I can see that I should start to use modern tools like Eclipse, I'm
still using Emacs for everything and it is not ideal for large
refactorings. OTH I don't create my own code with the same coding style
as in JXTG, so most of the time I don't have such a clear need for
automated refactoring help.
/Daniel
Re: [RFC] JXTG Refactoring
Posted by Leszek Gawron <lg...@mobilebox.pl>.
Daniel Fagerstrom wrote:
> Leszek and I have started refactoring JXTG, by breaking it up in its
> subclasses. Later we will work on creating more detailed interfaces
> between the different parts and all the other stuff that has been
> discussed on the list.
>
> Now the question is: where should this work take place?
>
> For the refactoring aspect its better to work on JXTG in core (BTW what
> is the correct terminology, before we refered to code not being in a
> block as core but now ECM++ is placed in a directory named core).
>
> But considering that we are going to add stuff, and make it a framework
> for further template experiments, it makes more sense to place it in a
> block. Also our long time plan is to remove as much as possible from
> core, AFAIU.
>
> So what I propose is that we do the refactoring in the template block.
> And that we call the refactored JXTG something else to avoid collisions
> with the original one, e.g. JXTemplateGenerator2 or
> o.a.c.template.generator.JXTemplateGenerator.
o.a.c.template.generator.JXTemplateGenerator in template block it is.
I will commit it as is and commit small steps further.
> Quite a lot of them, but mainly in other areas then the ones you have
> worked in. And I'm quite busy with other things the next few days so I
> think it is better that you commit your stuff than that I block the
> process.
>
> I would prefer to divede all the classes in some different directories
> like environment for the code that connects to the "cocoon object",
> expression for epression related functionality, tag for executable tags,
> script for parsing, execution, basic xml event classes and interface for
> executable tags. But there is no hurry with that we can do such things
> later, you can commit it as is.
You are totally right. Right now it was just the simplest thing to do.
It's still a little bit messy and will require lots of additional
refactoring.
> I set up some basic testing stuff that I can commit as soon as you have
> commited your stuff. I think creating a testing set for JXTG is an
> important part of our futire work.
Expect a commit soon.
--
Leszek Gawron lgawron@mobilebox.pl
Project Manager MobileBox sp. z o.o.
+48 (61) 855 06 67 http://www.mobilebox.pl
mobile: +48 (501) 720 812 fax: +48 (61) 853 29 65
Re: [RFC] JXTG Refactoring
Posted by Daniel Fagerstrom <da...@nada.kth.se>.
Leszek Gawron wrote:
> Daniel Fagerstrom wrote:
<snip/>
>> In a next step the cache object should be factored out from the
>> ExecutionContext and replaced by some kind of script manager, so that
>> we can have the code that compiles and caches the scripts at one place.
>>
>> Then execute takes a number of events as arguments, I would like to
>> abstact the coupling between execution and event implementation, but
>> how it should be done requires more thinking.
>
> I think the approach Jonas introduced could be used. Still it needs some
> extra features (like jx:parameter or jx:evalBody handling). We need a
> lot of regression tests to do that right I suppose.
Jonas prototype only handles part of it. The setup should occur at
compile time and also we must decide how the tag code should be allowed
to access the body of the content of the XML representation of the tag.
>> Each tag has code in three places: its start tag that contains data,
>> in the parser for setting it up and inexecute for executing it. That
>> is rather confusing, so we shoud put all the three parts in one class.
>> How this can be done has been discussed in the above thread among
>> other places.
>>
>> Hiding the jxpath and jexl specifc code beyond one interface would
>> also be nice.
>
> There is JXTExpression class. If we are not planning to implement
> pluggable ELs the only thing we have to do is to create JXTContext that
> would join jextContext and jxpathContext.
We are planning to implement pluggable ELs, but we don't need to do that
in the first step.
/Daniel
Re: [RFC] JXTG Refactoring
Posted by Leszek Gawron <lg...@mobilebox.pl>.
Daniel Fagerstrom wrote:
> Leszek Gawron wrote:
> <snip/>
>
>> I have commited an initial JXTemplateGenerator to
>> o.a.c.template.jxtg.JXTemplateGenerator and moved Jonas' templating
>> proposal to o.a.c.template.v2 package.
>>
>> Please review.
>
>
> Nice!
>
> Don't have time to review in any detail right now. I added some basic
> test cases. Two of them that tries to test that the cocoon object is
> accesible from expressions are faulty, I didn't get them to work even
> with the original JXTG, any idea about what goes wrong?
I will look into that as soon as I finish my post.
I also have a feeling that something got wrong with jexl/jxpath leniency
settings.
>
> --- o0o ---
>
> For further refactoring I think that we should try to factor out the
> execute method as you certainly have seen it is quite intermingled with
> other stuff. IMO it should be static (or better moved to an own class),
> and depend of three arguments:
>
> execute(XMLConsumer consumer, ExecutionContext context, ...)
>
> where ExecutionContext is a new class containing the jexl and jxpath
> contexts, ServiceManager (and SourceResolver but that is accesible from
> ServiceManager), Variables, cache and definitions. There was some
> discussion about context in
> http://marc.theaimsgroup.com/?t=110173410800002&r=1&w=2.
It should be quite easy IMO as it's mainly moving stuff around.
> In a next step the cache object should be factored out from the
> ExecutionContext and replaced by some kind of script manager, so that we
> can have the code that compiles and caches the scripts at one place.
>
> Then execute takes a number of events as arguments, I would like to
> abstact the coupling between execution and event implementation, but how
> it should be done requires more thinking.
I think the approach Jonas introduced could be used. Still it needs some
extra features (like jx:parameter or jx:evalBody handling). We need a
lot of regression tests to do that right I suppose.
> Each tag has code in three places: its start tag that contains data, in
> the parser for setting it up and inexecute for executing it. That is
> rather confusing, so we shoud put all the three parts in one class. How
> this can be done has been discussed in the above thread among other places.
>
> Hiding the jxpath and jexl specifc code beyond one interface would also
> be nice.
There is JXTExpression class. If we are not planning to implement
pluggable ELs the only thing we have to do is to create JXTContext that
would join jextContext and jxpathContext.
--
Leszek Gawron lgawron@mobilebox.pl
Project Manager MobileBox sp. z o.o.
+48 (61) 855 06 67 http://www.mobilebox.pl
mobile: +48 (501) 720 812 fax: +48 (61) 853 29 65
Re: [RFC] JXTG Refactoring
Posted by Leszek Gawron <lg...@mobilebox.pl>.
Leszek Gawron wrote:
> Daniel Fagerstrom wrote:
>
>> Leszek Gawron wrote:
>> <snip/>
>>
>>> I have commited an initial JXTemplateGenerator to
>>> o.a.c.template.jxtg.JXTemplateGenerator and moved Jonas' templating
>>> proposal to o.a.c.template.v2 package.
>>>
>>> Please review.
>>
>>
>>
>> Nice!
>>
>> Don't have time to review in any detail right now. I added some basic
>> test cases. Two of them that tries to test that the cocoon object is
>> accesible from expressions are faulty, I didn't get them to work even
>> with the original JXTG, any idea about what goes wrong?
>
>
>
> instead of:
>
>> <root>
>> protocol: ${cocoon.request.protocol}
>>
>> <item attr="** ${parameters.test} **">
>> Some text
>> </item>
>>
>> </root>
>
>
> you should do:
>
>> <root>
>> protocol: ${request.protocol}
>>
>> <item attr="** ${cocoon.parameters.test} **">
>> Some text
>> </item>
>>
>> </root>
>
>
> I. regarding the protocol property:
>
> JXTemplateGenerator.setContexts:
> There is a "request" registered populated from:
> final Request request = ObjectModelHelper.getRequest(objectModel);
>
> and there is cocoon.request registered populated from:
> cocoon.put("request", FOM_JavaScriptFlowHelper
> .getFOM_Request(objectModel));
>
> It itches me that cocoon.request will only work when working with flow
> controller. I do not know flow internals to fix that.
>
> II. regarding parameters property:
> you have cocoon.parameters:
> cocoon.put("parameters", Parameters.toProperties(parameters));
>
> and top level "parameters":
> map.put("parameters", parameters);
>
> Second case does not perform the conversion and this is the problem. I
> am commiting the fix right now.
Got me thinking. Maybe this is intentional? If it is it surely is not
intuitive. Still if I convert both cases to Properties there is no
elegant solution to provide a default value:
for parameters:
${parameters.getParameter('test','defaultTest')}
When converted one would have to use:
${parameters.getProperty('test', 'defaultTest')} (which is the current
use case for cocoon.parameters).
We have to either use Parameters in both cases or state explicitly in
docs that the parameters get converted to Properties for template scope.
What do we do?
--
Leszek Gawron lgawron@mobilebox.pl
Project Manager MobileBox sp. z o.o.
+48 (61) 855 06 67 http://www.mobilebox.pl
mobile: +48 (501) 720 812 fax: +48 (61) 853 29 65
Re: [RFC] JXTG Refactoring
Posted by Leszek Gawron <lg...@mobilebox.pl>.
Daniel Fagerstrom wrote:
> Leszek Gawron wrote:
> <snip/>
>
>> I have commited an initial JXTemplateGenerator to
>> o.a.c.template.jxtg.JXTemplateGenerator and moved Jonas' templating
>> proposal to o.a.c.template.v2 package.
>>
>> Please review.
>
>
> Nice!
>
> Don't have time to review in any detail right now. I added some basic
> test cases. Two of them that tries to test that the cocoon object is
> accesible from expressions are faulty, I didn't get them to work even
> with the original JXTG, any idea about what goes wrong?
instead of:
> <root>
> protocol: ${cocoon.request.protocol}
>
> <item attr="** ${parameters.test} **">
> Some text
> </item>
>
> </root>
you should do:
> <root>
> protocol: ${request.protocol}
>
> <item attr="** ${cocoon.parameters.test} **">
> Some text
> </item>
>
> </root>
I. regarding the protocol property:
JXTemplateGenerator.setContexts:
There is a "request" registered populated from:
final Request request = ObjectModelHelper.getRequest(objectModel);
and there is cocoon.request registered populated from:
cocoon.put("request", FOM_JavaScriptFlowHelper
.getFOM_Request(objectModel));
It itches me that cocoon.request will only work when working with flow
controller. I do not know flow internals to fix that.
II. regarding parameters property:
you have cocoon.parameters:
cocoon.put("parameters", Parameters.toProperties(parameters));
and top level "parameters":
map.put("parameters", parameters);
Second case does not perform the conversion and this is the problem. I
am commiting the fix right now.
--
Leszek Gawron lgawron@mobilebox.pl
Project Manager MobileBox sp. z o.o.
+48 (61) 855 06 67 http://www.mobilebox.pl
mobile: +48 (501) 720 812 fax: +48 (61) 853 29 65
Re: [RFC] JXTG Refactoring
Posted by Daniel Fagerstrom <da...@nada.kth.se>.
Leszek Gawron wrote:
> Daniel Fagerstrom wrote:
<snip/>
>> Concerning the strange difference between cocoon.request and request
>> I have some slight remembrance that it has been discussed on the list
>> and that it was deliberate. But I don't remember the reason and I
>> have not been able to find the relevant posts.
>
> There are two things whiche are not consistent:
> - request vs. cocoon.request. The latter does not work if you do not
> have a flowscript controller up front.
> - parameters vs. cocoon.parameters. First one is visible as Parameters,
> second one as Properties. so:
> ${parameters.param1} does not work !{cocoon.parameters.param1} does.
> ${parameters.getParameter('param1', 'default') is a proper syntax for
> passing a default value
> ${cocoon.parameters.getProperty('param1', 'default'} should do the
> same for cocoon.parameters.
I think we have to dive into the flow code and see why it not works
whithout flow controller and fix it.
> I haven't checked yet if there are issues with session.
>
>> As the use of request etc without cocoon prefix is deprecated, I
>> don't think there is any reasons to support it in our template work
>> that probably is directed to 2.2. So if no one protest I think we should
>
> Is it? As the cocoon.request does not work in all cases this is hard
> to believe this decision has been made.
I haven't been able to find a vote about it, but I remember that it was
a decision about it some time, and both in the documentation and in the
code there are comments about request etc being deprecated and that
cocoon.request is the supported syntax. And as a result we should make
that syntax work. Hopefully someone with more knowledge about FOM can
give some input.
/Daniel
Re: [RFC] JXTG Refactoring
Posted by Leszek Gawron <lg...@mobilebox.pl>.
Daniel Fagerstrom wrote:
> Leszek Gawron wrote:
>
>> Daniel Fagerstrom wrote:
>
> <snip/>
>
>>> In a next step the cache object should be factored out from the
>>> ExecutionContext and replaced by some kind of script manager, so that
>>> we can have the code that compiles and caches the scripts at one place.
>>
>>
>> I am on it if you haven't already started it.
>
>
> Nice! No I tried to commit the things that I have been working on
> dirrectly to decrease the risk of collisions with your work. Don't know
> if that succeded, at least I was first ;)
>
> My focus is to continue working on factoring out the tag code from the
> parser and invoker to the tag classes. I want them to become completely
> independent of JXTG specific stuff.
>
> If you have any possiblity to add test cases for the tags I would
> appriciate that very much.
Ok. I'll get into that.
>
> Concerning the strange difference between cocoon.request and request I
> have some slight remembrance that it has been discussed on the list and
> that it was deliberate. But I don't remember the reason and I have not
> been able to find the relevant posts.
There are two things whiche are not consistent:
- request vs. cocoon.request. The latter does not work if you do not
have a flowscript controller up front.
- parameters vs. cocoon.parameters. First one is visible as Parameters,
second one as Properties. so:
${parameters.param1} does not work !{cocoon.parameters.param1} does.
${parameters.getParameter('param1', 'default') is a proper syntax for
passing a default value
${cocoon.parameters.getProperty('param1', 'default'} should do the
same for cocoon.parameters.
I haven't checked yet if there are issues with session.
> As the use of request etc without cocoon prefix is deprecated, I don't
> think there is any reasons to support it in our template work that
> probably is directed to 2.2. So if no one protest I think we should
Is it? As the cocoon.request does not work in all cases this is hard to
believe this decision has been made.
--
Leszek Gawron lgawron@mobilebox.pl
Project Manager MobileBox sp. z o.o.
+48 (61) 855 06 67 http://www.mobilebox.pl
mobile: +48 (501) 720 812 fax: +48 (61) 853 29 65
Re: [RFC] JXTG Refactoring
Posted by Daniel Fagerstrom <da...@nada.kth.se>.
Leszek Gawron wrote:
> Daniel Fagerstrom wrote:
<snip/>
>> In a next step the cache object should be factored out from the
>> ExecutionContext and replaced by some kind of script manager, so that
>> we can have the code that compiles and caches the scripts at one place.
>
> I am on it if you haven't already started it.
Nice! No I tried to commit the things that I have been working on
dirrectly to decrease the risk of collisions with your work. Don't know
if that succeded, at least I was first ;)
My focus is to continue working on factoring out the tag code from the
parser and invoker to the tag classes. I want them to become completely
independent of JXTG specific stuff.
If you have any possiblity to add test cases for the tags I would
appriciate that very much.
Concerning the strange difference between cocoon.request and request I
have some slight remembrance that it has been discussed on the list and
that it was deliberate. But I don't remember the reason and I have not
been able to find the relevant posts.
As the use of request etc without cocoon prefix is deprecated, I don't
think there is any reasons to support it in our template work that
probably is directed to 2.2. So if no one protest I think we should
remove the deprecated stuff from template and try to get what is left to
work from test code. It is really frustrating that it is so tricky to
send data to the template. And it hinders my progress, that I have not
been able to write much more test cases.
/Daniel
Re: [RFC] JXTG Refactoring
Posted by Leszek Gawron <lg...@mobilebox.pl>.
Daniel Fagerstrom wrote:
> Leszek Gawron wrote:
> <snip/>
>
>> I have commited an initial JXTemplateGenerator to
>> o.a.c.template.jxtg.JXTemplateGenerator and moved Jonas' templating
>> proposal to o.a.c.template.v2 package.
>>
>> Please review.
>
>
> Nice!
>
> Don't have time to review in any detail right now. I added some basic
> test cases. Two of them that tries to test that the cocoon object is
> accesible from expressions are faulty, I didn't get them to work even
> with the original JXTG, any idea about what goes wrong?
>
> --- o0o ---
>
> For further refactoring I think that we should try to factor out the
> execute method as you certainly have seen it is quite intermingled with
> other stuff. IMO it should be static (or better moved to an own class),
> and depend of three arguments:
>
> execute(XMLConsumer consumer, ExecutionContext context, ...)
>
> where ExecutionContext is a new class containing the jexl and jxpath
> contexts, ServiceManager (and SourceResolver but that is accesible from
> ServiceManager), Variables, cache and definitions. There was some
> discussion about context in
> http://marc.theaimsgroup.com/?t=110173410800002&r=1&w=2.
>
> In a next step the cache object should be factored out from the
> ExecutionContext and replaced by some kind of script manager, so that we
> can have the code that compiles and caches the scripts at one place.
I am on it if you haven't already started it.
--
Leszek Gawron lgawron@mobilebox.pl
Project Manager MobileBox sp. z o.o.
+48 (61) 855 06 67 http://www.mobilebox.pl
mobile: +48 (501) 720 812 fax: +48 (61) 853 29 65
Re: [RFC] JXTG Refactoring
Posted by Leszek Gawron <lg...@mobilebox.pl>.
Daniel Fagerstrom wrote:
> Leszek Gawron wrote:
> <snip/>
>
>> I have commited an initial JXTemplateGenerator to
>> o.a.c.template.jxtg.JXTemplateGenerator and moved Jonas' templating
>> proposal to o.a.c.template.v2 package.
>>
>> Please review.
>
>
> Nice!
I am totally busy right now. I'll try to look into your test cases in
the evening as well as make some comments about further steps.
--
Leszek Gawron lgawron@mobilebox.pl
Project Manager MobileBox sp. z o.o.
+48 (61) 855 06 67 http://www.mobilebox.pl
mobile: +48 (501) 720 812 fax: +48 (61) 853 29 65
Re: [RFC] JXTG Refactoring
Posted by Daniel Fagerstrom <da...@nada.kth.se>.
Daniel Fagerstrom wrote:
<snip/>
> For further refactoring I think that we should ...
Even more important is of course to add lots of test cases, so that we
ensure high quality. For those who have corrected more or less subtle
bugs in JXTG, consider adding a test case for it so that we can avoid
regression.
Also, the refactored JXTG is supposed to be kept back compatible at all
time, so the really adventuerous can consider switching from the
original to the new JXTG in his/her sitemaps so that we can detect and
correct regression as fast as possible (if you not are the kind of
person that has a cron script that compile and install the latest Linux
kernel from CVS each night in your production servers, you might want to
avoid using the new JXTG for production yet, though ;) ).
/Daniel
Re: [RFC] JXTG Refactoring
Posted by Daniel Fagerstrom <da...@nada.kth.se>.
Leszek Gawron wrote:
<snip/>
> I have commited an initial JXTemplateGenerator to
> o.a.c.template.jxtg.JXTemplateGenerator and moved Jonas' templating
> proposal to o.a.c.template.v2 package.
>
> Please review.
Nice!
Don't have time to review in any detail right now. I added some basic
test cases. Two of them that tries to test that the cocoon object is
accesible from expressions are faulty, I didn't get them to work even
with the original JXTG, any idea about what goes wrong?
--- o0o ---
For further refactoring I think that we should try to factor out the
execute method as you certainly have seen it is quite intermingled with
other stuff. IMO it should be static (or better moved to an own class),
and depend of three arguments:
execute(XMLConsumer consumer, ExecutionContext context, ...)
where ExecutionContext is a new class containing the jexl and jxpath
contexts, ServiceManager (and SourceResolver but that is accesible from
ServiceManager), Variables, cache and definitions. There was some
discussion about context in
http://marc.theaimsgroup.com/?t=110173410800002&r=1&w=2.
In a next step the cache object should be factored out from the
ExecutionContext and replaced by some kind of script manager, so that we
can have the code that compiles and caches the scripts at one place.
Then execute takes a number of events as arguments, I would like to
abstact the coupling between execution and event implementation, but how
it should be done requires more thinking.
Each tag has code in three places: its start tag that contains data, in
the parser for setting it up and inexecute for executing it. That is
rather confusing, so we shoud put all the three parts in one class. How
this can be done has been discussed in the above thread among other places.
Hiding the jxpath and jexl specifc code beyond one interface would also
be nice.
WDYT?
/Daniel
Re: [RFC] JXTG Refactoring
Posted by Leszek Gawron <lg...@mobilebox.pl>.
Daniel Fagerstrom wrote:
> Leszek and I have started refactoring JXTG, by breaking it up in its
> subclasses. Later we will work on creating more detailed interfaces
> between the different parts and all the other stuff that has been
> discussed on the list.
>
> Now the question is: where should this work take place?
>
> For the refactoring aspect its better to work on JXTG in core (BTW what
> is the correct terminology, before we refered to code not being in a
> block as core but now ECM++ is placed in a directory named core).
>
> But considering that we are going to add stuff, and make it a framework
> for further template experiments, it makes more sense to place it in a
> block. Also our long time plan is to remove as much as possible from
> core, AFAIU.
>
> So what I propose is that we do the refactoring in the template block.
> And that we call the refactored JXTG something else to avoid collisions
> with the original one, e.g. JXTemplateGenerator2 or
> o.a.c.template.generator.JXTemplateGenerator.
I have commited an initial JXTemplateGenerator to
o.a.c.template.jxtg.JXTemplateGenerator and moved Jonas' templating
proposal to o.a.c.template.v2 package.
Please review.
--
Leszek Gawron lgawron@mobilebox.pl
Project Manager MobileBox sp. z o.o.
+48 (61) 855 06 67 http://www.mobilebox.pl
mobile: +48 (501) 720 812 fax: +48 (61) 853 29 65