SLING-7536 : Refactor implementation
diff --git a/src/main/java/org/apache/sling/servlets/resolver/internal/SlingServletResolver.java b/src/main/java/org/apache/sling/servlets/resolver/internal/SlingServletResolver.java
index e78ce68..842d144 100644
--- a/src/main/java/org/apache/sling/servlets/resolver/internal/SlingServletResolver.java
+++ b/src/main/java/org/apache/sling/servlets/resolver/internal/SlingServletResolver.java
@@ -41,12 +41,7 @@
import java.util.Iterator;
import java.util.List;
import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
-import javax.management.NotCompliantMBeanException;
-import javax.management.StandardMBean;
-import javax.script.ScriptEngineFactory;
-import javax.script.ScriptEngineManager;
import javax.servlet.Servlet;
import javax.servlet.ServletContext;
import javax.servlet.ServletException;
@@ -70,9 +65,6 @@
import org.apache.sling.api.resource.ResourceResolverFactory;
import org.apache.sling.api.resource.ResourceUtil;
import org.apache.sling.api.resource.SyntheticResource;
-import org.apache.sling.api.resource.observation.ExternalResourceChangeListener;
-import org.apache.sling.api.resource.observation.ResourceChange;
-import org.apache.sling.api.resource.observation.ResourceChangeListener;
import org.apache.sling.api.scripting.SlingScript;
import org.apache.sling.api.servlets.OptingServlet;
import org.apache.sling.api.servlets.ServletResolver;
@@ -85,9 +77,9 @@
import org.apache.sling.servlets.resolver.internal.helper.NamedScriptResourceCollector;
import org.apache.sling.servlets.resolver.internal.helper.ResourceCollector;
import org.apache.sling.servlets.resolver.internal.helper.SlingServletConfig;
+import org.apache.sling.servlets.resolver.internal.resolution.ResolutionCache;
import org.apache.sling.servlets.resolver.internal.resource.ServletResourceProvider;
import org.apache.sling.servlets.resolver.internal.resource.ServletResourceProviderFactory;
-import org.apache.sling.servlets.resolver.jmx.SlingServletResolverCacheMBean;
import org.apache.sling.spi.resource.provider.ResourceProvider;
import org.osgi.framework.Bundle;
import org.osgi.framework.BundleContext;
@@ -100,8 +92,6 @@
import org.osgi.service.component.annotations.Reference;
import org.osgi.service.component.annotations.ReferenceCardinality;
import org.osgi.service.component.annotations.ReferencePolicy;
-import org.osgi.service.event.Event;
-import org.osgi.service.event.EventHandler;
import org.osgi.service.metatype.annotations.AttributeDefinition;
import org.osgi.service.metatype.annotations.Designate;
import org.osgi.service.metatype.annotations.ObjectClassDefinition;
@@ -116,7 +106,7 @@
*
*/
@Component(name = SlingServletResolver.Config.PID,
- service = { ServletResolver.class,ErrorHandler.class, SlingRequestListener.class },
+ service = { ServletResolver.class, ErrorHandler.class, SlingRequestListener.class },
property = {
Constants.SERVICE_DESCRIPTION + "=Apache Sling Servlet Resolver and Error Handler",
Constants.SERVICE_VENDOR + "=The Apache Software Foundation"
@@ -125,9 +115,7 @@
public class SlingServletResolver
implements ServletResolver,
SlingRequestListener,
- ErrorHandler,
- EventHandler,
- ResourceChangeListener,ExternalResourceChangeListener {
+ ErrorHandler {
@ObjectClassDefinition(name = "Apache Sling Servlet/Script Resolver and Error Handler",
description= "The Sling Servlet and Script Resolver has "+
@@ -190,7 +178,7 @@
private ServiceUserMapped consoleServiceUserMapped;
@Reference
- private ScriptEngineManager scriptEngineManager;
+ private ResolutionCache resolutionCache;
private List<String> scriptEnginesExtensions = new ArrayList<>();
@@ -213,18 +201,6 @@
// a request. This field is set on demand by getDefaultErrorServlet()
private Servlet fallbackErrorServlet;
- /** The script resolution cache. */
- private volatile Map<AbstractResourceCollector, Servlet> cache;
-
- /** The cache size. */
- private int cacheSize;
-
- /** Flag to log warning if cache size exceed only once. */
- private volatile boolean logCacheSizeWarning;
-
- /** Registration as event handler. */
- private ServiceRegistration<?> eventHandlerReg;
-
/**
* The allowed execution paths.
*/
@@ -500,8 +476,6 @@
private final ThreadLocal<ResourceResolver> perThreadScriptResolver = new ThreadLocal<>();
- private ServiceRegistration<SlingServletResolverCacheMBean> mbeanRegistration;
-
/**
* @see org.apache.sling.api.request.SlingRequestListener#onEvent(org.apache.sling.api.request.SlingRequestEvent)
*/
@@ -608,8 +582,8 @@
final SlingHttpServletRequest request,
final ResourceResolver resolver) {
// use local variable to avoid race condition with activate
- final Map<AbstractResourceCollector, Servlet> localCache = this.cache;
- final Servlet scriptServlet = (localCache != null ? localCache.get(locationUtil) : null);
+ final ResolutionCache localCache = this.resolutionCache;
+ final Servlet scriptServlet = localCache.get(locationUtil);
if (scriptServlet != null) {
if ( LOGGER.isDebugEnabled() ) {
LOGGER.debug("Using cached servlet {}", RequestUtil.getServletName(scriptServlet));
@@ -639,14 +613,8 @@
final boolean isOptingServlet = candidate instanceof OptingServlet;
boolean servletAcceptsRequest = !isOptingServlet || (request != null && ((OptingServlet) candidate).accepts(request));
if (servletAcceptsRequest) {
- if (!hasOptingServlet && !isOptingServlet && localCache != null) {
- if ( localCache.size() < this.cacheSize ) {
- localCache.put(locationUtil, candidate);
- } else if ( this.logCacheSizeWarning ) {
- this.logCacheSizeWarning = false;
- LOGGER.warn("Script cache has reached its limit of {}. You might want to increase the cache size for the servlet resolver.",
- this.cacheSize);
- }
+ if (!hasOptingServlet && !isOptingServlet ) {
+ localCache.put(locationUtil, candidate);
}
LOGGER.debug("Using servlet provided by candidate resource {}", candidateResource.getPath());
return candidate;
@@ -785,57 +753,10 @@
this.executionPaths = AbstractResourceCollector.getExecutionPaths(config.servletresolver_paths());
this.defaultExtensions = config.servletresolver_defaultExtensions();
- // create cache - if a cache size is configured
- this.cacheSize = config.servletresolver_cacheSize();
- if (this.cacheSize > 5) {
- this.cache = new ConcurrentHashMap<>(cacheSize);
- this.logCacheSizeWarning = true;
- } else {
- this.cacheSize = 0;
- }
-
// setup default servlet
this.getDefaultServlet();
- // and finally register as event listener if we need to flush the cache
- if ( this.cache != null ) {
-
- final Dictionary<String, Object> props = new Hashtable<>();
- props.put("event.topics", new String[] {"javax/script/ScriptEngineFactory/*",
- "org/apache/sling/api/adapter/AdapterFactory/*","org/apache/sling/scripting/core/BindingsValuesProvider/*" });
- props.put(ResourceChangeListener.PATHS, "/");
- props.put("service.description", "Apache Sling Servlet Resolver and Error Handler");
- props.put("service.vendor","The Apache Software Foundation");
-
- this.eventHandlerReg = context
- .registerService(new String[] {ResourceChangeListener.class.getName(), EventHandler.class.getName()}, this, props);
- }
-
this.plugin = new ServletResolverWebConsolePlugin(context);
- if ( this.cache != null ) {
- try {
- Dictionary<String, String> mbeanProps = new Hashtable<>();
- mbeanProps.put("jmx.objectname", "org.apache.sling:type=servletResolver,service=SlingServletResolverCache");
-
- ServletResolverCacheMBeanImpl mbean = new ServletResolverCacheMBeanImpl();
- mbeanRegistration = context.registerService(SlingServletResolverCacheMBean.class, mbean, mbeanProps);
- } catch (Throwable t) {
- LOGGER.debug("Unable to register mbean");
- }
- }
- updateScriptEngineExtensions();
- }
-
- private void updateScriptEngineExtensions() {
- final ScriptEngineManager localScriptEngineManager = scriptEngineManager;
- // use local variable to avoid racing with deactivate
- if ( localScriptEngineManager != null ) {
- final List<String> scriptEnginesExtensions = new ArrayList<>();
- for (ScriptEngineFactory factory : localScriptEngineManager.getEngineFactories()) {
- scriptEnginesExtensions.addAll(factory.getExtensions());
- }
- this.scriptEnginesExtensions = Collections.unmodifiableList(scriptEnginesExtensions);
- }
}
/**
@@ -850,12 +771,6 @@
this.plugin.dispose();
}
- // unregister event handler
- if (this.eventHandlerReg != null) {
- this.eventHandlerReg.unregister();
- this.eventHandlerReg = null;
- }
-
// Copy the list of servlets first, to minimize the need for
// synchronization
final Collection<ServiceReference<Servlet>> refs;
@@ -886,13 +801,7 @@
this.sharedScriptResolver = null;
}
- this.cache = null;
this.servletResourceProviderFactory = null;
-
- if (this.mbeanRegistration != null) {
- this.mbeanRegistration.unregister();
- this.mbeanRegistration = null;
- }
}
// TODO
@@ -1050,28 +959,6 @@
}
}
- /**
- * @see org.osgi.service.event.EventHandler#handleEvent(org.osgi.service.event.Event)
- */
- @Override
- public void handleEvent(final Event event) {
- // return immediately if already deactivated
- if ( this.context == null ) {
- return;
- }
- flushCache();
- updateScriptEngineExtensions();
- }
-
- private void flushCache() {
- // use local variable to avoid racing with deactivate
- final Map<AbstractResourceCollector, Servlet> localCache = this.cache;
- if ( localCache != null ) {
- localCache.clear();
- this.logCacheSizeWarning = true;
- }
- }
-
/** The list of property names checked by {@link #getName(ServiceReference)} */
private static final String[] NAME_PROPERTIES = { SLING_SERVLET_NAME,
COMPONENT_NAME, SERVICE_PID, SERVICE_ID };
@@ -1371,54 +1258,4 @@
}
}
-
- class ServletResolverCacheMBeanImpl extends StandardMBean implements SlingServletResolverCacheMBean {
-
- ServletResolverCacheMBeanImpl() throws NotCompliantMBeanException {
- super(SlingServletResolverCacheMBean.class);
- }
-
- @Override
- public int getCacheSize() {
- // use local variable to avoid racing with deactivate
- final Map<AbstractResourceCollector, Servlet> localCache = cache;
- return localCache != null ? localCache.size() : 0;
- }
-
- @Override
- public void flushCache() {
- SlingServletResolver.this.flushCache();
- }
-
- @Override
- public int getMaximumCacheSize() {
- return cacheSize;
- }
-
- }
-
- @Override
- public void onChange(final List<ResourceChange> changes) {
- // return immediately if already deactivated
- if ( context == null ) {
- return;
- }
- boolean flushCache = false;
- for(final ResourceChange change : changes){
- // if the path of the event is a sub path of a search path
- // we flush the whole cache
- final String path = change.getPath();
- int index = 0;
- while (!flushCache && index < searchPaths.length) {
- if (path.startsWith(this.searchPaths[index])) {
- flushCache = true;
- }
- index++;
- }
- if ( flushCache ) {
- flushCache();
- break; // we can stop looping
- }
- }
- }
}
diff --git a/src/main/java/org/apache/sling/servlets/resolver/internal/resolution/ResolutionCache.java b/src/main/java/org/apache/sling/servlets/resolver/internal/resolution/ResolutionCache.java
new file mode 100644
index 0000000..2cf4034
--- /dev/null
+++ b/src/main/java/org/apache/sling/servlets/resolver/internal/resolution/ResolutionCache.java
@@ -0,0 +1,287 @@
+/*
+ * 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.servlets.resolver.internal.resolution;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Dictionary;
+import java.util.Hashtable;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+import javax.management.NotCompliantMBeanException;
+import javax.management.StandardMBean;
+import javax.script.ScriptEngineFactory;
+import javax.script.ScriptEngineManager;
+import javax.servlet.Servlet;
+
+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.api.resource.observation.ExternalResourceChangeListener;
+import org.apache.sling.api.resource.observation.ResourceChange;
+import org.apache.sling.api.resource.observation.ResourceChangeListener;
+import org.apache.sling.serviceusermapping.ServiceUserMapped;
+import org.apache.sling.servlets.resolver.internal.SlingServletResolver;
+import org.apache.sling.servlets.resolver.internal.helper.AbstractResourceCollector;
+import org.apache.sling.servlets.resolver.jmx.SlingServletResolverCacheMBean;
+import org.osgi.framework.BundleContext;
+import org.osgi.framework.Constants;
+import org.osgi.framework.ServiceRegistration;
+import org.osgi.service.component.annotations.Activate;
+import org.osgi.service.component.annotations.Component;
+import org.osgi.service.component.annotations.Deactivate;
+import org.osgi.service.component.annotations.Reference;
+import org.osgi.service.event.Event;
+import org.osgi.service.event.EventConstants;
+import org.osgi.service.event.EventHandler;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Cache for script resolution
+ *
+ */
+@Component(configurationPid = SlingServletResolver.Config.PID,
+ service = {ResolutionCache.class})
+public class ResolutionCache
+ implements EventHandler, ResourceChangeListener, ExternalResourceChangeListener {
+
+ private final Logger logger = LoggerFactory.getLogger(this.getClass());
+
+ @Reference
+ private ResourceResolverFactory resourceResolverFactory;
+
+ @Reference(target="("+ServiceUserMapped.SUBSERVICENAME+"=scripts)")
+ private ServiceUserMapped scriptServiceUserMapped;
+
+ @Reference
+ private ScriptEngineManager scriptEngineManager;
+
+ private volatile List<String> scriptEnginesExtensions = Collections.emptyList();
+
+ /** The script resolution cache. */
+ private volatile Map<AbstractResourceCollector, Servlet> cache;
+
+ /** The cache size. */
+ private int cacheSize;
+
+ /** Flag to log warning if cache size exceed only once. */
+ private volatile boolean logCacheSizeWarning;
+
+ /**
+ * The search paths
+ */
+ private String[] searchPaths;
+
+ /** Registration as event handler. */
+ private volatile ServiceRegistration<EventHandler> eventHandlerRegistration;
+
+ private volatile ServiceRegistration<ResourceChangeListener> resourceListenerRegistration;
+
+ private volatile ServiceRegistration<SlingServletResolverCacheMBean> mbeanRegistration;
+
+ /**
+ * Activate this component.
+ */
+ @Activate
+ protected void activate(final BundleContext context,
+ final SlingServletResolver.Config config) throws LoginException {
+ try ( final ResourceResolver scriptRR = resourceResolverFactory.getServiceResourceResolver(Collections.singletonMap(ResourceResolverFactory.SUBSERVICE, (Object)"scripts"))) {
+ this.searchPaths = scriptRR.getSearchPath();
+ }
+
+ // create cache - if a cache size is configured
+ this.cacheSize = config.servletresolver_cacheSize();
+ if (this.cacheSize > 5) {
+ this.cache = new ConcurrentHashMap<>(cacheSize);
+ this.logCacheSizeWarning = true;
+
+ // register MBean
+ try {
+ Dictionary<String, String> mbeanProps = new Hashtable<>();
+ mbeanProps.put("jmx.objectname", "org.apache.sling:type=servletResolver,service=SlingServletResolverCache");
+
+ ServletResolverCacheMBeanImpl mbean = new ServletResolverCacheMBeanImpl();
+ mbeanRegistration = context.registerService(SlingServletResolverCacheMBean.class, mbean, mbeanProps);
+ } catch (final Throwable t) {
+ logger.debug("Unable to register mbean");
+ }
+ } else {
+ this.cacheSize = 0;
+ }
+
+ // and finally register as event listener
+ // to invalidate cache and script extensions
+ final Dictionary<String, Object> props = new Hashtable<>();
+ props.put(Constants.SERVICE_DESCRIPTION, "Apache Sling Servlet Resolver Event Handler");
+ props.put(Constants.SERVICE_VENDOR,"The Apache Software Foundation");
+
+ props.put(EventConstants.EVENT_TOPIC, new String[] {"javax/script/ScriptEngineFactory/*",
+ "org/apache/sling/api/adapter/AdapterFactory/*","org/apache/sling/scripting/core/BindingsValuesProvider/*" });
+
+ this.eventHandlerRegistration = context.registerService(EventHandler.class, this, props);
+
+ // we need a resource change listener to invalidate the cache
+ if ( this.cache != null ) {
+ props.put(ResourceChangeListener.PATHS, "/");
+ this.resourceListenerRegistration = context.registerService(ResourceChangeListener.class, this, props);
+ }
+
+ updateScriptEngineExtensions();
+ }
+
+ /**
+ * Deactivate this component.
+ */
+ @Deactivate
+ protected void deactivate() {
+ // unregister mbean
+ if ( this.mbeanRegistration != null ) {
+ this.mbeanRegistration.unregister();
+ this.mbeanRegistration = null;
+ }
+
+ // unregister event handler
+ if (this.eventHandlerRegistration != null) {
+ this.eventHandlerRegistration.unregister();
+ this.eventHandlerRegistration = null;
+ }
+
+ // unregister event handler
+ if (this.resourceListenerRegistration != null) {
+ this.resourceListenerRegistration.unregister();
+ this.resourceListenerRegistration = null;
+ }
+
+ this.cache = null;
+ }
+
+
+ public List<String> getScriptEngineExtensions() {
+ return this.scriptEnginesExtensions;
+ }
+
+ private void updateScriptEngineExtensions() {
+ final ScriptEngineManager localScriptEngineManager = scriptEngineManager;
+ // use local variable to avoid racing with deactivate
+ if ( localScriptEngineManager != null ) {
+ final List<String> scriptEnginesExtensions = new ArrayList<>();
+ for (ScriptEngineFactory factory : localScriptEngineManager.getEngineFactories()) {
+ scriptEnginesExtensions.addAll(factory.getExtensions());
+ }
+ this.scriptEnginesExtensions = Collections.unmodifiableList(scriptEnginesExtensions);
+ }
+ }
+
+ /**
+ * @see org.osgi.service.event.EventHandler#handleEvent(org.osgi.service.event.Event)
+ */
+ @Override
+ public void handleEvent(final Event event) {
+ // return immediately if already deactivated
+ if ( this.eventHandlerRegistration == null ) {
+ return;
+ }
+ flushCache();
+ updateScriptEngineExtensions();
+ }
+
+ private void flushCache() {
+ // use local variable to avoid racing with deactivate
+ final Map<AbstractResourceCollector, Servlet> localCache = this.cache;
+ if ( localCache != null ) {
+ localCache.clear();
+ this.logCacheSizeWarning = true;
+ }
+ }
+
+ @Override
+ public void onChange(final List<ResourceChange> changes) {
+ // return immediately if already deactivated
+ if ( resourceListenerRegistration == null ) {
+ return;
+ }
+ boolean flushCache = false;
+ for(final ResourceChange change : changes){
+ // if the path of the event is a sub path of a search path
+ // we flush the whole cache
+ final String path = change.getPath();
+ int index = 0;
+ while (!flushCache && index < searchPaths.length) {
+ if (path.startsWith(this.searchPaths[index])) {
+ flushCache = true;
+ break;
+ }
+ index++;
+ }
+ if ( flushCache ) {
+ flushCache();
+ break; // we can stop looping
+ }
+ }
+ }
+
+ class ServletResolverCacheMBeanImpl extends StandardMBean implements SlingServletResolverCacheMBean {
+
+ ServletResolverCacheMBeanImpl() throws NotCompliantMBeanException {
+ super(SlingServletResolverCacheMBean.class);
+ }
+
+ @Override
+ public int getCacheSize() {
+ // use local variable to avoid racing with deactivate
+ final Map<AbstractResourceCollector, Servlet> localCache = cache;
+ return localCache != null ? localCache.size() : 0;
+ }
+
+ @Override
+ public void flushCache() {
+ ResolutionCache.this.flushCache();
+ }
+
+ @Override
+ public int getMaximumCacheSize() {
+ return cacheSize;
+ }
+
+ }
+
+ public Servlet get(final AbstractResourceCollector context) {
+ final Map<AbstractResourceCollector, Servlet> localCache = this.cache;
+ if ( localCache != null ) {
+ return localCache.get(context);
+ }
+ return null;
+ }
+
+ public void put(final AbstractResourceCollector context, final Servlet candidate) {
+ final Map<AbstractResourceCollector, Servlet> localCache = this.cache;
+ if ( localCache != null ) {
+ if ( localCache.size() < this.cacheSize ) {
+ localCache.put(context, candidate);
+ } else if ( this.logCacheSizeWarning ) {
+ this.logCacheSizeWarning = false;
+ logger.warn("Script cache has reached its limit of {}. You might want to increase the cache size for the servlet resolver.",
+ this.cacheSize);
+ }
+ }
+ }
+}
diff --git a/src/test/java/org/apache/sling/servlets/resolver/internal/SlingServletResolverTest.java b/src/test/java/org/apache/sling/servlets/resolver/internal/SlingServletResolverTest.java
index 2bf82d4..48b3732 100644
--- a/src/test/java/org/apache/sling/servlets/resolver/internal/SlingServletResolverTest.java
+++ b/src/test/java/org/apache/sling/servlets/resolver/internal/SlingServletResolverTest.java
@@ -30,7 +30,6 @@
import java.util.List;
import java.util.Map;
-import javax.script.ScriptEngineManager;
import javax.servlet.Servlet;
import javax.servlet.http.HttpServlet;
@@ -46,6 +45,7 @@
import org.apache.sling.commons.testing.sling.MockResource;
import org.apache.sling.commons.testing.sling.MockResourceResolver;
import org.apache.sling.commons.testing.sling.MockSlingHttpServletRequest;
+import org.apache.sling.servlets.resolver.internal.resolution.ResolutionCache;
import org.apache.sling.servlets.resolver.internal.resource.MockServletResource;
import org.apache.sling.servlets.resolver.internal.resource.ServletResourceProvider;
import org.apache.sling.servlets.resolver.internal.resource.ServletResourceProviderFactory;
@@ -135,10 +135,10 @@
resolverField.setAccessible(true);
resolverField.set(servletResolver, factory);
- // set script engine manager
- final Field scriptEngineManagerField = resolverClass.getDeclaredField("scriptEngineManager");
- scriptEngineManagerField.setAccessible(true);
- scriptEngineManagerField.set(servletResolver, Mockito.mock(ScriptEngineManager.class));
+ // set cache
+ final Field cacheField = resolverClass.getDeclaredField("resolutionCache");
+ cacheField.setAccessible(true);
+ cacheField.set(servletResolver, new ResolutionCache());
final Bundle bundle = Mockito.mock(Bundle.class);
Mockito.when(bundle.getBundleId()).thenReturn(1L);
