SLING-9233 - Skip registering the Dispatcher servlet if the resource types are not versioned
diff --git a/src/main/java/org/apache/sling/scripting/bundle/tracker/ResourceType.java b/src/main/java/org/apache/sling/scripting/bundle/tracker/ResourceType.java
index 263879b..b1399f1 100644
--- a/src/main/java/org/apache/sling/scripting/bundle/tracker/ResourceType.java
+++ b/src/main/java/org/apache/sling/scripting/bundle/tracker/ResourceType.java
@@ -19,6 +19,7 @@
package org.apache.sling.scripting.bundle.tracker;
import java.util.Objects;
+import java.util.regex.Pattern;
import org.apache.commons.lang3.StringUtils;
import org.jetbrains.annotations.NotNull;
@@ -39,9 +40,12 @@
*/
public final class ResourceType {
+ private static final Pattern versionPattern = Pattern.compile("[\\d\\.]+(-.*)*$");
+
private final String type;
private final String version;
private final String resourceLabel;
+ private final String toString;
private ResourceType(@NotNull String type, @Nullable String version) {
this.type = type;
@@ -53,6 +57,7 @@
} else {
resourceLabel = type;
}
+ toString = type + (version == null ? "" : "/" + version);
}
/**
@@ -90,7 +95,7 @@
@Override
public String toString() {
- return type + (version == null ? "" : "/" + version);
+ return toString;
}
/**
@@ -115,10 +120,15 @@
if (StringUtils.isNotEmpty(resourceTypeString)) {
int lastSlash = resourceTypeString.lastIndexOf('/');
if (lastSlash != -1 && !resourceTypeString.endsWith("/")) {
- try {
- version = Version.parseVersion(resourceTypeString.substring(lastSlash + 1)).toString();
- type = resourceTypeString.substring(0, lastSlash);
- } catch (IllegalArgumentException e) {
+ String versionString = resourceTypeString.substring(lastSlash + 1);
+ if (versionPattern.matcher(versionString).matches()) {
+ try {
+ version = Version.parseVersion(versionString).toString();
+ type = resourceTypeString.substring(0, lastSlash);
+ } catch (IllegalArgumentException e) {
+ type = resourceTypeString;
+ }
+ } else {
type = resourceTypeString;
}
} else {
diff --git a/src/main/java/org/apache/sling/scripting/bundle/tracker/internal/BundledScriptServlet.java b/src/main/java/org/apache/sling/scripting/bundle/tracker/internal/BundledScriptServlet.java
index 2cf4be3..0516ece 100644
--- a/src/main/java/org/apache/sling/scripting/bundle/tracker/internal/BundledScriptServlet.java
+++ b/src/main/java/org/apache/sling/scripting/bundle/tracker/internal/BundledScriptServlet.java
@@ -19,13 +19,12 @@
package org.apache.sling.scripting.bundle.tracker.internal;
import java.io.IOException;
-import java.util.HashMap;
-import java.util.HashSet;
+import java.util.Collection;
import java.util.LinkedHashSet;
-import java.util.Map;
-import java.util.Set;
-import java.util.concurrent.locks.ReadWriteLock;
-import java.util.concurrent.locks.ReentrantReadWriteLock;
+import java.util.Optional;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import java.util.stream.Collectors;
import javax.script.Bindings;
import javax.script.ScriptContext;
@@ -42,7 +41,6 @@
import org.apache.sling.api.request.RequestPathInfo;
import org.apache.sling.api.scripting.ScriptEvaluationException;
import org.apache.sling.api.scripting.SlingBindings;
-import org.apache.sling.scripting.bundle.tracker.ResourceType;
import org.apache.sling.scripting.core.ScriptHelper;
class BundledScriptServlet extends GenericServlet {
@@ -53,8 +51,7 @@
private final LinkedHashSet<TypeProvider> m_wiredTypeProviders;
private final boolean m_precompiledScripts;
- private Map<String, Executable> scriptsMap = new HashMap<>();
- private ReadWriteLock lock = new ReentrantReadWriteLock();
+ private ConcurrentMap<String, Optional<Executable>> scriptsMap = new ConcurrentHashMap<>();
@@ -83,55 +80,27 @@
}
String scriptsMapKey = getScriptsMapKey(request);
- Executable executable;
- lock.readLock().lock();
+ Executable executable = scriptsMap.computeIfAbsent(scriptsMapKey,
+ key -> Optional.ofNullable(m_bundledScriptFinder.getScript(request, m_wiredTypeProviders, m_precompiledScripts))
+ ).orElseThrow(() -> new ServletException("Unable to locate a " + (m_precompiledScripts ? "class" : "script") + " for rendering."));
+
+ RequestWrapper requestWrapper = new RequestWrapper(request,
+ m_wiredTypeProviders.stream().map(TypeProvider::getResourceTypes).flatMap(Collection::stream).collect(Collectors.toSet()));
+ ScriptContext scriptContext = m_scriptContextProvider.prepareScriptContext(requestWrapper, response, executable);
try {
- executable = scriptsMap.get(scriptsMapKey);
- if (executable == null) {
- lock.readLock().unlock();
- lock.writeLock().lock();
- try {
- executable = scriptsMap.get(scriptsMapKey);
- if (executable == null) {
- executable = m_bundledScriptFinder.getScript(request, m_wiredTypeProviders, m_precompiledScripts);
- if (executable != null) {
- scriptsMap.put(scriptsMapKey, executable);
- }
- }
- } finally {
- lock.readLock().lock();
- lock.writeLock().unlock();
- }
- }
+ executable.eval(scriptContext);
+ } catch (ScriptException se) {
+ Throwable cause = (se.getCause() == null) ? se : se.getCause();
+ throw new ScriptEvaluationException(executable.getName(), se.getMessage(), cause);
} finally {
- lock.readLock().unlock();
- }
- if (executable != null) {
- Set<String> wiredResourceTypes = new HashSet<>();
- for (TypeProvider typeProvider : m_wiredTypeProviders) {
- for (ResourceType resourceType : typeProvider.getResourceTypes()) {
- wiredResourceTypes.add(resourceType.toString());
+ Bindings engineBindings = scriptContext.getBindings(ScriptContext.ENGINE_SCOPE);
+ if (engineBindings != null && engineBindings.containsKey(SlingBindings.SLING)) {
+ Object scriptHelper = engineBindings.get(SlingBindings.SLING);
+ if (scriptHelper instanceof ScriptHelper) {
+ ((ScriptHelper) scriptHelper).cleanup();
}
}
- RequestWrapper requestWrapper = new RequestWrapper(request, wiredResourceTypes);
- ScriptContext scriptContext = m_scriptContextProvider.prepareScriptContext(requestWrapper, response, executable);
- try {
- executable.eval(scriptContext);
- } catch (ScriptException se) {
- Throwable cause = (se.getCause() == null) ? se : se.getCause();
- throw new ScriptEvaluationException(executable.getName(), se.getMessage(), cause);
- } finally {
- Bindings engineBindings = scriptContext.getBindings(ScriptContext.ENGINE_SCOPE);
- if (engineBindings != null && engineBindings.containsKey(SlingBindings.SLING)) {
- Object scriptHelper = engineBindings.get(SlingBindings.SLING);
- if (scriptHelper instanceof ScriptHelper) {
- ((ScriptHelper) scriptHelper).cleanup();
- }
- }
- executable.releaseDependencies();
- }
- } else {
- throw new ServletException("Unable to locate a " + (m_precompiledScripts ? "class" : "script") + " for rendering.");
+ executable.releaseDependencies();
}
} else {
throw new ServletException("Not a Sling HTTP request/response");
diff --git a/src/main/java/org/apache/sling/scripting/bundle/tracker/internal/BundledScriptTracker.java b/src/main/java/org/apache/sling/scripting/bundle/tracker/internal/BundledScriptTracker.java
index bbe4d15..6c8bd63 100644
--- a/src/main/java/org/apache/sling/scripting/bundle/tracker/internal/BundledScriptTracker.java
+++ b/src/main/java/org/apache/sling/scripting/bundle/tracker/internal/BundledScriptTracker.java
@@ -99,7 +99,7 @@
private volatile BundleContext m_context;
private volatile BundleTracker<List<ServiceRegistration<Servlet>>> m_tracker;
- private volatile Map<String, ServiceRegistration<Servlet>> m_dispatchers = new HashMap<>();
+ private volatile Map<Set<String>, ServiceRegistration<Servlet>> m_dispatchers = new HashMap<>();
@Activate
protected void activate(BundleContext context) {
@@ -126,7 +126,7 @@
{
Hashtable<String, Object> properties = new Hashtable<>();
properties.put(ServletResolverConstants.SLING_SERVLET_NAME, BundledScriptServlet.class.getName());
- properties.put(Constants.SERVICE_DESCRIPTION, cap.toString());
+ properties.put(Constants.SERVICE_DESCRIPTION, BundledScriptServlet.class.getName() + cap.getAttributes());
ResourceTypeCapability resourceTypeCapability = ResourceTypeCapability.fromBundleCapability(cap);
String[] resourceTypesRegistrationValue = new String[resourceTypeCapability.getResourceTypes().size()];
int rtIndex = 0;
@@ -220,12 +220,14 @@
}
private void refreshDispatcher(List<ServiceRegistration<Servlet>> regs) {
- Map<String, ServiceRegistration<Servlet>> dispatchers = new HashMap<>();
- Stream.concat(m_tracker.getTracked().values().stream(), Stream.of(regs)).flatMap(List::stream).map(this::toProperties).collect(
- Collectors.groupingBy(this::getResourceType)).forEach((rt, propList) -> {
+ Map<Set<String>, ServiceRegistration<Servlet>> dispatchers = new HashMap<>();
+ Stream.concat(m_tracker.getTracked().values().stream(), Stream.of(regs)).flatMap(List::stream)
+ .filter(ref -> getResourceTypeVersion(ref.getReference()) != null)
+ .map(this::toProperties)
+ .collect(Collectors.groupingBy(BundledScriptTracker::getResourceTypes)).forEach((rt, propList) -> {
Hashtable<String, Object> properties = new Hashtable<>();
properties.put(ServletResolverConstants.SLING_SERVLET_NAME, DispatcherServlet.class.getName());
- properties.put(ServletResolverConstants.SLING_SERVLET_RESOURCE_TYPES, rt);
+ properties.put(ServletResolverConstants.SLING_SERVLET_RESOURCE_TYPES, rt.toArray());
Set<String> methods = propList.stream()
.map(props -> props.getOrDefault(ServletResolverConstants.SLING_SERVLET_METHODS, new String[]{"GET", "HEAD"}))
.map(PropertiesUtil::toStringArray).map(Arrays::asList).flatMap(List::stream).collect(Collectors.toSet());
@@ -245,9 +247,11 @@
}
return null;
}).findFirst();
- properties.put(Constants.SERVICE_DESCRIPTION, ServletResolverConstants.SLING_SERVLET_RESOURCE_TYPES + "=" + rt + "; " +
+ properties.put(Constants.SERVICE_DESCRIPTION,
+ DispatcherServlet.class.getName() + "{" + ServletResolverConstants.SLING_SERVLET_RESOURCE_TYPES +
+ "=" + rt + "; " +
ServletResolverConstants.SLING_SERVLET_EXTENSIONS + "=" + extensions + "; " +
- ServletResolverConstants.SLING_SERVLET_METHODS + "=" + methods);
+ ServletResolverConstants.SLING_SERVLET_METHODS + "=" + methods + "}");
reg = registeringBundle.orElse(m_context).registerService(Servlet.class, new DispatcherServlet(rt), properties);
} else {
if (!new HashSet<>(Arrays.asList(PropertiesUtil
@@ -275,13 +279,6 @@
return result;
}
- private String getResourceType(Hashtable<String, Object> props) {
- String[] values = PropertiesUtil.toStringArray(props.get(ServletResolverConstants.SLING_SERVLET_RESOURCE_TYPES));
- String resourceTypeValue = values[0];
- ResourceType resourceType = ResourceType.parseResourceType(resourceTypeValue);
- return resourceType.getType();
- }
-
private void set(String key, ServiceReference<?> ref, Hashtable<String, Object> props) {
Object value = ref.getProperty(key);
if (value != null) {
@@ -302,9 +299,9 @@
}
private class DispatcherServlet extends GenericServlet {
- private final String m_rt;
+ private final Set<String> m_rt;
- DispatcherServlet(String rt) {
+ DispatcherServlet(Set<String> rt) {
m_rt = rt;
}
@@ -322,7 +319,7 @@
.filter(reg ->
{
Hashtable<String, Object> props = toProperties(reg);
- return getResourceType(props).equals(m_rt) &&
+ return getResourceTypes(props).equals(m_rt) &&
Arrays.asList(PropertiesUtil
.toStringArray(props.get(ServletResolverConstants.SLING_SERVLET_METHODS),
new String[]{"GET", "HEAD"}))
@@ -331,8 +328,7 @@
.toStringArray(props.get(ServletResolverConstants.SLING_SERVLET_EXTENSIONS), new String[]{"html"}))
.contains(slingRequest.getRequestPathInfo().getExtension() == null ? "html" :
slingRequest.getRequestPathInfo().getExtension());
- })
- .sorted((left, right) ->
+ }).min((left, right) ->
{
boolean la = Arrays.asList(PropertiesUtil
.toStringArray(toProperties(left).get(ServletResolverConstants.SLING_SERVLET_SELECTORS), new String[0]))
@@ -349,8 +345,7 @@
return 1;
}
- })
- .findFirst();
+ });
if (target.isPresent()) {
String[] targetRT =
@@ -382,12 +377,21 @@
((SlingHttpServletResponse) res).sendError(HttpServletResponse.SC_NOT_FOUND);
}
}
+ }
- private String getResourceTypeVersion(ServiceReference<?> ref) {
- String[] values = PropertiesUtil.toStringArray(ref.getProperty(ServletResolverConstants.SLING_SERVLET_RESOURCE_TYPES));
- String resourceTypeValue = values[0];
- ResourceType resourceType = ResourceType.parseResourceType(resourceTypeValue);
- return resourceType.getVersion();
+ private static String getResourceTypeVersion(ServiceReference<?> ref) {
+ String[] values = PropertiesUtil.toStringArray(ref.getProperty(ServletResolverConstants.SLING_SERVLET_RESOURCE_TYPES));
+ String resourceTypeValue = values[0];
+ ResourceType resourceType = ResourceType.parseResourceType(resourceTypeValue);
+ return resourceType.getVersion();
+ }
+
+ private static Set<String> getResourceTypes(Hashtable<String, Object> props) {
+ Set<String> resourceTypes = new HashSet<>();
+ String[] values = PropertiesUtil.toStringArray(props.get(ServletResolverConstants.SLING_SERVLET_RESOURCE_TYPES));
+ for (String resourceTypeValue : values) {
+ resourceTypes.add(ResourceType.parseResourceType(resourceTypeValue).getType());
}
+ return resourceTypes;
}
}
diff --git a/src/main/java/org/apache/sling/scripting/bundle/tracker/internal/RequestWrapper.java b/src/main/java/org/apache/sling/scripting/bundle/tracker/internal/RequestWrapper.java
index 7a9da1f..57ebfea 100644
--- a/src/main/java/org/apache/sling/scripting/bundle/tracker/internal/RequestWrapper.java
+++ b/src/main/java/org/apache/sling/scripting/bundle/tracker/internal/RequestWrapper.java
@@ -27,12 +27,13 @@
import org.apache.sling.api.request.RequestDispatcherOptions;
import org.apache.sling.api.resource.Resource;
import org.apache.sling.api.wrappers.SlingHttpServletRequestWrapper;
+import org.apache.sling.scripting.bundle.tracker.ResourceType;
class RequestWrapper extends SlingHttpServletRequestWrapper {
- private final Set<String> wiredResourceTypes;
+ private final Set<ResourceType> wiredResourceTypes;
- RequestWrapper(SlingHttpServletRequest wrappedRequest, Set<String> wiredResourceTypes) {
+ RequestWrapper(SlingHttpServletRequest wrappedRequest, Set<ResourceType> wiredResourceTypes) {
super(wrappedRequest);
this.wiredResourceTypes = wiredResourceTypes;
}
@@ -67,9 +68,10 @@
requestDispatcherOptions.setReplaceSuffix(options.getReplaceSuffix());
String forcedResourceType = options.getForceResourceType();
if (StringUtils.isNotEmpty(forcedResourceType)) {
- for (String wiredResourceType : wiredResourceTypes) {
- if (wiredResourceType.startsWith(forcedResourceType + "/")) {
- requestDispatcherOptions.setForceResourceType(wiredResourceType);
+ for (ResourceType wiredResourceType : wiredResourceTypes) {
+ String type = wiredResourceType.getType();
+ if (type.startsWith(forcedResourceType + "/")) {
+ requestDispatcherOptions.setForceResourceType(type);
break;
}
}
