You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ra...@apache.org on 2016/11/24 11:20:31 UTC

svn commit: r1771115 - in /sling/trunk/bundles/scripting: api/ api/src/main/java/org/apache/sling/scripting/api/ api/src/main/java/org/apache/sling/scripting/api/resolver/ core/src/main/java/org/apache/sling/scripting/core/impl/ core/src/test/java/org/...

Author: radu
Date: Thu Nov 24 11:20:31 2016
New Revision: 1771115

URL: http://svn.apache.org/viewvc?rev=1771115&view=rev
Log:
SLING-6165 - Expose a service for Sling Scripting that provides request-scoped Resource Resolvers for scripting dependencies

* moved the API in its own sub-package so that changes to the org.apache.sling.api.resource API don't force
artificial changes for org.apache.sling.scripting.api
* emphasised the fact that request based resolvers should be used only in a Servlet Request API context
* switched the implementation to listen for ServletRequestEvents instead of SlingRequestEvents
* removed useless synchronised block
* adapted unit tests

Added:
    sling/trunk/bundles/scripting/api/src/main/java/org/apache/sling/scripting/api/resolver/
    sling/trunk/bundles/scripting/api/src/main/java/org/apache/sling/scripting/api/resolver/ScriptingResourceResolverFactory.java
      - copied, changed from r1771114, sling/trunk/bundles/scripting/api/src/main/java/org/apache/sling/scripting/api/ScriptingResourceResolverFactory.java
    sling/trunk/bundles/scripting/api/src/main/java/org/apache/sling/scripting/api/resolver/package-info.java
      - copied, changed from r1771114, sling/trunk/bundles/scripting/api/src/main/java/org/apache/sling/scripting/api/package-info.java
Removed:
    sling/trunk/bundles/scripting/api/src/main/java/org/apache/sling/scripting/api/ScriptingResourceResolverFactory.java
Modified:
    sling/trunk/bundles/scripting/api/pom.xml
    sling/trunk/bundles/scripting/api/src/main/java/org/apache/sling/scripting/api/package-info.java
    sling/trunk/bundles/scripting/core/src/main/java/org/apache/sling/scripting/core/impl/ScriptingResourceResolver.java
    sling/trunk/bundles/scripting/core/src/main/java/org/apache/sling/scripting/core/impl/ScriptingResourceResolverFactoryImpl.java
    sling/trunk/bundles/scripting/core/src/test/java/org/apache/sling/scripting/core/impl/ScriptingResourceResolverFactoryImplTest.java

Modified: sling/trunk/bundles/scripting/api/pom.xml
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/scripting/api/pom.xml?rev=1771115&r1=1771114&r2=1771115&view=diff
==============================================================================
--- sling/trunk/bundles/scripting/api/pom.xml (original)
+++ sling/trunk/bundles/scripting/api/pom.xml Thu Nov 24 11:20:31 2016
@@ -57,7 +57,7 @@
         <dependency>
             <groupId>org.apache.sling</groupId>
             <artifactId>org.apache.sling.api</artifactId>
-            <version>2.9.0</version>
+            <version>2.11.0</version>
             <scope>provided</scope>
         </dependency>
     </dependencies>

Modified: sling/trunk/bundles/scripting/api/src/main/java/org/apache/sling/scripting/api/package-info.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/scripting/api/src/main/java/org/apache/sling/scripting/api/package-info.java?rev=1771115&r1=1771114&r2=1771115&view=diff
==============================================================================
--- sling/trunk/bundles/scripting/api/src/main/java/org/apache/sling/scripting/api/package-info.java (original)
+++ sling/trunk/bundles/scripting/api/src/main/java/org/apache/sling/scripting/api/package-info.java Thu Nov 24 11:20:31 2016
@@ -14,7 +14,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  ******************************************************************************/
-@Version("2.4.0")
+@Version("2.3.0")
 package org.apache.sling.scripting.api;
 
 import org.osgi.annotation.versioning.Version;

