You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by Felix Meschberger <fm...@gmail.com> on 2007/10/05 16:56:33 UTC

Move ContentManager to Sling API

Hi all,

Upon replying to Edgar's question regarding writing Content, I came to
thinking, that the ContentManager is a core interface of Sling and
probably should be made available more prominently. The most prominent
location for an interface in Sling would be the Sling API itself.

What do you think of moving the o.a.s.content.ContentManager interface
to the Sling API and provide it through the ComponentRequest instead of
having to access a request attribute:

    public interface ComponentRequest extends HttpServletRequest {
        ....
        public ContentManager getContentManager();
        ....
    }

What do you think ? If feedback is positive, I would also include this
in the Sling API proposal (SLING-28).

Regards
Felix


Re: Move ContentManager to Sling API

Posted by Felix Meschberger <fm...@gmail.com>.
Am Freitag, den 05.10.2007, 17:14 +0200 schrieb Carsten Ziegeler:
> > What do you think ? If feedback is positive, I would also include this
> > in the Sling API proposal (SLING-28).
> > 
> I'm not sure - it definitly makes repository based components much
> easier, but draws in some stuff into the (now simple) sling api and the
> core. As ContentManager does not have any further dependencies this
> seems to be ok.

That is the point: The ContentManager makes no actual references to any
concrete storage, it is only its (currently only one) extension
JcrContentManager which brings in the JCR Repository. But this would
remain in the content bundle.

> 
> So I'm +0 on this :)
> 
> I'm wondering if it is sufficient to have a single content manager in
> the application? Or could it be that some components might use a
> different content manager than others? For example some components
> writing/reading from one workspace or others doing it with another
> workspace?

Well having the getContentManager method would allow for
ComponentRequestWrapper implementations to overwrite this method and
return a different ContentManager while still allowing access to the
original one by just unwrapping and wrapped requests. This is not as
easy with the request attribute based solution.

On the other hand, nobody is hindered to get the
JcrContentManagerFactory service and acquire a different
JcrContentManager for specific use cases.

Regards
Felix


Re: Move ContentManager to Sling API

Posted by Carsten Ziegeler <cz...@apache.org>.
Felix Meschberger wrote:
> Hi all,
> 
> Upon replying to Edgar's question regarding writing Content, I came to
> thinking, that the ContentManager is a core interface of Sling and
> probably should be made available more prominently. The most prominent
> location for an interface in Sling would be the Sling API itself.
> 
> What do you think of moving the o.a.s.content.ContentManager interface
> to the Sling API and provide it through the ComponentRequest instead of
> having to access a request attribute:
> 
>     public interface ComponentRequest extends HttpServletRequest {
>         ....
>         public ContentManager getContentManager();
>         ....
>     
> 
> What do you think ? If feedback is positive, I would also include this
> in the Sling API proposal (SLING-28).
> 
I'm not sure - it definitly makes repository based components much
easier, but draws in some stuff into the (now simple) sling api and the
core. As ContentManager does not have any further dependencies this
seems to be ok.

So I'm +0 on this :)

I'm wondering if it is sufficient to have a single content manager in
the application? Or could it be that some components might use a
different content manager than others? For example some components
writing/reading from one workspace or others doing it with another
workspace?

Carsten

-- 
Carsten Ziegeler
cziegeler@apache.org

Re: Move ContentManager to Sling API

Posted by Felix Meschberger <fm...@gmail.com>.
Am Montag, den 08.10.2007, 10:44 +0100 schrieb Torgeir Veimo:
> My concern for subclassing core interfaces in the servlet API is due to
> deployment and testing concerns. If you have an API where you allow
> users of the API to rely on there always being subclassed instances of
> HttpServletRequest available, then you also lock any deployment
> configuration into always providing it. 

This was my initial concern with extending the
HttpServletRequest/Response a long time ago :-) In the meantime, I came
to believe, that it is far less problematic to do it this way to provide
some magical to get the additional data.

