You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cocoon.apache.org by Grzegorz Kossakowski <gr...@tuffmail.com> on 2007/07/24 11:51:56 UTC

How to register MockProcessInfoProvider?

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. Now, I would like to write tests for Spring beans depending on ProcessInfoProvider. I thought that 
it makes sense to provide mock implementation and register it in AbstractTestCase, similarly to how settings is registered; see my commit 
r558983[2].

I think, that it should work well and I think that it makes sense to register ProcessInfoProvider implementation in AbstractTestCase because 
this component provides access to environment and should be easily available for every test. The problem is that ProcessInfoProvider refers 
to javax.servlet.http.HttpServletRequest and in AbstractTestCase request is created with type org.apache.cocoon.environment.Request.

I'm puzzled what to do in such case, do you have any suggestion?

[1] http://article.gmane.org/gmane.text.xml.cocoon.devel/68706
[2] http://article.gmane.org/gmane.text.xml.cocoon.cvs/24845

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

Re: How to register MockProcessInfoProvider?

Posted by Carsten Ziegeler <cz...@apache.org>.
Grzegorz Kossakowski wrote:
> Carsten Ziegeler pisze:
>> Grzegorz Kossakowski wrote:
>>> Carsten Ziegeler pisze:
>>>
>>> Hmmm, something worth trying. Do you think that changing
>>> o.a.c.environment.* classes (like MockRequest) so they implement Servlet
>>> interfaces and wrapping them in AbstractTestCase won't break anything?
>> Hmm, it *should* not :) But I fear we'll only know afterwards..
> 
> I should have guessed the answer ;-)
> 
> Unfortunately, I face another problem. The only wrapper for
> HttpServletRequest we have is o.a.c.environment.http.HttpRequest but its
> only constructor looks like this:
> 
>   protected HttpRequest(HttpServletRequest req, HttpEnvironment env) {
>     super();
>     this.req = req;
>     this.env = env;
>   }
> 
> It's protected and it demands HttpEnvironment :-(
> Do you think that I should create MockHttpRequestWrapper that would
> extend HttpRequest? It all looks insane for me.
> 
> I'm wondering if previous approach (stub implementations) wouldn't be
> easier...
Hmm, what about mocking HttpServletRequest and HttpServletResponse and
then creating an HttpEnviromnent?
This in turn creates the HttpRequest object.

Carsten


-- 
Carsten Ziegeler
cziegeler@apache.org

Re: How to register MockProcessInfoProvider?

Posted by Grzegorz Kossakowski <gk...@apache.org>.
Carsten Ziegeler pisze:
> Grzegorz Kossakowski wrote:
>> Carsten Ziegeler pisze:
>>
>> Hmmm, something worth trying. Do you think that changing
>> o.a.c.environment.* classes (like MockRequest) so they implement Servlet
>> interfaces and wrapping them in AbstractTestCase won't break anything?
> Hmm, it *should* not :) But I fear we'll only know afterwards..

I should have guessed the answer ;-)

Unfortunately, I face another problem. The only wrapper for HttpServletRequest we have is o.a.c.environment.http.HttpRequest but its only 
constructor looks like this:

   protected HttpRequest(HttpServletRequest req, HttpEnvironment env) {
     super();
     this.req = req;
     this.env = env;
   }

It's protected and it demands HttpEnvironment :-(
Do you think that I should create MockHttpRequestWrapper that would extend HttpRequest? It all looks insane for me.

I'm wondering if previous approach (stub implementations) wouldn't be easier...

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

Re: How to register MockProcessInfoProvider?

Posted by Carsten Ziegeler <cz...@apache.org>.
Grzegorz Kossakowski wrote:
> Carsten Ziegeler pisze:
> 
>> Hmm, or the other way round - if the tests would create httpservlet
>> classes first, perhaps the current cocoon environment wrappers could
>> just be used to wrap them.
> 
> Hmmm, something worth trying. Do you think that changing
> o.a.c.environment.* classes (like MockRequest) so they implement Servlet
> interfaces and wrapping them in AbstractTestCase won't break anything?
Hmm, it *should* not :) But I fear we'll only know afterwards..

> I'm not sure if I understand you fully. Do you want to say that we
> should leave our environment interfaces untouched in general but I'm
> free to mess in AbstractTestCase or do you suggest to leave everything
> untouched?
Ah, sorry, yes, I meant to leave the environment abstraction untouched
but change the AbstractTestCase.

