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 2019/06/04 08:57:03 UTC

[sling-org-apache-sling-servlets-resolver] branch master updated: SLING-8470 : Improve servlets handling

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-servlets-resolver.git


The following commit(s) were added to refs/heads/master by this push:
     new aa77c2e  SLING-8470 : Improve servlets handling
aa77c2e is described below

commit aa77c2e61d4ed82deb1044337fae9962392445b9
Author: Carsten Ziegeler <cz...@apache.org>
AuthorDate: Tue Jun 4 10:56:54 2019 +0200

    SLING-8470 : Improve servlets handling
---
 pom.xml                                            | 17 ++++-
 .../resolver/internal/resource/ServletMounter.java | 80 +++-------------------
 .../internal/resource/ServletMounterTest.java      | 13 +---
 3 files changed, 27 insertions(+), 83 deletions(-)

diff --git a/pom.xml b/pom.xml
index 8260326..4f95d60 100644
--- a/pom.xml
+++ b/pom.xml
@@ -52,6 +52,7 @@
             <plugin>
                 <groupId>org.apache.felix</groupId>
                 <artifactId>maven-bundle-plugin</artifactId>
+                <version>4.2.0</version>
                 <extensions>true</extensions>
                 <configuration>
                     <instructions>
@@ -86,7 +87,19 @@
         <dependency>
             <groupId>org.osgi</groupId>
             <artifactId>org.osgi.annotation.versioning</artifactId>
-            <version>1.0.0</version>
+            <version>1.1.0</version>
+            <scope>provided</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.osgi</groupId>
+            <artifactId>org.osgi.service.component.annotations</artifactId>
+            <version>1.4.0</version>
+            <scope>provided</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.osgi</groupId>
+            <artifactId>org.osgi.service.metatype.annotations</artifactId>
+            <version>1.4.0</version>
             <scope>provided</scope>
         </dependency>
         <dependency>
@@ -115,7 +128,7 @@
         <dependency>
             <groupId>org.osgi</groupId>
             <artifactId>org.osgi.service.component</artifactId>
-            <version>1.3.0</version>
+            <version>1.4.0</version>
             <scope>provided</scope>
         </dependency>
         <dependency>
diff --git a/src/main/java/org/apache/sling/servlets/resolver/internal/resource/ServletMounter.java b/src/main/java/org/apache/sling/servlets/resolver/internal/resource/ServletMounter.java
index 30513cb..6c7d48a 100644
--- a/src/main/java/org/apache/sling/servlets/resolver/internal/resource/ServletMounter.java
+++ b/src/main/java/org/apache/sling/servlets/resolver/internal/resource/ServletMounter.java
@@ -28,7 +28,6 @@ import java.util.Collection;
 import java.util.Dictionary;
 import java.util.HashMap;
 import java.util.Hashtable;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 
