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 2020/01/24 15:57:54 UTC
[sling-org-apache-sling-serviceusermapper] 01/02: SLING-9026 :
Reduce unused code and move embedded classes to own package space
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-serviceusermapper.git
commit c324bff833ddcfa8b9ad714ef2280a299def1de8
Author: Carsten Ziegeler <cz...@apache.org>
AuthorDate: Fri Jan 24 16:56:22 2020 +0100
SLING-9026 : Reduce unused code and move embedded classes to own package space
---
pom.xml | 27 ++++++-
.../impl/ServiceUserMapperImpl.java | 88 +++++++++-------------
.../impl/ServiceUserMappedBundleFilterTest.java | 38 +++++-----
.../impl/ServiceUserMapperImplTest.java | 61 ++++++++-------
4 files changed, 117 insertions(+), 97 deletions(-)
diff --git a/pom.xml b/pom.xml
index 5db6822..ddb54e6 100644
--- a/pom.xml
+++ b/pom.xml
@@ -22,8 +22,8 @@
<modelVersion>4.0.0</modelVersion>
<parent>
<groupId>org.apache.sling</groupId>
- <artifactId>sling</artifactId>
- <version>34</version>
+ <artifactId>sling-bundle-parent</artifactId>
+ <version>36</version>
<relativePath />
</parent>
@@ -64,6 +64,29 @@
</excludePackageNames>
</configuration>
</plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-shade-plugin</artifactId>
+ <version>3.2.1</version>
+ <executions>
+ <execution>
+ <phase>package</phase>
+ <goals>
+ <goal>shade</goal>
+ </goals>
+ <configuration>
+ <createSourcesJar>true</createSourcesJar>
+ <shadeSourcesContent>true</shadeSourcesContent>
+ <relocations>
+ <relocation>
+ <pattern>org.apache.felix</pattern>
+ <shadedPattern>org.apache.sling.serviceusermapping.impl.felix</shadedPattern>
+ </relocation>
+ </relocations>
+ </configuration>
+ </execution>
+ </executions>
+ </plugin>
</plugins>
</build>
<dependencies>
diff --git a/src/main/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImpl.java b/src/main/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImpl.java
index 97224f7..5ec1f0b 100644
--- a/src/main/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImpl.java
+++ b/src/main/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImpl.java
@@ -91,39 +91,44 @@ public class ServiceUserMapperImpl implements ServiceUserMapper {
/** default log */
private final Logger log = LoggerFactory.getLogger(getClass());
- private Mapping[] globalServiceUserMappings = new Mapping[0];
+ private final BundleContext bundleContext;
- private String defaultUser;
+ private final ExecutorService executorService;
- volatile boolean useDefaultMapping;
+ private final List<ServiceUserValidator> userValidators = new CopyOnWriteArrayList<>();
- private Map<Long, MappingConfigAmendment> amendments = new HashMap<>();
+ private final List<ServicePrincipalsValidator> principalsValidators = new CopyOnWriteArrayList<>();
- private Mapping[] activeMappings = new Mapping[0];
+ private final AtomicReference<ServiceRegistration> defaultRegistration = new AtomicReference<>();
- private final List<ServiceUserValidator> userValidators = new CopyOnWriteArrayList<>();
+ private final Map<Long, MappingConfigAmendment> amendments = new HashMap<>();
- private final List<ServicePrincipalsValidator> principalsValidators = new CopyOnWriteArrayList<>();
+ private volatile Mapping[] globalServiceUserMappings;
- private SortedMap<Mapping, Registration> activeRegistrations = new TreeMap<>();
+ private volatile String defaultUser;
- private BundleContext bundleContext;
+ private volatile boolean useDefaultMapping;
- private ExecutorService executorService;
+ private volatile Mapping[] activeMappings = new Mapping[0];
- boolean registerAsync = true;
+ private volatile SortedMap<Mapping, Registration> activeRegistrations = new TreeMap<>();
private volatile boolean requireValidation = false;
- private final AtomicReference<ServiceRegistration> defaultRegistration = new AtomicReference<>();
-
@Activate
- @Modified
- synchronized void configure(BundleContext bundleContext, final Config config) {
- if (registerAsync && executorService == null) {
- executorService = Executors.newSingleThreadExecutor();
- }
+ public ServiceUserMapperImpl(final BundleContext bundleContext, final Config config) {
+ this(bundleContext, config, Executors.newSingleThreadExecutor());
+ }
+ public ServiceUserMapperImpl(final BundleContext bundleContext, final Config config,
+ final ExecutorService executor) {
+ this.bundleContext = bundleContext;
+ this.executorService = executor;
+ this.configure(config);
+ }
+
+ @Modified
+ synchronized void configure(final Config config) {
final String[] props = config.user_mapping();
if ( props != null ) {
@@ -144,11 +149,13 @@ public class ServiceUserMapperImpl implements ServiceUserMapper {
this.globalServiceUserMappings = new Mapping[0];
}
this.defaultUser = config.user_default();
+ if (this.defaultUser != null && this.defaultUser.isEmpty()) {
+ this.defaultUser = null;
+ }
this.useDefaultMapping = config.user_enable_default_mapping();
this.requireValidation = config.require_validation();
RegistrationSet registrationSet = null;
- this.bundleContext = bundleContext;
registrationSet = this.updateMappings();
this.executeServiceRegistrationsAsync(registrationSet);
@@ -159,11 +166,7 @@ public class ServiceUserMapperImpl implements ServiceUserMapper {
// this call does not unregister the mappings, but they should be unbound
// through the unbind methods anyway
updateServiceRegistrations(new Mapping[0]);
- bundleContext = null;
- if (executorService != null) {
- executorService.shutdown();
- executorService = null;
- }
+ executorService.shutdown();
}
private void restartAllActiveServiceUserMappedServices() {
@@ -319,10 +322,6 @@ public class ServiceUserMapperImpl implements ServiceUserMapper {
RegistrationSet updateServiceRegistrations(final Mapping[] newMappings) {
RegistrationSet result = new RegistrationSet();
- // do not do anything if not activated
- if (bundleContext == null) {
- return result;
- }
final SortedSet<Mapping> orderedNewMappings = new TreeSet<>(Arrays.asList(newMappings));
final SortedMap<Mapping, Registration> newRegistrations = new TreeMap<>();
@@ -358,23 +357,13 @@ public class ServiceUserMapperImpl implements ServiceUserMapper {
}
private void executeServiceRegistrationsAsync(final RegistrationSet registrationSet) {
-
- if (executorService == null) {
- executeServiceRegistrations(registrationSet);
- } else {
- executorService.submit(new Runnable() {
- @Override
- public void run() {
- executeServiceRegistrations(registrationSet);
- }
- });
- }
+ executorService.submit(() -> executeServiceRegistrations(registrationSet));
}
private void executeServiceRegistrations(final RegistrationSet registrationSet) {
- ServiceRegistration reg = defaultRegistration.getAndSet(null);
+ ServiceRegistration reg = defaultRegistration.getAndSet(null);
if (reg != null) {
reg.unregister();
}
@@ -397,12 +386,6 @@ public class ServiceUserMapperImpl implements ServiceUserMapper {
}
}
- BundleContext savedBundleContext = bundleContext;
-
- if (savedBundleContext == null) {
- return;
- }
-
for (final Registration registration : registrationSet.added) {
Mapping mapping = registration.mapping;
final Dictionary<String, Object> properties = new Hashtable<>();
@@ -411,7 +394,8 @@ public class ServiceUserMapperImpl implements ServiceUserMapper {
}
properties.put(Mapping.SERVICENAME, mapping.getServiceName());
- final ServiceRegistration serviceRegistration = savedBundleContext.registerService(ServiceUserMappedImpl.SERVICEUSERMAPPED,
+ final ServiceRegistration serviceRegistration = bundleContext.registerService(
+ ServiceUserMappedImpl.SERVICEUSERMAPPED,
new ServiceUserMappedImpl(), properties);
ServiceRegistration oldServiceRegistration = registration.setService(serviceRegistration);
@@ -426,10 +410,11 @@ public class ServiceUserMapperImpl implements ServiceUserMapper {
}
}
- if (this.useDefaultMapping || (defaultUser != null && !defaultUser.isEmpty())) {
+ if (this.useDefaultMapping || defaultUser != null) {
Dictionary<String, Object> properties = new Hashtable<>();
- properties.put(Mapping.SERVICENAME, getServiceName(savedBundleContext.getBundle()));
- final ServiceRegistration serviceRegistration = savedBundleContext.registerService(ServiceUserMappedImpl.SERVICEUSERMAPPED,
+ properties.put(Mapping.SERVICENAME, getServiceName(bundleContext.getBundle()));
+ final ServiceRegistration serviceRegistration = bundleContext
+ .registerService(ServiceUserMappedImpl.SERVICEUSERMAPPED,
new ServiceUserMappedImpl(), properties);
ServiceRegistration oldServiceRegistration = this.defaultRegistration.getAndSet(serviceRegistration);
if (oldServiceRegistration != null) {
@@ -465,7 +450,7 @@ public class ServiceUserMapperImpl implements ServiceUserMapper {
}
// use default mapping if configured and no default user
- if ( this.useDefaultMapping && (this.defaultUser == null || this.defaultUser.isEmpty() )) {
+ if (this.useDefaultMapping && this.defaultUser == null) {
final String userName = "serviceuser--" + serviceName + (subServiceName == null ? "" : "--" + subServiceName);
log.debug("internalGetUserId: no mapping found, using default mapping [{}]", userName);
return userName;
@@ -549,6 +534,7 @@ public class ServiceUserMapperImpl implements ServiceUserMapper {
return bundle.getSymbolicName();
}
+ @Override
public List<Mapping> getActiveMappings() {
return Collections.unmodifiableList(Arrays.asList(activeMappings));
}
diff --git a/src/test/java/org/apache/sling/serviceusermapping/impl/ServiceUserMappedBundleFilterTest.java b/src/test/java/org/apache/sling/serviceusermapping/impl/ServiceUserMappedBundleFilterTest.java
index 08fe051..d85edf9 100644
--- a/src/test/java/org/apache/sling/serviceusermapping/impl/ServiceUserMappedBundleFilterTest.java
+++ b/src/test/java/org/apache/sling/serviceusermapping/impl/ServiceUserMappedBundleFilterTest.java
@@ -20,7 +20,14 @@
package org.apache.sling.serviceusermapping.impl;
-import junit.framework.TestCase;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
import org.apache.sling.serviceusermapping.Mapping;
import org.junit.Test;
@@ -29,18 +36,9 @@ import org.osgi.framework.BundleContext;
import org.osgi.framework.Constants;
import org.osgi.framework.ServiceEvent;
import org.osgi.framework.ServiceReference;
-import org.osgi.framework.hooks.service.EventListenerHook;
-import org.osgi.framework.hooks.service.FindHook;
import org.osgi.framework.hooks.service.ListenerHook;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
+import junit.framework.TestCase;
/**
* Test reference and bundle filtering based on <code>Mapping.SERVICENAME</code>
@@ -95,7 +93,8 @@ public class ServiceUserMappedBundleFilterTest {
ServiceUserMappedBundleFilter eventListenerHook = new ServiceUserMappedBundleFilter();
- eventListenerHook.mapper = new ServiceUserMapperImpl();
+ ServiceUserMapperImpl.Config config = mock(ServiceUserMapperImpl.Config.class);
+ eventListenerHook.mapper = new ServiceUserMapperImpl(null, config);
eventListenerHook.start(mapperContext);
Map<BundleContext, Collection<ListenerHook.ListenerInfo>> map1 = new HashMap<>(map);
@@ -103,8 +102,9 @@ public class ServiceUserMappedBundleFilterTest {
TestCase.assertEquals(0, map1.size());
+ when(config.user_enable_default_mapping()).thenReturn(true);
+ eventListenerHook.mapper.configure(config);
Map<BundleContext, Collection<ListenerHook.ListenerInfo>> map2 = new HashMap<>(map);
- eventListenerHook.mapper.useDefaultMapping = true;
eventListenerHook.event(serviceEvent, map2);
TestCase.assertEquals(1, map2.size());
@@ -114,7 +114,7 @@ public class ServiceUserMappedBundleFilterTest {
@Test
public void testFind() {
- List collection = new ArrayList<ServiceReference>();
+ List<ServiceReference> collection = new ArrayList<>();
ServiceReference serviceReference1 = mock(ServiceReference.class);
ServiceReference serviceReference2 = mock(ServiceReference.class);
@@ -129,17 +129,19 @@ public class ServiceUserMappedBundleFilterTest {
ServiceUserMappedBundleFilter findHook = new ServiceUserMappedBundleFilter();
- findHook.mapper = new ServiceUserMapperImpl();
+ ServiceUserMapperImpl.Config config = mock(ServiceUserMapperImpl.Config.class);
+ findHook.mapper = new ServiceUserMapperImpl(null, config);
findHook.start(mapperContext);
- List collection1 = new ArrayList(collection);
+ List<ServiceReference> collection1 = new ArrayList<>(collection);
findHook.find(bundleContext1, null, null, false, collection1);
TestCase.assertEquals(0, collection1.size());
- findHook.mapper.useDefaultMapping = true;
+ when(config.user_enable_default_mapping()).thenReturn(true);
+ findHook.mapper.configure(config);
- List collection2 = new ArrayList(collection);
+ List<ServiceReference> collection2 = new ArrayList<>(collection);
findHook.find(bundleContext1, null, null, false, collection2);
TestCase.assertEquals(1, collection2.size());
diff --git a/src/test/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImplTest.java b/src/test/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImplTest.java
index 1fc5abb..dfd9ec1 100644
--- a/src/test/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImplTest.java
+++ b/src/test/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImplTest.java
@@ -32,10 +32,13 @@ import java.util.HashSet;
import java.util.Iterator;
import java.util.Map;
import java.util.Set;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
import org.apache.sling.serviceusermapping.ServicePrincipalsValidator;
import org.apache.sling.serviceusermapping.ServiceUserValidator;
import org.junit.Test;
+import org.mockito.ArgumentCaptor;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
import org.osgi.framework.Bundle;
@@ -107,8 +110,7 @@ public class ServiceUserMapperImplTest {
when(config.user_default()).thenReturn(NONE);
when(config.user_enable_default_mapping()).thenReturn(false);
- final ServiceUserMapperImpl sum = new ServiceUserMapperImpl();
- sum.configure(null, config);
+ final ServiceUserMapperImpl sum = new ServiceUserMapperImpl(null, config);
TestCase.assertEquals(SAMPLE, sum.getServiceUserID(BUNDLE1, null));
TestCase.assertEquals(ANOTHER, sum.getServiceUserID(BUNDLE2, null));
@@ -132,8 +134,7 @@ public class ServiceUserMapperImplTest {
when(config.user_default()).thenReturn(NONE);
when(config.user_enable_default_mapping()).thenReturn(true);
- final ServiceUserMapperImpl sum = new ServiceUserMapperImpl();
- sum.configure(null, config);
+ final ServiceUserMapperImpl sum = new ServiceUserMapperImpl(null, config);
TestCase.assertEquals(SAMPLE, sum.getServiceUserID(BUNDLE1, null));
TestCase.assertEquals(ANOTHER, sum.getServiceUserID(BUNDLE2, null));
@@ -157,8 +158,7 @@ public class ServiceUserMapperImplTest {
when(config.user_default()).thenReturn(null);
when(config.user_enable_default_mapping()).thenReturn(true);
- final ServiceUserMapperImpl sum = new ServiceUserMapperImpl();
- sum.configure(null, config);
+ final ServiceUserMapperImpl sum = new ServiceUserMapperImpl(null, config);
TestCase.assertEquals(SAMPLE, sum.getServiceUserID(BUNDLE1, null));
TestCase.assertEquals(ANOTHER, sum.getServiceUserID(BUNDLE2, null));
@@ -182,8 +182,7 @@ public class ServiceUserMapperImplTest {
when(config.user_default()).thenReturn(NONE);
when(config.user_enable_default_mapping()).thenReturn(false);
- final ServiceUserMapperImpl sum = new ServiceUserMapperImpl();
- sum.configure(null, config);
+ final ServiceUserMapperImpl sum = new ServiceUserMapperImpl(null, config);
ServiceUserValidator serviceUserValidator = new ServiceUserValidator() {
@Override
@@ -218,8 +217,7 @@ public class ServiceUserMapperImplTest {
BUNDLE_SYMBOLIC2 + ":" + SUB + "=[" + SAMPLE_SUB + "," + ANOTHER_SUB + "]" //
});
- final ServiceUserMapperImpl sum = new ServiceUserMapperImpl();
- sum.configure(null, config);
+ final ServiceUserMapperImpl sum = new ServiceUserMapperImpl(null, config);
assertEqualPrincipalNames(sum.getServicePrincipalNames(BUNDLE1, null), SAMPLE);
assertEqualPrincipalNames(sum.getServicePrincipalNames(BUNDLE2, null), ANOTHER);
@@ -238,8 +236,7 @@ public class ServiceUserMapperImplTest {
BUNDLE_SYMBOLIC2 + "=[ " + ANOTHER + " ]", //
});
- final ServiceUserMapperImpl sum = new ServiceUserMapperImpl();
- sum.configure(null, config);
+ final ServiceUserMapperImpl sum = new ServiceUserMapperImpl(null, config);
assertEqualPrincipalNames(sum.getServicePrincipalNames(BUNDLE1, ""), SAMPLE);
assertEqualPrincipalNames(sum.getServicePrincipalNames(BUNDLE2, ""), ANOTHER);
@@ -255,8 +252,7 @@ public class ServiceUserMapperImplTest {
when(config.user_default()).thenReturn(NONE);
when(config.user_enable_default_mapping()).thenReturn(false);
- final ServiceUserMapperImpl sum = new ServiceUserMapperImpl();
- sum.configure(null, config);
+ final ServiceUserMapperImpl sum = new ServiceUserMapperImpl(null, config);
assertNull(sum.getServicePrincipalNames(BUNDLE1, null));
assertNull(SAMPLE_SUB, sum.getServicePrincipalNames(BUNDLE1, SUB));
@@ -268,8 +264,7 @@ public class ServiceUserMapperImplTest {
when(config.user_default()).thenReturn(NONE);
when(config.user_enable_default_mapping()).thenReturn(true);
- final ServiceUserMapperImpl sum = new ServiceUserMapperImpl();
- sum.configure(null, config);
+ final ServiceUserMapperImpl sum = new ServiceUserMapperImpl(null, config);
assertNull(sum.getServicePrincipalNames(BUNDLE1, null));
assertNull(sum.getServicePrincipalNames(BUNDLE1, SUB));
@@ -285,8 +280,7 @@ public class ServiceUserMapperImplTest {
BUNDLE_SYMBOLIC2 + ":" + SUB + "=[" + ANOTHER_SUB + "," + SAMPLE_SUB + "," + SAMPLE + "]"//
});
- final ServiceUserMapperImpl sum = new ServiceUserMapperImpl();
- sum.configure(null, config);
+ final ServiceUserMapperImpl sum = new ServiceUserMapperImpl(null, config);
ServicePrincipalsValidator validator = new ServicePrincipalsValidator() {
@Override
public boolean isValid(Iterable<String> servicePrincipalNames, String serviceName, String subServiceName) {
@@ -334,8 +328,7 @@ public class ServiceUserMapperImplTest {
when(config.user_default()).thenReturn(NONE);
when(config.user_enable_default_mapping()).thenReturn(false);
- final ServiceUserMapperImpl sum = new ServiceUserMapperImpl();
- sum.configure(null, config);
+ final ServiceUserMapperImpl sum = new ServiceUserMapperImpl(null, config);
final MappingConfigAmendment mca1 = new MappingConfigAmendment();
MappingConfigAmendment.Config mca1Config = mock(MappingConfigAmendment.Config.class);
@@ -376,8 +369,7 @@ public class ServiceUserMapperImplTest {
when(config.user_default()).thenReturn(NONE);
when(config.user_enable_default_mapping()).thenReturn(false);
- final ServiceUserMapperImpl sum = new ServiceUserMapperImpl();
- sum.configure(null, config);
+ final ServiceUserMapperImpl sum = new ServiceUserMapperImpl(null, config);
final MappingConfigAmendment mca1 = new MappingConfigAmendment();
@@ -419,11 +411,28 @@ public class ServiceUserMapperImplTest {
when(config.user_default()).thenReturn(NONE);
when(config.user_enable_default_mapping()).thenReturn(false);
- final ServiceUserMapperImpl sum = new ServiceUserMapperImpl();
- sum.registerAsync = false;
- final ServiceRegistrationContextHelper context = new ServiceRegistrationContextHelper();
- sum.configure(context.getBundleContext(), config);
+ ArgumentCaptor<Runnable> argument = ArgumentCaptor.forClass(Runnable.class);
+
+ final ExecutorService executor = mock(ExecutorService.class);
+ when(executor.submit(argument.capture())).thenAnswer(new Answer<Future<Object>>() {
+ @Override
+ public Future<Object> answer(InvocationOnMock invocation) {
+ argument.getValue().run();
+ return null;
+ }
+ });
+ final ServiceRegistrationContextHelper context = new ServiceRegistrationContextHelper();
+ final ServiceUserMapperImpl sum = new ServiceUserMapperImpl(context.getBundleContext(), config, executor);
+
+ while (context.getRegistrations(ServiceUserMappedImpl.SERVICEUSERMAPPED) == null
+ || context.getRegistrations(ServiceUserMappedImpl.SERVICEUSERMAPPED).size() < 3) {
+ try {
+ Thread.sleep(100);
+ } catch (InterruptedException e) {
+ // ignore
+ }
+ }
TestCase.assertEquals(3, context.getRegistrations(ServiceUserMappedImpl.SERVICEUSERMAPPED).size());
final MappingConfigAmendment mca1 = new MappingConfigAmendment();