You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by Carsten Ziegeler <cz...@apache.org> on 2009/09/18 14:05:36 UTC

Applying SLING-864

I'm currently looking through open issues for our API and I stumbled
upon https://issues.apache.org/jira/browse/SLING-864.

What is the general opinion? Should we include it? Is is required?

I would like to either apply or close it to reduce the issue count :)

Regards
Carsten
-- 
Carsten Ziegeler
cziegeler@apache.org

Re: Applying SLING-864

Posted by Bertrand Delacretaz <bd...@gmail.com>.
On Fri, Sep 18, 2009 at 2:05 PM, Carsten Ziegeler <cz...@apache.org> wrote:
> I'm currently looking through open issues for our API and I stumbled
> upon https://issues.apache.org/jira/browse/SLING-864.
>
> What is the general opinion? Should we include it? Is is required?

+1 for applying, makes things more consistent.
-Bertrand

Re: Applying SLING-864

Posted by Jonathan 'J5' Cook <jo...@gmail.com>.
+1 from me to apply



On Sep 18, 2009, at 8:39 AM, Alexander Klimetschek <ak...@day.com>  
wrote:

> On Fri, Sep 18, 2009 at 14:05, Carsten Ziegeler  
> <cz...@apache.org> wrote:
>> I'm currently looking through open issues for our API and I stumbled
>> upon https://issues.apache.org/jira/browse/SLING-864.
>>
>> What is the general opinion? Should we include it? Is is required?
>>
>> I would like to either apply or close it to reduce the issue count :)
>
> +1 from me, of course ;-)
>
> Regards,
> Alex
>
> -- 
> Alexander Klimetschek
> alexander.klimetschek@day.com

Re: Applying SLING-864

Posted by Alexander Klimetschek <ak...@day.com>.
On Fri, Sep 18, 2009 at 14:05, Carsten Ziegeler <cz...@apache.org> wrote:
> I'm currently looking through open issues for our API and I stumbled
> upon https://issues.apache.org/jira/browse/SLING-864.
>
> What is the general opinion? Should we include it? Is is required?
>
> I would like to either apply or close it to reduce the issue count :)

+1 from me, of course ;-)

Regards,
Alex

-- 
Alexander Klimetschek
alexander.klimetschek@day.com

Re: Applying SLING-864

Posted by Alexander Klimetschek <ak...@day.com>.
On Wed, Sep 23, 2009 at 15:43, Felix Meschberger <fm...@gmail.com> wrote:
> And here [1] is the patch for setting the metadata in the
> JcrResourceResolver2 instead of the NonExistingResource constructor,
> where it probably does not belong anyway.

ACK for changing it. This keeps the NonExistingResource class clean.

Regards,
Alex

-- 
Alexander Klimetschek
alexander.klimetschek@day.com

Re: Applying SLING-864

Posted by Felix Meschberger <fm...@gmail.com>.
And here [1] is the patch for setting the metadata in the
JcrResourceResolver2 instead of the NonExistingResource constructor,
where it probably does not belong anyway.

Regards
Felix

[1]https://issues.apache.org/jira/browse/SLING-864?focusedCommentId=12758699&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#action_12758699

Felix Meschberger schrieb:
> Hi,
> 
> Speaking up as I am kind of opposed to this patch, too ;-)
> 
> Just to clarify: This patch modifies the NonExistingResource constructor
> such that it reads:
> 
>      public NonExistingResource(ResourceResolver resourceResolver,
>              String resourceURI) {
>          super(resourceResolver, resourceURI,
>                RESOURCE_TYPE_NON_EXISTING);
>          // separate "virtual" path and path info at first dot
>          int index = resourceURI.indexOf('.');
>          if ( index != -1 ) {
>              getResourceMetadata().setResolutionPathInfo(
>                        resourceURI.substring(index));
>          }
>      }
> 
> 
> This constructor may be called from anywhere, most notably it is called
> from the JcrResourceResolver2.resolve method.
> 
> After this constructor, the path of the resource remains unmodified
> (including any potential selectors and extensions). Only the resolution
> path info metadata is set, which used by RequestData.initServlet to
> setup the RequestPathInfo for the request.
> 
> First of all, I think Bertrand is perfectly right. It is probably a
> better idea to set this resource metadata in the
> JcrResourceResolver2.resolve method creating the non existing resource.
> Since this is where the use of this metadata is of most interest.
> 
> Second, I could be ok with setting this field in the resolve method and
> not in the constructor itself. It is also conceptually more correct
> because for any other resources this metadata is also set by the
> JcrResourceResolver2.
> 
> Regards
> Felix
> 
> Bertrand Delacretaz schrieb:
>> On Wed, Sep 23, 2009 at 1:22 PM, Ian Boston <ie...@tfd.co.uk> wrote:
>>> ...What I don't see justification for, is parsing the path into selectors,
>>> extensions etc when we have no evidence what is the right parsing.
>>> The patch adopts a convention as justification for the parsing but IIUC that
>>> convention hasn't been adopted in a,b or c1...
>> I agree that hardcoding this parsing in the api bundle feels weird.
>>
>> If we want to avoid that, the changes should go into
>> JcrResourceResolver2 instead, where the NonExistingResource is
>> created.
>>
>> The parsing can then use a configurable regexp (or a service? we
>> probably don't need to go that far) to extract the resource path from
>> the request path.
>>
>> -Bertrand
>>
> 

Re: Applying SLING-864

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

Speaking up as I am kind of opposed to this patch, too ;-)