@@ -72,40 +71,24 @@ public class ServletMounter {
 
     private static final String REF_SERVLET = "Servlet";
 
-    @Reference(target="(name=org.apache.sling)")
-    private ServletContext servletContext;
+    private final ServletContext servletContext;
 
     private final Map<ServiceReference<Servlet>, ServletReg> servletsByReference = new HashMap<>();
 
-    private final List<PendingServlet> pendingServlets = new ArrayList<>();
+    private volatile boolean active = true;
 
-    /** The bundle context. */
-    private volatile BundleContext context;
-
-    private volatile ServletResourceProviderFactory servletResourceProviderFactory;
-
-    @Reference
-    private ResourceResolverFactory resourceResolverFactory;
+    private final ServletResourceProviderFactory servletResourceProviderFactory;
 
     /**
      * Activate this component.
      */
     @Activate
-    protected void activate(final BundleContext context,
+    public ServletMounter(final BundleContext context, @Reference final ResourceResolverFactory resourceResolverFactory,
+            @Reference(target = "(name=org.apache.sling)") ServletContext servletContext,
             final ResolverConfig config) {
-        final Collection<PendingServlet> refs;
-        synchronized (this.pendingServlets) {
-
-            refs = new ArrayList<>(pendingServlets);
-            pendingServlets.clear();
-
-            servletResourceProviderFactory = new ServletResourceProviderFactory(config.servletresolver_servletRoot(),
-                    resourceResolverFactory.getSearchPath());
-
-            // register servlets immediately from now on
-            this.context = context;
-        }
-        createAllServlets(refs);
+        this.servletContext = servletContext;
+        servletResourceProviderFactory = new ServletResourceProviderFactory(config.servletresolver_servletRoot(),
+                resourceResolverFactory.getSearchPath());
     }
 
     /**
@@ -113,9 +96,7 @@ public class ServletMounter {
      */
     @Deactivate
     protected void deactivate() {
-        // stop registering of servlets immediately
-        this.context = null;
-
+        this.active = false;
         // Copy the list of servlets first, to minimize the need for
         // synchronization
         final Collection<ServiceReference<Servlet>> refs;
@@ -129,13 +110,8 @@ public class ServletMounter {
         synchronized ( this.servletsByReference ) {
             this.servletsByReference.clear();
         }
-
-        this.servletResourceProviderFactory = null;
     }
 
-    // TODO
-    // This can be simplified once we can use DS from R7 with constructor injection
-    // as we can inject the bundle context through the constructor
     @Reference(
             name = REF_SERVLET,
             service = Servlet.class,
@@ -143,41 +119,15 @@ public class ServletMounter {
             policy = ReferencePolicy.DYNAMIC,
             target="(|(" + ServletResolverConstants.SLING_SERVLET_PATHS + "=*)(" + ServletResolverConstants.SLING_SERVLET_RESOURCE_TYPES + "=*))")
     protected void bindServlet(final Servlet servlet, final ServiceReference<Servlet> reference) {
-        boolean directCreate = true;
-        if (context == null) {
-            synchronized ( pendingServlets ) {
-                if (context == null) {
-                    pendingServlets.add(new PendingServlet(servlet, reference));
-                    directCreate = false;
-                }
-            }
-        }
-        if ( directCreate ) {
+        if (this.active) {
             createServlet(servlet, reference);
         }
     }
 
     protected void unbindServlet(final ServiceReference<Servlet> reference) {
-        synchronized ( pendingServlets ) {
-            final Iterator<PendingServlet> iter = pendingServlets.iterator();
-            while ( iter.hasNext() ) {
-                final PendingServlet ps = iter.next();
-                if ( ps.reference.compareTo(reference) == 0 ) {
-                    iter.remove();
-                    break;
-                }
-            }
-        }
         destroyServlet(reference);
     }
 
-    private void createAllServlets(final Collection<PendingServlet> pendingServlets) {
-        for (final PendingServlet ps : pendingServlets) {
-            createServlet(ps.servlet, ps.reference);
-        }
-    }
-
-
     private boolean createServlet(final Servlet servlet, final ServiceReference<Servlet> reference) {
         // check for a name, this is required
         final String name = getName(reference);
@@ -316,14 +266,4 @@ public class ServletMounter {
             this.registrations = srs;
         }
     }
-
-    private static final class PendingServlet {
-        public final Servlet servlet;
-        public final ServiceReference<Servlet> reference;
-
-        public PendingServlet(final Servlet s, final ServiceReference<Servlet> ref) {
-            this.servlet = s;
-            this.reference = ref;
-        }
-    }
 }
diff --git a/src/test/java/org/apache/sling/servlets/resolver/internal/resource/ServletMounterTest.java b/src/test/java/org/apache/sling/servlets/resolver/internal/resource/ServletMounterTest.java
index cf90c49..26c457f 100644
--- a/src/test/java/org/apache/sling/servlets/resolver/internal/resource/ServletMounterTest.java
+++ b/src/test/java/org/apache/sling/servlets/resolver/internal/resource/ServletMounterTest.java
@@ -22,7 +22,6 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 
-import java.lang.reflect.Field;
 import java.lang.reflect.Method;
 import java.util.Collections;
 import java.util.Dictionary;
@@ -56,14 +55,6 @@ public class ServletMounterTest {
         final ResourceResolverFactory factory = Mockito.mock(ResourceResolverFactory.class);
         Mockito.when(factory.getSearchPath()).thenReturn(Collections.singletonList("/"));
 
-        // create mounter
-        this.mounter = new ServletMounter();
-
-        // set resource resolver factory
-        final Field resolverField = ServletMounter.class.getDeclaredField("resourceResolverFactory");
-        resolverField.setAccessible(true);
-        resolverField.set(mounter, factory);
-
         // create mock bundle
         final Bundle bundle = Mockito.mock(Bundle.class);
         Mockito.when(bundle.getBundleId()).thenReturn(1L);
@@ -72,8 +63,8 @@ public class ServletMounterTest {
         final BundleContext bundleContext = Mockito.mock(BundleContext.class);
         Mockito.when(bundle.getBundleContext()).thenReturn(bundleContext);
 
-        // activate mounter
-        this.mounter.activate(bundleContext, config);
+        // create mounter
+        this.mounter = new ServletMounter(bundleContext, factory, null, config);
     }