- PR for JIRA GEOS-7619

 - PR for JIRA GEOS-7619

 - PR for JIRA GEOS-7619

src/main/src/main/java/org/geoserver/GeoServerConfigurationLock.java

@@ -5,11 +5,13 @@
  */
 package org.geoserver;
 
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
+import org.geoserver.platform.GeoServerExtensions;
 import org.geotools.util.logging.Logging;
 
 /**
@@ -25,6 +27,12 @@
  * 
  */
 public class GeoServerConfigurationLock {
+
+    /** DEFAULT_TRY_LOCK_TIMEOUT_MS */
+    private static final long DEFAULT_TRY_LOCK_TIMEOUT_MS =
+            (GeoServerExtensions.getProperty("CONFIGURATION_TRYLOCK_TIMEOUT") != null ? 
+                Long.valueOf(GeoServerExtensions.getProperty("CONFIGURATION_TRYLOCK_TIMEOUT")) : 5000);
+
     private static final Level LEVEL = Level.FINE;
 
     private static final Logger LOGGER = Logging.getLogger(GeoServerConfigurationLock.class);
@@ -59,22 +67,67 @@ public void lock(LockType type) {
             return;
         }
 
-        Lock lock;
-        if (type == LockType.WRITE) {
-            lock = readWriteLock.writeLock();
-        } else {
-            lock = readWriteLock.readLock();
-        }
-        if (LOGGER.isLoggable(LEVEL)) {
-            LOGGER.log(LEVEL, "Thread " + Thread.currentThread().getId() + " locking in mode "
-                    + type);
-        }
+        Lock lock = getLock(type);
+
         lock.lock();
+
         if (LOGGER.isLoggable(LEVEL)) {
             LOGGER.log(LEVEL, "Thread " + Thread.currentThread().getId() + " got the lock in mode "
                     + type);
         }
     }
+
+    /**
+     * Tries to open a lock in the specified mode. Acquires the lock if it is available and returns immediately with the value true. 
+     * If the lock is not available then this method will return immediately with the value false.
+     * 
+     * A typical usage idiom for this method would be:
+     * 
+     * <pre>
+     *  Lock lock = ...;
+     *  if (lock.tryLock()) {
+     *    try {
+     *      // manipulate protected state
+     *    } finally {
+     *      lock.unlock();
+     *    }
+     *  } else {
+     *    // perform alternative actions
+     *  }}
+     * </pre>
+     * 
+     * This usage ensures that the lock is unlocked if it was acquired, and doesn't try to unlock if the lock was not acquired.
+     *  
+     * @param type
+     * @return true if the lock was acquired and false otherwise
+     */
+    public boolean tryLock(LockType type) {
+        if (!enabled) {
+            return true;
+        }
+
+        Lock lock = getLock(type);
+
+        boolean res = false;
+        try {
+            res = lock.tryLock(DEFAULT_TRY_LOCK_TIMEOUT_MS, TimeUnit.MILLISECONDS);
+        } catch (InterruptedException e) {
+            LOGGER.log(Level.WARNING, "Thread " + Thread.currentThread().getId() + " thrown an InterruptedException on GeoServerConfigurationLock TryLock.", e);
+            res = false;
+        }
+
+        if (LOGGER.isLoggable(LEVEL)) {
+            if (res) {
+                LOGGER.log(LEVEL, "Thread " + Thread.currentThread().getId() + " got the lock in mode "
+                        + type);
+            } else {
+                LOGGER.log(LEVEL, "Thread " + Thread.currentThread().getId() + " could not get the lock in mode "
+                        + type);
+            }
+        }
+
+        return res;
+    }
 
     /**
      * Unlocks a previously acquired lock. The lock type must match the previous
@@ -87,12 +140,8 @@ public void unlock(LockType type) {
             return;
         }
 
-        Lock lock;
-        if (type == LockType.WRITE) {
-            lock = readWriteLock.writeLock();
-        } else {
-            lock = readWriteLock.readLock();
-        }
+        Lock lock  = getLock(type);
+
         if (LOGGER.isLoggable(LEVEL)) {
             LOGGER.log(LEVEL, "Thread " + Thread.currentThread().getId()
                     + " releasing the lock in mode " + type);
@@ -108,4 +157,22 @@ public void setEnabled(boolean enabled) {
         this.enabled = enabled;
     }
 
+    /**
+     * @param type
+     * @return
+     */
+    private Lock getLock(LockType type) {
+        Lock lock;
+        if (type == LockType.WRITE) {
+            lock = readWriteLock.writeLock();
+        } else {
+            lock = readWriteLock.readLock();
+        }
+        if (LOGGER.isLoggable(LEVEL)) {
+            LOGGER.log(LEVEL, "Thread " + Thread.currentThread().getId() + " locking in mode "
+                    + type);
+        }
+        return lock;
+    }
+
 }