Just to clarify: This patch modifies the NonExistingResource constructor
such that it reads:

     public NonExistingResource(ResourceResolver resourceResolver,
             String resourceURI) {
         super(resourceResolver, resourceURI,
               RESOURCE_TYPE_NON_EXISTING);
         // separate "virtual" path and path info at first dot
         int index = resourceURI.indexOf('.');
         if ( index != -1 ) {
             getResourceMetadata().setResolutionPathInfo(
                       resourceURI.substring(index));
         }
     }


This constructor may be called from anywhere, most notably it is called
from the JcrResourceResolver2.resolve method.

After this constructor, the path of the resource remains unmodified
(including any potential selectors and extensions). Only the resolution
path info metadata is set, which used by RequestData.initServlet to
setup the RequestPathInfo for the request.

First of all, I think Bertrand is perfectly right. It is probably a
better idea to set this resource metadata in the
JcrResourceResolver2.resolve method creating the non existing resource.
Since this is where the use of this metadata is of most interest.

Second, I could be ok with setting this field in the resolve method and
not in the constructor itself. It is also conceptually more correct
because for any other resources this metadata is also set by the
JcrResourceResolver2.

Regards
Felix

Bertrand Delacretaz schrieb:
> On Wed, Sep 23, 2009 at 1:22 PM, Ian Boston <ie...@tfd.co.uk> wrote:
>> ...What I don't see justification for, is parsing the path into selectors,
>> extensions etc when we have no evidence what is the right parsing.
>> The patch adopts a convention as justification for the parsing but IIUC that
>> convention hasn't been adopted in a,b or c1...
> 
> I agree that hardcoding this parsing in the api bundle feels weird.
> 
> If we want to avoid that, the changes should go into
> JcrResourceResolver2 instead, where the NonExistingResource is
> created.
> 
> The parsing can then use a configurable regexp (or a service? we
> probably don't need to go that far) to extract the resource path from
> the request path.
> 
> -Bertrand
> 

Re: Applying SLING-864

Posted by Bertrand Delacretaz <bd...@apache.org>.
On Wed, Sep 23, 2009 at 1:22 PM, Ian Boston <ie...@tfd.co.uk> wrote:
> ...What I don't see justification for, is parsing the path into selectors,
> extensions etc when we have no evidence what is the right parsing.
> The patch adopts a convention as justification for the parsing but IIUC that
> convention hasn't been adopted in a,b or c1...

I agree that hardcoding this parsing in the api bundle feels weird.

If we want to avoid that, the changes should go into
JcrResourceResolver2 instead, where the NonExistingResource is
created.

The parsing can then use a configurable regexp (or a service? we
probably don't need to go that far) to extract the resource path from
the request path.

-Bertrand

Re: Applying SLING-864

Posted by Ian Boston <ie...@tfd.co.uk>.
On 23 Sep 2009, at 13:38, Alexander Klimetschek wrote:

> On Wed, Sep 23, 2009 at 13:22, Ian Boston <ie...@tfd.co.uk> wrote:
>> yes I agree that at c2, after all processing is performed, no  
>> resource is
>> found and, in addition the patch only modifies the behavior of c2.
>
> Ok, so we can agree that the patch won't break existing things. This
> leaves the open point a conceptual one, as you note...


we should qualify that, it wont break anything in Sling because there  
is nothing that binds to NonExsistantResource, but it will break  
anything that binds to NonExistantResource  outside Sling, as a  
resource type since the content of NonExistingResource has changed.  
(the path)

But thats not a good enough reason to reject the patch... more below.

>
>> What I don't see justification for, is parsing the path into  
>> selectors,
>> extensions etc when we have no evidence what is the right parsing.
>> The patch adopts a convention as justification for the parsing but  
>> IIUC that
>> convention hasn't been adopted in a,b or c1
>
> ...which is right to question. My justification of bringing in a
> convention into the parsing is because I think that what is gained is
> so useful, that we can live with that convention.
>
> Also, I think it's not an arbitrary convention, but one that follows
> for the majority of selector/extension cases. And when considering
> that a request to a non-existent resource is typically made *very
> specifically* to that URL, we even get a 100% match (because the
> server gives clients a certain URL to POST to).
>
> For example (the same as given in SLING-864), to build an importer
> servlet for eg. a special file format such as icalendar files (ics),
> you would like to do a POST or PUT onto a non-existent resource:
>
> POST /content/calendar.ics
>
> Now if you want to update the file, you do the exact same POST, but
> now going the way over the existing resource (but you could use the
> same servlet for that, registered under non-existent resource type and
> the calendar resource type):
>
> POST /content/calendar.ics
>
> And later you get it via the corresponding export servlet
>
> GET /content/calendar.ics
>
> The problem is that since in the first case the resource does not
> exist yet, you currently (ie. w/o the patch) cannot register anything
> under the ics extension for non-existing resources. That's why I put
> this parsing "convention" into the patch, as it matches the typical
> and most-often used case for extensions and selectors.


These are strong overriding reasons to apply this patch.
On this basis I am +1, even it it does change API behavior and break  
other code.

Thanks for taking time to work this through.
Ian


>
>> IMHO, either the patch should go wide and adopt the convention  
>> throughout or
>> it should avoid imposing a convention leaving it to things that  
>> *might* be
>> able to operate on the non existent resource.
>
> It's right that for other special cases the convention might seem
> arbitrary, but without the patch they would be lost as well. You can
> still apply your custom handling if you want to follow another
> convention by simply registering as the default servlet for
> non-existing resources and take the plain request URI while ignoring
> the pre-parsed selectors and extensions.
>
> Summary: no existing code is broken, and no convention is forced upon;
> but if you like the convention, you gain a lot.
>
> Regards,
> Alex
>
> -- 
> Alexander Klimetschek
> alexander.klimetschek@day.com


Re: Applying SLING-864

Posted by Alexander Klimetschek <ak...@day.com>.
On Wed, Sep 23, 2009 at 13:22, Ian Boston <ie...@tfd.co.uk> wrote:
> yes I agree that at c2, after all processing is performed, no resource is
> found and, in addition the patch only modifies the behavior of c2.

Ok, so we can agree that the patch won't break existing things. This
leaves the open point a conceptual one, as you note...

> What I don't see justification for, is parsing the path into selectors,
> extensions etc when we have no evidence what is the right parsing.
> The patch adopts a convention as justification for the parsing but IIUC that
> convention hasn't been adopted in a,b or c1

...which is right to question. My justification of bringing in a
convention into the parsing is because I think that what is gained is
so useful, that we can live with that convention.

Also, I think it's not an arbitrary convention, but one that follows
for the majority of selector/extension cases. And when considering
that a request to a non-existent resource is typically made *very
specifically* to that URL, we even get a 100% match (because the
server gives clients a certain URL to POST to).

For example (the same as given in SLING-864), to build an importer
servlet for eg. a special file format such as icalendar files (ics),
you would like to do a POST or PUT onto a non-existent resource:

POST /content/calendar.ics

Now if you want to update the file, you do the exact same POST, but
now going the way over the existing resource (but you could use the
same servlet for that, registered under non-existent resource type and
the calendar resource type):

POST /content/calendar.ics

And later you get it via the corresponding export servlet

GET /content/calendar.ics

The problem is that since in the first case the resource does not
exist yet, you currently (ie. w/o the patch) cannot register anything
under the ics extension for non-existing resources. That's why I put
this parsing "convention" into the patch, as it matches the typical
and most-often used case for extensions and selectors.

> IMHO, either the patch should go wide and adopt the convention throughout or
> it should avoid imposing a convention leaving it to things that *might* be
> able to operate on the non existent resource.

It's right that for other special cases the convention might seem
arbitrary, but without the patch they would be lost as well. You can
still apply your custom handling if you want to follow another
convention by simply registering as the default servlet for
non-existing resources and take the plain request URI while ignoring
the pre-parsed selectors and extensions.

Summary: no existing code is broken, and no convention is forced upon;
but if you like the convention, you gain a lot.

Regards,
Alex

-- 
Alexander Klimetschek
alexander.klimetschek@day.com

Re: Applying SLING-864

Posted by Ian Boston <ie...@tfd.co.uk>.
On 23 Sep 2009, at 11:32, Alexander Klimetschek wrote:

> On Wed, Sep 23, 2009 at 11:40, Ian Boston <ie...@tfd.co.uk> wrote:
>> IMHO this comes down to if you believe that the resolution of the  
>> resource
>> should be based on parsing the URI path alone, or should be  
>> informed by the
>> existence of a real resource in the content system.
>>
>> At the moment most resources in Sling derive the fields of the  
>> Resource
>> object from the real resources, NonExistentResource is one of the  
>> exceptions
>> since it has no resource to inform it.
>
> Exactly. And my argument is that this case is
>
> a) special, because no resource present that could be mistakenly
> "missed" or incorrectly addressed through another extension and
> b) it's obvious to the API user / Sling developer through the special
> resource type.
>
> Note that before Sling comes up with a non-existent resource, it has
> already checked for any longest-matching resource to be present.
> Because there is none, the non-existent resource is created and then
> the path info is parsed. Thus the path info could never match any
> existing resource.
>
>> Applying the patch resolved the inconsistency but generates a  
>> deeper and
>> more fundamental inconsistency that if also resolved would not allow
>> resources to have extensions in the content system (ie /content/ 
>> readme.doc
>> would not be a valid resource name since parsing by path alone  
>> would make
>> the resource /content/readme )
>
> This is not the case. As noted above, the path info parsing for
> non-existent resources that the patch introduces is only applied
> *after* Sling searched for resources in its standard way. Thus a
> /content/readme.doc would always be found before something like
> /content/readme.doc.whatever as a non-existent resource could ever be
> set by Sling.
>
>> On that basis I am of the opinion that the current behavior is  
>> consistent
>> with Resource resolution being based on real resources and so the  
>> patch
>> should not be applied.
>>
>> You will notice I haven't "voted", as I am prepared to be persuaded  
>> that my
>> assumptions and conclusions are incorrect.
>
> I assume you thought that this patch will change the general resource
> resolution behavior to cut off the resource path at the first dot.
> This is not the case. This is how it looks like:
>
> a) sling gets request with uri, starts searching for the longest
> matching resource (IIRC cutting off at the last dot first)
> b) if nothing is found, this is repeated by cutting off at the
> second-last dot, etc.
> c1) resource found: use the given resource path and derive selectors,
> extensions etc. from the rest of the request uri after the resource
> path
> c2) no resource found: create a NonExistingResource and parse
> selectors, extensions etc. after the first dot found in the request
> path
>
> For c2) this means there cannot be a resource at the "virtual"
> resource path, or any other part of the rest the full URI.