Carsten

-- 
Carsten Ziegeler
cziegeler@apache.org

Re: How to register MockProcessInfoProvider?

Posted by Grzegorz Kossakowski <gk...@apache.org>.
Carsten Ziegeler pisze:

> Hmm, or the other way round - if the tests would create httpservlet
> classes first, perhaps the current cocoon environment wrappers could
> just be used to wrap them.

Hmmm, something worth trying. Do you think that changing o.a.c.environment.* classes (like MockRequest) so they implement Servlet interfaces 
and wrapping them in AbstractTestCase won't break anything?

> It's a long time ago....well, the basic idea was that, as Daniel pointed
> out in the mentioned thread, there are a lot of libraries/code out there
> which require servlet classes (req/resp/context) are required.
> 
> The ProcessInfoProvider is a bean which just delivers these and does
> internally the required wrapping or unwrapping. In addition this creates
> a migration path as new stuff can be written which depends just on the
> servlet api and the bridge to the old stuff is this info provider.
> 
> I totally agree that we should remove our own environment as soon as
> possible, but this would create a lot of incompatibilities (as outlined
> in one of those threads). The biggest is that req.getSession() returns a
> o.a.c.e.Session currently and with switching to the servlet api it
> becomes a httpsession.
> 
> Personally I would strongly suggest to leave this stuff as it is and
> release 2.2 and see what happens.

I'm not sure if I understand you fully. Do you want to say that we should leave our environment interfaces untouched in general but I'm free 
to mess in AbstractTestCase or do you suggest to leave everything untouched?

Of course, I need something to do in order to be able to test Object Model implementation and parts using it.

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

Re: How to register MockProcessInfoProvider?

Posted by Peter Hunsberger <pe...@gmail.com>.
On 7/30/07, Daniel Fagerstrom <da...@nada.kth.se> wrote:
> Carsten Ziegeler skrev:
> > Grzegorz Kossakowski wrote:

<snip/>

+100

>
> So to summarize, it would be a great benefit to let our environment
> abstractions extend the http servlet ones and the cost would be low. So
> it is time to do it.
>
-- 
Peter Hunsberger

Re: How to register MockProcessInfoProvider?

Posted by Daniel Fagerstrom <da...@nada.kth.se>.
Carsten Ziegeler skrev:
> Daniel Fagerstrom wrote:
>> "A lot of incompatibilities" is an exaggeration. If we just let or
>> Request, Response, Session and Cookie extend the corresponding http
>> servlet ones, we would just get a few minor incompatibilities that IMO
>> would be rather minor in comparison with the benefits of using a
>> standardized API.
> :) exaggeration is better than downplaying (not implying that you're
> doing this here). With "a lot of" I meant for the users. I know a lot of
> code where a request.getSession() is called and stored in a variable of
> type o.a.c.e.Session. If we change the signature of this, well all these
> places have to be changed.
> 
> But as you outlined, it is not that hard to do the changes.

It is of course a trade of, but I think we gaining much more than we lose.

> Doesn't
> eclipse have this out-refactoring feature (or whatever it is called)?
> Where you add a refactoring description to a new jar and through this
> description the client code is automatically refactored?

No idea, sounds great though.

>> So to summarize, it would be a great benefit to let our environment
>> abstractions extend the http servlet ones and the cost would be low. So
>> it is time to do it.
>>
> I still think we should release before those changes.

I think we should consider the time plan for these changes and for the 
release orthogonal.

Grek has to respect GSoC project deadlines, so we cannot just ask him to 
wait. OTH, he or others cannot do major changes close to releases. If we 
have a clear release plan with fixed release dates, this shouldn't be to 
much of a problem. We'll have a code freeze a week before a release. If 
Grek doesn't think that his ongoing piece of work will be finished 
before the code freeze, he has to work in a branch. Also work during the 
freeze will need to be in a branch. Then he can merge again after the 
release.

This will of course be a little bit inconvenient. But much better than 
waiting until after the release.

/Daniel

Re: How to register MockProcessInfoProvider?

