Re: svn commit: r1771782 - /sling/trunk/bundles/api/src/main/java/org/apache/sling/api/resource/ResourceResolverWrapper.java

[Radu Cotescu]

Hi Julian,

You're right, we should also wrap the returned resources. I've just replied
on JIRA a couple of minutes ago.

Thanks,
Radu

On Mon, 28 Nov 2016 at 19:15 Julian Sedding  wrote:

> Hi Radu
>
> I would argue that this implementation is incorrect.
>
> Consider that with this implementation resolver !=
> resolver.getResource("/foo").getResourceResolver(), which should be
> the case IMHO.
>
> I commented on the issue with two tickets, where the same feature was
> previously discussed. The tickets also have patches attached, which
> implement proper (or deep) wrapping of the ResourceResolver.
>
> Regards
> Julian
>
>
>
> On Mon, Nov 28, 2016 at 7:11 PM,   wrote:
> > Author: radu
> > Date: Mon Nov 28 18:11:34 2016
> > New Revision: 1771782
> >
> > URL: http://svn.apache.org/viewvc?rev=1771782=rev
> > Log:
> > SLING-6336 - Implement a ResourceResolverWrapper
> >
> > Added:
> >
>  
> sling/trunk/bundles/api/src/main/java/org/apache/sling/api/resource/ResourceResolverWrapper.java
> >
> > Added:
> sling/trunk/bundles/api/src/main/java/org/apache/sling/api/resource/ResourceResolverWrapper.java
> > URL:
> http://svn.apache.org/viewvc/sling/trunk/bundles/api/src/main/java/org/apache/sling/api/resource/ResourceResolverWrapper.java?rev=1771782=auto
> >
> ==
> > ---
> sling/trunk/bundles/api/src/main/java/org/apache/sling/api/resource/ResourceResolverWrapper.java
> (added)
> > +++
> sling/trunk/bundles/api/src/main/java/org/apache/sling/api/resource/ResourceResolverWrapper.java
> Mon Nov 28 18:11:34 2016
> > @@ -0,0 +1,210 @@
> >
> +/***
> > + * Licensed to the Apache Software Foundation (ASF) under one or more
> > + * contributor license agreements.  See the NOTICE file distributed with
> > + * this work for additional information regarding copyright ownership.
> > + * The ASF licenses this file to You under the Apache License, Version
> 2.0
> > + * (the "License"); you may not use this file except in compliance with
> > + * the License.  You may obtain a copy of the License at
> > + *
> > + * http://www.apache.org/licenses/LICENSE-2.0
> > + *
> > + * Unless required by applicable law or agreed to in writing, software
> > + * distributed under the License is distributed on an "AS IS" BASIS,
> > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> implied.
> > + * See the License for the specific language governing permissions and
> > + * limitations under the License.
> > +
> **/
> > +package org.apache.sling.api.resource;
> > +
> > +import java.util.Iterator;
> > +import java.util.Map;
> > +import javax.annotation.Nonnull;
> > +import javax.servlet.http.HttpServletRequest;
> > +
> > +import org.osgi.annotation.versioning.ConsumerType;
> > +
> > +/**
> > + * The {@code ResourceResolverWrapper} is a wrapper for any {@code
> ResourceResolver}, delegating all method calls to the wrapped resource
> > + * resolver by default. Extensions of this class may overwrite any
> method to return different values as appropriate.
> > + */
> > +@ConsumerType
> > +public class ResourceResolverWrapper implements ResourceResolver {
> > +
> > +private ResourceResolver wrapped;
> > +
> > +public ResourceResolverWrapper(ResourceResolver resolver) {
> > +wrapped = resolver;
> > +}
> > +
> > +@Nonnull
> > +@Override
> > +public Resource resolve(@Nonnull HttpServletRequest request,
> @Nonnull String absPath) {
> > +return wrapped.resolve(request, absPath);
> > +}
> > +
> > +@Nonnull
> > +@Override
> > +public Resource resolve(@Nonnull String absPath) {
> > +return wrapped.resolve(absPath);
> > +}
> > +
> > +@Nonnull
> > +@Override
> > +public Resource resolve(@Nonnull HttpServletRequest request) {
> > +return wrapped.resolve(request);
> > +}
> > +
> > +@Nonnull
> > +@Override
> > +public String map(@Nonnull String resourcePath) {
> > +return wrapped.map(resourcePath);
> > +}
> > +
> > +@Override
> > +public String map(@Nonnull HttpServletRequest request, @Nonnull
> String resourcePath) {
> > +return wrapped.map(request, resourcePath);
> > +}
> > +
> > +@Override
> > +public Resource getResource(@Nonnull String path) {
> > +return wrapped.getResource(path);
> > +}
> > +
> > +@Override
> > +public Resource getResource(Resource base, @Nonnull String path) {
> > +return wrapped.getResource(base, path);
> > +}
> > +
> > +@Nonnull
> > +@Override
> > +public String[] getSearchPath() {
> > +return wrapped.getSearchPath();
> > +}
> > +
> > +@Nonnull
> > +@Override
> > +public Iterator listChildren(@Nonnull Resource parent) {
> > +return 

Re: svn commit: r1771782 - /sling/trunk/bundles/api/src/main/java/org/apache/sling/api/resource/ResourceResolverWrapper.java

[Julian Sedding]

Hi Radu

I would argue that this implementation is incorrect.

Consider that with this implementation resolver !=
resolver.getResource("/foo").getResourceResolver(), which should be
the case IMHO.

I commented on the issue with two tickets, where the same feature was
previously discussed. The tickets also have patches attached, which
implement proper (or deep) wrapping of the ResourceResolver.

Regards
Julian



On Mon, Nov 28, 2016 at 7:11 PM,   wrote:
> Author: radu
> Date: Mon Nov 28 18:11:34 2016
> New Revision: 1771782
>
> URL: http://svn.apache.org/viewvc?rev=1771782=rev
> Log:
> SLING-6336 - Implement a ResourceResolverWrapper
>
> Added:
> 
> sling/trunk/bundles/api/src/main/java/org/apache/sling/api/resource/ResourceResolverWrapper.java
>
> Added: 
> sling/trunk/bundles/api/src/main/java/org/apache/sling/api/resource/ResourceResolverWrapper.java
> URL: 
> http://svn.apache.org/viewvc/sling/trunk/bundles/api/src/main/java/org/apache/sling/api/resource/ResourceResolverWrapper.java?rev=1771782=auto
> ==
> --- 
> sling/trunk/bundles/api/src/main/java/org/apache/sling/api/resource/ResourceResolverWrapper.java
>  (added)
> +++ 
> sling/trunk/bundles/api/src/main/java/org/apache/sling/api/resource/ResourceResolverWrapper.java
>  Mon Nov 28 18:11:34 2016
> @@ -0,0 +1,210 @@
> +/***
> + * Licensed to the Apache Software Foundation (ASF) under one or more
> + * contributor license agreements.  See the NOTICE file distributed with
> + * this work for additional information regarding copyright ownership.
> + * The ASF licenses this file to You under the Apache License, Version 2.0
> + * (the "License"); you may not use this file except in compliance with
> + * the License.  You may obtain a copy of the License at
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + 
> **/
> +package org.apache.sling.api.resource;
> +
> +import java.util.Iterator;
> +import java.util.Map;
> +import javax.annotation.Nonnull;
> +import javax.servlet.http.HttpServletRequest;
> +
> +import org.osgi.annotation.versioning.ConsumerType;
> +
> +/**
> + * The {@code ResourceResolverWrapper} is a wrapper for any {@code 
> ResourceResolver}, delegating all method calls to the wrapped resource
> + * resolver by default. Extensions of this class may overwrite any method to 
> return different values as appropriate.
> + */
> +@ConsumerType
> +public class ResourceResolverWrapper implements ResourceResolver {
> +
> +private ResourceResolver wrapped;
> +
> +public ResourceResolverWrapper(ResourceResolver resolver) {
> +wrapped = resolver;
> +}
> +
> +@Nonnull
> +@Override
> +public Resource resolve(@Nonnull HttpServletRequest request, @Nonnull 
> String absPath) {
> +return wrapped.resolve(request, absPath);
> +}
> +
> +@Nonnull
> +@Override
> +public Resource resolve(@Nonnull String absPath) {
> +return wrapped.resolve(absPath);
> +}
> +
> +@Nonnull
> +@Override
> +public Resource resolve(@Nonnull HttpServletRequest request) {
> +return wrapped.resolve(request);
> +}
> +
> +@Nonnull
> +@Override
> +public String map(@Nonnull String resourcePath) {
> +return wrapped.map(resourcePath);
> +}
> +
> +@Override
> +public String map(@Nonnull HttpServletRequest request, @Nonnull String 
> resourcePath) {
> +return wrapped.map(request, resourcePath);
> +}
> +
> +@Override
> +public Resource getResource(@Nonnull String path) {
> +return wrapped.getResource(path);
> +}
> +
> +@Override
> +public Resource getResource(Resource base, @Nonnull String path) {
> +return wrapped.getResource(base, path);
> +}
> +
> +@Nonnull
> +@Override
> +public String[] getSearchPath() {
> +return wrapped.getSearchPath();
> +}
> +
> +@Nonnull
> +@Override
> +public Iterator listChildren(@Nonnull Resource parent) {
> +return wrapped.listChildren(parent);
> +}
> +
> +@Override
> +public Resource getParent(@Nonnull Resource child) {
> +return wrapped.getParent(child);
> +}
> +
> +@Nonnull
> +@Override
> +public Iterable getChildren(@Nonnull Resource parent) {
> +return wrapped.getChildren(parent);
> +}
> +
> +@Nonnull
> +@Override
> +public Iterator findResources(@Nonnull String query, String 
> language) {
> +