yes I agree that at c2, after all processing is performed, no resource  
is found and, in addition the patch only modifies the behavior of c2.

What I don't see justification for, is parsing the path into  
selectors, extensions etc when we have no evidence what is the right  
parsing.
The patch adopts a convention as justification for the parsing but  
IIUC that convention hasn't been adopted in a,b or c1

IMHO, either the patch should go wide and adopt the convention  
throughout or it should avoid imposing a convention leaving it to  
things that *might* be able to operate on the non existent resource.

Perhaps this is really ideology over what a URI is, in which case I am  
happy to adopt any consistent convention.

Ian


BTW, my concern is not driven by code outside Sling that I will have  
to change, I am 100% ok with making those changes regardless of the  
outcome here. Progress even in API related behavior is vital.

>
> Regards,
> Alex
>
> -- 
> Alexander Klimetschek
> alexander.klimetschek@day.com


Re: Applying SLING-864

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

Vidar Ramdal schrieb:
> Just to check if I understand correctly:
> If you have an existing resource /content/readme.doc, you will not be
> able to address a non-existing resource /content/readme with the
> extension ".doc"?

Yes, that is correct.

Regards
Felix

> 
> That possible limitation is far-fetched, and acceptable in my opinion.
> 


Re: Applying SLING-864

Posted by Alexander Klimetschek <ak...@day.com>.
On Wed, Sep 23, 2009 at 13:08, Vidar Ramdal <vi...@idium.no> wrote:
> On Wed, Sep 23, 2009 at 12:32 PM, Alexander Klimetschek
> <ak...@day.com> wrote:
>> For c2) this means there cannot be a resource at the "virtual"
>> resource path, or any other part of the rest the full URI.
>
> Just to check if I understand correctly:
> If you have an existing resource /content/readme.doc, you will not be
> able to address a non-existing resource /content/readme with the
> extension ".doc"?