Posted by Carsten Ziegeler <cz...@apache.org>.
Daniel Fagerstrom wrote:
> 
> "A lot of incompatibilities" is an exaggeration. If we just let or
> Request, Response, Session and Cookie extend the corresponding http
> servlet ones, we would just get a few minor incompatibilities that IMO
> would be rather minor in comparison with the benefits of using a
> standardized API.
:) exaggeration is better than downplaying (not implying that you're
doing this here). With "a lot of" I meant for the users. I know a lot of
code where a request.getSession() is called and stored in a variable of
type o.a.c.e.Session. If we change the signature of this, well all these
places have to be changed.

But as you outlined, it is not that hard to do the changes. Doesn't
eclipse have this out-refactoring feature (or whatever it is called)?
Where you add a refactoring description to a new jar and through this
description the client code is automatically refactored?

> 
> So to summarize, it would be a great benefit to let our environment
> abstractions extend the http servlet ones and the cost would be low. So
> it is time to do it.
> 
I still think we should release before those changes.

Carsten


-- 
Carsten Ziegeler
cziegeler@apache.org

Re: How to register MockProcessInfoProvider?

Posted by Daniel Fagerstrom <da...@nada.kth.se>.
Carsten Ziegeler skrev:
> Grzegorz Kossakowski wrote:
>> After exploring code for a while I came to conclusion that I need to
>> implement stub implementation of HttpServletRequest, HttpServletResponse
>> and ServletContext that will forward to Cocoon's corresponding classes.
> Hmm, or the other way round - if the tests would create httpservlet
> classes first, perhaps the current cocoon environment wrappers could
> just be used to wrap them.
> 
>> Additionally, I've read /[RT] Ditching the environment abstraction/
>> thread[1] where Daniel proposed to switch to standard servlet interfaces
>> from Cocoon's own ones. I've seen that several folks were rather
>> reluctant[2][3] to allow these changes. There were concerns raised[4]
>> that some methods in our interfaces has no counterparts in Servlet API
>> interfaces.
>>
>> Since ProcessInfoProvider operates on interfaces from Servlet API I
>> wonder if community's standpoint changed to date.
>>
>> Carsten, it was you who introduced ProcessInfoProvider interface, could
>> you comment?
>>
> It's a long time ago....well, the basic idea was that, as Daniel pointed
> out in the mentioned thread, there are a lot of libraries/code out there
> which require servlet classes (req/resp/context) are required.
> 
> The ProcessInfoProvider is a bean which just delivers these and does
> internally the required wrapping or unwrapping. In addition this creates
> a migration path as new stuff can be written which depends just on the
> servlet api and the bridge to the old stuff is this info provider.
> 
> I totally agree that we should remove our own environment as soon as
> possible, but this would create a lot of incompatibilities (as outlined
> in one of those threads).

"A lot of incompatibilities" is an exaggeration. If we just let or 
Request, Response, Session and Cookie extend the corresponding http 
servlet ones, we would just get a few minor incompatibilities that IMO 
would be rather minor in comparison with the benefits of using a 
standardized API.

> The biggest is that req.getSession() returns a
> o.a.c.e.Session currently and with switching to the servlet api it
> becomes a httpsession.

As o.a.c.e.Session in the above proposal would extend HttpSession an 
object implementing o.a.c.e.Session can still be returned, so it 
shouldn't be such a big deal.

I took a more detailed look at it, we would need to change the following 
methods in Request:

Cookie[] getCookies();
InputStream getInputStream();
Session getSession(boolean create);
Session getSession();

I discussed the session methods above. The o.a.c.e.Cookie is a little 
bit worse as it is an interface and javax.servlet.http.Cookie is a 
class. But we could let o.a.c.e.http.HttpCookie (which is the only 
implementation of o.a.c.e.Cookie) extend javax.servlet.http.Cookie and 
just remove o.a.c.e.Cookie (which has exactly the same signature as 
javax.servlet.http.Cookie). The getInputStream method needs to return a 
ServletInputStream instead of an InputStream. As the 
o.a.c.e.http.HttpRequest.getInputStream returns a ServletInputStream 
anyway, I don't think it would lead to any problems.

Response and Session lacks a few methods from their http servlet counter 
parts. We could let them throw unsupported operation exceptions.

So to summarize, it would be a great benefit to let our environment 
abstractions extend the http servlet ones and the cost would be low. So 
it is time to do it.

/Daniel