Copied: sling/trunk/bundles/scripting/api/src/main/java/org/apache/sling/scripting/api/resolver/ScriptingResourceResolverFactory.java (from r1771114, sling/trunk/bundles/scripting/api/src/main/java/org/apache/sling/scripting/api/ScriptingResourceResolverFactory.java)
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/scripting/api/src/main/java/org/apache/sling/scripting/api/resolver/ScriptingResourceResolverFactory.java?p2=sling/trunk/bundles/scripting/api/src/main/java/org/apache/sling/scripting/api/resolver/ScriptingResourceResolverFactory.java&p1=sling/trunk/bundles/scripting/api/src/main/java/org/apache/sling/scripting/api/ScriptingResourceResolverFactory.java&r1=1771114&r2=1771115&rev=1771115&view=diff
==============================================================================
--- sling/trunk/bundles/scripting/api/src/main/java/org/apache/sling/scripting/api/ScriptingResourceResolverFactory.java (original)
+++ sling/trunk/bundles/scripting/api/src/main/java/org/apache/sling/scripting/api/resolver/ScriptingResourceResolverFactory.java Thu Nov 24 11:20:31 2016
@@ -14,7 +14,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  ******************************************************************************/
-package org.apache.sling.scripting.api;
+package org.apache.sling.scripting.api.resolver;
 
 import org.apache.sling.api.resource.Resource;
 import org.apache.sling.api.resource.ResourceResolver;
