Mailing-List: contact commits-help@sling.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@sling.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1239916 - in
 /sling/trunk/bundles/jcr/resource/src/main/java/org/apache/sling/jcr/resource/internal:
 JcrResourceResolver.java helper/MapEntries.java
Date: Thu, 02 Feb 2012 23:13:22 -0000
To: commits@sling.apache.org
From: fmeschbe@apache.org
Message-Id: <20120202231322.6DBF42388865@eris.apache.org>

Author: fmeschbe
Date: Thu Feb  2 23:13:21 2012
New Revision: 1239916

URL: http://svn.apache.org/viewvc?rev=1239916&view=rev
Log:
SLING-2321 Enhance event handler filter to also capture /etc/map updates
SLING-2398 Refactor asynchronous map initialization with a single thread

Modified:
    sling/trunk/bundles/jcr/resource/src/main/java/org/apache/sling/jcr/resource/internal/JcrResourceResolver.java
    sling/trunk/bundles/jcr/resource/src/main/java/org/apache/sling/jcr/resource/internal/helper/MapEntries.java

Modified: sling/trunk/bundles/jcr/resource/src/main/java/org/apache/sling/jcr/resource/internal/JcrResourceResolver.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/jcr/resource/src/main/java/org/apache/sling/jcr/resource/internal/JcrResourceResolver.java?rev=1239916&r1=1239915&r2=1239916&view=diff
==============================================================================
--- sling/trunk/bundles/jcr/resource/src/main/java/org/apache/sling/jcr/resource/internal/JcrResourceResolver.java (original)
+++ sling/trunk/bundles/jcr/resource/src/main/java/org/apache/sling/jcr/resource/internal/JcrResourceResolver.java Thu Feb  2 23:13:21 2012
@@ -98,6 +98,8 @@ public class JcrResourceResolver
 
     public static final String PROP_REDIRECT_EXTERNAL_STATUS = "sling:status";
 
+    public static final String PROP_REDIRECT_EXTERNAL_REDIRECT_STATUS = "sling:status";
+
     // The suffix of a resource being a content node of some parent
     // such as nt:file. The slash is included to prevent false
     // positives for the String.endsWith check for names like

Modified: sling/trunk/bundles/jcr/resource/src/main/java/org/apache/sling/jcr/resource/internal/helper/MapEntries.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/jcr/resource/src/main/java/org/apache/sling/jcr/resource/internal/helper/MapEntries.java?rev=1239916&r1=1239915&r2=1239916&view=diff
==============================================================================
--- sling/trunk/bundles/jcr/resource/src/main/java/org/apache/sling/jcr/resource/internal/helper/MapEntries.java (original)
+++ sling/trunk/bundles/jcr/resource/src/main/java/org/apache/sling/jcr/resource/internal/helper/MapEntries.java Thu Feb  2 23:13:21 2012
@@ -34,6 +34,9 @@ import java.util.Map.Entry;
 import java.util.SortedMap;
 import java.util.TreeMap;
 import java.util.TreeSet;
+import java.util.concurrent.Semaphore;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.locks.ReentrantLock;
 
 import javax.servlet.http.HttpServletResponse;
 
@@ -69,7 +72,7 @@ public class MapEntries implements Event
 
     private JcrResourceResolverFactoryImpl factory;
 
-    private ResourceResolver resolver;
+    private volatile ResourceResolver resolver;
 
     private final String mapRoot;
 
@@ -79,11 +82,13 @@ public class MapEntries implements Event
 
     private Collection<String> vanityTargets;
 
-    private boolean initializing = false;
+    private ServiceRegistration registration;
 
-    private final ServiceRegistration registration;
+    private ServiceTracker eventAdminTracker;
 