src/web/core/src/main/java/org/geoserver/web/GeoServerApplication.java

@@ -340,27 +340,27 @@ public void onDetach(org.apache.wicket.request.cycle.RequestCycle cycle) {
 
         @Override
         public void onRequestHandlerScheduled(RequestCycle cycle, IRequestHandler handler) {
-            processHandler(handler);
+            processHandler(cycle, handler);
         }
 
-        private void processHandler(IRequestHandler handler) {
+        private void processHandler(RequestCycle cycle, IRequestHandler handler) {
             if(handler instanceof IPageRequestHandler) {
                 IPageRequestHandler pageHandler = (IPageRequestHandler) handler;
                 Class<? extends IRequestablePage> pageClass = pageHandler.getPageClass();
                 for (WicketCallback callback : callbacks) {
-                    callback.onRequestTargetSet(pageClass);
+                    callback.onRequestTargetSet(cycle, pageClass);
                 }
             } else if(handler instanceof IRequestHandlerDelegate) {
                 IRequestHandlerDelegate delegator = (IRequestHandlerDelegate) handler;
-                processHandler(delegator.getDelegateHandler());
+                processHandler(cycle, delegator.getDelegateHandler());
             }
 
         }
 
         @Override
         public void onRequestHandlerResolved(org.apache.wicket.request.cycle.RequestCycle cycle,
                 IRequestHandler handler) {
-            processHandler(handler);
+            processHandler(cycle, handler);
         }
 
         @Override

src/web/core/src/main/java/org/geoserver/web/GeoServerHomePage.java

@@ -57,7 +57,7 @@
  * @author Andrea Aime - TOPP
  * 
  */
-public class GeoServerHomePage extends GeoServerBasePage {
+public class GeoServerHomePage extends GeoServerBasePage implements GeoServerUnlockablePage {
 
     @SuppressWarnings({ "rawtypes", "unchecked" })
     public GeoServerHomePage() {

src/web/core/src/main/java/org/geoserver/web/GeoServerUnlockablePage.java

@@ -0,0 +1,17 @@
+/* (c) 2016 Open Source Geospatial Foundation - all rights reserved
+ * This code is licensed under the GPL 2.0 license, available at the root
+ * application directory.
+ */
+package org.geoserver.web;
+
+/**
+ * Interface for GeoServer Pages which can simply ignore ConfigurationLock since they are safe.
+ * 
+ * The developer *MUST* know what he is doing and manage the configuration read/write safely!
+ * 
+ * @author Alessio Fabiani, GeoSolutions S.A.S.
+ *
+ */
+public interface GeoServerUnlockablePage {
+
+}

src/web/core/src/main/java/org/geoserver/web/ServerBusyPage.html

@@ -0,0 +1,7 @@
+<html xmlns:wicket="http://wicket.apache.org/">
+<body>
+<wicket:extend>
+<p wicket:id="serverBusyMessage">Server is currently busy. Please wait...</p>
+</wicket:extend>
+</body>
+</html>

src/web/core/src/main/java/org/geoserver/web/ServerBusyPage.java

@@ -0,0 +1,20 @@
+/* (c) 2016 Open Source Geospatial Foundation - all rights reserved
+ * This code is licensed under the GPL 2.0 license, available at the root
+ * application directory.
+ */
+package org.geoserver.web;
+
+import org.apache.wicket.markup.html.basic.Label;
+import org.apache.wicket.model.IModel;
+import org.apache.wicket.model.ResourceModel;
+
+/**
+ * Displays a message suggesting the user to login or to elevate his privileges
+ */
+public class ServerBusyPage extends GeoServerBasePage implements GeoServerUnlockablePage {
+
+    public ServerBusyPage() {
+        IModel model = new ResourceModel( "ServerBusyPage.serverBusyMessage" );
+        add(new Label("serverBusyMessage", model));
+    }
+}

src/web/core/src/main/java/org/geoserver/web/WicketCallback.java

@@ -34,10 +34,24 @@ public interface WicketCallback {
     /**
      * Called when a request target is set on the request cycle
      * 
+     * This method is taken for retro-compatibility with old GeoServer verisons
+     * 
      * @param requestTarget
+     * @deprecated replaced by {@link #onRequestTargetSet(cycle, requestTarget)}
      */
     void onRequestTargetSet(Class<? extends IRequestablePage> requestTarget);
 
+    /**
+     * Called when a request target is set on the request cycle
+     * 
+     * @param cycle
+     * @param requestTarget
+     */
+    default void onRequestTargetSet(RequestCycle cycle, 
+            Class<? extends IRequestablePage> requestTarget) {
+        onRequestTargetSet(requestTarget);
+    }
+
     /**
      * Called when a runtime exception is thrown, just before the actual handling of the runtime
      * exception.

src/web/core/src/main/java/org/geoserver/web/WicketConfigurationLockCallback.java

@@ -25,7 +25,7 @@ public class WicketConfigurationLockCallback implements WicketCallback {
     GeoServerConfigurationLock locker;
 
     static ThreadLocal<LockType> THREAD_LOCK = new ThreadLocal<GeoServerConfigurationLock.LockType>();
-
+    
     public WicketConfigurationLockCallback(GeoServerConfigurationLock locker) {
         this.locker = locker;
     }
@@ -50,25 +50,56 @@ public void onEndRequest() {
     }
 
     @Override
+    @Deprecated
     public void onRequestTargetSet(Class<? extends IRequestablePage> requestTarget) {
-        // we can have many of these calls per http call, avoid locking multiple times,
-        // onEndRequest will be called just once
-        LockType type = THREAD_LOCK.get();
-        if (type != null || requestTarget == null) {
-            return;
+        onRequestTargetSet(null, requestTarget);
+    }
+
+    @Override
+    public void onRequestTargetSet(RequestCycle cycle,
+            Class<? extends IRequestablePage> requestTarget) {
+
+        if (!GeoServerUnlockablePage.class.isAssignableFrom(requestTarget)) {
+            LockType type = THREAD_LOCK.get();
+            if (type != null || requestTarget == null) {
+                return;
+            }
+
+            boolean lockTaken = false;
+            if (type == null) {
+                type = getLockType(requestTarget);
+
+                // and lock
+                lockTaken = locker.tryLock(type);
+
+                if (lockTaken) {
+                    THREAD_LOCK.set(type);
+                }
+            }
+
+            // Check if the configuration is locked and the page is safe...
+            if (cycle != null && !lockTaken) {
+                cycle.setResponsePage(ServerBusyPage.class);
+            }
+
         }
+    }
 
+    /**
+     * @param requestTarget
+     * @param type
+     * @return
+     */
+    private LockType getLockType(Class<? extends IRequestablePage> requestTarget) {
+        LockType type = null;
         // setup a write lock for secured pages, a read one for the others
         if (GeoServerSecuredPage.class.isAssignableFrom(requestTarget)) {
             type = LockType.WRITE;
         }
         if (type == null) {
             type = LockType.READ;
         }
-
-        // and lock
-        THREAD_LOCK.set(type);
-        locker.lock(type);
+        return type;
     }
 
     @Override

src/web/core/src/main/resources/GeoServerApplication.properties

@@ -672,6 +672,10 @@ UnauthorizedPage.insufficientPrivileges = The current user does not have suffici
 UnauthorizedPage.unauthorizedMessage = The current user does not have sufficient privileges to access the page or \
         perform the requested operation
 
+ServerBusyPage.title               = Server busy
+ServerBusyPage.description         =
+ServerBusyPage.serverBusyMessage   = Server is currently busy. Please wait...
+
 WFSDataStoreFactory\:BUFFER_SIZE = Feature buffer size
 
 WFSDataStoreFactory\:ENCODING = Character encoding for XML messages

src/web/core/src/test/java/org/geoserver/web/admin/AbstractAdminPrivilegeTest.java

@@ -14,13 +14,10 @@
 import java.util.Arrays;
 import java.util.Iterator;
 
-import org.apache.wicket.core.request.handler.PageProvider;
 import org.apache.wicket.core.request.handler.RenderPageRequestHandler;
 import org.apache.wicket.markup.html.form.DropDownChoice;
 import org.apache.wicket.markup.repeater.Item;
 import org.apache.wicket.markup.repeater.data.DataView;
-import org.apache.wicket.request.IRequestHandler;
-import org.apache.wicket.request.Response;
 import org.apache.wicket.request.cycle.RequestCycle;
 import org.apache.wicket.request.mapper.parameter.PageParameters;
 import org.geoserver.catalog.Catalog;