Re: How to register MockProcessInfoProvider?

Posted by Carsten Ziegeler <cz...@apache.org>.
Grzegorz Kossakowski wrote:

> 
> What about wrapping these few classes by dynamic proxies that would
> implement lacking methods? These wrapping would happen in
> CocoonObjectModelProvider[1], would be narrowed to the Object Model only
> and could be quite transparent to the rest.
> 
Sounds good to me.


Carsten

-- 
Carsten Ziegeler
cziegeler@apache.org

Re: How to register MockProcessInfoProvider?

Posted by Grzegorz Kossakowski <gk...@apache.org>.
Carsten Ziegeler pisze:
> Yes, this is an open question - we should avoid these methods if
> possible :) Now, I can guess that you don't like the answer, but to have
> general components which might be reusable outside of Cocoon we should
> stick to available interfaces where possible.

Generally speaking, I'm not happy with our own interfaces either so have no problem with getting rid of them where it's possible.

> Now, the request has a getParametersMap(), perhaps we could "somehow"
> use this for cocoon.request.parameters? It would be great if we could
> just use the servlet interfaces whereever possible. This makes a
> transition later on much easier.

What about wrapping these few classes by dynamic proxies that would implement lacking methods? These wrapping would happen in 
CocoonObjectModelProvider[1], would be narrowed to the Object Model only and could be quite transparent to the rest.

[1] 
http://svn.apache.org/repos/asf/cocoon/trunk/core/cocoon-expression-language/cocoon-expression-language-impl/src/main/java/org/apache/cocoon/objectmodel/provider/CocoonEntryObjectModelProvider.java

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

Re: How to register MockProcessInfoProvider?

Posted by Carsten Ziegeler <cz...@apache.org>.
Grzegorz Kossakowski wrote:

> 
> What about getParameters() sort of methods that enable you to have
> expressions like cocoon.request.parameters.param1 ?
> 
> Since ProcessInfoProvider returns implementation of HttpServletRequest
> there is no access to these methods. Any idea on this?
> 
Yes, this is an open question - we should avoid these methods if
possible :) Now, I can guess that you don't like the answer, but to have
general components which might be reusable outside of Cocoon we should
stick to available interfaces where possible.

Now, the request has a getParametersMap(), perhaps we could "somehow"
use this for cocoon.request.parameters? It would be great if we could
just use the servlet interfaces whereever possible. This makes a
transition later on much easier.

Carsten

-- 
Carsten Ziegeler
cziegeler@apache.org

Re: How to register MockProcessInfoProvider?

Posted by Grzegorz Kossakowski <gk...@apache.org>.
Carsten Ziegeler pisze:
> Grzegorz Kossakowski wrote:
>> After exploring code for a while I came to conclusion that I need to
>> implement stub implementation of HttpServletRequest, HttpServletResponse
>> and ServletContext that will forward to Cocoon's corresponding classes.
> Hmm, or the other way round - if the tests would create httpservlet
> classes first, perhaps the current cocoon environment wrappers could
> just be used to wrap them.
> 
>> Additionally, I've read /[RT] Ditching the environment abstraction/
>> thread[1] where Daniel proposed to switch to standard servlet interfaces
>> from Cocoon's own ones. I've seen that several folks were rather
>> reluctant[2][3] to allow these changes. There were concerns raised[4]
>> that some methods in our interfaces has no counterparts in Servlet API
>> interfaces.
>>
>> Since ProcessInfoProvider operates on interfaces from Servlet API I
>> wonder if community's standpoint changed to date.
>>
>> Carsten, it was you who introduced ProcessInfoProvider interface, could
>> you comment?
>>
> It's a long time ago....well, the basic idea was that, as Daniel pointed
> out in the mentioned thread, there are a lot of libraries/code out there
> which require servlet classes (req/resp/context) are required.
> 
> The ProcessInfoProvider is a bean which just delivers these and does
> internally the required wrapping or unwrapping. In addition this creates
> a migration path as new stuff can be written which depends just on the
> servlet api and the bridge to the old stuff is this info provider.
> 
> I totally agree that we should remove our own environment as soon as
> possible, but this would create a lot of incompatibilities (as outlined
> in one of those threads). The biggest is that req.getSession() returns a
> o.a.c.e.Session currently and with switching to the servlet api it
> becomes a httpsession.