AFAIK not (Felix can probably clarify that). I think the resource
resolution starts with the full URI path first and then cuts off at
the last dot and so on (not as I mentioned before that it first cuts
off at the last dot).

Also note that the integration tests all run without problems with the
patch applied, and they should contain some tests targeted at resource
resolution.

Regards,
Alex

-- 
Alexander Klimetschek
alexander.klimetschek@day.com

Re: Applying SLING-864

Posted by Vidar Ramdal <vi...@idium.no>.
On Wed, Sep 23, 2009 at 12:32 PM, Alexander Klimetschek
<ak...@day.com> wrote:
> For c2) this means there cannot be a resource at the "virtual"
> resource path, or any other part of the rest the full URI.

Just to check if I understand correctly:
If you have an existing resource /content/readme.doc, you will not be
able to address a non-existing resource /content/readme with the
extension ".doc"?

That possible limitation is far-fetched, and acceptable in my opinion.

-- 
Vidar S. Ramdal <vi...@idium.no> - http://www.idium.no
Sommerrogata 13-15, N-0255 Oslo, Norway
+ 47 22 00 84 00 / +47 21 531941, ext 2070

Re: Applying SLING-864

Posted by Alexander Klimetschek <ak...@day.com>.
On Wed, Sep 23, 2009 at 11:40, Ian Boston <ie...@tfd.co.uk> wrote:
> IMHO this comes down to if you believe that the resolution of the resource
> should be based on parsing the URI path alone, or should be informed by the
> existence of a real resource in the content system.
>
> At the moment most resources in Sling derive the fields of the Resource
> object from the real resources, NonExistentResource is one of the exceptions
> since it has no resource to inform it.

