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 2007/07/30 11:14:34 UTC

Sitemap scope

was: How to register MockProcessInfoProvider?

Grzegorz Kossakowski skrev:
> Hi,
> 
> I'll give you some background to my problem. I use ProcessInfoProvider 
> for accessing environmental data because I've read[1] that it is a 
> proffered way to do this in Cocoon 2.2.
...

IMO, ProcessorInfoProvider shouldn't be the preferred way to access 
environment info. For sure it is a much better way than using the static 
methods of the EnvironmentHelper class directly. But if you think about 
it, many user components (e.g. sitemap components) needs to access 
environment info. So any "preferred" way of accessing environment info 
will be heavily used and will be an important part of our API. Thus it 
deserves some attention before we decide that it is "the preferred way".

So why am I not convinced about the ProcessorInfoProvider? First the 
interface is somewhat "asymmetric". It has get methods for the http 
servlet request, response and context which is fine, but then it also 
makes the object model available that contains these objects as well as 
some other ones. That is redundant and confusing. And I would prefer if 
we could get rid of the need for users to access the object model. 
Second and what is more important, we should minimize the set of APIs 
that users needs to depend on. So it would IMO be much preferable if the 
user could just depend on the http servlet request, response and 
contexts objects directly through DI, than needing to depend on a 
provider object. It i easier to test and less intrusive in the user code.

So how do we achieve this? This can be done by defining a special 
"sitemap scope" as I actually suggested in the thread that you are 
citing http://article.gmane.org/gmane.text.xml.cocoon.devel/68676. Back 
then it was just an idea, but since then I have implemented the call 
scope in the servlet service fw, so we know both that it works and how 
to implement it.

A sitemap scope could be implemented similar to the call scope, but it 
would use the EnvironmentHelper, EnvironmentStack and EnvironmentInfo as 
underlying store instead of the CallStack and CallFrame. Also factory 
beans similar to the ones in o.a.c.callstack.environment are needed for 
accessing the various environment objects.

