You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cocoon.apache.org by Reinhard Poetz <re...@apache.org> on 2007/03/06 09:52:11 UTC

Apples enhancements

I added/changed three things in Apples:

  - Apples can be Spring beans. In order to lookup a bean, prepand #, e.g. to 
lookup a bean of name 'myBean', use <map:call function="#myBean"/>.

  - Add an AppleNotFoundException which informs why an Apple can't be initialized.

  - Add sendStatus() to the AppleResponse interface.

Comments?



Carsten, may you have a look at 
http://svn.apache.org/repos/asf/cocoon/trunk/blocks/cocoon-apples/cocoon-apples-impl/src/main/java/org/apache/cocoon/components/flow/apples/ApplesProcessor.java?

I'm not sure about

sitemapManager = (ServiceManager) 
avalonContext.get(ContextHelper.CONTEXT_SITEMAP_SERVICE_MANAGER);

and

LifecycleHelper.setupComponent(app, getLogger(), appleContext, sitemapManager, 
null, true);

as both statements use deprecated code.


-- 
Reinhard Pötz           Independent Consultant, Trainer & (IT)-Coach 

{Software Engineering, Open Source, Web Applications, Apache Cocoon}

                                        web(log): http://www.poetz.cc
--------------------------------------------------------------------

Re: Apples enhancements

Posted by Reinhard Poetz <re...@apache.org>.
Carsten Ziegeler wrote:
> Reinhard Poetz wrote:
>> Carsten, may you have a look at 
>> http://svn.apache.org/repos/asf/cocoon/trunk/blocks/cocoon-apples/cocoon-apples-impl/src/main/java/org/apache/cocoon/components/flow/apples/ApplesProcessor.java?
>>
>> I'm not sure about
>>
>> sitemapManager = (ServiceManager) 
>> avalonContext.get(ContextHelper.CONTEXT_SITEMAP_SERVICE_MANAGER);
>>
>> and
>>
>> LifecycleHelper.setupComponent(app, getLogger(), appleContext, sitemapManager, 
>> null, true);
>>
>> as both statements use deprecated code.
>>
> Yes, the stuff is deprecated because it's avalon based code. The first
> one can be changed (I just commited it),
> but there is no replacement for the lifecycle helper. You can only
> remove this code by either making a real Avalon component out of 'app',
> so the container does the stuff for you. Or you can remove the avalon
> support and use spring beans.

Removing Avalon support is out of question for cocoon-apples that should work 
with cocoon-core 2.2.

Thanks for doing the refactoring!

-- 
Reinhard Pötz           Independent Consultant, Trainer & (IT)-Coach 

{Software Engineering, Open Source, Web Applications, Apache Cocoon}

                                        web(log): http://www.poetz.cc
--------------------------------------------------------------------

Re: Apples enhancements

Posted by Carsten Ziegeler <cz...@apache.org>.
Reinhard Poetz wrote:
> 
> Carsten, may you have a look at 
> http://svn.apache.org/repos/asf/cocoon/trunk/blocks/cocoon-apples/cocoon-apples-impl/src/main/java/org/apache/cocoon/components/flow/apples/ApplesProcessor.java?
> 
> I'm not sure about
> 
> sitemapManager = (ServiceManager) 
> avalonContext.get(ContextHelper.CONTEXT_SITEMAP_SERVICE_MANAGER);
> 
> and
> 
> LifecycleHelper.setupComponent(app, getLogger(), appleContext, sitemapManager, 
> null, true);
> 
> as both statements use deprecated code.
> 
Yes, the stuff is deprecated because it's avalon based code. The first
one can be changed (I just commited it),
but there is no replacement for the lifecycle helper. You can only
remove this code by either making a real Avalon component out of 'app',
so the container does the stuff for you. Or you can remove the avalon
support and use spring beans.

Carsten

-- 
Carsten Ziegeler
http://www.osoco.org/weblogs/rael/

Re: Apples enhancements

