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 <[email protected]> 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
> 

Reply via email to