The implementation of the CallScope is rather straight forward 
(considering that you understand Spring custom scopes 
http://static.springframework.org/spring/docs/2.0.x/reference/beans.html#beans-factory-scopes-custom). 
One subtle thing is that the factory beans needs (AFAICS) to be stored 
at the stack as well as the actual environment objects (see 
CallScope.get and CallScope.remove). For a sitemap scope this could be 
achieved by extending the StackInfo with a map that can contain the 
factory beans. For the currently intended use, 
Scope.registerDestructionCallBack could be a no op.

          --- o0o ---

Now, I understand if you feel that what I write above seem overly 
pedantic and that ProcessorInfoProvider should be a good enough solution 
to the environment handling problems. I have spent so much times 
fighting the internals of Cocoon, and IMO one of the main reasons that 
the code is so hard to follow is that the handling of the current 
context is based on a patchwork of "good enough" hacks, rather than a 
overall design. So I am strongly convinced that we should spend some 
effort on getting it right. The greatest advantage with the "sitemap 
scope" idea, is that it is non intrusive and isolates the user 
components from the environment handling subtleness.

WDYT?

/Daniel

Re: Sitemap scope

Posted by Grzegorz Kossakowski <gk...@apache.org>.
Daniel Fagerstrom pisze:
> was: How to register MockProcessInfoProvider?
> 
> Grzegorz Kossakowski skrev:
>> Hi,
>>
>> I'll give you some background to my problem. I use ProcessInfoProvider 
>> for accessing environmental data because I've read[1] that it is a 
>> proffered way to do this in Cocoon 2.2.
> ...
> 
> IMO, ProcessorInfoProvider shouldn't be the preferred way to access 
> environment info. For sure it is a much better way than using the static 
> methods of the EnvironmentHelper class directly. But if you think about 
> it, many user components (e.g. sitemap components) needs to access 
> environment info. So any "preferred" way of accessing environment info 
> will be heavily used and will be an important part of our API. Thus it 
> deserves some attention before we decide that it is "the preferred way".
> 
> So why am I not convinced about the ProcessorInfoProvider? First the 
> interface is somewhat "asymmetric". It has get methods for the http 
> servlet request, response and context which is fine, but then it also 
> makes the object model available that contains these objects as well as 
> some other ones. That is redundant and confusing. And I would prefer if 
> we could get rid of the need for users to access the object model. 
> Second and what is more important, we should minimize the set of APIs 
> that users needs to depend on. So it would IMO be much preferable if the 
> user could just depend on the http servlet request, response and 
> contexts objects directly through DI, than needing to depend on a 
> provider object. It i easier to test and less intrusive in the user code.
> 
> So how do we achieve this? This can be done by defining a special 
> "sitemap scope" as I actually suggested in the thread that you are 
> citing http://article.gmane.org/gmane.text.xml.cocoon.devel/68676. Back 
> then it was just an idea, but since then I have implemented the call 
> scope in the servlet service fw, so we know both that it works and how 
> to implement it.
> 
> A sitemap scope could be implemented similar to the call scope, but it 
> would use the EnvironmentHelper, EnvironmentStack and EnvironmentInfo as 
> underlying store instead of the CallStack and CallFrame. Also factory 
> beans similar to the ones in o.a.c.callstack.environment are needed for 
> accessing the various environment objects.
> 
> The implementation of the CallScope is rather straight forward 
> (considering that you understand Spring custom scopes 
> http://static.springframework.org/spring/docs/2.0.x/reference/beans.html#beans-factory-scopes-custom). 
> One subtle thing is that the factory beans needs (AFAICS) to be stored 
> at the stack as well as the actual environment objects (see 
> CallScope.get and CallScope.remove). For a sitemap scope this could be 
> achieved by extending the StackInfo with a map that can contain the 
> factory beans. For the currently intended use, 
> Scope.registerDestructionCallBack could be a no op.

Thanks for all suggestions. I will comment implementation details as soon as I start implementing sitemap scope because it will be the time 
I will have something valuable to say. :)

> Now, I understand if you feel that what I write above seem overly 
> pedantic and that ProcessorInfoProvider should be a good enough solution 
> to the environment handling problems. I have spent so much times 
> fighting the internals of Cocoon, and IMO one of the main reasons that 
> the code is so hard to follow is that the handling of the current 
> context is based on a patchwork of "good enough" hacks, rather than a 
> overall design. So I am strongly convinced that we should spend some 
> effort on getting it right. The greatest advantage with the "sitemap 
> scope" idea, is that it is non intrusive and isolates the user 
> components from the environment handling subtleness.

Daniel, I know that you have spent a lot of time in Cocoon's internals so I trust to your opinion. Moreover, my little journey to the 
"banished code" that no one wants to look at resulted in the same feeling. I already created an issue for this task:
http://issues.apache.org/jira/browse/COCOON-2099

Thanks for your suggestion.

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

Re: Sitemap scope

Posted by Carsten Ziegeler <cz...@apache.org>.
Grzegorz Kossakowski wrote:
> 
> Do you think all three pieces are always needed? My answer would be: no,
> usually they are not.
> 
:) In my use cases, I often need all three of them and sometimes request
and context.

> I have no strong preference on ProcessInfoProvider or on direct
> dependencies. Daniel's argument that it is a less APIs to learn is right
> and I would add another one: it's easier to see what particular
> component really needs.
Well, fine - although I hate to write to much code, we could remove the
interface all together.

> 
>> Today, the object model is still of great value as we messed up the
>> implementation and the semantics of internal pipeline calls: request
>> attributes are shared between the original request and all child
>> requests - therefore setting one in a child request overwrites it in all
>> related requests. This is something we can't change for compatibility.
>> The object model, however, is a per request map which inherits values
>> from the parent request. So some components rely on this behaviour.
> 
> Which object model? I'm not sure what exactly you are talking about.
The object model map which is passed to each and every sitemap component
in the setup() method. This is the only way of storing per request
information.

> 
> Despite things you have pointed out I'm wondering about the type
> returned by methods from ProcessInfoProvider.
> 
> If we want ProcessInfoProvider to return request object aware of sitemap
> sub-calls I think it makes sense to switch to our own Request interface
> just after we make it extending HttpServletRequest.
> On the other hand, we want to get rid of extensive use of Request so it
> would make sense to stay with HttpServletRequest but returned object
> would implement Request interface. This way it would be possible to cast
> where it's really needed (there are only few such places) and we could
> stay with clean interfaces.
Yes, let's use HttpServletRequest etc. - this makes the code usable
"outside" of Cocoon without any problems.

Carsten
-- 
Carsten Ziegeler
cziegeler@apache.org

Re: Sitemap scope