Exactly. And my argument is that this case is

a) special, because no resource present that could be mistakenly
"missed" or incorrectly addressed through another extension and
b) it's obvious to the API user / Sling developer through the special
resource type.

Note that before Sling comes up with a non-existent resource, it has
already checked for any longest-matching resource to be present.
Because there is none, the non-existent resource is created and then
the path info is parsed. Thus the path info could never match any
existing resource.

> Applying the patch resolved the inconsistency but generates a deeper and
> more fundamental inconsistency that if also resolved would not allow
> resources to have extensions in the content system (ie /content/readme.doc
> would not be a valid resource name since parsing by path alone would make
> the resource /content/readme )

This is not the case. As noted above, the path info parsing for
non-existent resources that the patch introduces is only applied
*after* Sling searched for resources in its standard way. Thus a
/content/readme.doc would always be found before something like
/content/readme.doc.whatever as a non-existent resource could ever be
set by Sling.

> On that basis I am of the opinion that the current behavior is consistent
> with Resource resolution being based on real resources and so the patch
> should not be applied.
>
> You will notice I haven't "voted", as I am prepared to be persuaded that my
> assumptions and conclusions are incorrect.

I assume you thought that this patch will change the general resource
resolution behavior to cut off the resource path at the first dot.
This is not the case. This is how it looks like:

a) sling gets request with uri, starts searching for the longest
matching resource (IIRC cutting off at the last dot first)
b) if nothing is found, this is repeated by cutting off at the
second-last dot, etc.
c1) resource found: use the given resource path and derive selectors,
extensions etc. from the rest of the request uri after the resource
path
c2) no resource found: create a NonExistingResource and parse
selectors, extensions etc. after the first dot found in the request
path

For c2) this means there cannot be a resource at the "virtual"
resource path, or any other part of the rest the full URI.

Regards,
Alex

-- 
Alexander Klimetschek
alexander.klimetschek@day.com

Re: Applying SLING-864

Posted by Ian Boston <ie...@tfd.co.uk>.
On 23 Sep 2009, at 09:49, Carsten Ziegeler wrote:

> Ok, it seems that this issue is rather a controversial one.
>
> The question remains if we should apply it or not. As far as I could
> follow the discussion, Felix and Ian are not happy with the patch  
> while
> Alex claims that it doesn't do any harm.
>
> So I think, if you two, Felix and Ian, could shortly state what your
> current feelings are about this issue we can proceed. Either we can
> apply it then or I'll close it.

IMHO this comes down to if you believe that the resolution of the  
resource should be based on parsing the URI path alone, or should be  
informed by the existence of a real resource in the content system.

At the moment most resources in Sling derive the fields of the  
Resource object from the real resources, NonExistentResource is one of  
the exceptions since it has no resource to inform it.

Applying the patch resolved the inconsistency but generates a deeper  
and more fundamental inconsistency that if also resolved would not  
allow resources to have extensions in the content system (ie /content/ 
readme.doc would not be a valid resource name since parsing by path  
alone would make the resource /content/readme )

On that basis I am of the opinion that the current behavior is  
consistent with Resource resolution being based on real resources and  
so the patch should not be applied.

You will notice I haven't "voted", as I am prepared to be persuaded  
that my assumptions and conclusions are incorrect.

Ian



>
> Thanks
> Carsten
> -- 
> Carsten Ziegeler
> cziegeler@apache.org


Re: Applying SLING-864

Posted by Carsten Ziegeler <cz...@apache.org>.
Ok, it seems that this issue is rather a controversial one.

The question remains if we should apply it or not. As far as I could
follow the discussion, Felix and Ian are not happy with the patch while
Alex claims that it doesn't do any harm.

So I think, if you two, Felix and Ian, could shortly state what your
current feelings are about this issue we can proceed. Either we can
apply it then or I'll close it.

Thanks
Carsten
-- 
Carsten Ziegeler
cziegeler@apache.org

Re: Applying SLING-864

Posted by Alexander Klimetschek <ak...@day.com>.
On Mon, Sep 21, 2009 at 12:28, Felix Meschberger <fm...@gmail.com> wrote:
> I agree also with Ian's POV (and have also argumented the same way in
> the past), the unless there is an actual resource around we cannot tell
> where the name ends and the selectors/extension/suffix starts ...

Of course this is just a "guess", but it is the same guess that the
non-existent servlet handler (which is one single-big thing at the
moment) would have to do itself. And in 99% cases this guess will be
the right one - for the other 1% the handler can do it himself.

But I wonder how such a case (the 1%) should be handled: it's all
about resources that contain a dot inside (because the path will force
path info parsing starting at the first dot), and programming against
that case sounds a bit crazy to me: if this post to a non-existing URL
starts with xyz.ext.bla (which does not exist!), then cut off at the
third dot...

So far I haven't seen a real use-case where this patch would introduce
a problem (that is larger than the very useful gain it brings, namely
to allow for selectors/extensions on those requests).

> Create a generic servlet handling requests to non-existing resources.