Regarding Sling, the goal is to implement a piece of code to run inside
Sling and here you expect the additional data available from Sling. If
you implement something, which you might want to run inside of Sling and
inside a standard Servlet Container, you would implement your something
in a way as to be able to handle "missing Sling".

> 
> With my experience with different servlet containers and deployment
> models, one cannot always ensure that specific servlet filters are
> handling requests at all times in a given servlet container. Eg. try
> getting servlet filters to work 100% of the time for error pages in
> tomcat. Thus, I always prefer a mechanism for retrieving manager
> instances which do not rely on subclassing HttpServletRequest or
> employing filters.

ServletFilters in Servlet Containers is always an issue, I agree.

> 
> The testing issue is a bit more simple, mainly concerned with making it
> easier to test code by using interfaces and classes that can easily be
> mocked. HttpServletRequest is not the best in that regard.

Agreed, but ... Objection, this is suggestive :-) 

Regards
Felix


Re: Move ContentManager to Sling API

Posted by Torgeir Veimo <to...@pobox.com>.
On Mon, 2007-10-08 at 08:22 +0200, Felix Meschberger wrote:
> Am Freitag, den 05.10.2007, 21:36 +0100 schrieb Torgeir Veimo:
> > I feel it's cleaner to fetch the manager from some sort of factory  
> > instance. It wouldn't expose the implementation details of how the  
> > manager is kept local to the request, nor does it make any dependent  
> > code reliant on any such implementation details staying stable.
> 
> Well, the implementation in fact is to use a factory to get the
> ContentManager. Look at the ContentResolverFilter class.
> 
> But this is different from the question on how we provide the
> ContentManager to components: As the return value of a declared method
> or as a well known request attribute. The first solution locks the
> Component into the API and forces the implementation of the API itself
> into providing a ContentManager object.
> 
> The second solution of just having the request attribute makes the
> existence of a ContentManager an implementation detail of the API
> implementation but locks the Component into the API implementation,
> which IMHO is more of a problem than being locked into the API.
> 
> > As you say yourself, you now keep the manager as a request-scoped  
> > attribute, but would like to make a subclass of HttpServletRequest  
> > include a method to retrieve it. If you used a factory to retrieve  
> > the manager, that change wouldn't have any impact on any code that  
> > uses that manager.
> 
> The ContentManager is core to Sling - without the ContentManager Sling
> cannot operate because it is used by Sling - the ContentResolver
> implementation to be precise - to resolve paths to Content instances. So
> Sling cares to have the ContentManager handy and just provides it to
> components to manage the Content.
> 
> Regarding extending HttpServletRequest: The ComponentRequest interface
> already extends the HttpServletRequest interface, so adding a getter for
> the ContentManager would just add to the ComponentRequest interface.

I have to admit I haven't started working closely with Sling yet, so I
don't have the full overview of the API yet.

My concern for subclassing core interfaces in the servlet API is due to
deployment and testing concerns. If you have an API where you allow
users of the API to rely on there always being subclassed instances of
HttpServletRequest available, then you also lock any deployment
configuration into always providing it. 

With my experience with different servlet containers and deployment
models, one cannot always ensure that specific servlet filters are
handling requests at all times in a given servlet container. Eg. try
getting servlet filters to work 100% of the time for error pages in
tomcat. Thus, I always prefer a mechanism for retrieving manager
instances which do not rely on subclassing HttpServletRequest or
employing filters.

The testing issue is a bit more simple, mainly concerned with making it
easier to test code by using interfaces and classes that can easily be
mocked. HttpServletRequest is not the best in that regard.

-- 
-Tor


Re: Move ContentManager to Sling API

Posted by Felix Meschberger <fm...@gmail.com>.
Am Freitag, den 05.10.2007, 21:36 +0100 schrieb Torgeir Veimo:
> I feel it's cleaner to fetch the manager from some sort of factory  
> instance. It wouldn't expose the implementation details of how the  
> manager is kept local to the request, nor does it make any dependent  
> code reliant on any such implementation details staying stable.