Posted by Grzegorz Kossakowski <gk...@apache.org>.
Carsten Ziegeler pisze:
> Daniel Fagerstrom wrote:
> 
>> Now, I understand if you feel that what I write above seem overly
>> pedantic and that ProcessorInfoProvider should be a good enough solution
>> to the environment handling problems. I have spent so much times
>> fighting the internals of Cocoon, and IMO one of the main reasons that
>> the code is so hard to follow is that the handling of the current
>> context is based on a patchwork of "good enough" hacks, rather than a
>> overall design. So I am strongly convinced that we should spend some
>> effort on getting it right. The greatest advantage with the "sitemap
>> scope" idea, is that it is non intrusive and isolates the user
>> components from the environment handling subtleness.
>>
> I think we are mixing things a little bit - one is the interface and the
> other one is the implementation. The sitemap scope is definitly an
> implementation detail which is irrelevant for the user of the interface.
> 
> The main idea of the ProcessInfoProvider was to have one single
> interface to get all relevant information. If we forget about the object
> model for a moment, I think it makes sense to have one interface for
> getting the request, the response and the context. Instead of having to
> implement three setter methods to get all pieces of information.

Do you think all three pieces are always needed? My answer would be: no, usually they are not.

I have no strong preference on ProcessInfoProvider or on direct dependencies. Daniel's argument that it is a less APIs to learn is right and 
I would add another one: it's easier to see what particular component really needs.

> Today, the object model is still of great value as we messed up the
> implementation and the semantics of internal pipeline calls: request
> attributes are shared between the original request and all child
> requests - therefore setting one in a child request overwrites it in all
> related requests. This is something we can't change for compatibility.
> The object model, however, is a per request map which inherits values
> from the parent request. So some components rely on this behaviour.

Which object model? I'm not sure what exactly you are talking about.

> We should therefore define what the behaviour of the request/response is
> that you get from the ProcessInfoProvider. If it's similar to the
> current object model and if I get a request object for the current
> request (being it interal or the original one) than we can remove this
> method from the interface. We can even rename the interface in need of a
> better name. (For implementing this behaviour) we might need the sitemap
> scope :)
> 
> But let's define the interfaces and contracts first.

Despite things you have pointed out I'm wondering about the type returned by methods from ProcessInfoProvider.

If we want ProcessInfoProvider to return request object aware of sitemap sub-calls I think it makes sense to switch to our own Request 
interface just after we make it extending HttpServletRequest.
On the other hand, we want to get rid of extensive use of Request so it would make sense to stay with HttpServletRequest but returned object 
would implement Request interface. This way it would be possible to cast where it's really needed (there are only few such places) and we 
could stay with clean interfaces.

WDYT?

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

Re: Sitemap scope

Posted by Carsten Ziegeler <cz...@apache.org>.
Daniel Fagerstrom wrote:

> 
> Now, I understand if you feel that what I write above seem overly
> pedantic and that ProcessorInfoProvider should be a good enough solution
> to the environment handling problems. I have spent so much times
> fighting the internals of Cocoon, and IMO one of the main reasons that
> the code is so hard to follow is that the handling of the current
> context is based on a patchwork of "good enough" hacks, rather than a
> overall design. So I am strongly convinced that we should spend some
> effort on getting it right. The greatest advantage with the "sitemap
> scope" idea, is that it is non intrusive and isolates the user
> components from the environment handling subtleness.
> 
I think we are mixing things a little bit - one is the interface and the
other one is the implementation. The sitemap scope is definitly an
implementation detail which is irrelevant for the user of the interface.

The main idea of the ProcessInfoProvider was to have one single
interface to get all relevant information. If we forget about the object
model for a moment, I think it makes sense to have one interface for
getting the request, the response and the context. Instead of having to
implement three setter methods to get all pieces of information.

Today, the object model is still of great value as we messed up the
implementation and the semantics of internal pipeline calls: request
attributes are shared between the original request and all child
requests - therefore setting one in a child request overwrites it in all
related requests. This is something we can't change for compatibility.
The object model, however, is a per request map which inherits values
from the parent request. So some components rely on this behaviour.

We should therefore define what the behaviour of the request/response is
that you get from the ProcessInfoProvider. If it's similar to the
current object model and if I get a request object for the current
request (being it interal or the original one) than we can remove this
method from the interface. We can even rename the interface in need of a
better name. (For implementing this behaviour) we might need the sitemap
scope :)

But let's define the interfaces and contracts first.

Carsten
-- 
Carsten Ziegeler
cziegeler@apache.org