Posted by Leszek Gawron <lg...@mobilebox.pl>.
Reinhard Poetz wrote:
> Leszek Gawron wrote:
>> Reinhard Poetz wrote:
>>> Leszek Gawron wrote:
>>>> Reinhard Poetz wrote:
>>>>> Leszek Gawron wrote:
>>>>>> What 'apple initialization options' are you refering to ?
>>>>>
>>>>>  - 
>>>>> Thread.currentThread().getContextClassLoader().loadClass(appleName);
>>>>>  - sitemapManager.lookup(beanName);
>>>>
>>>> And why would you like to mix those? Either you want managed apples 
>>>> or you don't. If some do not need to be managed the only thing you 
>>>> need to do is:
>>>>
>>>> <bean id="appleName" class="o.a.c.apples.AppleName" scope="prototype"/>
>>>>
>>>> and you have the same functionality but more consistent.
>>>
>>> right and as your solution was in place first, I will remove the 
>>> support for #[bean-name] apples (btw, sooner or later we should 
>>> deprecate the "call function" as it doesn't fit for a generic 
>>> controller concept at all)
>>
>> Do not get me wrong. You are way more experienced so I don't feel like 
>> enforcing my solution (which is btw a 5 liner). Still some discussion 
>> won't hurt :)
> 
> The only argument for making both options available is less 
> configuration. But thinking more about it, I probably want all my Apples 
> being managed by Spring in the future.
> 
>>>> There is one more thing that I dislike about current apples + spring 
>>>> integration: you can mix ApplesController vs. 
>>>> StatelessApplesController (which triggers different continuations 
>>>> handling) and prototype scope vs. singleton scope.
>>>>
>>>> The most dangerous situation is marking a bean singleton and not 
>>>> implementing StatelessAppleController. This way a continuation will 
>>>> be created with a singleton apple which other threads also will use.
>>>>
>>>> Simply put: ApplesProcessor.callFunction should not depend on marker 
>>>> interfaces if used with managed apples.
>>>
>>> What do you want to say? Shall we disallow singleton scoped beans 
>>> that do not implement StatelessAppleController?
>>
>> Exactly. There is no point in such construct (at least I don't see it) 
>> and it may be a very dangerous mistake to be made.
> 
> I Will look into it. I think it is possible to prevent it since with 
> Carsten's help we have access to the Spring app context in the 
> interpreter now which should make it possible to find out a bean's scope.

There is a small gotcha': currently managed apples are looked up using a 
service manager, not spring directly. Should we switch to spring lookup?

One more thing: even if a bean is spring managed it is being 
instantiated with LifecycleHelper anyway.

I'll try to commit some fix (laaack of time).

-- 
Leszek Gawron                         http://www.mobilebox.pl/krs.html
CTO at MobileBox Ltd.


Re: Apples enhancements

Posted by Reinhard Poetz <re...@apache.org>.
Leszek Gawron wrote:
> Reinhard Poetz wrote:
>> Leszek Gawron wrote:
>>> Reinhard Poetz wrote:
>>>> Leszek Gawron wrote:
>>>>> What 'apple initialization options' are you refering to ?
>>>>
>>>>  - Thread.currentThread().getContextClassLoader().loadClass(appleName);
>>>>  - sitemapManager.lookup(beanName);
>>>
>>> And why would you like to mix those? Either you want managed apples 
>>> or you don't. If some do not need to be managed the only thing you 
>>> need to do is:
>>>
>>> <bean id="appleName" class="o.a.c.apples.AppleName" scope="prototype"/>
>>>
>>> and you have the same functionality but more consistent.
>>
>> right and as your solution was in place first, I will remove the 
>> support for #[bean-name] apples (btw, sooner or later we should 
>> deprecate the "call function" as it doesn't fit for a generic 
>> controller concept at all)
> 
> Do not get me wrong. You are way more experienced so I don't feel like 
> enforcing my solution (which is btw a 5 liner). Still some discussion 
> won't hurt :)

The only argument for making both options available is less configuration. But 
thinking more about it, I probably want all my Apples being managed by Spring in 
the future.

