You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by cz...@apache.org on 2022/02/01 09:38:22 UTC
[sling-org-apache-sling-rewriter] branch master updated: SLING-11103 - Improvements in synchronization and additional logging - Clean up code
This is an automated email from the ASF dual-hosted git repository.
cziegeler pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-rewriter.git
The following commit(s) were added to refs/heads/master by this push:
new 463914a SLING-11103 - Improvements in synchronization and additional logging - Clean up code
463914a is described below
commit 463914adfa3fef746b5792507e2eff640fe16529
Author: Carsten Ziegeler <cz...@apache.org>
AuthorDate: Tue Feb 1 10:38:16 2022 +0100
SLING-11103 - Improvements in synchronization and additional logging - Clean up code
---
pom.xml | 4 +-
.../apache/sling/rewriter/impl/FactoryCache.java | 20 ++--------
.../impl/HashingServiceTrackerCustomizer.java | 19 +++++----
.../sling/rewriter/impl/ProcessorManagerImpl.java | 46 ++++++----------------
.../impl/TransformerFactoryServiceTracker.java | 31 ++++++++-------
5 files changed, 43 insertions(+), 77 deletions(-)
diff --git a/pom.xml b/pom.xml
index cbaf452..cb5fcff 100644
--- a/pom.xml
+++ b/pom.xml
@@ -109,7 +109,7 @@
<dependency>
<groupId>org.apache.sling</groupId>
<artifactId>org.apache.sling.api</artifactId>
- <version>2.11.0</version>
+ <version>2.18.0</version>
<scope>provided</scope>
</dependency>
<dependency>
@@ -236,7 +236,7 @@
<dependency>
<groupId>org.apache.felix</groupId>
<artifactId>org.apache.felix.framework</artifactId>
- <version>6.0.3</version>
+ <version>7.0.3</version>
<scope>test</scope>
</dependency>
<!-- jsoup -->
diff --git a/src/main/java/org/apache/sling/rewriter/impl/FactoryCache.java b/src/main/java/org/apache/sling/rewriter/impl/FactoryCache.java
index 62384a5..2437715 100644
--- a/src/main/java/org/apache/sling/rewriter/impl/FactoryCache.java
+++ b/src/main/java/org/apache/sling/rewriter/impl/FactoryCache.java
@@ -16,8 +16,6 @@
*/
package org.apache.sling.rewriter.impl;
-import java.util.Comparator;
-
import org.apache.sling.commons.osgi.OsgiUtil;
import org.apache.sling.rewriter.Generator;
import org.apache.sling.rewriter.GeneratorFactory;
@@ -75,7 +73,7 @@ public class FactoryCache {
private final HashingServiceTrackerCustomizer<SerializerFactory> serializerTracker;
/** The tracker for transformer factories. */
- private final TransformerFactoryServiceTracker<TransformerFactory> transformerTracker;
+ private final TransformerFactoryServiceTracker transformerTracker;
/** The tracker for processor factories. */
private final HashingServiceTrackerCustomizer<ProcessorFactory> processorTracker;
@@ -86,7 +84,7 @@ public class FactoryCache {
GeneratorFactory.class.getName());
this.serializerTracker = new HashingServiceTrackerCustomizer<SerializerFactory>(context,
SerializerFactory.class.getName());
- this.transformerTracker = new TransformerFactoryServiceTracker<TransformerFactory>(context,
+ this.transformerTracker = new TransformerFactoryServiceTracker(context,
TransformerFactory.class.getName());
this.processorTracker = new HashingServiceTrackerCustomizer<ProcessorFactory>(context,
ProcessorFactory.class.getName());
@@ -214,24 +212,12 @@ public class FactoryCache {
return transformers;
}
- /**
- * Comparator for service references.
- */
- static final class ServiceReferenceComparator implements Comparator<ServiceReference> {
- public static ServiceReferenceComparator INSTANCE = new ServiceReferenceComparator();
-
- @Override
- public int compare(ServiceReference o1, ServiceReference o2) {
- return o1.compareTo(o2);
- }
- }
-
static final class TransformerFactoryEntry {
public final TransformerFactory factory;
private final ProcessorConfiguration configuration;
- public TransformerFactoryEntry(final TransformerFactory factory, final ServiceReference ref) {
+ public TransformerFactoryEntry(final TransformerFactory factory, final ServiceReference<TransformerFactory> ref) {
this.factory = factory;
final String[] paths = OsgiUtil.toStringArray(ref.getProperty(PROPERTY_PATHS), null);
final String[] extensions = OsgiUtil.toStringArray(ref.getProperty(PROPERTY_EXTENSIONS), null);
diff --git a/src/main/java/org/apache/sling/rewriter/impl/HashingServiceTrackerCustomizer.java b/src/main/java/org/apache/sling/rewriter/impl/HashingServiceTrackerCustomizer.java
index bcca737..595fc8d 100644
--- a/src/main/java/org/apache/sling/rewriter/impl/HashingServiceTrackerCustomizer.java
+++ b/src/main/java/org/apache/sling/rewriter/impl/HashingServiceTrackerCustomizer.java
@@ -31,14 +31,14 @@ import org.osgi.util.tracker.ServiceTracker;
/**
* This service tracker stores all services into a hash map.
*/
-class HashingServiceTrackerCustomizer<T> extends ServiceTracker {
+class HashingServiceTrackerCustomizer<T> extends ServiceTracker<T, T> {
public static final class Pair<T> {
- public final ServiceReference reference;
+ public final ServiceReference<T> reference;
public final T service;
- public Pair(final ServiceReference r, final T s) {
+ public Pair(final ServiceReference<T> r, final T s) {
this.reference = r;
this.service = s;
}
@@ -50,7 +50,7 @@ class HashingServiceTrackerCustomizer<T> extends ServiceTracker {
public final List<Pair<T>> references = new ArrayList<Pair<T>>();
- public void add(final ServiceReference ref, final T service) {
+ public void add(final ServiceReference<T> ref, final T service) {
references.add(new Pair<T>(ref, service));
Collections.sort(references, new Comparator<Pair<T>>() {
@@ -64,7 +64,7 @@ class HashingServiceTrackerCustomizer<T> extends ServiceTracker {
}
}
- public void remove(final ServiceReference ref) {
+ public void remove(final ServiceReference<T> ref) {
if ( !references.isEmpty() ) {
boolean update = references.get(0).reference == ref;
final Iterator<Pair<T>> i = references.iterator();
@@ -101,7 +101,7 @@ class HashingServiceTrackerCustomizer<T> extends ServiceTracker {
return entry == null ? null : entry.service;
}
- private String getType(final ServiceReference ref) {
+ String getType(final ServiceReference<T> ref) {
final String type = (String) ref.getProperty(FactoryCache.PROPERTY_TYPE);
return type;
}
@@ -110,10 +110,9 @@ class HashingServiceTrackerCustomizer<T> extends ServiceTracker {
* @see org.osgi.util.tracker.ServiceTrackerCustomizer#addingService(org.osgi.framework.ServiceReference)
*/
@Override
- public Object addingService(final ServiceReference reference) {
+ public T addingService(final ServiceReference<T> reference) {
final String type = this.getType(reference);
- @SuppressWarnings("unchecked")
- final T factory = (type == null ? null : (T) this.context.getService(reference));
+ final T factory = (type == null ? null : this.context.getService(reference));
if ( factory != null ) {
if ( FactoryCache.LOGGER.isDebugEnabled() ) {
FactoryCache.LOGGER.debug("Found service {}, type={}.", factory, type);
@@ -134,7 +133,7 @@ class HashingServiceTrackerCustomizer<T> extends ServiceTracker {
* @see org.osgi.util.tracker.ServiceTrackerCustomizer#removedService(org.osgi.framework.ServiceReference, java.lang.Object)
*/
@Override
- public void removedService(final ServiceReference reference, final Object service) {
+ public void removedService(final ServiceReference<T> reference, final T service) {
final String type = this.getType(reference);
if ( type != null ) {
synchronized ( this ) {
diff --git a/src/main/java/org/apache/sling/rewriter/impl/ProcessorManagerImpl.java b/src/main/java/org/apache/sling/rewriter/impl/ProcessorManagerImpl.java
index 6b6a0aa..5640ef6 100644
--- a/src/main/java/org/apache/sling/rewriter/impl/ProcessorManagerImpl.java
+++ b/src/main/java/org/apache/sling/rewriter/impl/ProcessorManagerImpl.java
@@ -21,9 +21,7 @@ import java.io.PrintWriter;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
-import java.util.Dictionary;
import java.util.HashMap;
-import java.util.Hashtable;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
@@ -46,7 +44,6 @@ import org.apache.sling.serviceusermapping.ServiceUserMapped;
import org.osgi.framework.BundleContext;
import org.osgi.framework.Constants;
import org.osgi.framework.InvalidSyntaxException;
-import org.osgi.framework.ServiceRegistration;
import org.osgi.service.component.ComponentContext;
import org.osgi.service.component.annotations.Activate;
import org.osgi.service.component.annotations.Component;
@@ -60,24 +57,27 @@ import org.slf4j.LoggerFactory;
* This manager keeps track of configured processors.
*
*/
-@Component(service = ProcessorManager.class,
+@Component(service = {ProcessorManager.class, ResourceChangeListener.class},
property = {
- Constants.SERVICE_VENDOR + "=The Apache Software Foundation"
+ Constants.SERVICE_VENDOR + "=The Apache Software Foundation",
+ ResourceChangeListener.CHANGES + "=ADDED",
+ ResourceChangeListener.CHANGES + "=CHANGED",
+ ResourceChangeListener.CHANGES + "=REMOVED",
+ ResourceChangeListener.CHANGES + "=PROVIDER_ADDED",
+ ResourceChangeListener.CHANGES + "=PROVIDER_REMOVED",
+ ResourceChangeListener.PATHS + "=glob:*" + ProcessorManagerImpl.CONFIG_PATH + "/**"
})
public class ProcessorManagerImpl
implements ProcessorManager, ResourceChangeListener, ExternalResourceChangeListener {
- private static final String CONFIG_REL_PATH = "config/rewriter";
- private static final String CONFIG_PATH = "/" + CONFIG_REL_PATH;
+ static final String CONFIG_REL_PATH = "config/rewriter";
+ static final String CONFIG_PATH = "/" + CONFIG_REL_PATH;
protected static final String MIME_TYPE_HTML = "text/html";
/** The logger */
private final Logger log = LoggerFactory.getLogger(this.getClass());
- /** The bundle context. */
- private BundleContext bundleContext;
-
@Reference
private ResourceResolverFactory resourceResolverFactory;
@@ -93,9 +93,6 @@ public class ProcessorManagerImpl
/** Ordered processor configurations. */
private List<ProcessorConfiguration> orderedProcessors = new ArrayList<>();
- /** Event handler registration */
- private volatile ServiceRegistration<ResourceChangeListener> eventHandlerRegistration;
-
/** Search path */
private String[] searchPath;
@@ -109,24 +106,13 @@ public class ProcessorManagerImpl
@Activate
protected void activate(final BundleContext ctx)
throws LoginException, InvalidSyntaxException {
- this.bundleContext = ctx;
- this.factoryCache = new FactoryCache(this.bundleContext);
+ this.factoryCache = new FactoryCache(ctx);
// create array of search paths for actions and constraints
this.searchPath = this.initProcessors();
- // register event handler
- final Dictionary<String, Object> props = new Hashtable<>();
- props.put(ResourceChangeListener.CHANGES,
- new String[] { ChangeType.ADDED.toString(), ChangeType.CHANGED.toString(),
- ChangeType.REMOVED.toString(), ChangeType.PROVIDER_ADDED.toString(), ChangeType.PROVIDER_REMOVED.toString() });
- props.put(ResourceChangeListener.PATHS, "glob:*" + CONFIG_PATH + "/**");
- props.put("service.description", "Processor Configuration/Modification Handler");
- props.put("service.vendor", "The Apache Software Foundation");
- this.eventHandlerRegistration = this.bundleContext.registerService(ResourceChangeListener.class, this,
- props);
this.factoryCache.start();
- WebConsoleConfigPrinter.register(this.bundleContext, this);
+ WebConsoleConfigPrinter.register(ctx, this);
}
private ResourceResolver createResourceResolver() throws LoginException {
@@ -138,16 +124,10 @@ public class ProcessorManagerImpl
* @param ctx
*/
protected void deactivate(final ComponentContext ctx) {
- if ( this.eventHandlerRegistration != null ) {
- this.eventHandlerRegistration.unregister();
- this.eventHandlerRegistration = null;
- }
this.factoryCache.stop();
this.factoryCache = null;
WebConsoleConfigPrinter.unregister();
-
- this.bundleContext = null;
}
@Override
@@ -211,7 +191,7 @@ public class ProcessorManagerImpl
/**
* Initializes the current processors
*/
- private synchronized String[] initProcessors() throws LoginException {
+ private String[] initProcessors() throws LoginException {
try ( final ResourceResolver resolver = this.createResourceResolver()) {
for(final String path : resolver.getSearchPath() ) {
// check if the search path exists
diff --git a/src/main/java/org/apache/sling/rewriter/impl/TransformerFactoryServiceTracker.java b/src/main/java/org/apache/sling/rewriter/impl/TransformerFactoryServiceTracker.java
index ae7082f..de28364 100644
--- a/src/main/java/org/apache/sling/rewriter/impl/TransformerFactoryServiceTracker.java
+++ b/src/main/java/org/apache/sling/rewriter/impl/TransformerFactoryServiceTracker.java
@@ -20,7 +20,6 @@ import java.util.Arrays;
import org.apache.sling.rewriter.ProcessingContext;
import org.apache.sling.rewriter.TransformerFactory;
-import org.apache.sling.rewriter.impl.FactoryCache.ServiceReferenceComparator;
import org.apache.sling.rewriter.impl.FactoryCache.TransformerFactoryEntry;
import org.osgi.framework.BundleContext;
import org.osgi.framework.Constants;
@@ -28,16 +27,16 @@ import org.osgi.framework.ServiceReference;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-final class TransformerFactoryServiceTracker<T> extends HashingServiceTrackerCustomizer<T> {
+final class TransformerFactoryServiceTracker extends HashingServiceTrackerCustomizer<TransformerFactory> {
private final Logger LOGGER = LoggerFactory.getLogger(TransformerFactoryServiceTracker.class);
- private String getMode(final ServiceReference ref) {
+ private String getMode(final ServiceReference<TransformerFactory> ref) {
final String mode = (String) ref.getProperty(FactoryCache.PROPERTY_MODE);
return mode;
}
- private boolean isGlobal(final ServiceReference ref) {
+ private boolean isGlobal(final ServiceReference<TransformerFactory> ref) {
return FactoryCache.MODE_GLOBAL.equalsIgnoreCase(this.getMode(ref));
}
@@ -61,12 +60,11 @@ final class TransformerFactoryServiceTracker<T> extends HashingServiceTrackerCus
* @see org.osgi.util.tracker.ServiceTracker#addingService(org.osgi.framework.ServiceReference)
*/
@Override
- public Object addingService(ServiceReference reference) {
+ public TransformerFactory addingService(ServiceReference<TransformerFactory> reference) {
final boolean isGlobal = isGlobal(reference);
LOGGER.debug("Adding service {}, isGlobal={}", reference.getClass(), isGlobal);
- Object obj = null;
- obj = super.addingService(reference);
- if ( obj == null && isGlobal ) {
+ TransformerFactory obj = super.addingService(reference);
+ if ( isGlobal && getType(reference) == null ) {
obj = this.context.getService(reference);
}
return obj;
@@ -76,10 +74,13 @@ final class TransformerFactoryServiceTracker<T> extends HashingServiceTrackerCus
* @see org.osgi.util.tracker.ServiceTracker#removedService(org.osgi.framework.ServiceReference, java.lang.Object)
*/
@Override
- public void removedService(ServiceReference reference, Object service) {
+ public void removedService(ServiceReference<TransformerFactory> reference, TransformerFactory service) {
final boolean isGlobal = isGlobal(reference);
LOGGER.debug("Removing service {}, isGlobal={}", reference.getClass(), isGlobal);
super.removedService(reference, service);
+ if ( isGlobal && getType(reference) == null ) {
+ this.context.ungetService(reference);
+ }
}
/**
@@ -90,16 +91,16 @@ final class TransformerFactoryServiceTracker<T> extends HashingServiceTrackerCus
if (this.currentTrackingCount != this.getTrackingCount()) {
synchronized ( this ) {
if (this.currentTrackingCount != this.getTrackingCount()) {
- final ServiceReference[] refs = this.getServiceReferences();
+ final ServiceReference<TransformerFactory>[] refs = this.getServiceReferences();
LOGGER.debug("Found {} service references", refs.length);
if ( refs == null || refs.length == 0 ) {
this.cached = EMPTY_DOUBLE_ENTRY_ARRAY;
} else {
- Arrays.sort(refs, ServiceReferenceComparator.INSTANCE);
+ Arrays.sort(refs);
int preCount = 0;
int postCount = 0;
- for(final ServiceReference ref : refs) {
+ for(final ServiceReference<TransformerFactory> ref : refs) {
if ( isGlobal(ref) ) {
final Object r = ref.getProperty(Constants.SERVICE_RANKING);
int ranking = (r instanceof Integer ? (Integer)r : 0);
@@ -122,14 +123,14 @@ final class TransformerFactoryServiceTracker<T> extends HashingServiceTrackerCus
globalFactories[1] = new TransformerFactoryEntry[postCount];
}
int index = 0;
- for(final ServiceReference ref : refs) {
+ for(final ServiceReference<TransformerFactory> ref : refs) {
if ( isGlobal(ref) ) {
if ( index < preCount ) {
LOGGER.debug("Initializing pre global TransformerFactory for service ref: {}", ref.getClass());
- globalFactories[0][index] = new TransformerFactoryEntry((TransformerFactory) this.getService(ref), ref);
+ globalFactories[0][index] = new TransformerFactoryEntry(this.getService(ref), ref);
} else {
LOGGER.debug("Initializing post global TransformerFactory for service ref: {}", ref.getClass());
- globalFactories[1][index - preCount] = new TransformerFactoryEntry((TransformerFactory) this.getService(ref), ref);
+ globalFactories[1][index - preCount] = new TransformerFactoryEntry(this.getService(ref), ref);
}
index++;
}