This is an automated email from the ASF dual-hosted git repository.
joerghoh pushed a commit to branch master
in repository
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-servlets-resolver.git
The following commit(s) were added to refs/heads/master by this push:
new 29b4739 SLING-12461 improve logging (#48)
29b4739 is described below
commit 29b473968cfc4e6014bfbd93dd33a8cb6781b397
Author: Jörg Hoh <[email protected]>
AuthorDate: Mon Oct 28 17:44:50 2024 +0100
SLING-12461 improve logging (#48)
* Split the large addingBundle() method
* add a dedicated log message when servlets from a bundle were registered
* add explicit logging in case the DefaultServlet is used (which should
never happen when the DefaultGetServlet is around)
---
.../internal/bundle/BundledScriptTracker.java | 281 +++++++++++----------
.../resolver/internal/defaults/DefaultServlet.java | 18 +-
2 files changed, 159 insertions(+), 140 deletions(-)
diff --git
a/src/main/java/org/apache/sling/servlets/resolver/internal/bundle/BundledScriptTracker.java
b/src/main/java/org/apache/sling/servlets/resolver/internal/bundle/BundledScriptTracker.java
index 6b9144d..70bb82b 100644
---
a/src/main/java/org/apache/sling/servlets/resolver/internal/bundle/BundledScriptTracker.java
+++
b/src/main/java/org/apache/sling/servlets/resolver/internal/bundle/BundledScriptTracker.java
@@ -22,6 +22,8 @@ import java.io.IOException;
import java.lang.reflect.InvocationHandler;
import java.lang.reflect.Method;
import java.lang.reflect.Proxy;
+import java.time.Duration;
+import java.time.Instant;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
@@ -176,157 +178,166 @@ public class BundledScriptTracker implements
BundleTrackerCustomizer<List<Servic
});
Set<TypeProvider> requiresChain =
collectRequiresChain(bundleWiring, cache);
if (!capabilities.isEmpty()) {
+ Instant registerStart = Instant.now();
Set<BundledRenderUnitCapability> bundledRenderUnitCapabilities
= new HashSet<>(cache.values());
bundledRenderUnitCapabilities =
reduce(bundledRenderUnitCapabilities);
- List<ServiceRegistration<Servlet>> serviceRegistrations =
bundledRenderUnitCapabilities.stream().flatMap(bundledRenderUnitCapability ->
- {
- Hashtable<String, Object> properties = new Hashtable<>();
- BundledRenderUnit executable = null;
- TypeProvider baseTypeProvider = new
TypeProviderImpl(bundledRenderUnitCapability, bundle);
- LinkedHashSet<TypeProvider> inheritanceChain = new
LinkedHashSet<>();
- inheritanceChain.add(baseTypeProvider);
- if
(!bundledRenderUnitCapability.getResourceTypes().isEmpty()) {
- LinkedHashSet<String>
resourceTypesRegistrationValueSet = new LinkedHashSet<>();
- for (ResourceType resourceType :
bundledRenderUnitCapability.getResourceTypes()) {
-
resourceTypesRegistrationValueSet.add(resourceType.toString());
- }
- String[] resourceTypesRegistrationValue =
resourceTypesRegistrationValueSet.stream().filter(rt -> {
- if (!rt.startsWith("/")) {
- for (String prefix : this.searchPaths) {
- if
(resourceTypesRegistrationValueSet.contains(prefix.concat(rt))) {
- return false;
- }
- }
- }
- return true;
- }).toArray(String[]::new);
-
properties.put(ServletResolverConstants.SLING_SERVLET_RESOURCE_TYPES,
resourceTypesRegistrationValue);
+ List<ServiceRegistration<Servlet>> serviceRegistrations =
bundledRenderUnitCapabilities.stream()
+ .flatMap(bundledRenderUnitCapability ->
registerServicesWithinBundle(bundle, bundleWiring, cache, requiresChain,
+ bundledRenderUnitCapability))
+ .collect(Collectors.toList());
+ refreshDispatcher(serviceRegistrations);
+ long duration = Duration.between(registerStart,
Instant.now()).toMillis();
+ LOGGER.info("Took {}ms to register {} servlets from bundle
{}.", duration, serviceRegistrations.size(), bundle.getSymbolicName());
+ return serviceRegistrations;
+ } else {
+ return Collections.emptyList();
+ }
+ } else {
+ return Collections.emptyList();
+ }
+ }
- String extension =
bundledRenderUnitCapability.getExtension();
- if (!StringUtils.isEmpty(extension)) {
-
properties.put(ServletResolverConstants.SLING_SERVLET_EXTENSIONS, extension);
+ Stream<? extends ServiceRegistration<Servlet>>
registerServicesWithinBundle(Bundle bundle,
+ BundleWiring bundleWiring, Map<BundleCapability,
BundledRenderUnitCapability> cache,
+ Set<TypeProvider> requiresChain, BundledRenderUnitCapability
bundledRenderUnitCapability) {
+ Hashtable<String, Object> properties = new Hashtable<>();
+ BundledRenderUnit executable = null;
+ TypeProvider baseTypeProvider = new
TypeProviderImpl(bundledRenderUnitCapability, bundle);
+ LinkedHashSet<TypeProvider> inheritanceChain = new LinkedHashSet<>();
+ inheritanceChain.add(baseTypeProvider);
+ if (!bundledRenderUnitCapability.getResourceTypes().isEmpty()) {
+ LinkedHashSet<String> resourceTypesRegistrationValueSet = new
LinkedHashSet<>();
+ for (ResourceType resourceType :
bundledRenderUnitCapability.getResourceTypes()) {
+ resourceTypesRegistrationValueSet.add(resourceType.toString());
+ }
+ String[] resourceTypesRegistrationValue =
resourceTypesRegistrationValueSet.stream().filter(rt -> {
+ if (!rt.startsWith("/")) {
+ for (String prefix : this.searchPaths) {
+ if
(resourceTypesRegistrationValueSet.contains(prefix.concat(rt))) {
+ return false;
}
+ }
+ }
+ return true;
+ }).toArray(String[]::new);
+
properties.put(ServletResolverConstants.SLING_SERVLET_RESOURCE_TYPES,
resourceTypesRegistrationValue);
- if
(!bundledRenderUnitCapability.getSelectors().isEmpty()) {
-
properties.put(ServletResolverConstants.SLING_SERVLET_SELECTORS,
bundledRenderUnitCapability.getSelectors().toArray());
- }
+ String extension = bundledRenderUnitCapability.getExtension();
+ if (!StringUtils.isEmpty(extension)) {
+
properties.put(ServletResolverConstants.SLING_SERVLET_EXTENSIONS, extension);
+ }
- if
(StringUtils.isNotEmpty(bundledRenderUnitCapability.getMethod())) {
-
properties.put(ServletResolverConstants.SLING_SERVLET_METHODS,
bundledRenderUnitCapability.getMethod());
- }
+ if (!bundledRenderUnitCapability.getSelectors().isEmpty()) {
+
properties.put(ServletResolverConstants.SLING_SERVLET_SELECTORS,
bundledRenderUnitCapability.getSelectors().toArray());
+ }
- String extendedResourceTypeString =
bundledRenderUnitCapability.getExtendedResourceType();
- if
(StringUtils.isNotEmpty(extendedResourceTypeString)) {
- collectInheritanceChain(inheritanceChain,
bundleWiring, extendedResourceTypeString, cache);
- inheritanceChain.stream().filter(typeProvider ->
typeProvider.getBundledRenderUnitCapability().getResourceTypes().stream()
- .anyMatch(resourceType ->
resourceType.getType().equals(extendedResourceTypeString))).findFirst()
- .ifPresent(typeProvider -> {
- for (ResourceType type :
typeProvider.getBundledRenderUnitCapability().getResourceTypes()) {
- if
(type.getType().equals(extendedResourceTypeString)) {
-
properties.put(ServletResolverConstants.SLING_SERVLET_RESOURCE_SUPER_TYPE,
type.toString());
- }
- }
- });
- }
- Set<TypeProvider> aggregate =
- Stream.concat(inheritanceChain.stream(),
requiresChain.stream()).collect(Collectors.toCollection(LinkedHashSet::new));
- if
(properties.containsKey(ServletResolverConstants.SLING_SERVLET_RESOURCE_SUPER_TYPE)
&&
-
baseTypeProvider.getBundledRenderUnitCapability().getScriptEngineName() !=
null) {
- executable =
bundledRenderUnitFinder.findUnit(bundle.getBundleContext(), new
HashSet<>(Arrays.asList(baseTypeProvider)), aggregate);
- } else {
- executable =
bundledRenderUnitFinder.findUnit(bundle.getBundleContext(), inheritanceChain,
aggregate);
- }
- } else if
(StringUtils.isNotEmpty(bundledRenderUnitCapability.getPath()) &&
StringUtils.isNotEmpty(
-
bundledRenderUnitCapability.getScriptEngineName())) {
- Set<TypeProvider> aggregate =
- Stream.concat(inheritanceChain.stream(),
requiresChain.stream()).collect(Collectors.toCollection(LinkedHashSet::new));
- executable =
bundledRenderUnitFinder.findUnit(bundle.getBundleContext(), baseTypeProvider,
aggregate);
- }
- List<ServiceRegistration<Servlet>> regs = new
ArrayList<>();
+ if
(StringUtils.isNotEmpty(bundledRenderUnitCapability.getMethod())) {
+ properties.put(ServletResolverConstants.SLING_SERVLET_METHODS,
bundledRenderUnitCapability.getMethod());
+ }
- if (executable != null) {
- String executablePath = executable.getPath();
- final String executableParentPath =
ResourceUtil.getParent(executablePath);
- if
(executablePath.equals(bundledRenderUnitCapability.getPath())) {
-
properties.put(ServletResolverConstants.SLING_SERVLET_PATHS, executablePath);
- } else {
- if
(!bundledRenderUnitCapability.getResourceTypes().isEmpty() &&
bundledRenderUnitCapability.getSelectors().isEmpty() &&
-
StringUtils.isEmpty(bundledRenderUnitCapability.getExtension()) &&
-
StringUtils.isEmpty(bundledRenderUnitCapability.getMethod())) {
- String scriptName =
FilenameUtils.getName(executable.getPath());
- String scriptNameNoExtension =
scriptName.substring(0, scriptName.lastIndexOf('.'));
- boolean noMatch =
-
bundledRenderUnitCapability.getResourceTypes().stream().noneMatch(resourceType
-> {
- String resourceTypePath =
resourceType.toString();
- String label;
- int lastSlash =
resourceTypePath.lastIndexOf('/');
- if (lastSlash > -1) {
- label =
resourceTypePath.substring(lastSlash + 1);
- } else {
- label = resourceTypePath;
- }
- return
label.equals(scriptNameNoExtension);
- });
- if (noMatch) {
- List<String> paths = new ArrayList<>();
- paths.add(executablePath);
-
bundledRenderUnitCapability.getResourceTypes().forEach(resourceType -> {
- String resourceTypePath =
resourceType.toString();
- String label;
- int lastSlash =
resourceTypePath.lastIndexOf('/');
- if (lastSlash > -1) {
- label =
resourceTypePath.substring(lastSlash + 1);
- } else {
- label = resourceTypePath;
- }
- if
(StringUtils.isNotEmpty(executableParentPath) &&
executableParentPath.equals(resourceTypePath)) {
- paths.add(resourceTypePath + "/" +
label + ".servlet");
- }
- });
-
properties.put(ServletResolverConstants.SLING_SERVLET_PATHS, paths.toArray(new
String[0]));
- }
- if
(!properties.containsKey(ServletResolverConstants.SLING_SERVLET_PATHS)) {
- String[] rts =
Converters.standardConverter().convert(properties.get(ServletResolverConstants.SLING_SERVLET_RESOURCE_TYPES)).to(String[].class);
- for (String resourceType : rts) {
- String path;
- if (resourceType.startsWith("/")) {
- path = resourceType + "/" +
resourceType.substring(resourceType.lastIndexOf('/') + 1) + "." +
FilenameUtils.getExtension(scriptName);
- } else {
- path = this.searchPaths.get(0) +
resourceType + "/" + resourceType.substring(resourceType.lastIndexOf('/') + 1)
+ "." + FilenameUtils.getExtension(scriptName);
- }
-
properties.put(ServletResolverConstants.SLING_SERVLET_PATHS, path);
- }
+ String extendedResourceTypeString =
bundledRenderUnitCapability.getExtendedResourceType();
+ if (StringUtils.isNotEmpty(extendedResourceTypeString)) {
+ collectInheritanceChain(inheritanceChain, bundleWiring,
extendedResourceTypeString, cache);
+ inheritanceChain.stream().filter(typeProvider ->
typeProvider.getBundledRenderUnitCapability().getResourceTypes().stream()
+ .anyMatch(resourceType ->
resourceType.getType().equals(extendedResourceTypeString))).findFirst()
+ .ifPresent(typeProvider -> {
+ for (ResourceType type :
typeProvider.getBundledRenderUnitCapability().getResourceTypes()) {
+ if
(type.getType().equals(extendedResourceTypeString)) {
+
properties.put(ServletResolverConstants.SLING_SERVLET_RESOURCE_SUPER_TYPE,
type.toString());
}
}
- if
(!properties.containsKey(ServletResolverConstants.SLING_SERVLET_PATHS)) {
-
bundledRenderUnitCapability.getResourceTypes().forEach(resourceType -> {
- if
(StringUtils.isNotEmpty(executableParentPath) && (executableParentPath +
"/").startsWith(resourceType.toString() + "/")) {
-
properties.put(ServletResolverConstants.SLING_SERVLET_PATHS, executablePath);
- }
- });
+ });
+ }
+ Set<TypeProvider> aggregate =
+ Stream.concat(inheritanceChain.stream(),
requiresChain.stream()).collect(Collectors.toCollection(LinkedHashSet::new));
+ if
(properties.containsKey(ServletResolverConstants.SLING_SERVLET_RESOURCE_SUPER_TYPE)
&&
+
baseTypeProvider.getBundledRenderUnitCapability().getScriptEngineName() !=
null) {
+ executable =
bundledRenderUnitFinder.findUnit(bundle.getBundleContext(), new
HashSet<>(Arrays.asList(baseTypeProvider)), aggregate);
+ } else {
+ executable =
bundledRenderUnitFinder.findUnit(bundle.getBundleContext(), inheritanceChain,
aggregate);
+ }
+ } else if
(StringUtils.isNotEmpty(bundledRenderUnitCapability.getPath()) &&
StringUtils.isNotEmpty(
+ bundledRenderUnitCapability.getScriptEngineName())) {
+ Set<TypeProvider> aggregate =
+ Stream.concat(inheritanceChain.stream(),
requiresChain.stream()).collect(Collectors.toCollection(LinkedHashSet::new));
+ executable =
bundledRenderUnitFinder.findUnit(bundle.getBundleContext(), baseTypeProvider,
aggregate);
+ }
+ List<ServiceRegistration<Servlet>> regs = new ArrayList<>();
+
+ if (executable != null) {
+ String executablePath = executable.getPath();
+ final String executableParentPath =
ResourceUtil.getParent(executablePath);
+ if (executablePath.equals(bundledRenderUnitCapability.getPath())) {
+ properties.put(ServletResolverConstants.SLING_SERVLET_PATHS,
executablePath);
+ } else {
+ if (!bundledRenderUnitCapability.getResourceTypes().isEmpty()
&& bundledRenderUnitCapability.getSelectors().isEmpty() &&
+
StringUtils.isEmpty(bundledRenderUnitCapability.getExtension()) &&
+
StringUtils.isEmpty(bundledRenderUnitCapability.getMethod())) {
+ String scriptName =
FilenameUtils.getName(executable.getPath());
+ String scriptNameNoExtension = scriptName.substring(0,
scriptName.lastIndexOf('.'));
+ boolean noMatch =
+
bundledRenderUnitCapability.getResourceTypes().stream().noneMatch(resourceType
-> {
+ String resourceTypePath = resourceType.toString();
+ String label;
+ int lastSlash = resourceTypePath.lastIndexOf('/');
+ if (lastSlash > -1) {
+ label = resourceTypePath.substring(lastSlash +
1);
+ } else {
+ label = resourceTypePath;
+ }
+ return label.equals(scriptNameNoExtension);
+ });
+ if (noMatch) {
+ List<String> paths = new ArrayList<>();
+ paths.add(executablePath);
+
bundledRenderUnitCapability.getResourceTypes().forEach(resourceType -> {
+ String resourceTypePath = resourceType.toString();
+ String label;
+ int lastSlash = resourceTypePath.lastIndexOf('/');
+ if (lastSlash > -1) {
+ label = resourceTypePath.substring(lastSlash +
1);
+ } else {
+ label = resourceTypePath;
+ }
+ if (StringUtils.isNotEmpty(executableParentPath)
&& executableParentPath.equals(resourceTypePath)) {
+ paths.add(resourceTypePath + "/" + label +
".servlet");
}
+ });
+
properties.put(ServletResolverConstants.SLING_SERVLET_PATHS, paths.toArray(new
String[0]));
+ }
+ if
(!properties.containsKey(ServletResolverConstants.SLING_SERVLET_PATHS)) {
+ String[] rts =
Converters.standardConverter().convert(properties.get(ServletResolverConstants.SLING_SERVLET_RESOURCE_TYPES)).to(String[].class);
+ for (String resourceType : rts) {
+ String path;
+ if (resourceType.startsWith("/")) {
+ path = resourceType + "/" +
resourceType.substring(resourceType.lastIndexOf('/') + 1) + "." +
FilenameUtils.getExtension(scriptName);
+ } else {
+ path = this.searchPaths.get(0) + resourceType
+ "/" + resourceType.substring(resourceType.lastIndexOf('/') + 1) + "." +
FilenameUtils.getExtension(scriptName);
+ }
+
properties.put(ServletResolverConstants.SLING_SERVLET_PATHS, path);
}
-
properties.put(ServletResolverConstants.SLING_SERVLET_NAME,
- String.format("%s (%s)",
BundledScriptServlet.class.getSimpleName(), executablePath));
- properties.put(Constants.SERVICE_DESCRIPTION,
- BundledScriptServlet.class.getName() + "{" +
bundledRenderUnitCapability + "}");
- regs.add(
- register(bundle.getBundleContext(), new
BundledScriptServlet(inheritanceChain, executable), properties)
- );
- } else {
- LOGGER.debug(String.format("Unable to locate an
executable for capability %s.", bundledRenderUnitCapability.toString()));
}
-
- return regs.stream();
- }).collect(Collectors.toList());
- refreshDispatcher(serviceRegistrations);
- return serviceRegistrations;
- } else {
- return Collections.emptyList();
+ }
+ if
(!properties.containsKey(ServletResolverConstants.SLING_SERVLET_PATHS)) {
+
bundledRenderUnitCapability.getResourceTypes().forEach(resourceType -> {
+ if (StringUtils.isNotEmpty(executableParentPath) &&
(executableParentPath + "/").startsWith(resourceType.toString() + "/")) {
+
properties.put(ServletResolverConstants.SLING_SERVLET_PATHS, executablePath);
+ }
+ });
+ }
}
+ properties.put(ServletResolverConstants.SLING_SERVLET_NAME,
+ String.format("%s (%s)",
BundledScriptServlet.class.getSimpleName(), executablePath));
+ properties.put(Constants.SERVICE_DESCRIPTION,
+ BundledScriptServlet.class.getName() + "{" +
bundledRenderUnitCapability + "}");
+ regs.add(
+ register(bundle.getBundleContext(), new
BundledScriptServlet(inheritanceChain, executable), properties)
+ );
} else {
- return Collections.emptyList();
+ LOGGER.debug(String.format("Unable to locate an executable for
capability %s.", bundledRenderUnitCapability.toString()));
}
+
+ return regs.stream();
}
private final AtomicLong idCounter = new AtomicLong(0);
diff --git
a/src/main/java/org/apache/sling/servlets/resolver/internal/defaults/DefaultServlet.java
b/src/main/java/org/apache/sling/servlets/resolver/internal/defaults/DefaultServlet.java
index c2d1110..57809c4 100644
---
a/src/main/java/org/apache/sling/servlets/resolver/internal/defaults/DefaultServlet.java
+++
b/src/main/java/org/apache/sling/servlets/resolver/internal/defaults/DefaultServlet.java
@@ -27,6 +27,8 @@ import org.apache.sling.api.SlingHttpServletResponse;
import org.apache.sling.api.resource.NonExistingResource;
import org.apache.sling.api.resource.Resource;
import org.apache.sling.api.servlets.SlingSafeMethodsServlet;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* The <code>DefaultServlet</code> is a very simple default resource handler.
@@ -37,6 +39,8 @@ import org.apache.sling.api.servlets.SlingSafeMethodsServlet;
public class DefaultServlet extends SlingSafeMethodsServlet {
private static final long serialVersionUID = 3806788918045433043L;
+
+ private static final Logger LOG =
LoggerFactory.getLogger(DefaultServlet.class);
@Override
protected void doGet(SlingHttpServletRequest request,
@@ -44,13 +48,17 @@ public class DefaultServlet extends SlingSafeMethodsServlet
{
Resource resource = request.getResource();
- // cannot handle the request for missing resources
+ // Also log that this servlet was invoked, as there are circumstances
where setting the statuscode
+ // might not have any effect anymore (for example when the response is
already committed).
if (resource instanceof NonExistingResource) {
- response.sendError(HttpServletResponse.SC_NOT_FOUND,
- "Resource not found at path " + resource.getPath());
+ // cannot handle the request for missing resources
+ String msg = String.format("Resource not found at path %s",
resource.getPath());
+ LOG.error(msg);
+ response.sendError(HttpServletResponse.SC_NOT_FOUND,msg);
} else {
- response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
- "Cannot find servlet to handle resource " +
resource.getPath());
+ String msg = String.format("Cannot find servlet to handle resource
%s", resource.getPath());
+ LOG.error(msg);
+
response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR,msg);
}
}