Introducing another servlet/mechanism for that only introduces more
complexity that isn't needed IMHO.

Regards,
Alex

-- 
Alexander Klimetschek
alexander.klimetschek@day.com

Re: Applying SLING-864

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

I have had this patch lying around in my workspace for quite some time,
simply because I was not sure about how to treat this.

Basically, I understand and agree with the issue, but ...

I agree also with Ian's POV (and have also argumented the same way in
the past), the unless there is an actual resource around we cannot tell
where the name ends and the selectors/extension/suffix starts ...

So, here is my -1 to applying the patch.

And here is my proposed solution: Rather I propose to add another
pluggable functionality to support non existing resources (as I already
proposed in [1]):

Create a generic servlet handling requests to non-existing resources. A
new interface is provided helping in further deciding on the actual
servlet to call for handling.

If no such servlet is actually registered, the non existing servlet
would just delegate to the default servlet.

WDYT ?

Regards
Felix


[1] http://markmail.org/message/rgxhqtkretocniki

Ian Boston schrieb:
> I could put that a bit more clearly.
> 
> Until the resource exists, there is no telling if where the resource ends.
> Current behavior is consistent where there is an extension.
> But is not consistent where there is not an extension.
> 
> IMHO, until there is a resource to indicate what the resource is and
> what the extension is all we can say for sure is what the path is.
> 
> Ian
> 
> eg
> curl http://localhost:8080/testing/universal.swf
> 
>       0 (2009-09-20 08:10:16) Starting Request Processing
>       0 (2009-09-20 08:10:16) Method=GET, PathInfo=/test/universal.swf
>       1 (2009-09-20 08:10:16) Starting ResourceResolution
>      14 (2009-09-20 08:10:16) URI=/test/universal.swf resolves to
> Resource=NonExistingResource, path=/test/universal.swf, elapsed = 13ms
>      14 (2009-09-20 08:10:16) Resource Path Info: SlingRequestPathInfo:
> path='/test/universal.swf', selectorString='null', extension='null',
> suffix='null'
>      14 (2009-09-20 08:10:16) Starting ServletResolution
>      14 (2009-09-20 08:10:16) Starting
> resolveServlet(NonExistingResource, path=/test/universal.swf)
>      14 (2009-09-20 08:10:16) Using servlet
> org.apache.sling.servlets.get.DefaultGetServlet, elapsed = 0ms
>      14 (2009-09-20 08:10:16) URI=/test/universal.swf handled by
> Servlet=org.apache.sling.servlets.get.DefaultGetServlet, elapsed = 0ms
>      14 (2009-09-20 08:10:16) Applying request filters
>      14 (2009-09-20 08:10:16) Calling filter:
> org.apache.sling.engine.impl.debug.RequestProgressTrackerLogFilter
>      14 (2009-09-20 08:10:16) Starting
> org.apache.sling.servlets.get.DefaultGetServlet#0
>      14 (2009-09-20 08:10:16)
> org.apache.sling.servlets.get.DefaultGetServlet#0, elapsed = 0ms
>      14 (2009-09-20 08:10:16) Starting handleError:status=404
>      14 (2009-09-20 08:10:16) Using handler Sling Default Error Handler
> Servlet, elapsed = 0ms
>      15 (2009-09-20 08:10:16) Error handler finished, elapsed = 1ms
> 
> 
> Upload universal.swf and then
> 
> 
>       0 (2009-09-20 08:12:10) Starting Request Processing
>       0 (2009-09-20 08:12:10) Method=GET, PathInfo=/test/universal.swf
>      61 (2009-09-20 08:12:10) Starting ResourceResolution
>      62 (2009-09-20 08:12:10) URI=/test/universal.swf resolves to
> Resource=JcrNodeResource, type=nt:file, superType=null,
> path=/test/universal.swf, elapsed = 1ms
>      62 (2009-09-20 08:12:10) Resource Path Info: SlingRequestPathInfo:
> path='/test/universal.swf', selectorString='null', extension='null',
> suffix='null'
>      62 (2009-09-20 08:12:10) Starting ServletResolution
>      62 (2009-09-20 08:12:10) Starting resolveServlet(JcrNodeResource,
> type=nt:file, superType=null, path=/test/universal.swf)
>      62 (2009-09-20 08:12:10) Using servlet
> org.apache.sling.servlets.get.DefaultGetServlet, elapsed = 0ms
>      62 (2009-09-20 08:12:10) URI=/test/universal.swf handled by
> Servlet=org.apache.sling.servlets.get.DefaultGetServlet, elapsed = 0ms
>      62 (2009-09-20 08:12:10) Applying request filters
>      62 (2009-09-20 08:12:10) Calling filter:
> org.apache.sling.engine.impl.debug.RequestProgressTrackerLogFilter
>      62 (2009-09-20 08:12:10) Starting
> org.apache.sling.servlets.get.DefaultGetServlet#0
>      62 (2009-09-20 08:12:10) Using
> org.apache.sling.servlets.get.impl.helpers.StreamRendererServlet to
> render for extension=null
>     169 (2009-09-20 08:12:10)
> org.apache.sling.servlets.get.DefaultGetServlet#0, elapsed = 107ms
>     169 (2009-09-20 08:12:10) Request Processing ends, elapsed = 169ms
> 
> 
> 
> On 19 Sep 2009, at 17:49, Ian Boston wrote:
> 
>> I think this might break some behavior, as there is no way for the non
>> existent resource to know if an extension is a serialization extension
>> or the real extension of the file in the jcr, at the moment, code that
>> uses non existent resource assumes that the extension is not
>> determined, and so would need changing to be compatible with this change.
>>
>> Ian
>>
>> On 18 Sep 2009, at 22:05, Carsten Ziegeler wrote:
>>
>>> I'm currently looking through open issues for our API and I stumbled
>>> upon https://issues.apache.org/jira/browse/SLING-864.
>>>
>>> What is the general opinion? Should we include it? Is is required?
>>>
>>> I would like to either apply or close it to reduce the issue count :)
>>>
>>> Regards
>>> Carsten
>>> -- 
>>> Carsten Ziegeler
>>> cziegeler@apache.org
>>
> 
> 