What about getParameters() sort of methods that enable you to have expressions like cocoon.request.parameters.param1 ?

Since ProcessInfoProvider returns implementation of HttpServletRequest there is no access to these methods. Any idea on this?

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

Re: How to register MockProcessInfoProvider?

Posted by Carsten Ziegeler <cz...@apache.org>.
Grzegorz Kossakowski wrote:
> 
> After exploring code for a while I came to conclusion that I need to
> implement stub implementation of HttpServletRequest, HttpServletResponse
> and ServletContext that will forward to Cocoon's corresponding classes.
Hmm, or the other way round - if the tests would create httpservlet
classes first, perhaps the current cocoon environment wrappers could
just be used to wrap them.

> 
> Additionally, I've read /[RT] Ditching the environment abstraction/
> thread[1] where Daniel proposed to switch to standard servlet interfaces
> from Cocoon's own ones. I've seen that several folks were rather
> reluctant[2][3] to allow these changes. There were concerns raised[4]
> that some methods in our interfaces has no counterparts in Servlet API
> interfaces.
> 
> Since ProcessInfoProvider operates on interfaces from Servlet API I
> wonder if community's standpoint changed to date.
> 
> Carsten, it was you who introduced ProcessInfoProvider interface, could
> you comment?
> 
It's a long time ago....well, the basic idea was that, as Daniel pointed
out in the mentioned thread, there are a lot of libraries/code out there
which require servlet classes (req/resp/context) are required.

The ProcessInfoProvider is a bean which just delivers these and does
internally the required wrapping or unwrapping. In addition this creates
a migration path as new stuff can be written which depends just on the
servlet api and the bridge to the old stuff is this info provider.

I totally agree that we should remove our own environment as soon as
possible, but this would create a lot of incompatibilities (as outlined
in one of those threads). The biggest is that req.getSession() returns a
o.a.c.e.Session currently and with switching to the servlet api it
becomes a httpsession.

Personally I would strongly suggest to leave this stuff as it is and
release 2.2 and see what happens.

Carsten
-- 
Carsten Ziegeler
cziegeler@apache.org

Re: How to register MockProcessInfoProvider?

Posted by Grzegorz Kossakowski <gk...@apache.org>.
Grzegorz Kossakowski pisze:
> 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. Now, I would like to write tests 
> for Spring beans depending on ProcessInfoProvider. I thought that it 
> makes sense to provide mock implementation and register it in 
> AbstractTestCase, similarly to how settings is registered; see my commit 
> r558983[2].
> 
> I think, that it should work well and I think that it makes sense to 
> register ProcessInfoProvider implementation in AbstractTestCase because 
> this component provides access to environment and should be easily 
> available for every test. The problem is that ProcessInfoProvider refers 
> to javax.servlet.http.HttpServletRequest and in AbstractTestCase request 
> is created with type org.apache.cocoon.environment.Request.
> 
> I'm puzzled what to do in such case, do you have any suggestion?
> 
> [1] http://article.gmane.org/gmane.text.xml.cocoon.devel/68706
> [2] http://article.gmane.org/gmane.text.xml.cocoon.cvs/24845

After exploring code for a while I came to conclusion that I need to implement stub implementation of HttpServletRequest, 
HttpServletResponse and ServletContext that will forward to Cocoon's corresponding classes.

Additionally, I've read /[RT] Ditching the environment abstraction/ thread[1] where Daniel proposed to switch to standard servlet interfaces 
from Cocoon's own ones. I've seen that several folks were rather reluctant[2][3] to allow these changes. There were concerns raised[4] that 
some methods in our interfaces has no counterparts in Servlet API interfaces.

Since ProcessInfoProvider operates on interfaces from Servlet API I wonder if community's standpoint changed to date.

Carsten, it was you who introduced ProcessInfoProvider interface, could you comment?

[1] http://thread.gmane.org/gmane.text.xml.cocoon.devel/59035
[2] http://article.gmane.org/gmane.text.xml.cocoon.devel/59143
[3] http://article.gmane.org/gmane.text.xml.cocoon.devel/59163
[4] http://article.gmane.org/gmane.text.xml.cocoon.devel/59118

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

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

Sitemap scope

Posted by Daniel Fagerstrom <da...@nada.kth.se>.
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