>>> There is one more thing that I dislike about current apples + spring 
>>> integration: you can mix ApplesController vs. 
>>> StatelessApplesController (which triggers different continuations 
>>> handling) and prototype scope vs. singleton scope.
>>>
>>> The most dangerous situation is marking a bean singleton and not 
>>> implementing StatelessAppleController. This way a continuation will 
>>> be created with a singleton apple which other threads also will use.
>>>
>>> Simply put: ApplesProcessor.callFunction should not depend on marker 
>>> interfaces if used with managed apples.
>>
>> What do you want to say? Shall we disallow singleton scoped beans that 
>> do not implement StatelessAppleController?
> 
> Exactly. There is no point in such construct (at least I don't see it) 
> and it may be a very dangerous mistake to be made.

I Will look into it. I think it is possible to prevent it since with Carsten's 
help we have access to the Spring app context in the interpreter now which 
should make it possible to find out a bean's scope.

-- 
Reinhard Pötz           Independent Consultant, Trainer & (IT)-Coach 

{Software Engineering, Open Source, Web Applications, Apache Cocoon}

                                        web(log): http://www.poetz.cc
--------------------------------------------------------------------

Re: Apples enhancements

Posted by Leszek Gawron <lg...@mobilebox.pl>.
Reinhard Poetz wrote:
> Leszek Gawron wrote:
>> Reinhard Poetz wrote:
>>> Leszek Gawron wrote:
>>>> What 'apple initialization options' are you refering to ?
>>>
>>>  - Thread.currentThread().getContextClassLoader().loadClass(appleName);
>>>  - sitemapManager.lookup(beanName);
>>
>> And why would you like to mix those? Either you want managed apples or 
>> you don't. If some do not need to be managed the only thing you need 
>> to do is:
>>
>> <bean id="appleName" class="o.a.c.apples.AppleName" scope="prototype"/>
>>
>> and you have the same functionality but more consistent.
> 
> right and as your solution was in place first, I will remove the support 
> for #[bean-name] apples (btw, sooner or later we should deprecate the 
> "call function" as it doesn't fit for a generic controller concept at all)

Do not get me wrong. You are way more experienced so I don't feel like 
enforcing my solution (which is btw a 5 liner). Still some discussion 
won't hurt :)

> 
>> There is one more thing that I dislike about current apples + spring 
>> integration: you can mix ApplesController vs. 
>> StatelessApplesController (which triggers different continuations 
>> handling) and prototype scope vs. singleton scope.
>>
>> The most dangerous situation is marking a bean singleton and not 
>> implementing StatelessAppleController. This way a continuation will be 
>> created with a singleton apple which other threads also will use.
>>
>> Simply put: ApplesProcessor.callFunction should not depend on marker 
>> interfaces if used with managed apples.
> 
> What do you want to say? Shall we disallow singleton scoped beans that 
> do not implement StatelessAppleController?

Exactly. There is no point in such construct (at least I don't see it) 
and it may be a very dangerous mistake to be made.

-- 
Leszek Gawron                         http://www.mobilebox.pl/krs.html
CTO at MobileBox Ltd.


Re: Apples enhancements

Posted by Reinhard Poetz <re...@apache.org>.
Leszek Gawron wrote:
> Reinhard Poetz wrote:
>> Leszek Gawron wrote:
>>> What 'apple initialization options' are you refering to ?
>>
>>  - Thread.currentThread().getContextClassLoader().loadClass(appleName);
>>  - sitemapManager.lookup(beanName);
> 
> And why would you like to mix those? Either you want managed apples or 
> you don't. If some do not need to be managed the only thing you need to 
> do is:
> 
> <bean id="appleName" class="o.a.c.apples.AppleName" scope="prototype"/>
> 
> and you have the same functionality but more consistent.

right and as your solution was in place first, I will remove the support for 
#[bean-name] apples (btw, sooner or later we should deprecate the "call 
function" as it doesn't fit for a generic controller concept at all)

> There is one more thing that I dislike about current apples + spring 
> integration: you can mix ApplesController vs. StatelessApplesController 
> (which triggers different continuations handling) and prototype scope 
> vs. singleton scope.
> 
> The most dangerous situation is marking a bean singleton and not 
> implementing StatelessAppleController. This way a continuation will be 
> created with a singleton apple which other threads also will use.
> 
> Simply put: ApplesProcessor.callFunction should not depend on marker 
> interfaces if used with managed apples.

What do you want to say? Shall we disallow singleton scoped beans that do not 
implement StatelessAppleController?

-- 
Reinhard Pötz           Independent Consultant, Trainer & (IT)-Coach 

{Software Engineering, Open Source, Web Applications, Apache Cocoon}

                                        web(log): http://www.poetz.cc
--------------------------------------------------------------------

Re: Apples enhancements

Posted by Leszek Gawron <lg...@mobilebox.pl>.
Reinhard Poetz wrote:
> Leszek Gawron wrote:
>> Reinhard Poetz wrote:
>>> Leszek Gawron wrote:
>>>> Reinhard Poetz wrote:
>>>>>
>>>>> I added/changed three things in Apples:
>>>>>
>>>>>  - Apples can be Spring beans. In order to lookup a bean, prepand 
>>>>> #, e.g. to lookup a bean of name 'myBean', use <map:call 
>>>>> function="#myBean"/>.
>>>>>
>>>>>  - Add an AppleNotFoundException which informs why an Apple can't 
>>>>> be initialized.
>>>>>
>>>>>  - Add sendStatus() to the AppleResponse interface.
>>>>>
>>>>> Comments?
>>>> I think you implemented a duplicate. Some time ago I have commited 
>>>> ServiceApplesProcessor which looks up apples using service manager.
>>>>
>>>> The only thing you have to to is to use different "flow language"
>>>
>>> I see, but I don't like to have another flow language because I want 
>>> to use both Apple initialization options in the same sitemap. WDYT?
>>
>> What 'apple initialization options' are you refering to ?
> 
>  - Thread.currentThread().getContextClassLoader().loadClass(appleName);
>  - sitemapManager.lookup(beanName);

And why would you like to mix those? Either you want managed apples or 
you don't. If some do not need to be managed the only thing you need to 
do is:

<bean id="appleName" class="o.a.c.apples.AppleName" scope="prototype"/>

and you have the same functionality but more consistent.


There is one more thing that I dislike about current apples + spring 
integration: you can mix ApplesController vs. StatelessApplesController 
(which triggers different continuations handling) and prototype scope 
vs. singleton scope.

The most dangerous situation is marking a bean singleton and not 
implementing StatelessAppleController. This way a continuation will be 
created with a singleton apple which other threads also will use.

Simply put: ApplesProcessor.callFunction should not depend on marker 
interfaces if used with managed apples.

-- 
Leszek Gawron                         http://www.mobilebox.pl/krs.html
CTO at MobileBox Ltd.


Re: Apples enhancements

Posted by Reinhard Poetz <re...@apache.org>.
Leszek Gawron wrote:
> Reinhard Poetz wrote:
>> Leszek Gawron wrote:
>>> Reinhard Poetz wrote:
>>>>
>>>> I added/changed three things in Apples:
>>>>
>>>>  - Apples can be Spring beans. In order to lookup a bean, prepand #, 
>>>> e.g. to lookup a bean of name 'myBean', use <map:call 
>>>> function="#myBean"/>.
>>>>
>>>>  - Add an AppleNotFoundException which informs why an Apple can't be 
>>>> initialized.
>>>>
>>>>  - Add sendStatus() to the AppleResponse interface.
>>>>
>>>> Comments?
>>> I think you implemented a duplicate. Some time ago I have commited 
>>> ServiceApplesProcessor which looks up apples using service manager.
>>>
>>> The only thing you have to to is to use different "flow language"
>>
>> I see, but I don't like to have another flow language because I want 
>> to use both Apple initialization options in the same sitemap. WDYT?
> 
> What 'apple initialization options' are you refering to ?

  - Thread.currentThread().getContextClassLoader().loadClass(appleName);
  - sitemapManager.lookup(beanName);

-- 
Reinhard Pötz           Independent Consultant, Trainer & (IT)-Coach 

{Software Engineering, Open Source, Web Applications, Apache Cocoon}

                                        web(log): http://www.poetz.cc
--------------------------------------------------------------------

Re: Apples enhancements

Posted by Leszek Gawron <lg...@mobilebox.pl>.
Reinhard Poetz wrote:
> Leszek Gawron wrote:
>> Reinhard Poetz wrote:
>>>
>>> I added/changed three things in Apples:
>>>
>>>  - Apples can be Spring beans. In order to lookup a bean, prepand #, 
>>> e.g. to lookup a bean of name 'myBean', use <map:call 
>>> function="#myBean"/>.
>>>
>>>  - Add an AppleNotFoundException which informs why an Apple can't be 
>>> initialized.
>>>
>>>  - Add sendStatus() to the AppleResponse interface.
>>>
>>> Comments?
>> I think you implemented a duplicate. Some time ago I have commited 
>> ServiceApplesProcessor which looks up apples using service manager.
>>
>> The only thing you have to to is to use different "flow language"
> 
> I see, but I don't like to have another flow language because I want to 
> use both Apple initialization options in the same sitemap. WDYT?

What 'apple initialization options' are you refering to ?


-- 
Leszek Gawron                         http://www.mobilebox.pl/krs.html
CTO at MobileBox Ltd.


Re: Apples enhancements

Posted by Reinhard Poetz <re...@apache.org>.
Leszek Gawron wrote:
> Reinhard Poetz wrote:
>>
>> I added/changed three things in Apples:
>>
>>  - Apples can be Spring beans. In order to lookup a bean, prepand #, 
>> e.g. to lookup a bean of name 'myBean', use <map:call 
>> function="#myBean"/>.
>>
>>  - Add an AppleNotFoundException which informs why an Apple can't be 
>> initialized.
>>
>>  - Add sendStatus() to the AppleResponse interface.
>>
>> Comments?
> I think you implemented a duplicate. Some time ago I have commited 
> ServiceApplesProcessor which looks up apples using service manager.
> 
> The only thing you have to to is to use different "flow language"

I see, but I don't like to have another flow language because I want to use both 
Apple initialization options in the same sitemap. WDYT?

-- 
Reinhard Pötz           Independent Consultant, Trainer & (IT)-Coach 

{Software Engineering, Open Source, Web Applications, Apache Cocoon}

                                        web(log): http://www.poetz.cc
--------------------------------------------------------------------

Re: Apples enhancements

Posted by Leszek Gawron <lg...@mobilebox.pl>.
Reinhard Poetz wrote:
> 
> I added/changed three things in Apples:
> 
>  - Apples can be Spring beans. In order to lookup a bean, prepand #, 
> e.g. to lookup a bean of name 'myBean', use <map:call function="#myBean"/>.
> 
>  - Add an AppleNotFoundException which informs why an Apple can't be 
> initialized.
> 
>  - Add sendStatus() to the AppleResponse interface.
> 
> Comments?
I think you implemented a duplicate. Some time ago I have commited 
ServiceApplesProcessor which looks up apples using service manager.

The only thing you have to to is to use different "flow language"

> <components>
>   <flow-interpreters>
>     <component-instance 
>       class="org.apache.cocoon.components.flow.apples.ApplesProcessor" 
>       name="apples" logger="apples">
>     </component-instance>
>   	<component-instance 
>   		class="org.apache.cocoon.components.flow.apples.ServiceApplesProcessor" 
>   		name="service-apples" logger="service-apples">
>   	</component-instance>
>   </flow-interpreters>
> </components>


-- 
Leszek Gawron                         http://www.mobilebox.pl/krs.html
CTO at MobileBox Ltd.