Re: Applying SLING-864

Posted by Ian Boston <ie...@tfd.co.uk>.
I could put that a bit more clearly.

Until the resource exists, there is no telling if where the resource  
ends.
Current behavior is consistent where there is an extension.
But is not consistent where there is not an extension.

IMHO, until there is a resource to indicate what the resource is and  
what the extension is all we can say for sure is what the path is.

Ian

eg
curl http://localhost:8080/testing/universal.swf

       0 (2009-09-20 08:10:16) Starting Request Processing
       0 (2009-09-20 08:10:16) Method=GET, PathInfo=/test/universal.swf
       1 (2009-09-20 08:10:16) Starting ResourceResolution
      14 (2009-09-20 08:10:16) URI=/test/universal.swf resolves to  
Resource=NonExistingResource, path=/test/universal.swf, elapsed = 13ms
      14 (2009-09-20 08:10:16) Resource Path Info:  
SlingRequestPathInfo: path='/test/universal.swf',  
selectorString='null', extension='null', suffix='null'
      14 (2009-09-20 08:10:16) Starting ServletResolution
      14 (2009-09-20 08:10:16) Starting  
resolveServlet(NonExistingResource, path=/test/universal.swf)
      14 (2009-09-20 08:10:16) Using servlet  
org.apache.sling.servlets.get.DefaultGetServlet, elapsed = 0ms
      14 (2009-09-20 08:10:16) URI=/test/universal.swf handled by  
Servlet=org.apache.sling.servlets.get.DefaultGetServlet, elapsed = 0ms
      14 (2009-09-20 08:10:16) Applying request filters
      14 (2009-09-20 08:10:16) Calling filter:  
org.apache.sling.engine.impl.debug.RequestProgressTrackerLogFilter
      14 (2009-09-20 08:10:16) Starting  
org.apache.sling.servlets.get.DefaultGetServlet#0
      14 (2009-09-20 08:10:16)  
org.apache.sling.servlets.get.DefaultGetServlet#0, elapsed = 0ms
      14 (2009-09-20 08:10:16) Starting handleError:status=404
      14 (2009-09-20 08:10:16) Using handler Sling Default Error  
Handler Servlet, elapsed = 0ms
      15 (2009-09-20 08:10:16) Error handler finished, elapsed = 1ms


Upload universal.swf and then


       0 (2009-09-20 08:12:10) Starting Request Processing
       0 (2009-09-20 08:12:10) Method=GET, PathInfo=/test/universal.swf
      61 (2009-09-20 08:12:10) Starting ResourceResolution
      62 (2009-09-20 08:12:10) URI=/test/universal.swf resolves to  
Resource=JcrNodeResource, type=nt:file, superType=null, path=/test/ 
universal.swf, elapsed = 1ms
      62 (2009-09-20 08:12:10) Resource Path Info:  
SlingRequestPathInfo: path='/test/universal.swf',  
selectorString='null', extension='null', suffix='null'
      62 (2009-09-20 08:12:10) Starting ServletResolution
      62 (2009-09-20 08:12:10) Starting  
resolveServlet(JcrNodeResource, type=nt:file, superType=null, path=/ 
test/universal.swf)
      62 (2009-09-20 08:12:10) Using servlet  
org.apache.sling.servlets.get.DefaultGetServlet, elapsed = 0ms
      62 (2009-09-20 08:12:10) URI=/test/universal.swf handled by  
Servlet=org.apache.sling.servlets.get.DefaultGetServlet, elapsed = 0ms
      62 (2009-09-20 08:12:10) Applying request filters
      62 (2009-09-20 08:12:10) Calling filter:  
org.apache.sling.engine.impl.debug.RequestProgressTrackerLogFilter
      62 (2009-09-20 08:12:10) Starting  