Well, the implementation in fact is to use a factory to get the
ContentManager. Look at the ContentResolverFilter class.

But this is different from the question on how we provide the
ContentManager to components: As the return value of a declared method
or as a well known request attribute. The first solution locks the
Component into the API and forces the implementation of the API itself
into providing a ContentManager object.

The second solution of just having the request attribute makes the
existence of a ContentManager an implementation detail of the API
implementation but locks the Component into the API implementation,
which IMHO is more of a problem than being locked into the API.

> As you say yourself, you now keep the manager as a request-scoped  
> attribute, but would like to make a subclass of HttpServletRequest  
> include a method to retrieve it. If you used a factory to retrieve  
> the manager, that change wouldn't have any impact on any code that  
> uses that manager.

The ContentManager is core to Sling - without the ContentManager Sling
cannot operate because it is used by Sling - the ContentResolver
implementation to be precise - to resolve paths to Content instances. So
Sling cares to have the ContentManager handy and just provides it to
components to manage the Content.

Regarding extending HttpServletRequest: The ComponentRequest interface
already extends the HttpServletRequest interface, so adding a getter for
the ContentManager would just add to the ComponentRequest interface.

Regards
Felix


Re: Move ContentManager to Sling API

Posted by Torgeir Veimo <to...@pobox.com>.
On 5 Oct 2007, at 17:30, Felix Meschberger wrote:

> Hi Torgeir,
>
> Am Freitag, den 05.10.2007, 17:25 +0100 schrieb Torgeir Veimo:
>> Having done some work recently with Day Communique, one of the
>> annoying things was that it subclasses HttpServletRequest.
>
> Can you please elaborate on this ? Because we initially did not extend
> HttpServletRequest but found that extending it would make the
> implementation much easier in multiple places not the least support  
> for
> running JSP scripts.

I feel it's cleaner to fetch the manager from some sort of factory  
instance. It wouldn't expose the implementation details of how the  
manager is kept local to the request, nor does it make any dependent  
code reliant on any such implementation details staying stable.

As you say yourself, you now keep the manager as a request-scoped  
attribute, but would like to make a subclass of HttpServletRequest  
include a method to retrieve it. If you used a factory to retrieve  
the manager, that change wouldn't have any impact on any code that  
uses that manager.

The factory method to retrieve the manager could easily take an  
HttpServletRequest as parameter, which would allow you to implement  
it in any way you choose.

-- 
Torgeir Veimo
torgeir@pobox.com




Re: Move ContentManager to Sling API

Posted by Felix Meschberger <fm...@gmail.com>.
Hi Torgeir,

Am Freitag, den 05.10.2007, 17:25 +0100 schrieb Torgeir Veimo:
> Having done some work recently with Day Communique, one of the  
> annoying things was that it subclasses HttpServletRequest.

Can you please elaborate on this ? Because we initially did not extend
HttpServletRequest but found that extending it would make the
implementation much easier in multiple places not the least support for
running JSP scripts.

Regards
Felix


Re: Move ContentManager to Sling API

Posted by Torgeir Veimo <to...@pobox.com>.
On 5 Oct 2007, at 15:56, Felix Meschberger wrote:

> What do you think of moving the o.a.s.content.ContentManager interface
> to the Sling API and provide it through the ComponentRequest  
> instead of
> having to access a request attribute:
>
>     public interface ComponentRequest extends HttpServletRequest {
>         ....
>         public ContentManager getContentManager();
>         ....
>     }
>
> What do you think ? If feedback is positive, I would also include this
> in the Sling API proposal (SLING-28).

Having done some work recently with Day Communique, one of the  
annoying things was that it subclasses HttpServletRequest.

-1

-- 
Torgeir Veimo
torgeir@pobox.com