@@ -37,6 +37,9 @@ public interface ScriptingResourceResolv
      * ResourceResolver} should not be closed by consumers (calling {@link ResourceResolver#close} doesn't do anything), since this
      * service will handle the closing operation automatically. The {@code ResourceResolver} will be shared between scripting
      * dependencies that render parts of the response for the same request.</p>
+     *
+     * <p><b>NOTE:</b> Usage of this {@link ResourceResolver} outside of a Servlet Request API context might lead to improper cleaning
+     * (e.g. reusing the same resolver for multiple threads).</p>
      */
     ResourceResolver getRequestScopedResourceResolver();
 

Copied: sling/trunk/bundles/scripting/api/src/main/java/org/apache/sling/scripting/api/resolver/package-info.java (from r1771114, sling/trunk/bundles/scripting/api/src/main/java/org/apache/sling/scripting/api/package-info.java)
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/scripting/api/src/main/java/org/apache/sling/scripting/api/resolver/package-info.java?p2=sling/trunk/bundles/scripting/api/src/main/java/org/apache/sling/scripting/api/resolver/package-info.java&p1=sling/trunk/bundles/scripting/api/src/main/java/org/apache/sling/scripting/api/package-info.java&r1=1771114&r2=1771115&rev=1771115&view=diff
==============================================================================
--- sling/trunk/bundles/scripting/api/src/main/java/org/apache/sling/scripting/api/package-info.java (original)
+++ sling/trunk/bundles/scripting/api/src/main/java/org/apache/sling/scripting/api/resolver/package-info.java Thu Nov 24 11:20:31 2016
@@ -14,7 +14,18 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  ******************************************************************************/
-@Version("2.4.0")
-package org.apache.sling.scripting.api;
+/**
+ * <p>The {@code org.apache.sling.scripting.api.resolver} package provides a unified API for scripting bundles that need to perform script
+ * resolution across the {@link org.apache.sling.api.resource.Resource} space.</p>
+ *
+ * <p>Some API methods might indicate that they are <i>request-bound</i>. In this case it should be noted that <i>usage outside of the
+ * context of a Servlet API Request might lead to improper cleaning of objects whose life-cycle should not be longer than the request to
+ * which they're bound to (for example per-thread objects)</i>.</p>
+ *
+ * <p>This package depends on the {@link org.apache.sling.api.resource} API, version 2.9.0 (bundle {@code org.apache.sling.api}, version
+ * 2.11.0).</p>
+ */
+@Version("1.0.0")
+package org.apache.sling.scripting.api.resolver;
 
 import org.osgi.annotation.versioning.Version;

Modified: sling/trunk/bundles/scripting/core/src/main/java/org/apache/sling/scripting/core/impl/ScriptingResourceResolver.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/scripting/core/src/main/java/org/apache/sling/scripting/core/impl/ScriptingResourceResolver.java?rev=1771115&r1=1771114&r2=1771115&view=diff
==============================================================================
--- sling/trunk/bundles/scripting/core/src/main/java/org/apache/sling/scripting/core/impl/ScriptingResourceResolver.java (original)
+++ sling/trunk/bundles/scripting/core/src/main/java/org/apache/sling/scripting/core/impl/ScriptingResourceResolver.java Thu Nov 24 11:20:31 2016
@@ -27,7 +27,7 @@ import org.apache.sling.api.resource.Log
 import org.apache.sling.api.resource.PersistenceException;
 import org.apache.sling.api.resource.Resource;
 import org.apache.sling.api.resource.ResourceResolver;
-import org.apache.sling.scripting.api.ScriptingResourceResolverFactory;
+import org.apache.sling.scripting.api.resolver.ScriptingResourceResolverFactory;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 

Modified: sling/trunk/bundles/scripting/core/src/main/java/org/apache/sling/scripting/core/impl/ScriptingResourceResolverFactoryImpl.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/scripting/core/src/main/java/org/apache/sling/scripting/core/impl/ScriptingResourceResolverFactoryImpl.java?rev=1771115&r1=1771114&r2=1771115&view=diff
==============================================================================
--- sling/trunk/bundles/scripting/core/src/main/java/org/apache/sling/scripting/core/impl/ScriptingResourceResolverFactoryImpl.java (original)
+++ sling/trunk/bundles/scripting/core/src/main/java/org/apache/sling/scripting/core/impl/ScriptingResourceResolverFactoryImpl.java Thu Nov 24 11:20:31 2016
@@ -16,12 +16,13 @@
  ******************************************************************************/
 package org.apache.sling.scripting.core.impl;
 
-import org.apache.sling.api.request.SlingRequestEvent;
-import org.apache.sling.api.request.SlingRequestListener;
+import javax.servlet.ServletRequestEvent;
+import javax.servlet.ServletRequestListener;
+
 import org.apache.sling.api.resource.LoginException;
 import org.apache.sling.api.resource.ResourceResolver;
 import org.apache.sling.api.resource.ResourceResolverFactory;
-import org.apache.sling.scripting.api.ScriptingResourceResolverFactory;
+import org.apache.sling.scripting.api.resolver.ScriptingResourceResolverFactory;
 import org.osgi.service.component.annotations.Activate;
 import org.osgi.service.component.annotations.Component;
 import org.osgi.service.component.annotations.Reference;
@@ -33,17 +34,17 @@ import org.slf4j.LoggerFactory;
 
 @Component(
         name = "Apache Sling Scripting Resource Resolver Factory",
-        service = {ScriptingResourceResolverFactory.class, SlingRequestListener.class},
+        service = {ScriptingResourceResolverFactory.class, ServletRequestListener.class},
         configurationPid = "org.apache.sling.scripting.core.impl.ScriptingResourceResolverFactoryImpl"
 )
 @Designate(
         ocd = ScriptingResourceResolverFactoryImpl.Configuration.class
 )
-public class ScriptingResourceResolverFactoryImpl implements ScriptingResourceResolverFactory, SlingRequestListener {
+public class ScriptingResourceResolverFactoryImpl implements ScriptingResourceResolverFactory, ServletRequestListener {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(ScriptingResourceResolverFactoryImpl.class);
 
-    private static final ThreadLocal<ScriptingResourceResolver> perThreadResourceResolver = new ThreadLocal<>();
+    private final ThreadLocal<ScriptingResourceResolver> perThreadResourceResolver = new ThreadLocal<>();
     private boolean logStackTraceOnResolverClose;
 
     @Reference
@@ -63,25 +64,21 @@ public class ScriptingResourceResolverFa
         )
         boolean log_stacktrace_onclose() default false;
 
-
     }
 
     @Override
     public ResourceResolver getRequestScopedResourceResolver() {
         ScriptingResourceResolver threadResolver = perThreadResourceResolver.get();
         if (threadResolver == null) {
-            // no per thread; need to synchronize access
-            synchronized (perThreadResourceResolver) {
-                try {
-                    ResourceResolver delegate = rrf.getServiceResourceResolver(null);
-                    threadResolver = new ScriptingResourceResolver(logStackTraceOnResolverClose, delegate);
-                    perThreadResourceResolver.set(threadResolver);
-                    if (LOGGER.isDebugEnabled()) {
-                        LOGGER.debug("Setting per thread resource resolver for thread {}.", Thread.currentThread().getName());
-                    }
-                } catch (LoginException e) {
-                    throw new IllegalStateException("Cannot create per thread resource resolver.", e);
+            try {
+                ResourceResolver delegate = rrf.getServiceResourceResolver(null);
+                threadResolver = new ScriptingResourceResolver(logStackTraceOnResolverClose, delegate);
+                perThreadResourceResolver.set(threadResolver);
+                if (LOGGER.isDebugEnabled()) {
+                    LOGGER.debug("Setting per thread resource resolver for thread {}.", Thread.currentThread().getName());
                 }
+            } catch (LoginException e) {
+                throw new IllegalStateException("Cannot create per thread resource resolver.", e);
             }
         }
         return threadResolver;
@@ -97,16 +94,19 @@ public class ScriptingResourceResolverFa
     }
 
     @Override
-    public void onEvent(SlingRequestEvent sre) {
-        if (sre.getType().equals(SlingRequestEvent.EventType.EVENT_DESTROY)) {
-            ScriptingResourceResolver scriptingResourceResolver = perThreadResourceResolver.get();
-            if (scriptingResourceResolver != null) {
-                scriptingResourceResolver._close();
-                perThreadResourceResolver.remove();
-            }
-            if (LOGGER.isDebugEnabled()) {
-                LOGGER.debug("Removing per thread resource resolver for thread {}.", Thread.currentThread().getName());
-            }
+    public void requestInitialized(ServletRequestEvent sre) {
+        // we don't care about this event; the request scoped resource resolver is created lazily
+    }
+
+    @Override
+    public void requestDestroyed(ServletRequestEvent sre) {
+        ScriptingResourceResolver scriptingResourceResolver = perThreadResourceResolver.get();
+        if (scriptingResourceResolver != null) {
+            scriptingResourceResolver._close();
+            perThreadResourceResolver.remove();
+        }
+        if (LOGGER.isDebugEnabled()) {
+            LOGGER.debug("Removing per thread resource resolver for thread {}.", Thread.currentThread().getName());
         }
     }
 

Modified: sling/trunk/bundles/scripting/core/src/test/java/org/apache/sling/scripting/core/impl/ScriptingResourceResolverFactoryImplTest.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/scripting/core/src/test/java/org/apache/sling/scripting/core/impl/ScriptingResourceResolverFactoryImplTest.java?rev=1771115&r1=1771114&r2=1771115&view=diff
==============================================================================
--- sling/trunk/bundles/scripting/core/src/test/java/org/apache/sling/scripting/core/impl/ScriptingResourceResolverFactoryImplTest.java (original)
+++ sling/trunk/bundles/scripting/core/src/test/java/org/apache/sling/scripting/core/impl/ScriptingResourceResolverFactoryImplTest.java Thu Nov 24 11:20:31 2016
@@ -18,6 +18,7 @@ package org.apache.sling.scripting.core.
 
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -25,10 +26,8 @@ import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
-import javax.servlet.ServletContext;
-import javax.servlet.ServletRequest;
+import javax.servlet.ServletRequestEvent;
 
-import org.apache.sling.api.request.SlingRequestEvent;
 import org.apache.sling.api.resource.LoginException;
 import org.apache.sling.api.resource.ResourceResolver;
 import org.apache.sling.api.resource.ResourceResolverFactory;
@@ -51,7 +50,7 @@ public class ScriptingResourceResolverFa
 
     @Before
     public void setUp() throws LoginException {
-        delegates = new HashSet<>();
+        delegates = Collections.synchronizedSet(new HashSet<ResourceResolver>());
         ResourceResolverFactory rrf = mock(ResourceResolverFactory.class);
         when(rrf.getServiceResourceResolver(null)).thenAnswer(new Answer<ResourceResolver>() {
             @Override
@@ -75,9 +74,7 @@ public class ScriptingResourceResolverFa
     public void testGetRequestScopedResourceResolver() throws Exception {
         ResourceResolver resourceResolver = scriptingResourceResolverFactory.getRequestScopedResourceResolver();
         assertEquals("sling-scripting", resourceResolver.getUserID());
-        SlingRequestEvent sre2 = new SlingRequestEvent(mock(ServletContext.class), mock(ServletRequest.class), SlingRequestEvent.EventType
-                .EVENT_DESTROY);
-        scriptingResourceResolverFactory.onEvent(sre2);
+        scriptingResourceResolverFactory.requestDestroyed(mock(ServletRequestEvent.class));
         assertEquals(1, delegates.size());
         for (ResourceResolver delegate : delegates) {
             verify(delegate).close();
@@ -114,8 +111,7 @@ public class ScriptingResourceResolverFa
                     assertEquals("Expected that subsequent calls to ScriptingResourceResolverFactory#getRequestScopedResourceResolver() " +
                             "from the same thread will not create additional resolvers.", resourceResolver, subsequentResolver);
                 }
-                scriptingResourceResolverFactory.onEvent(new SlingRequestEvent(mock(ServletContext.class), mock(ServletRequest.class),
-                            SlingRequestEvent.EventType.EVENT_DESTROY));
+                scriptingResourceResolverFactory.requestDestroyed(mock(ServletRequestEvent.class));
                 return resourceResolver;
             }
         };