org.apache.sling.servlets.get.DefaultGetServlet#0
      62 (2009-09-20 08:12:10) Using  
org.apache.sling.servlets.get.impl.helpers.StreamRendererServlet to  
render for extension=null
     169 (2009-09-20 08:12:10)  
org.apache.sling.servlets.get.DefaultGetServlet#0, elapsed = 107ms
     169 (2009-09-20 08:12:10) Request Processing ends, elapsed = 169ms



On 19 Sep 2009, at 17:49, Ian Boston wrote:

> I think this might break some behavior, as there is no way for the  
> non existent resource to know if an extension is a serialization  
> extension or the real extension of the file in the jcr, at the  
> moment, code that uses non existent resource assumes that the  
> extension is not determined, and so would need changing to be  
> compatible with this change.
>
> Ian
>
> On 18 Sep 2009, at 22:05, Carsten Ziegeler wrote:
>
>> I'm currently looking through open issues for our API and I stumbled
>> upon https://issues.apache.org/jira/browse/SLING-864.
>>
>> What is the general opinion? Should we include it? Is is required?
>>
>> I would like to either apply or close it to reduce the issue count :)
>>
>> Regards
>> Carsten
>> -- 
>> Carsten Ziegeler
>> cziegeler@apache.org
>


Re: Applying SLING-864

Posted by Carsten Ziegeler <cz...@apache.org>.
Ian Boston wrote:
> I think this might break some behavior, as there is no way for the non
> existent resource to know if an extension is a serialization extension
> or the real extension of the file in the jcr, at the moment, code that
> uses non existent resource assumes that the extension is not determined,
> and so would need changing to be compatible with this change.
> 
I haven't looked at the details, but if you're right that this will
break existing behaviour than we should not apply it.

Carsten

> Ian
> 
> On 18 Sep 2009, at 22:05, Carsten Ziegeler wrote:
> 
>> I'm currently looking through open issues for our API and I stumbled
>> upon https://issues.apache.org/jira/browse/SLING-864.
>>
>> What is the general opinion? Should we include it? Is is required?
>>
>> I would like to either apply or close it to reduce the issue count :)
>>
>> Regards
>> Carsten
>> -- 
>> Carsten Ziegeler
>> cziegeler@apache.org
> 
> 


-- 
Carsten Ziegeler
cziegeler@apache.org

Re: Applying SLING-864

Posted by Alexander Klimetschek <ak...@day.com>.
On Sat, Sep 19, 2009 at 09:49, Ian Boston <ie...@tfd.co.uk> wrote:
> I think this might break some behavior, as there is no way for the non
> existent resource to know if an extension is a serialization extension or
> the real extension of the file in the jcr, at the moment, code that uses non
> existent resource assumes that the extension is not determined, and so would
> need changing to be compatible with this change.

But I would say that code that depends on all the request path infos
(selectors, extensions, suffix) to be "null" should be very rare and I
think this is an acceptable change, especially if this code misses the
quite important check if the resource is actually existing or not. The
(sling) resource is clearly a NonExistingResource (easy to check) and
you still have the full request path to do your own path mangling.

This also means that the servlet resolution process is not affected in
general, as the resource type will always be of sling/nonexisting,
hence only here a different script, eg. for the swf extension, might
be called - which is unlikely to be present in existing code as it
would never be called with the current way. Scripts/servlets for this
special resource type might be changed, but this is what this patch is
all about - make them simpler and more capable.

Regards,
Alex

-- 
Alexander Klimetschek
alexander.klimetschek@day.com

Re: Applying SLING-864

Posted by Ian Boston <ie...@tfd.co.uk>.
I think this might break some behavior, as there is no way for the non  
existent resource to know if an extension is a serialization extension  
or the real extension of the file in the jcr, at the moment, code that  
uses non existent resource assumes that the extension is not  
determined, and so would need changing to be compatible with this  
change.

Ian

On 18 Sep 2009, at 22:05, Carsten Ziegeler wrote:

> I'm currently looking through open issues for our API and I stumbled
> upon https://issues.apache.org/jira/browse/SLING-864.
>
> What is the general opinion? Should we include it? Is is required?
>
> I would like to either apply or close it to reduce the issue count :)
>
> Regards
> Carsten
> -- 
> Carsten Ziegeler
> cziegeler@apache.org


Re: Applying SLING-864

Posted by Vidar Ramdal <vi...@idium.no>.
On Fri, Sep 18, 2009 at 2:05 PM, Carsten Ziegeler <cz...@apache.org> wrote:
> I'm currently looking through open issues for our API and I stumbled
> upon https://issues.apache.org/jira/browse/SLING-864.
>
> What is the general opinion? Should we include it? Is is required?
>
> I would like to either apply or close it to reduce the issue count :)

+1 for applying

-- 
Vidar S. Ramdal <vi...@idium.no> - http://www.idium.no
Sommerrogata 13-15, N-0255 Oslo, Norway
+ 47 22 00 84 00 / +47 21 531941, ext 2070