-    private final ServiceTracker eventAdminTracker;
+    private final Semaphore initTrigger = new Semaphore(0);
+
+    private final ReentrantLock initializing = new ReentrantLock();
 
     private MapEntries() {
         this.factory = null;
@@ -106,16 +111,25 @@ public class MapEntries implements Event
         this.mapRoot = factory.getMapRoot();
         this.eventAdminTracker = eventAdminTracker;
 
-        init();
+        this.resolveMaps = Collections.<MapEntry> emptyList();
+        this.mapMaps = Collections.<MapEntry> emptyList();
+        this.vanityTargets = Collections.<String> emptySet();
+
+        doInit();
 
         // build a filter which matches if any of the nodeProps (JCR
         // properties modified) is listed in any of the eventProps (event
         // properties listing modified JCR properties)
         // this allows to only get events interesting for updating the
         // internal structure
-        final String[] nodeProps = { "sling:vanityPath", "sling:vanityOrder", "sling:redirect" };
-        final String[] eventProps = { "resourceAddedAttributes", "resourceChangedAttributes",
-            "resourceRemovedAttributes" };
+        final String[] nodeProps = {
+            "sling:vanityPath", "sling:vanityOrder", JcrResourceResolver.PROP_REDIRECT_EXTERNAL_REDIRECT_STATUS,
+            JcrResourceResolver.PROP_REDIRECT_EXTERNAL, JcrResourceResolver.PROP_REDIRECT_INTERNAL,
+            JcrResourceResolver.PROP_REDIRECT_EXTERNAL_STATUS
+        };
+        final String[] eventProps = {
+            "resourceAddedAttributes", "resourceChangedAttributes", "resourceRemovedAttributes"
+        };
         StringBuilder filter = new StringBuilder();
         filter.append("(|");
         for (String eventProp : eventProps) {
@@ -134,20 +148,56 @@ public class MapEntries implements Event
         props.put(Constants.SERVICE_DESCRIPTION, "Map Entries Observation");
         props.put(Constants.SERVICE_VENDOR, "The Apache Software Foundation");
         this.registration = bundleContext.registerService(EventHandler.class.getName(), this, props);
-    }
 
-    private void init() {
-        synchronized (this) {
-            // no initialization if the session has already been reset
-            if (resolver == null) {
-                return;
+        Thread updateThread = new Thread(new Runnable() {
+            public void run() {
+                MapEntries.this.init();
             }
+        }, "MapEntries Update");
+        updateThread.start();
+    }
+
+    /**
+     * Signals the init method that a the doInit method should be
+     * called.
+     */
+    private void triggerInit() {
+        // only release if there is not one in the queue already
+        if (initTrigger.availablePermits() < 1) {
+            initTrigger.release();
+        }
+    }
 
-            // set the flag
-            initializing = true;
+    /**
+     * Runs as the method of the update thread. Waits for the triggerInit
+     * method to trigger a call to doInit. Terminates when the resolver
+     * has been null-ed after having been triggered.
+     */
+    void init() {
+        while (MapEntries.this.resolver != null) {
+            try {
+                MapEntries.this.initTrigger.acquire();
+                MapEntries.this.doInit();
+            } catch (InterruptedException ie) {
+                // just continue acquisition
+            }
         }
 
+    }
+
+    /**
+     * Actual initializer. Guards itself agains concurrent use by
+     * using a ReentrantLock. Does nothing if the resource resolver
+     * has already been null-ed.
+     */
+    private void doInit() {
+
+        this.initializing.lock();
         try {
+            final ResourceResolver resolver = this.resolver;
+            if (resolver == null) {
+                return;
+            }
 
             List<MapEntry> newResolveMaps = new ArrayList<MapEntry>();
             SortedMap<String, MapEntry> newMapMaps = new TreeMap<String, MapEntry>();
@@ -171,45 +221,72 @@ public class MapEntries implements Event
 
             sendChangeEvent();
 
+        } catch (Exception e) {
+
+            log.warn("doInit: Unexpected problem during initialization", e);
+
         } finally {
 
-            // reset the flag and notify listeners
-            synchronized (this) {
-                initializing = false;
-                notifyAll();
-            }
+            this.initializing.unlock();
+
         }
     }
 
+    /**
+     * Cleans up this class.
+     */
     public void dispose() {
-        final ResourceResolver oldResolver;
+        if ( this.registration != null ) {
+            this.registration.unregister();
+            this.registration = null;
+        }
+
+        /*
+         * Cooperation with doInit: The same lock as used by doInit
+         * is acquired thus preventing doInit from running and waiting
+         * for a concurrent doInit to terminate.
+         * Once the lock has been acquired, the resource resolver is
+         * null-ed (thus causing the init to terminate when triggered
+         * the right after and prevent the doInit method from doing any
+         * thing).
+         */
 
         // wait at most 10 seconds for a notifcation during initialization
-        synchronized (this) {
-            if (initializing) {
-                try {
-                    wait(10L * 1000L);
-                } catch (InterruptedException ie) {
-                    // ignore
-                }
+        boolean initLocked;
+        try {
+            initLocked = this.initializing.tryLock(10, TimeUnit.SECONDS);
+        } catch (InterruptedException ie) {
+            initLocked = false;
+        }
+
+        try {
+            if (!initLocked) {
+                log.warn("dispose: Could not acquire initialization lock within 10 seconds; ongoing intialization may fail");
             }
 
             // immediately set the resolver field to null to indicate
             // that we have been disposed (this also signals to the
             // event handler to stop working
-            oldResolver = resolver;
-            resolver = null;
-        }
-        if ( this.registration != null ) {
-            this.registration.unregister();
-        }
+            final ResourceResolver oldResolver = this.resolver;
+            this.resolver = null;
+
+            // trigger initialization to terminate init thread
+            triggerInit();
 
-        if (oldResolver != null) {
-            oldResolver.close();
+            if (oldResolver != null) {
+                oldResolver.close();
+            } else {
+                log.warn("dispose: ResourceResolver has already been cleared before; duplicate call to dispose ?");
+            }
+        } finally {
+            if (initLocked) {
+                this.initializing.unlock();
+            }
         }
 
         // clear the rest of the fields
-        factory = null;
+        this.factory = null;
+        this.eventAdminTracker = null;
     }
 
     public List<MapEntry> getResolveMaps() {
@@ -237,22 +314,21 @@ public class MapEntries implements Event
             final Object p = event.getProperty(SlingConstants.PROPERTY_PATH);
             if (p instanceof String) {
                 final String path = (String) p;
-                for (String target : this.vanityTargets) {
-                    if (target.startsWith(path)) {
-                        doInit = true;
-                        break;
+                doInit = path.startsWith(this.mapRoot);
+                if (!doInit) {
+                    for (String target : this.vanityTargets) {
+                        if (target.startsWith(path)) {
+                            doInit = true;
+                            break;
+                        }
                     }
                 }
             }
         }
 
+        // trigger an update
         if (doInit) {
-            final Thread t = new Thread() {
-                public void run() {
-                    init();
-                }
-            };
-            t.start();
+            triggerInit();
         }
     }
 
@@ -263,10 +339,12 @@ public class MapEntries implements Event
      */
     private void sendChangeEvent() {
         final EventAdmin ea = (EventAdmin) this.eventAdminTracker.getService();
-        if ( ea != null ) {
-            // we hard code the topic here and don't use SlingConstants.TOPIC_RESOURCE_RESOLVER_MAPPING_CHANGED
+        if (ea != null) {
+            // we hard code the topic here and don't use
+            // SlingConstants.TOPIC_RESOURCE_RESOLVER_MAPPING_CHANGED
             // to avoid requiring the latest API version for this bundle to work
-            final Event event = new Event("org/apache/sling/api/resource/ResourceResolverMapping/CHANGED", (Dictionary<?,?>)null);
+            final Event event = new Event("org/apache/sling/api/resource/ResourceResolverMapping/CHANGED",
+                (Dictionary<?, ?>) null);
             ea.postEvent(event);
         }
     }
@@ -362,7 +440,7 @@ public class MapEntries implements Event
                     // whether the target is attained by a 302/FOUND or by an
                     // internal redirect is defined by the sling:redirect property
                     int status = row.get("sling:redirect", false)
-                            ? row.get("sling:redirectStatus", HttpServletResponse.SC_FOUND)
+                            ? row.get(JcrResourceResolver.PROP_REDIRECT_EXTERNAL_REDIRECT_STATUS, HttpServletResponse.SC_FOUND)
                             : -1;
 
                     // 1. entry with exact match