You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by rg...@apache.org on 2014/10/10 11:59:58 UTC

svn commit: r1630749 [2/6] - in /qpid/branches/QPID-6125-ProtocolRefactoring/java: ./ bdbstore/jmx/src/main/java/org/apache/qpid/server/store/berkeleydb/jmx/ bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/ bdbstore/src/main/java/org/apa...

Modified: qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/model/JsonSystemConfigImpl.java
URL: http://svn.apache.org/viewvc/qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/model/JsonSystemConfigImpl.java?rev=1630749&r1=1630748&r2=1630749&view=diff
==============================================================================
--- qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/model/JsonSystemConfigImpl.java (original)
+++ qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/model/JsonSystemConfigImpl.java Fri Oct 10 09:59:55 2014
@@ -38,9 +38,10 @@ public class JsonSystemConfigImpl extend
     public JsonSystemConfigImpl(final TaskExecutor taskExecutor,
                                 final EventLogger eventLogger,
                                 final LogRecorder logRecorder,
-                                final BrokerOptions brokerOptions)
+                                final BrokerOptions brokerOptions,
+                                final BrokerShutdownProvider brokerShutdownProvider)
     {
-        super(taskExecutor, eventLogger, logRecorder, brokerOptions);
+        super(taskExecutor, eventLogger, logRecorder, brokerOptions, brokerShutdownProvider);
     }
 
     public String getStorePath()

Modified: qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/model/SystemConfig.java
URL: http://svn.apache.org/viewvc/qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/model/SystemConfig.java?rev=1630749&r1=1630748&r2=1630749&view=diff
==============================================================================
--- qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/model/SystemConfig.java (original)
+++ qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/model/SystemConfig.java Fri Oct 10 09:59:55 2014
@@ -37,4 +37,6 @@ public interface SystemConfig<X extends 
     LogRecorder getLogRecorder();
 
     DurableConfigurationStore getConfigurationStore();
+
+    BrokerShutdownProvider getBrokerShutdownProvider();
 }

Modified: qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java
URL: http://svn.apache.org/viewvc/qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java?rev=1630749&r1=1630748&r2=1630749&view=diff
==============================================================================
--- qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java (original)
+++ qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java Fri Oct 10 09:59:55 2014
@@ -943,6 +943,28 @@ public class BrokerAdapter extends Abstr
         _eventLogger = eventLogger;
     }
 
+    @Override
+    protected void onExceptionInOpen(RuntimeException e)
+    {
+        SystemConfig systemConfig = getParent(SystemConfig.class);
+        if (systemConfig != null)
+        {
+            BrokerShutdownProvider shutdownProvider = systemConfig.getBrokerShutdownProvider();
+            if (shutdownProvider != null)
+            {
+                shutdownProvider.shutdown();
+            }
+            else
+            {
+                throw new IllegalStateException("Shutdown provider is not found in system config");
+            }
+        }
+        else
+        {
+            throw new IllegalStateException("SystemConfig is not found among broker parents");
+        }
+    }
+
     public void registerMessageDelivered(long messageSize)
     {
         _messagesDelivered.registerEvent(1L);

Modified: qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/FileBasedGroupProviderImpl.java
URL: http://svn.apache.org/viewvc/qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/FileBasedGroupProviderImpl.java?rev=1630749&r1=1630748&r2=1630749&view=diff
==============================================================================
--- qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/FileBasedGroupProviderImpl.java (original)
+++ qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/FileBasedGroupProviderImpl.java Fri Oct 10 09:59:55 2014
@@ -114,22 +114,27 @@ public class FileBasedGroupProviderImpl
             throw new IllegalArgumentException("Cannot change the path");
         }
     }
+
+    @Override
     protected void onOpen()
     {
         super.onOpen();
-        if(_groupDatabase == null)
+        FileGroupDatabase groupDatabase = new FileGroupDatabase();
+        try
         {
-            _groupDatabase = new FileGroupDatabase();
-            try
-            {
-                _groupDatabase.setGroupFile(getPath());
-            }
-            catch (IOException e)
+            groupDatabase.setGroupFile(getPath());
+        }
+        catch(IOException | RuntimeException e)
+        {
+            if (e instanceof IllegalConfigurationException)
             {
-                setState(State.ERRORED);
-                LOGGER.warn(("Unable to open preferences file at " + _path));
+                throw (IllegalConfigurationException) e;
             }
+            throw new IllegalConfigurationException(String.format("Cannot load groups from '%s'", getPath()), e);
         }
+
+        _groupDatabase = groupDatabase;
+
         Set<Principal> groups = getGroupPrincipals();
         Collection<Group> principals = new ArrayList<Group>(groups.size());
         for (Principal group : groups)
@@ -150,43 +155,47 @@ public class FileBasedGroupProviderImpl
     protected void onCreate()
     {
         super.onCreate();
-        _groupDatabase = new FileGroupDatabase();
-
         File file = new File(_path);
         if (!file.exists())
         {
             File parent = file.getParentFile();
-            if (!parent.exists())
+            if (!parent.exists() && !file.getParentFile().mkdirs())
             {
-                parent.mkdirs();
+                throw new IllegalConfigurationException(String.format("Cannot create groups file at '%s'",_path));
             }
-            if (parent.exists())
+            try
             {
-                try
-                {
-                    file.createNewFile();
-                }
-                catch (IOException e)
-                {
-                    throw new IllegalConfigurationException("Cannot create group file");
-                }
+                file.createNewFile();
             }
-            else
+            catch (IOException e)
             {
-                throw new IllegalConfigurationException("Cannot create group file");
+                throw new IllegalConfigurationException(String.format("Cannot create groups file at '%s'", _path), e);
             }
         }
-        try
-        {
-            _groupDatabase.setGroupFile(getPath());
-        }
-        catch (IOException e)
-        {
-            setState(State.ERRORED);
-            LOGGER.warn(("Unable to open preferences file at " + _path));
-        }
+    }
 
+    @Override
+    protected void validateOnCreate()
+    {
+        super.validateOnCreate();
+        File groupsFile = new File(_path);
+        if (groupsFile.exists())
+        {
+            if (!groupsFile.canRead())
+            {
+                throw new IllegalConfigurationException(String.format("Cannot read groups file '%s'. Please check permissions.", _path));
+            }
 
+            FileGroupDatabase groupDatabase = new FileGroupDatabase();
+            try
+            {
+                groupDatabase.setGroupFile(_path);
+            }
+            catch (Exception e)
+            {
+                throw new IllegalConfigurationException(String.format("Cannot load groups from '%s'", _path), e);
+            }
+        }
     }
 
     @Override
@@ -205,6 +214,11 @@ public class FileBasedGroupProviderImpl
 
             getSecurityManager().authoriseGroupOperation(Operation.CREATE, groupName);
 
+            if (getState() != State.ACTIVE)
+            {
+                throw new IllegalConfigurationException(String.format("Group provider '%s' is not activated. Cannot create a group.", getName()));
+            }
+
             _groupDatabase.createGroup(groupName);
 
             Map<String,Object> attrMap = new HashMap<String, Object>();
@@ -247,20 +261,22 @@ public class FileBasedGroupProviderImpl
         return _broker.getSecurityManager();
     }
 
-    @StateTransition( currentState = { State.UNINITIALIZED, State.QUIESCED }, desiredState = State.ACTIVE )
+    @StateTransition( currentState = { State.UNINITIALIZED, State.QUIESCED, State.ERRORED }, desiredState = State.ACTIVE )
     private void activate()
     {
-        try
+        if (_groupDatabase != null)
         {
-            _groupDatabase.setGroupFile(getPath());
             setState(State.ACTIVE);
         }
-        catch(IOException | RuntimeException e)
+        else
         {
-            setState(State.ERRORED);
             if (_broker.isManagementMode())
             {
-                LOGGER.warn("Failed to activate group provider: " + getName(), e);
+                        LOGGER.warn("Failed to activate group provider: " + getName());
+            }
+            else
+            {
+                throw new IllegalConfigurationException(String.format("Cannot load groups from '%s'", getPath()));
             }
         }
     }
@@ -268,6 +284,7 @@ public class FileBasedGroupProviderImpl
     @StateTransition( currentState = { State.QUIESCED, State.ACTIVE, State.ERRORED}, desiredState = State.DELETED )
     private void doDelete()
     {
+        close();
         File file = new File(getPath());
         if (file.exists())
         {
@@ -289,7 +306,7 @@ public class FileBasedGroupProviderImpl
 
     public Set<Principal> getGroupPrincipalsForUser(String username)
     {
-        Set<String> groups = _groupDatabase.getGroupsForUser(username);
+        Set<String> groups = _groupDatabase == null ? Collections.<String>emptySet(): _groupDatabase.getGroupsForUser(username);
         if (groups.isEmpty())
         {
             return Collections.emptySet();

Modified: qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/FileSystemPreferencesProviderImpl.java
URL: http://svn.apache.org/viewvc/qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/FileSystemPreferencesProviderImpl.java?rev=1630749&r1=1630748&r2=1630749&view=diff
==============================================================================
--- qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/FileSystemPreferencesProviderImpl.java (original)
+++ qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/FileSystemPreferencesProviderImpl.java Fri Oct 10 09:59:55 2014
@@ -76,20 +76,64 @@ public class FileSystemPreferencesProvid
         _authenticationProvider = authenticationProvider;
     }
 
-    @StateTransition( currentState = State.UNINITIALIZED, desiredState = State.ACTIVE )
+    @Override
+    protected void validateOnCreate()
+    {
+        super.validateOnCreate();
+        File storeFile  = new File(_path);
+        if (storeFile.exists() )
+        {
+            if (!storeFile.canRead())
+            {
+                throw new IllegalConfigurationException(String.format("Cannot read preferences file '%s'. Please check permissions.", _path));
+            }
+
+            FileSystemPreferencesStore store = null;
+            try
+            {
+                store = new FileSystemPreferencesStore(storeFile);
+                store.open();
+            }
+            catch (RuntimeException e)
+            {
+                if (e instanceof IllegalConfigurationException)
+                {
+                    throw e;
+                }
+                throw new IllegalConfigurationException(String.format("Cannot open preferences store at '%s'", _path), e);
+            }
+            finally
+            {
+                if (store != null)
+                {
+                    store.close();
+                }
+            }
+        }
+    }
+
+    @Override
+    protected void onOpen()
+    {
+        FileSystemPreferencesStore store = new FileSystemPreferencesStore(new File(_path));
+
+        // we need to check and create file if it does not exist every time on open
+        store.createIfNotExist();
+        store.open();
+        _store = store;
+        _open = true;
+    }
+
+    @StateTransition( currentState = {State.UNINITIALIZED, State.ERRORED}, desiredState = State.ACTIVE )
     private void activate()
     {
-        try
+        if (_store != null)
         {
-            _store = new FileSystemPreferencesStore(new File(_path));
-            createStoreIfNotExist();
-            _store.open();
-            _open = true;
             setState(State.ACTIVE);
         }
-        catch( RuntimeException e )
+        else
         {
-            setState(State.ERRORED);
+            throw new IllegalStateException("Cannot open preferences provider " + getName() + " in state " + getState() );
         }
     }
 
@@ -148,9 +192,14 @@ public class FileSystemPreferencesProvid
         setState(State.DELETED);
     }
 
-    @StateTransition(currentState = { State.QUIESCED, State.ERRORED }, desiredState = State.ACTIVE )
+    @StateTransition(currentState = State.QUIESCED, desiredState = State.ACTIVE )
     private void restart()
     {
+        if (_store == null)
+        {
+            throw new IllegalStateException("Cannot open preferences provider " + getName() + " in state " + getState() );
+        }
+
         _store.open();
         setState(State.ACTIVE);
     }
@@ -158,24 +207,39 @@ public class FileSystemPreferencesProvid
     @Override
     public Map<String, Object> getPreferences(String userId)
     {
-        return _store.getPreferences(userId);
+        return _store == null? Collections.<String, Object>emptyMap() : _store.getPreferences(userId);
     }
 
     @Override
     public Map<String, Object> setPreferences(String userId, Map<String, Object> preferences)
     {
+        if (_store == null)
+        {
+            throw new IllegalStateException("Cannot set preferences with preferences provider " + getName() + " in state " + getState() );
+        }
+
         return _store.setPreferences(userId, preferences);
     }
 
     @Override
     public String[] deletePreferences(String... userIDs)
     {
+        if (_store == null)
+        {
+            throw new IllegalStateException("Cannot delete preferences with preferences provider " + getName() + " in state " + getState() );
+        }
+
         return _store.deletePreferences(userIDs);
     }
 
     @Override
     public Set<String> listUserIDs()
     {
+        if (_store == null)
+        {
+            return Collections.emptySet();
+        }
+
         return _store.listUserIDs();
     }
 
@@ -215,9 +279,10 @@ public class FileSystemPreferencesProvid
             }
             else
             {
-                _store = new FileSystemPreferencesStore(new File(_path));
-                createStoreIfNotExist();
-                _store.open();
+                FileSystemPreferencesStore store = new FileSystemPreferencesStore(new File(_path));
+                store.createIfNotExist();
+                store.open();
+                _store = store;
             }
         }
     }
@@ -265,11 +330,6 @@ public class FileSystemPreferencesProvid
 
     }
 
-    private void createStoreIfNotExist()
-    {
-        _store.createIfNotExist();
-    }
-
     public static class FileSystemPreferencesStore
     {
         private final ObjectMapper _objectMapper;
@@ -294,18 +354,18 @@ public class FileSystemPreferencesProvid
                 File parent = _storeFile.getParentFile();
                 if (!parent.exists() && !parent.mkdirs())
                 {
-                    throw new IllegalConfigurationException("Cannot create preferences store folders");
+                    throw new IllegalConfigurationException(String.format("Cannot create preferences store folder at '%s'", _storeFile.getAbsolutePath()));
                 }
                 try
                 {
                     if (_storeFile.createNewFile() && !_storeFile.exists())
                     {
-                        throw new IllegalConfigurationException("Preferences store file was not created:" + _storeFile.getAbsolutePath());
+                        throw new IllegalConfigurationException(String.format("Cannot create preferences store file at '%s'", _storeFile.getAbsolutePath()));
                     }
                 }
                 catch (IOException e)
                 {
-                    throw new IllegalConfigurationException("Cannot create preferences store file");
+                    throw new IllegalConfigurationException(String.format("Cannot create preferences store file at '%s'", _storeFile.getAbsolutePath()), e);
                 }
             }
         }

Modified: qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/model/port/AbstractPort.java
URL: http://svn.apache.org/viewvc/qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/model/port/AbstractPort.java?rev=1630749&r1=1630748&r2=1630749&view=diff
==============================================================================
--- qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/model/port/AbstractPort.java (original)
+++ qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/model/port/AbstractPort.java Fri Oct 10 09:59:55 2014
@@ -317,7 +317,7 @@ abstract public class AbstractPort<X ext
         setState(State.DELETED);
     }
 
-    @StateTransition( currentState = {State.UNINITIALIZED, State.QUIESCED}, desiredState = State.ACTIVE )
+    @StateTransition( currentState = {State.UNINITIALIZED, State.QUIESCED, State.ERRORED}, desiredState = State.ACTIVE )
     protected void activate()
     {
         try
@@ -327,8 +327,7 @@ abstract public class AbstractPort<X ext
         catch (RuntimeException e)
         {
             setState(State.ERRORED);
-            LOGGER.error("Unable to active port '" + getName() + "'of type " + getType() + " on port " + getPort(),
-                         e);
+            throw new IllegalConfigurationException("Unable to active port '" + getName() + "'of type " + getType() + " on " + getPort(), e);
         }
     }
 

Modified: qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/model/port/AmqpPortImpl.java
URL: http://svn.apache.org/viewvc/qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/model/port/AmqpPortImpl.java?rev=1630749&r1=1630748&r2=1630749&view=diff
==============================================================================
--- qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/model/port/AmqpPortImpl.java (original)
+++ qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/model/port/AmqpPortImpl.java Fri Oct 10 09:59:55 2014
@@ -34,6 +34,7 @@ import javax.net.ssl.SSLContext;
 import javax.net.ssl.TrustManager;
 import javax.net.ssl.X509TrustManager;
 
+import org.apache.qpid.server.util.PortUtil;
 import org.codehaus.jackson.map.ObjectMapper;
 
 import org.apache.qpid.server.configuration.BrokerProperties;
@@ -187,6 +188,18 @@ public class AmqpPortImpl extends Abstra
         }
     }
 
+    @Override
+    public void validateOnCreate()
+    {
+        super.validateOnCreate();
+        String bindingAddress = getBindingAddress();
+        if (!PortUtil.isPortAvailable(bindingAddress, getPort()))
+        {
+            throw new IllegalConfigurationException(String.format("Cannot bind to port %d and binding address '%s'. Port is already is use.",
+                    getPort(), bindingAddress == null || "".equals(bindingAddress) ? "*" : bindingAddress));
+        }
+    }
+
     private SSLContext createSslContext()
     {
         KeyStore keyStore = getKeyStore();

Modified: qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/model/port/HttpPortImpl.java
URL: http://svn.apache.org/viewvc/qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/model/port/HttpPortImpl.java?rev=1630749&r1=1630748&r2=1630749&view=diff
==============================================================================
--- qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/model/port/HttpPortImpl.java (original)
+++ qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/model/port/HttpPortImpl.java Fri Oct 10 09:59:55 2014
@@ -22,10 +22,12 @@ package org.apache.qpid.server.model.por
 
 import java.util.Map;
 
+import org.apache.qpid.server.configuration.IllegalConfigurationException;
 import org.apache.qpid.server.model.Broker;
 import org.apache.qpid.server.model.ManagedAttributeField;
 import org.apache.qpid.server.model.ManagedObjectFactoryConstructor;
 import org.apache.qpid.server.model.State;
+import org.apache.qpid.server.util.PortUtil;
 
 public class HttpPortImpl extends AbstractClientAuthCapablePortWithAuthProvider<HttpPortImpl> implements HttpPort<HttpPortImpl>
 {
@@ -65,4 +67,16 @@ public class HttpPortImpl extends Abstra
             return State.QUIESCED;
         }
     }
+
+    @Override
+    public void validateOnCreate()
+    {
+        super.validateOnCreate();
+        String bindingAddress = getBindingAddress();
+        if (!PortUtil.isPortAvailable(bindingAddress, getPort()))
+        {
+            throw new IllegalConfigurationException(String.format("Cannot bind to port %d and binding address '%s'. Port is already is use.",
+                    getPort(), bindingAddress == null || "".equals(bindingAddress) ? "*" : bindingAddress));
+        }
+    }
 }

Modified: qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/plugin/SystemConfigFactory.java
URL: http://svn.apache.org/viewvc/qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/plugin/SystemConfigFactory.java?rev=1630749&r1=1630748&r2=1630749&view=diff
==============================================================================
--- qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/plugin/SystemConfigFactory.java (original)
+++ qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/plugin/SystemConfigFactory.java Fri Oct 10 09:59:55 2014
@@ -24,6 +24,7 @@ import org.apache.qpid.server.BrokerOpti
 import org.apache.qpid.server.configuration.updater.TaskExecutor;
 import org.apache.qpid.server.logging.EventLogger;
 import org.apache.qpid.server.logging.LogRecorder;
+import org.apache.qpid.server.model.BrokerShutdownProvider;
 import org.apache.qpid.server.model.SystemConfig;
 
 public interface SystemConfigFactory<X extends SystemConfig<X>> extends Pluggable
@@ -31,5 +32,6 @@ public interface SystemConfigFactory<X e
     public X newInstance(final TaskExecutor taskExecutor,
                          final EventLogger eventLogger,
                          final LogRecorder logRecorder,
-                         final BrokerOptions brokerOptions);
+                         final BrokerOptions brokerOptions,
+                         final BrokerShutdownProvider brokerShutdownProvider);
 }

Modified: qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/queue/AbstractQueue.java
URL: http://svn.apache.org/viewvc/qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/queue/AbstractQueue.java?rev=1630749&r1=1630748&r2=1630749&view=diff
==============================================================================
--- qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/queue/AbstractQueue.java (original)
+++ qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/queue/AbstractQueue.java Fri Oct 10 09:59:55 2014
@@ -254,6 +254,12 @@ public abstract class AbstractQueue<X ex
     }
 
     @Override
+    protected void validateOnCreate()
+    {
+        _virtualHost.getSecurityManager().authoriseCreateQueue(this);
+    }
+
+    @Override
     protected void onCreate()
     {
         super.onCreate();
@@ -304,6 +310,7 @@ public abstract class AbstractQueue<X ex
         }
     }
 
+    @Override
     protected void onOpen()
     {
         super.onOpen();
@@ -319,17 +326,6 @@ public abstract class AbstractQueue<X ex
 
         _logSubject = new QueueLogSubject(this);
 
-        try
-        {
-
-            _virtualHost.getSecurityManager().authoriseCreateQueue(this);
-        }
-        catch(AccessControlException e)
-        {
-            deleted();
-            throw e;
-        }
-
         Subject activeSubject = Subject.getSubject(AccessController.getContext());
         Set<SessionPrincipal> sessionPrincipals = activeSubject == null ? Collections.<SessionPrincipal>emptySet() : activeSubject.getPrincipals(SessionPrincipal.class);
         AMQSessionModel<?,?> sessionModel;
@@ -2798,7 +2794,7 @@ public abstract class AbstractQueue<X ex
 
     //=============
 
-    @StateTransition(currentState = State.UNINITIALIZED, desiredState = State.ACTIVE)
+    @StateTransition(currentState = {State.UNINITIALIZED,State.ERRORED}, desiredState = State.ACTIVE)
     private void activate()
     {
         setState(State.ACTIVE);
@@ -2965,7 +2961,10 @@ public abstract class AbstractQueue<X ex
         {
             throw new IllegalConfigurationException("Flow resume size can't be greater than flow control size");
         }
-
+        else if (changedAttributes.contains(DURABLE) && proxyForValidation.isDurable() != isDurable())
+        {
+            throw new IllegalConfigurationException("Message durability cannot be modified after queue creation");
+        }
 
         for (String attrName : NON_NEGATIVE_NUMBERS)
         {

Modified: qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/PrincipalDatabaseAuthenticationManager.java
URL: http://svn.apache.org/viewvc/qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/PrincipalDatabaseAuthenticationManager.java?rev=1630749&r1=1630748&r2=1630749&view=diff
==============================================================================
--- qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/PrincipalDatabaseAuthenticationManager.java (original)
+++ qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/PrincipalDatabaseAuthenticationManager.java Fri Oct 10 09:59:55 2014
@@ -76,25 +76,32 @@ public abstract class PrincipalDatabaseA
     }
 
     @Override
+    protected void validateOnCreate()
+    {
+        super.validateOnCreate();
+        File passwordFile = new File(_path);
+        if (passwordFile.exists() && !passwordFile.canRead())
+        {
+            throw new IllegalConfigurationException(String.format("Cannot read password file '%s'. Please check permissions.", _path));
+        }
+    }
+
+    @Override
     protected void onCreate()
     {
         super.onCreate();
-        try
+        File passwordFile = new File(_path);
+        if (!passwordFile.exists())
         {
-            File passwordFile = new File(_path);
-            if (!passwordFile.exists())
+            try
             {
                 passwordFile.createNewFile();
             }
-            else if (!passwordFile.canRead())
+            catch (IOException e)
             {
-                throw new IllegalConfigurationException("Cannot read password file" + _path + ". Check permissions.");
+                throw new IllegalConfigurationException(String.format("Cannot create password file at '%s'", _path), e);
             }
         }
-        catch (IOException e)
-        {
-            throw new IllegalConfigurationException("Cannot use password database at :" + _path, e);
-        }
     }
 
     @Override
@@ -102,23 +109,14 @@ public abstract class PrincipalDatabaseA
     {
         super.onOpen();
         _principalDatabase = createDatabase();
-        try
-        {
-            initialise();
-            List<Principal> users =
-                    _principalDatabase == null ? Collections.<Principal>emptyList() : _principalDatabase.getUsers();
-            for (Principal user : users)
-            {
-                PrincipalAdapter principalAdapter = new PrincipalAdapter(user);
-                principalAdapter.registerWithParents();
-                principalAdapter.open();
-                _userMap.put(user, principalAdapter);
-            }
-        }
-        catch(IllegalConfigurationException e)
-        {
-            setState(State.ERRORED);
-
+        initialise();
+        List<Principal> users = _principalDatabase == null ? Collections.<Principal>emptyList() : _principalDatabase.getUsers();
+        for (Principal user : users)
+        {
+            PrincipalAdapter principalAdapter = new PrincipalAdapter(user);
+            principalAdapter.registerWithParents();
+            principalAdapter.open();
+            _userMap.put(user, principalAdapter);
         }
     }
 
@@ -457,7 +455,7 @@ public abstract class PrincipalDatabaseA
             return super.changeAttribute(name, expected, desired);
         }
 
-        @StateTransition(currentState = State.UNINITIALIZED, desiredState = State.ACTIVE)
+        @StateTransition(currentState = {State.UNINITIALIZED,State.ERRORED}, desiredState = State.ACTIVE)
         private void activate()
         {
             setState(State.ACTIVE);

Modified: qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/SimpleLDAPAuthenticationManager.java
URL: http://svn.apache.org/viewvc/qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/SimpleLDAPAuthenticationManager.java?rev=1630749&r1=1630748&r2=1630749&view=diff
==============================================================================
--- qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/SimpleLDAPAuthenticationManager.java (original)
+++ qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/SimpleLDAPAuthenticationManager.java Fri Oct 10 09:59:55 2014
@@ -22,6 +22,7 @@ package org.apache.qpid.server.security.
 
 import org.apache.qpid.server.model.AuthenticationProvider;
 import org.apache.qpid.server.model.ManagedAttribute;
+import org.apache.qpid.server.model.ManagedContextDefault;
 import org.apache.qpid.server.model.ManagedObject;
 import org.apache.qpid.server.model.TrustStore;
 
@@ -29,24 +30,34 @@ import org.apache.qpid.server.model.Trus
 public interface SimpleLDAPAuthenticationManager<X extends SimpleLDAPAuthenticationManager<X>> extends AuthenticationProvider<X>
 {
     String PROVIDER_TYPE = "SimpleLDAP";
+    String PROVIDER_URL = "providerUrl";
+    String PROVIDER_AUTH_URL = "providerAuthUrl";
+    String SEARCH_CONTEXT = "searchContext";
+    String LDAP_CONTEXT_FACTORY = "ldapContextFactory";
+    String SEARCH_USERNAME = "getSearchUsername";
+    String SEARCH_PASSWORD = "getSearchPassword";
     String TRUST_STORE = "trustStore";
 
-    @ManagedAttribute( description = "LDAP server URL" )
+
+    @ManagedAttribute( description = "LDAP server URL", mandatory = true)
     String getProviderUrl();
 
     @ManagedAttribute( description = "LDAP authentication URL")
     String getProviderAuthUrl();
 
-    @ManagedAttribute( description = "Search context")
+    @ManagedAttribute( description = "Search context", mandatory = true)
     String getSearchContext();
 
-    @ManagedAttribute( description = "Search filter")
+    @ManagedAttribute( description = "Search filter", mandatory = true)
     String getSearchFilter();
 
     @ManagedAttribute( description = "Bind without search")
     boolean isBindWithoutSearch();
 
-    @ManagedAttribute( description = "LDAP context factory")
+    @ManagedContextDefault( name = "ldap.context.factory")
+    String DEFAULT_LDAP_CONTEXT_FACTORY = "com.sun.jndi.ldap.LdapCtxFactory";
+
+    @ManagedAttribute( description = "LDAP context factory", defaultValue = "${ldap.context.factory}")
     String getLdapContextFactory();
 
     @ManagedAttribute( description = "Trust store name")

Modified: qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/SimpleLDAPAuthenticationManagerImpl.java
URL: http://svn.apache.org/viewvc/qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/SimpleLDAPAuthenticationManagerImpl.java?rev=1630749&r1=1630748&r2=1630749&view=diff
==============================================================================
--- qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/SimpleLDAPAuthenticationManagerImpl.java (original)
+++ qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/SimpleLDAPAuthenticationManagerImpl.java Fri Oct 10 09:59:55 2014
@@ -19,15 +19,18 @@
 
 package org.apache.qpid.server.security.auth.manager;
 
+import static java.util.Collections.disjoint;
+import static java.util.Collections.unmodifiableList;
+import static java.util.Collections.singletonList;
+
 import java.io.IOException;
 import java.security.GeneralSecurityException;
-import java.security.KeyManagementException;
-import java.security.NoSuchAlgorithmException;
 import java.security.Principal;
-import java.util.Collections;
+import java.util.Arrays;
 import java.util.Hashtable;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import javax.naming.AuthenticationException;
 import javax.naming.Context;
@@ -48,7 +51,9 @@ import javax.security.sasl.SaslServer;
 
 import org.apache.log4j.Logger;
 
+import org.apache.qpid.server.configuration.IllegalConfigurationException;
 import org.apache.qpid.server.model.Broker;
+import org.apache.qpid.server.model.ConfiguredObject;
 import org.apache.qpid.server.model.ManagedAttributeField;
 import org.apache.qpid.server.model.ManagedObjectFactoryConstructor;
 import org.apache.qpid.server.model.TrustStore;
@@ -59,7 +64,6 @@ import org.apache.qpid.server.security.a
 import org.apache.qpid.server.security.auth.manager.ldap.LDAPSSLSocketFactoryGenerator;
 import org.apache.qpid.server.security.auth.sasl.plain.PlainPasswordCallback;
 import org.apache.qpid.server.security.auth.sasl.plain.PlainSaslServer;
-import org.apache.qpid.server.util.ServerScopedRuntimeException;
 import org.apache.qpid.server.util.StringUtil;
 import org.apache.qpid.ssl.SSLContextFactory;
 
@@ -68,6 +72,14 @@ public class SimpleLDAPAuthenticationMan
 {
     private static final Logger _logger = Logger.getLogger(SimpleLDAPAuthenticationManagerImpl.class);
 
+    private static final List<String> CONNECTIVITY_ATTRS = unmodifiableList(Arrays.asList(PROVIDER_URL,
+                                                                             PROVIDER_AUTH_URL,
+                                                                             SEARCH_CONTEXT,
+                                                                             LDAP_CONTEXT_FACTORY,
+                                                                             SEARCH_USERNAME,
+                                                                             SEARCH_PASSWORD,
+                                                                             TRUST_STORE));
+
     /**
      * Environment key to instruct {@link InitialDirContext} to override the socket factory.
      */
@@ -111,15 +123,37 @@ public class SimpleLDAPAuthenticationMan
         super(attributes, broker);
     }
 
+    @Override
+    protected void validateOnCreate()
+    {
+        super.validateOnCreate();
+
+        Class<? extends SocketFactory> sslSocketFactoryOverrideClass = _trustStore == null ? null : createSslSocketFactoryOverrideClass(_trustStore);
+        validateInitialDirContext(sslSocketFactoryOverrideClass, _providerUrl, _searchUsername, _searchPassword);
+    }
+
+    @Override
+    protected void validateChange(final ConfiguredObject<?> proxyForValidation, final Set<String> changedAttributes)
+    {
+        super.validateChange(proxyForValidation, changedAttributes);
+
+        if (!disjoint(changedAttributes, CONNECTIVITY_ATTRS))
+        {
+            SimpleLDAPAuthenticationManager changed = (SimpleLDAPAuthenticationManager)proxyForValidation;
+            TrustStore changedTruststore = changed.getTrustStore();
+            Class<? extends SocketFactory> sslSocketFactoryOverrideClass = changedTruststore == null ? null : createSslSocketFactoryOverrideClass(
+                    changedTruststore);
+            validateInitialDirContext(sslSocketFactoryOverrideClass, changed.getProviderUrl(), changed.getSearchUsername(),
+                                      changed.getSearchPassword());
+        }
+    }
 
     @Override
     protected void onOpen()
     {
         super.onOpen();
 
-        _sslSocketFactoryOverrideClass = createSslSocketFactoryOverrideClass();
-
-      //  validateInitialDirContext();
+        _sslSocketFactoryOverrideClass = _trustStore == null ? null : createSslSocketFactoryOverrideClass(_trustStore);
     }
 
     @Override
@@ -174,7 +208,7 @@ public class SimpleLDAPAuthenticationMan
     @Override
     public List<String> getMechanisms()
     {
-        return Collections.singletonList(PlainSaslServer.MECHANISM);
+        return singletonList(PlainSaslServer.MECHANISM);
     }
 
     @Override
@@ -259,7 +293,7 @@ public class SimpleLDAPAuthenticationMan
         InitialDirContext ctx = null;
         try
         {
-            ctx = createInitialDirContext(env);
+            ctx = createInitialDirContext(env, _sslSocketFactoryOverrideClass);
 
             //Authentication succeeded
             return new AuthenticationResult(new UsernamePrincipal(name));
@@ -291,7 +325,8 @@ public class SimpleLDAPAuthenticationMan
         return env;
     }
 
-    private InitialDirContext createInitialDirContext(Hashtable<String, Object> env) throws NamingException
+    private InitialDirContext createInitialDirContext(Hashtable<String, Object> env,
+                                                      Class<? extends SocketFactory> sslSocketFactoryOverrideClass) throws NamingException
     {
         ClassLoader existingContextClassLoader = null;
 
@@ -300,11 +335,11 @@ public class SimpleLDAPAuthenticationMan
         boolean revertContentClassLoader = false;
         try
         {
-            if (isLdaps && _sslSocketFactoryOverrideClass != null)
+            if (isLdaps && sslSocketFactoryOverrideClass != null)
             {
                 existingContextClassLoader = Thread.currentThread().getContextClassLoader();
-                env.put(JAVA_NAMING_LDAP_FACTORY_SOCKET, _sslSocketFactoryOverrideClass.getName());
-                Thread.currentThread().setContextClassLoader(_sslSocketFactoryOverrideClass.getClassLoader());
+                env.put(JAVA_NAMING_LDAP_FACTORY_SOCKET, sslSocketFactoryOverrideClass.getName());
+                Thread.currentThread().setContextClassLoader(sslSocketFactoryOverrideClass.getClassLoader());
                 revertContentClassLoader = true;
             }
             return new InitialDirContext(env);
@@ -323,59 +358,59 @@ public class SimpleLDAPAuthenticationMan
      * associated with the {@link SSLContext} generated from that trust store.
      *
      * @return generated socket factory class
+     * @param trustStore
      */
-    private Class<? extends SocketFactory> createSslSocketFactoryOverrideClass()
+    private Class<? extends SocketFactory> createSslSocketFactoryOverrideClass(final TrustStore trustStore)
     {
-        if (_trustStore != null)
+        String clazzName = new StringUtil().createUniqueJavaName(getName() + "_" + trustStore.getName());
+        SSLContext sslContext = null;
+        try
         {
-            String clazzName = new StringUtil().createUniqueJavaName(getName());
-            SSLContext sslContext = null;
-            try
-            {
-                sslContext = SSLContext.getInstance("TLS");
-                sslContext.init(null, _trustStore.getTrustManagers(), null);
-            }
-            catch (NoSuchAlgorithmException e)
-            {
-                _logger.error("Exception creating SSLContext", e);
-                throw new ServerScopedRuntimeException("Error creating SSLContext for trust store : " + _trustStore.getName() , e);
-            }
-            catch (KeyManagementException e)
-            {
-                _logger.error("Exception creating SSLContext", e);
-                throw new ServerScopedRuntimeException("Error creating SSLContext for trust store : " + _trustStore.getName() , e);
-            }
-            catch (GeneralSecurityException e)
-            {
-                _logger.error("Exception creating SSLContext", e);
-                throw new ServerScopedRuntimeException("Error creating SSLContext for trust store : " + _trustStore.getName() , e);
-            }
+            sslContext = SSLContext.getInstance("TLS");
+            sslContext.init(null, trustStore.getTrustManagers(), null);
+        }
+        catch (GeneralSecurityException e)
+        {
+            _logger.error("Exception creating SSLContext", e);
+            throw new IllegalConfigurationException("Error creating SSLContext with trust store : " + trustStore.getName() , e);
+        }
 
-            Class<? extends AbstractLDAPSSLSocketFactory> clazz = LDAPSSLSocketFactoryGenerator.createSubClass(clazzName, sslContext.getSocketFactory());
-            if (_logger.isDebugEnabled())
-            {
-                _logger.debug("Connection to Directory will use custom SSL socket factory : " +  clazz);
-            }
-            return clazz;
+        Class<? extends AbstractLDAPSSLSocketFactory> clazz = LDAPSSLSocketFactoryGenerator.createSubClass(clazzName, sslContext.getSocketFactory());
+        if (_logger.isDebugEnabled())
+        {
+            _logger.debug("Connection to Directory will use custom SSL socket factory : " +  clazz);
         }
+        return clazz;
+    }
 
-        return null;
+    @Override
+    public String toString()
+    {
+        return "SimpleLDAPAuthenticationManagerImpl [id=" + getId() + ", name=" + getName() +
+               ", providerUrl=" + _providerUrl + ", providerAuthUrl=" + _providerAuthUrl +
+               ", searchContext=" + _searchContext + ", state=" + getState() +
+               ", searchFilter=" + _searchFilter + ", ldapContextFactory=" + _ldapContextFactory +
+               ", bindWithoutSearch=" + _bindWithoutSearch  + ", trustStore=" + _trustStore  +
+               ", searchUsername=" + _searchUsername + "]";
     }
 
-    private void validateInitialDirContext()
+    private void validateInitialDirContext(Class<? extends SocketFactory> sslSocketFactoryOverrideClass,
+                                           final String providerUrl,
+                                           final String searchUsername, final String searchPassword)
     {
-        Hashtable<String,Object> env = createInitialDirContextEnvironment(_providerUrl);
+        Hashtable<String,Object> env = createInitialDirContextEnvironment(providerUrl);
 
-        setupSearchContext(env);
+        setupSearchContext(env, searchUsername, searchPassword);
 
         InitialDirContext ctx = null;
         try
         {
-            ctx = createInitialDirContext(env);
+            ctx = createInitialDirContext(env, sslSocketFactoryOverrideClass);
         }
         catch (NamingException e)
         {
-            throw new ServerScopedRuntimeException("Unable to establish connection to the ldap server at " + _providerUrl, e);
+            _logger.error("Failed to establish connectivity to the ldap server for " + providerUrl, e);
+            throw new IllegalConfigurationException("Failed to establish connectivity to the ldap server." , e);
         }
         finally
         {
@@ -383,13 +418,14 @@ public class SimpleLDAPAuthenticationMan
         }
     }
 
-    private void setupSearchContext(final Hashtable<String, Object> env)
+    private void setupSearchContext(final Hashtable<String, Object> env,
+                                    final String searchUsername, final String searchPassword)
     {
         if(_searchUsername != null && _searchUsername.trim().length()>0)
         {
             env.put(Context.SECURITY_AUTHENTICATION, "simple");
-            env.put(Context.SECURITY_PRINCIPAL, _searchUsername);
-            env.put(Context.SECURITY_CREDENTIALS, _searchPassword);
+            env.put(Context.SECURITY_PRINCIPAL, searchUsername);
+            env.put(Context.SECURITY_CREDENTIALS, searchPassword);
         }
         else
         {
@@ -454,9 +490,9 @@ public class SimpleLDAPAuthenticationMan
         {
             Hashtable<String, Object> env = createInitialDirContextEnvironment(_providerUrl);
 
-            setupSearchContext(env);
+            setupSearchContext(env, _searchUsername, _searchPassword);
 
-            InitialDirContext ctx = createInitialDirContext(env);
+            InitialDirContext ctx = createInitialDirContext(env, _sslSocketFactoryOverrideClass);
 
             try
             {

Modified: qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/store/AbstractJDBCConfigurationStore.java
URL: http://svn.apache.org/viewvc/qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/store/AbstractJDBCConfigurationStore.java?rev=1630749&r1=1630748&r2=1630749&view=diff
==============================================================================
--- qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/store/AbstractJDBCConfigurationStore.java (original)
+++ qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/store/AbstractJDBCConfigurationStore.java Fri Oct 10 09:59:55 2014
@@ -572,7 +572,7 @@ public abstract class AbstractJDBCConfig
         }
         catch (SQLException e)
         {
-            throw new StoreException("Error creating ConfiguredObject " + object);
+            throw new StoreException("Error creating ConfiguredObject " + object, e);
         }
     }
 

Modified: qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/store/AbstractJDBCMessageStore.java
URL: http://svn.apache.org/viewvc/qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/store/AbstractJDBCMessageStore.java?rev=1630749&r1=1630748&r2=1630749&view=diff
==============================================================================
--- qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/store/AbstractJDBCMessageStore.java (original)
+++ qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/store/AbstractJDBCMessageStore.java Fri Oct 10 09:59:55 2014
@@ -133,7 +133,7 @@ public abstract class AbstractJDBCMessag
         }
         catch (SQLException e)
         {
-            throw new StoreException(e);
+            throw new StoreException("Failed to determine maximum ids", e);
         }
 
     }
@@ -816,7 +816,6 @@ public abstract class AbstractJDBCMessag
 
     private void commitTran(ConnectionWrapper connWrapper) throws StoreException
     {
-
         try
         {
             Connection conn = connWrapper.getConnection();
@@ -833,10 +832,6 @@ public abstract class AbstractJDBCMessag
         {
             throw new StoreException("Error commit tx: " + e.getMessage(), e);
         }
-        finally
-        {
-
-        }
     }
 
     private StoreFuture commitTranAsync(ConnectionWrapper connWrapper) throws StoreException
@@ -1448,7 +1443,7 @@ public abstract class AbstractJDBCMessag
                 }
                 catch (SQLException e)
                 {
-                    throw new StoreException(e);
+                    throw new StoreException("Failed to get metadata for message id: " + _messageId, e);
                 }
             }
 
@@ -1507,7 +1502,7 @@ public abstract class AbstractJDBCMessag
                         }
                         catch (SQLException e)
                         {
-                            throw new StoreException(e);
+                            throw new StoreException("Failed to get content for message id " + _messageId, e);
                         }
                     }
                     else
@@ -1551,7 +1546,7 @@ public abstract class AbstractJDBCMessag
                         }
                         catch (SQLException e)
                         {
-                            throw new StoreException(e);
+                            throw new StoreException("Failed to get content for message id: " + _messageId, e);
                         }
                     }
                     else
@@ -1598,11 +1593,7 @@ public abstract class AbstractJDBCMessag
             }
             catch (SQLException e)
             {
-                throw new StoreException(e);
-            }
-            finally
-            {
-
+                throw new StoreException("Failed to flow to disk", e);
             }
             return true;
         }

Modified: qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java
URL: http://svn.apache.org/viewvc/qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java?rev=1630749&r1=1630748&r2=1630749&view=diff
==============================================================================
--- qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java (original)
+++ qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java Fri Oct 10 09:59:55 2014
@@ -37,7 +37,6 @@ import java.util.concurrent.ScheduledThr
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicLong;
-import java.util.concurrent.atomic.AtomicReference;
 
 import javax.security.auth.Subject;
 
@@ -231,6 +230,47 @@ public abstract class AbstractVirtualHos
     }
 
     @Override
+    public void validateOnCreate()
+    {
+        super.validateOnCreate();
+        validateMessageStoreCreation();
+    }
+
+    private void validateMessageStoreCreation()
+    {
+        MessageStore store = createMessageStore();
+        if (store != null)
+        {
+            try
+            {
+                store.openMessageStore(this);
+            }
+            catch (Exception e)
+            {
+                throw new IllegalConfigurationException("Cannot open virtual host message store:" + e.getMessage(), e);
+            }
+            finally
+            {
+                try
+                {
+                    store.closeMessageStore();
+                }
+                catch(Exception e)
+                {
+                    _logger.warn("Failed to close database", e);
+                }
+            }
+        }
+    }
+
+    @Override
+    protected void onExceptionInOpen(RuntimeException e)
+    {
+        super.onExceptionInOpen(e);
+        closeMessageStore();
+    }
+
+    @Override
     protected void onOpen()
     {
         super.onOpen();
@@ -638,6 +678,7 @@ public abstract class AbstractVirtualHos
 
     protected void onClose()
     {
+        setState(State.UNAVAILABLE);
         //Stop Connections
         _connectionRegistry.close();
         _dtxRegistry.close();
@@ -659,11 +700,11 @@ public abstract class AbstractVirtualHos
             {
                 _logger.error("Failed to close message store", e);
             }
-        }
 
-        if (!(_virtualHostNode.getConfigurationStore() instanceof MessageStoreProvider))
-        {
-            getEventLogger().message(getMessageStoreLogSubject(), MessageStoreMessages.CLOSED());
+            if (!(_virtualHostNode.getConfigurationStore() instanceof MessageStoreProvider))
+            {
+                getEventLogger().message(getMessageStoreLogSubject(), MessageStoreMessages.CLOSED());
+            }
         }
     }
 
@@ -1355,7 +1396,7 @@ public abstract class AbstractVirtualHos
         getDurableConfigurationStore().create(new ConfiguredObjectRecordImpl(record.getId(), record.getType(), record.getAttributes()));
     }
 
-    @StateTransition( currentState = { State.UNINITIALIZED }, desiredState = State.ACTIVE )
+    @StateTransition( currentState = { State.UNINITIALIZED,State.ERRORED }, desiredState = State.ACTIVE )
     private void onActivate()
     {
         _houseKeepingTasks = new ScheduledThreadPoolExecutor(getHousekeepingThreadCount());

Modified: qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/AbstractStandardVirtualHostNode.java
URL: http://svn.apache.org/viewvc/qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/AbstractStandardVirtualHostNode.java?rev=1630749&r1=1630748&r2=1630749&view=diff
==============================================================================
--- qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/AbstractStandardVirtualHostNode.java (original)
+++ qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/AbstractStandardVirtualHostNode.java Fri Oct 10 09:59:55 2014
@@ -41,6 +41,7 @@ import org.apache.qpid.server.model.Virt
 import org.apache.qpid.server.security.SecurityManager;
 import org.apache.qpid.server.store.ConfiguredObjectRecord;
 import org.apache.qpid.server.store.ConfiguredObjectRecordImpl;
+import org.apache.qpid.server.store.DurableConfigurationStore;
 import org.apache.qpid.server.store.VirtualHostStoreUpgraderAndRecoverer;
 
 public abstract class AbstractStandardVirtualHostNode<X extends AbstractStandardVirtualHostNode<X>> extends AbstractVirtualHostNode<X>
@@ -169,4 +170,33 @@ public abstract class AbstractStandardVi
     {
         return Collections.emptyList();
     }
+
+    @Override
+    public void validateOnCreate()
+    {
+        super.validateOnCreate();
+        DurableConfigurationStore store = createConfigurationStore();
+        if (store != null)
+        {
+            try
+            {
+                store.openConfigurationStore(this, false);
+            }
+            catch (Exception e)
+            {
+                throw new IllegalConfigurationException("Cannot open node configuration store:" + e.getMessage(), e);
+            }
+            finally
+            {
+                try
+                {
+                    store.closeConfigurationStore();
+                }
+                catch(Exception e)
+                {
+                    LOGGER.warn("Failed to close database", e);
+                }
+            }
+        }
+    }
 }

Modified: qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/AbstractVirtualHostNode.java
URL: http://svn.apache.org/viewvc/qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/AbstractVirtualHostNode.java?rev=1630749&r1=1630748&r2=1630749&view=diff
==============================================================================
--- qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/AbstractVirtualHostNode.java (original)
+++ qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/AbstractVirtualHostNode.java Fri Oct 10 09:59:55 2014
@@ -36,7 +36,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.UUID;
-import java.util.concurrent.atomic.AtomicReference;
 
 import org.apache.log4j.Logger;
 
@@ -100,8 +99,6 @@ public abstract class AbstractVirtualHos
     {
         super.onOpen();
         _durableConfigurationStore = createConfigurationStore();
-        _configurationStoreLogSubject = new MessageStoreLogSubject(getName(), _durableConfigurationStore.getClass().getSimpleName());
-
     }
 
     @Override
@@ -167,11 +164,6 @@ public abstract class AbstractVirtualHos
         return _eventLogger;
     }
 
-    protected DurableConfigurationStore getDurableConfigurationStore()
-    {
-        return _durableConfigurationStore;
-    }
-
     protected MessageStoreLogSubject getConfigurationStoreLogSubject()
     {
         return _configurationStoreLogSubject;
@@ -184,7 +176,11 @@ public abstract class AbstractVirtualHos
         deleteVirtualHostIfExists();
         close();
         deleted();
-        getConfigurationStore().onDelete(this);
+        DurableConfigurationStore configurationStore = getConfigurationStore();
+        if (configurationStore != null)
+        {
+            configurationStore.onDelete(this);
+        }
     }
 
     protected void deleteVirtualHostIfExists()
@@ -205,11 +201,30 @@ public abstract class AbstractVirtualHos
     protected void stopAndSetStateTo(State stoppedState)
     {
         closeChildren();
-        closeConfigurationStore();
+        closeConfigurationStoreSafely();
         setState(stoppedState);
     }
 
     @Override
+    protected void onExceptionInOpen(RuntimeException e)
+    {
+        super.onExceptionInOpen(e);
+        closeConfigurationStoreSafely();
+    }
+
+    @Override
+    protected void postResolve()
+    {
+        super.postResolve();
+        DurableConfigurationStore store = getConfigurationStore();
+        if (store == null)
+        {
+            store = createConfigurationStore();
+        }
+        _configurationStoreLogSubject = new MessageStoreLogSubject(getName(), store.getClass().getSimpleName());
+    }
+
+    @Override
     protected void onClose()
     {
         closeConfigurationStore();
@@ -262,6 +277,18 @@ public abstract class AbstractVirtualHos
         }
     }
 
+    private void closeConfigurationStoreSafely()
+    {
+        try
+        {
+            closeConfigurationStore();
+        }
+        catch(Exception e)
+        {
+            LOGGER.warn("Unexpected exception on close of configuration store", e);
+        }
+    }
+
     @Override
     public String getVirtualHostInitialConfiguration()
     {

Modified: qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/test/java/org/apache/qpid/server/configuration/startup/FileKeyStoreCreationTest.java
URL: http://svn.apache.org/viewvc/qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/test/java/org/apache/qpid/server/configuration/startup/FileKeyStoreCreationTest.java?rev=1630749&r1=1630748&r2=1630749&view=diff
==============================================================================
--- qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/test/java/org/apache/qpid/server/configuration/startup/FileKeyStoreCreationTest.java (original)
+++ qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/test/java/org/apache/qpid/server/configuration/startup/FileKeyStoreCreationTest.java Fri Oct 10 09:59:55 2014
@@ -63,7 +63,7 @@ public class FileKeyStoreCreationTest ex
         Map<String, Object> attributesCopy = new HashMap<String, Object>(attributes);
 
         Broker broker = mock(Broker.class);
-        TaskExecutor executor = new CurrentThreadTaskExecutor();
+        TaskExecutor executor = CurrentThreadTaskExecutor.newStartedInstance();
         when(broker.getObjectFactory()).thenReturn(_factory);
         when(broker.getModel()).thenReturn(_factory.getModel());
         when(broker.getTaskExecutor()).thenReturn(executor);

Modified: qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/test/java/org/apache/qpid/server/configuration/store/ManagementModeStoreHandlerTest.java
URL: http://svn.apache.org/viewvc/qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/test/java/org/apache/qpid/server/configuration/store/ManagementModeStoreHandlerTest.java?rev=1630749&r1=1630748&r2=1630749&view=diff
==============================================================================
--- qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/test/java/org/apache/qpid/server/configuration/store/ManagementModeStoreHandlerTest.java (original)
+++ qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/test/java/org/apache/qpid/server/configuration/store/ManagementModeStoreHandlerTest.java Fri Oct 10 09:59:55 2014
@@ -34,6 +34,7 @@ import java.util.HashSet;
 import java.util.Map;
 import java.util.UUID;
 
+import org.apache.qpid.server.model.BrokerShutdownProvider;
 import org.mockito.ArgumentCaptor;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
@@ -78,7 +79,8 @@ public class ManagementModeStoreHandlerT
         _taskExecutor.start();
 
         _systemConfig = new JsonSystemConfigImpl(_taskExecutor, mock(EventLogger.class),
-                                               mock(LogRecorder.class), new BrokerOptions());
+                                               mock(LogRecorder.class), new BrokerOptions(),
+                                               mock(BrokerShutdownProvider.class));
 
 
         ConfiguredObjectRecord systemContextRecord = _systemConfig.asObjectRecord();

Modified: qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java
URL: http://svn.apache.org/viewvc/qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java?rev=1630749&r1=1630748&r2=1630749&view=diff
==============================================================================
--- qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java (original)
+++ qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java Fri Oct 10 09:59:55 2014
@@ -26,7 +26,9 @@ import java.util.Map;
 
 import junit.framework.TestCase;
 
+import org.apache.qpid.server.configuration.IllegalConfigurationException;
 import org.apache.qpid.server.model.testmodel.TestChildCategory;
+import org.apache.qpid.server.model.testmodel.TestConfiguredObject;
 import org.apache.qpid.server.model.testmodel.TestModel;
 import org.apache.qpid.server.model.testmodel.TestRootCategory;
 import org.apache.qpid.server.store.ConfiguredObjectRecord;
@@ -257,4 +259,189 @@ public class AbstractConfiguredObjectTes
                    parent.getChildren(TestChildCategory.class).isEmpty());
     }
 
+    public void testOpeningResultsInErroredStateWhenResolutionFails() throws Exception
+    {
+        TestConfiguredObject object = new TestConfiguredObject(getName());
+        object.setThrowExceptionOnPostResolve(true);
+        object.open();
+        assertFalse("Unexpected opened", object.isOpened());
+        assertEquals("Unexpected state", State.ERRORED, object.getState());
+
+        object.setThrowExceptionOnPostResolve(false);
+        object.setAttributes(Collections.<String, Object>singletonMap(Port.DESIRED_STATE, State.ACTIVE));
+        assertTrue("Unexpected opened", object.isOpened());
+        assertEquals("Unexpected state", State.ACTIVE, object.getState());
+    }
+
+    public void testOpeningInERROREDStateAfterFailedOpenOnDesiredStateChangeToActive() throws Exception
+    {
+        TestConfiguredObject object = new TestConfiguredObject(getName());
+        object.setThrowExceptionOnOpen(true);
+        object.open();
+        assertFalse("Unexpected opened", object.isOpened());
+        assertEquals("Unexpected state", State.ERRORED, object.getState());
+
+        object.setThrowExceptionOnOpen(false);
+        object.setAttributes(Collections.<String, Object>singletonMap(Port.DESIRED_STATE, State.ACTIVE));
+        assertTrue("Unexpected opened", object.isOpened());
+        assertEquals("Unexpected state", State.ACTIVE, object.getState());
+    }
+
+    public void testOpeningInERROREDStateAfterFailedOpenOnStart() throws Exception
+    {
+        TestConfiguredObject object = new TestConfiguredObject(getName());
+        object.setThrowExceptionOnOpen(true);
+        object.open();
+        assertFalse("Unexpected opened", object.isOpened());
+        assertEquals("Unexpected state", State.ERRORED, object.getState());
+
+        object.setThrowExceptionOnOpen(false);
+        object.start();
+        assertTrue("Unexpected opened", object.isOpened());
+        assertEquals("Unexpected state", State.ACTIVE, object.getState());
+    }
+
+    public void testDeletionERROREDStateAfterFailedOpenOnDelete() throws Exception
+    {
+        TestConfiguredObject object = new TestConfiguredObject(getName());
+        object.setThrowExceptionOnOpen(true);
+        object.open();
+        assertFalse("Unexpected opened", object.isOpened());
+        assertEquals("Unexpected state", State.ERRORED, object.getState());
+
+        object.delete();
+        assertFalse("Unexpected opened", object.isOpened());
+        assertEquals("Unexpected state", State.DELETED, object.getState());
+    }
+
+    public void testDeletionInERROREDStateAfterFailedOpenOnDesiredStateChangeToDelete() throws Exception
+    {
+        TestConfiguredObject object = new TestConfiguredObject(getName());
+        object.setThrowExceptionOnOpen(true);
+        object.open();
+        assertFalse("Unexpected opened", object.isOpened());
+        assertEquals("Unexpected state", State.ERRORED, object.getState());
+
+        object.setAttributes(Collections.<String, Object>singletonMap(Port.DESIRED_STATE, State.DELETED));
+        assertFalse("Unexpected opened", object.isOpened());
+        assertEquals("Unexpected state", State.DELETED, object.getState());
+    }
+
+
+    public void testCreationWithExceptionThrownFromValidationOnCreate() throws Exception
+    {
+        TestConfiguredObject object = new TestConfiguredObject(getName());
+        object.setThrowExceptionOnValidationOnCreate(true);
+        try
+        {
+            object.create();
+            fail("IllegalConfigurationException is expected to be thrown");
+        }
+        catch(IllegalConfigurationException e)
+        {
+            //pass
+        }
+        assertFalse("Unexpected opened", object.isOpened());
+    }
+
+    public void testCreationWithoutExceptionThrownFromValidationOnCreate() throws Exception
+    {
+        TestConfiguredObject object = new TestConfiguredObject(getName());
+        object.setThrowExceptionOnValidationOnCreate(false);
+        object.create();
+        assertTrue("Unexpected opened", object.isOpened());
+        assertEquals("Unexpected state", State.ACTIVE, object.getState());
+    }
+
+    public void testCreationWithExceptionThrownFromOnOpen() throws Exception
+    {
+        TestConfiguredObject object = new TestConfiguredObject(getName());
+        object.setThrowExceptionOnOpen(true);
+        try
+        {
+            object.create();
+            fail("Exception should have been re-thrown");
+        }
+        catch (RuntimeException re)
+        {
+            // pass
+        }
+
+        assertFalse("Unexpected opened", object.isOpened());
+        assertEquals("Unexpected state", State.DELETED, object.getState());
+    }
+
+    public void testCreationWithExceptionThrownFromOnCreate() throws Exception
+    {
+        TestConfiguredObject object = new TestConfiguredObject(getName());
+        object.setThrowExceptionOnCreate(true);
+        try
+        {
+            object.create();
+            fail("Exception should have been re-thrown");
+        }
+        catch (RuntimeException re)
+        {
+            // pass
+        }
+
+        assertFalse("Unexpected opened", object.isOpened());
+        assertEquals("Unexpected state", State.DELETED, object.getState());
+    }
+
+    public void testUnresolvedChildInERROREDStateIsNotValidatedOrOpenedOrAttainedDesiredStateOnParentOpen() throws Exception
+    {
+        TestConfiguredObject parent = new TestConfiguredObject("parent");
+        TestConfiguredObject child1 = new TestConfiguredObject("child1", parent, parent.getTaskExecutor());
+        child1.registerWithParents();
+        TestConfiguredObject child2 = new TestConfiguredObject("child2", parent, parent.getTaskExecutor());
+        child2.registerWithParents();
+
+        child1.setThrowExceptionOnPostResolve(true);
+
+        parent.open();
+
+        assertTrue("Parent should be resolved", parent.isResolved());
+        assertTrue("Parent should be validated", parent.isValidated());
+        assertTrue("Parent should be opened", parent.isOpened());
+        assertEquals("Unexpected parent state", State.ACTIVE, parent.getState());
+
+        assertTrue("Child2 should be resolved", child2.isResolved());
+        assertTrue("Child2 should be validated", child2.isValidated());
+        assertTrue("Child2 should be opened", child2.isOpened());
+        assertEquals("Unexpected child2 state", State.ACTIVE, child2.getState());
+
+        assertFalse("Child2 should not be resolved", child1.isResolved());
+        assertFalse("Child1 should not be validated", child1.isValidated());
+        assertFalse("Child1 should not be opened", child1.isOpened());
+        assertEquals("Unexpected child1 state", State.ERRORED, child1.getState());
+    }
+
+    public void testUnvalidatedChildInERROREDStateIsNotOpenedOrAttainedDesiredStateOnParentOpen() throws Exception
+    {
+        TestConfiguredObject parent = new TestConfiguredObject("parent");
+        TestConfiguredObject child1 = new TestConfiguredObject("child1", parent, parent.getTaskExecutor());
+        child1.registerWithParents();
+        TestConfiguredObject child2 = new TestConfiguredObject("child2", parent, parent.getTaskExecutor());
+        child2.registerWithParents();
+
+        child1.setThrowExceptionOnValidate(true);
+
+        parent.open();
+
+        assertTrue("Parent should be resolved", parent.isResolved());
+        assertTrue("Parent should be validated", parent.isValidated());
+        assertTrue("Parent should be opened", parent.isOpened());
+        assertEquals("Unexpected parent state", State.ACTIVE, parent.getState());
+
+        assertTrue("Child2 should be resolved", child2.isResolved());
+        assertTrue("Child2 should be validated", child2.isValidated());
+        assertTrue("Child2 should be opened", child2.isOpened());
+        assertEquals("Unexpected child2 state", State.ACTIVE, child2.getState());
+
+        assertTrue("Child1 should be resolved", child1.isResolved());
+        assertFalse("Child1 should not be validated", child1.isValidated());
+        assertFalse("Child1 should not be opened", child1.isOpened());
+        assertEquals("Unexpected child1 state", State.ERRORED, child1.getState());
+    }
 }

Modified: qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/test/java/org/apache/qpid/server/model/VirtualHostTest.java
URL: http://svn.apache.org/viewvc/qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/test/java/org/apache/qpid/server/model/VirtualHostTest.java?rev=1630749&r1=1630748&r2=1630749&view=diff
==============================================================================
--- qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/test/java/org/apache/qpid/server/model/VirtualHostTest.java (original)
+++ qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/test/java/org/apache/qpid/server/model/VirtualHostTest.java Fri Oct 10 09:59:55 2014
@@ -65,6 +65,7 @@ public class VirtualHostTest extends Qpi
     private TaskExecutor _taskExecutor;
     private VirtualHostNode<?> _virtualHostNode;
     private DurableConfigurationStore _configStore;
+    private VirtualHost<?, ?, ?> _virtualHost;
 
     @Override
     protected void setUp() throws Exception
@@ -94,7 +95,17 @@ public class VirtualHostTest extends Qpi
     {
         try
         {
-            _taskExecutor.stopImmediately();
+            try
+            {
+                _taskExecutor.stopImmediately();
+            }
+            finally
+            {
+                if (_virtualHost != null)
+                {
+                    _virtualHost.close();
+                }
+            }
         }
         finally
         {
@@ -386,6 +397,7 @@ public class VirtualHostTest extends Qpi
 
         TestMemoryVirtualHost host = new TestMemoryVirtualHost(attributes, _virtualHostNode);
         host.create();
+        _virtualHost = host;
         return host;
     }
 

Modified: qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/test/java/org/apache/qpid/server/model/adapter/FileSystemPreferencesProviderTest.java
URL: http://svn.apache.org/viewvc/qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/test/java/org/apache/qpid/server/model/adapter/FileSystemPreferencesProviderTest.java?rev=1630749&r1=1630748&r2=1630749&view=diff
==============================================================================
--- qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/test/java/org/apache/qpid/server/model/adapter/FileSystemPreferencesProviderTest.java (original)
+++ qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/test/java/org/apache/qpid/server/model/adapter/FileSystemPreferencesProviderTest.java Fri Oct 10 09:59:55 2014
@@ -31,6 +31,7 @@ import java.util.Map;
 import java.util.Set;
 import java.util.UUID;
 
+import org.apache.qpid.server.configuration.IllegalConfigurationException;
 import org.apache.qpid.server.configuration.updater.CurrentThreadTaskExecutor;
 import org.apache.qpid.server.configuration.updater.TaskExecutor;
 import org.apache.qpid.server.model.AuthenticationProvider;
@@ -109,7 +110,7 @@ public class FileSystemPreferencesProvid
             attributes.put(ConfiguredObject.ID, UUID.randomUUID());
             attributes.put(ConfiguredObject.NAME, getTestName());
             _preferencesProvider = new FileSystemPreferencesProviderImpl(attributes, _authenticationProvider);
-            _preferencesProvider.open();
+            _preferencesProvider.create();
 
             assertEquals(State.ACTIVE, _preferencesProvider.getState());
             assertTrue("Preferences file was not created", nonExistingFile.exists());
@@ -120,6 +121,57 @@ public class FileSystemPreferencesProvid
         }
     }
 
+    public void testValidationOnCreateForInvalidPath() throws Exception
+    {
+        File file = new File(TMP_FOLDER + File.separator + getTestName() + System.nanoTime() );
+        file.createNewFile();
+        String path = file.getAbsolutePath() + File.separator + "users";
+
+        Map<String, Object> attributes = new HashMap<String, Object>();
+        attributes.put(FileSystemPreferencesProvider.PATH, path);
+        attributes.put(ConfiguredObject.ID, UUID.randomUUID());
+        attributes.put(ConfiguredObject.NAME, getTestName());
+        _preferencesProvider = new FileSystemPreferencesProviderImpl(attributes, _authenticationProvider);
+
+        try
+        {
+
+            _preferencesProvider.create();
+
+            fail("Creation of preferences provider with invalid path should have failed");
+        }
+        catch(IllegalConfigurationException e)
+        {
+            assertEquals("Unexpected exception message:" + e.getMessage(), String.format("Cannot create preferences store file at '%s'", path), e.getMessage());
+        }
+    }
+
+    public void testValidationOnCreateWithInvalidPreferences()
+    {
+        File tmp = TestFileUtils.createTempFile(this, "preferences", "{blah:=boo}");
+        try
+        {
+            Map<String, Object> attributes = new HashMap<String, Object>();
+            attributes.put(FileSystemPreferencesProvider.PATH, tmp.getAbsolutePath());
+            attributes.put(ConfiguredObject.ID, UUID.randomUUID());
+            attributes.put(ConfiguredObject.NAME, getTestName());
+            _preferencesProvider = new FileSystemPreferencesProviderImpl(attributes, _authenticationProvider);
+            try
+            {
+                _preferencesProvider.create();
+                fail("Exception is expected on validation of groups provider with invalid preferences format");
+            }
+            catch (IllegalConfigurationException e)
+            {
+                assertEquals("Unexpected exception message:" + e.getMessage(), "Cannot parse preferences json in " + tmp.getName(), e.getMessage());
+            }
+        }
+        finally
+        {
+            tmp.delete();
+        }
+    }
+
     public void testConstructionWithEmptyFile() throws Exception
     {
         File emptyPrefsFile = new File(TMP_FOLDER, "preferences-" + getTestName() + ".json");

Modified: qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/test/java/org/apache/qpid/server/model/adapter/PortFactoryTest.java
URL: http://svn.apache.org/viewvc/qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/test/java/org/apache/qpid/server/model/adapter/PortFactoryTest.java?rev=1630749&r1=1630748&r2=1630749&view=diff
==============================================================================
--- qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/test/java/org/apache/qpid/server/model/adapter/PortFactoryTest.java (original)
+++ qpid/branches/QPID-6125-ProtocolRefactoring/java/broker-core/src/test/java/org/apache/qpid/server/model/adapter/PortFactoryTest.java Fri Oct 10 09:59:55 2014
@@ -54,7 +54,7 @@ import org.apache.qpid.test.utils.QpidTe
 public class PortFactoryTest extends QpidTestCase
 {
     private UUID _portId = UUID.randomUUID();
-    private int _portNumber = 123;
+    private int _portNumber;
     private Set<String> _tcpStringSet = Collections.singleton(Transport.TCP.name());
     private Set<Transport> _tcpTransports = Collections.singleton(Transport.TCP);
     private Set<String> _sslStringSet = Collections.singleton(Transport.SSL.name());
@@ -68,11 +68,13 @@ public class PortFactoryTest extends Qpi
     private String _authProviderName = "authProvider";
     private AuthenticationProvider _authProvider = mock(AuthenticationProvider.class);
     private ConfiguredObjectFactoryImpl _factory;
+    private Port<?> _port;
 
 
     @Override
     protected void setUp() throws Exception
     {
+        _portNumber = findFreePort();
         TaskExecutor executor = CurrentThreadTaskExecutor.newStartedInstance();
         when(_authProvider.getName()).thenReturn(_authProviderName);
         when(_broker.getChildren(eq(AuthenticationProvider.class))).thenReturn(Collections.singleton(_authProvider));
@@ -109,30 +111,45 @@ public class PortFactoryTest extends Qpi
         _attributes.put(Port.BINDING_ADDRESS, "127.0.0.1");
     }
 
+    public void tearDown() throws Exception
+    {
+        try
+        {
+            if (_port != null)
+            {
+                _port.close();
+            }
+        }
+        finally
+        {
+            super.tearDown();
+        }
+    }
+
     public void testCreatePortWithMinimumAttributes()
     {
         Map<String, Object> attributes = new HashMap<String, Object>();
-        attributes.put(Port.PORT, 1);
+        attributes.put(Port.PORT, _portNumber);
         attributes.put(Port.NAME, getName());
         attributes.put(Port.AUTHENTICATION_PROVIDER, _authProviderName);
         attributes.put(Port.DESIRED_STATE, State.QUIESCED);
 
-        Port<?> port = _factory.create(Port.class, attributes, _broker);
+        _port = _factory.create(Port.class, attributes, _broker);
 
-        assertNotNull(port);
-        assertTrue(port instanceof AmqpPort);
-        assertEquals("Unexpected port", 1, port.getPort());
-        assertEquals("Unexpected transports", Collections.singleton(PortFactory.DEFAULT_TRANSPORT), port.getTransports());
+        assertNotNull(_port);
+        assertTrue(_port instanceof AmqpPort);
+        assertEquals("Unexpected _port", _portNumber, _port.getPort());
+        assertEquals("Unexpected transports", Collections.singleton(PortFactory.DEFAULT_TRANSPORT), _port.getTransports());
         assertEquals("Unexpected send buffer size", PortFactory.DEFAULT_AMQP_SEND_BUFFER_SIZE,
-                port.getAttribute(AmqpPort.SEND_BUFFER_SIZE));
+                _port.getAttribute(AmqpPort.SEND_BUFFER_SIZE));
         assertEquals("Unexpected receive buffer size", PortFactory.DEFAULT_AMQP_RECEIVE_BUFFER_SIZE,
-                port.getAttribute(AmqpPort.RECEIVE_BUFFER_SIZE));
+                _port.getAttribute(AmqpPort.RECEIVE_BUFFER_SIZE));
         assertEquals("Unexpected need client auth", PortFactory.DEFAULT_AMQP_NEED_CLIENT_AUTH,
-                port.getAttribute(Port.NEED_CLIENT_AUTH));
+                _port.getAttribute(Port.NEED_CLIENT_AUTH));
         assertEquals("Unexpected want client auth", PortFactory.DEFAULT_AMQP_WANT_CLIENT_AUTH,
-                port.getAttribute(Port.WANT_CLIENT_AUTH));
-        assertEquals("Unexpected tcp no delay", PortFactory.DEFAULT_AMQP_TCP_NO_DELAY, port.getAttribute(Port.TCP_NO_DELAY));
-        assertEquals("Unexpected binding", PortFactory.DEFAULT_AMQP_BINDING, port.getAttribute(Port.BINDING_ADDRESS));
+                _port.getAttribute(Port.WANT_CLIENT_AUTH));
+        assertEquals("Unexpected tcp no delay", PortFactory.DEFAULT_AMQP_TCP_NO_DELAY, _port.getAttribute(Port.TCP_NO_DELAY));
+        assertEquals("Unexpected binding", PortFactory.DEFAULT_AMQP_BINDING, _port.getAttribute(Port.BINDING_ADDRESS));
     }
 
     public void testCreateAmqpPort()
@@ -256,27 +273,27 @@ public class PortFactoryTest extends Qpi
 
         _attributes.put(Port.DESIRED_STATE, State.QUIESCED);
 
-        Port<?> port = _factory.create(Port.class, _attributes, _broker);
+        _port = _factory.create(Port.class, _attributes, _broker);
 
-        assertNotNull(port);
-        assertTrue(port instanceof AmqpPort);
-        assertEquals(_portId, port.getId());
-        assertEquals(_portNumber, port.getPort());
+        assertNotNull(_port);
+        assertTrue(_port instanceof AmqpPort);
+        assertEquals(_portId, _port.getId());
+        assertEquals(_portNumber, _port.getPort());
         if(useSslTransport)
         {
-            assertEquals(_sslTransports, port.getTransports());
+            assertEquals(_sslTransports, _port.getTransports());
         }
         else
         {
-            assertEquals(_tcpTransports, port.getTransports());
+            assertEquals(_tcpTransports, _port.getTransports());
         }
-        assertEquals(amqp010ProtocolSet, port.getProtocols());
-        assertEquals("Unexpected send buffer size", 2, port.getAttribute(AmqpPort.SEND_BUFFER_SIZE));
-        assertEquals("Unexpected receive buffer size", 1, port.getAttribute(AmqpPort.RECEIVE_BUFFER_SIZE));
-        assertEquals("Unexpected need client auth", needClientAuth, port.getAttribute(Port.NEED_CLIENT_AUTH));
-        assertEquals("Unexpected want client auth", wantClientAuth, port.getAttribute(Port.WANT_CLIENT_AUTH));
-        assertEquals("Unexpected tcp no delay", true, port.getAttribute(Port.TCP_NO_DELAY));
-        assertEquals("Unexpected binding", "127.0.0.1", port.getAttribute(Port.BINDING_ADDRESS));
+        assertEquals(amqp010ProtocolSet, _port.getProtocols());
+        assertEquals("Unexpected send buffer size", 2, _port.getAttribute(AmqpPort.SEND_BUFFER_SIZE));
+        assertEquals("Unexpected receive buffer size", 1, _port.getAttribute(AmqpPort.RECEIVE_BUFFER_SIZE));
+        assertEquals("Unexpected need client auth", needClientAuth, _port.getAttribute(Port.NEED_CLIENT_AUTH));
+        assertEquals("Unexpected want client auth", wantClientAuth, _port.getAttribute(Port.WANT_CLIENT_AUTH));
+        assertEquals("Unexpected tcp no delay", true, _port.getAttribute(Port.TCP_NO_DELAY));
+        assertEquals("Unexpected binding", "127.0.0.1", _port.getAttribute(Port.BINDING_ADDRESS));
     }
 
     public void testCreateNonAmqpPort()
@@ -291,14 +308,14 @@ public class PortFactoryTest extends Qpi
         _attributes.put(Port.NAME, getName());
         _attributes.put(Port.ID, _portId);
 
-        Port<?> port = _factory.create(Port.class, _attributes, _broker);
+        _port = _factory.create(Port.class, _attributes, _broker);
 
-        assertNotNull(port);
-        assertFalse("Port should not be an AMQP-specific subclass", port instanceof AmqpPort);
-        assertEquals(_portId, port.getId());
-        assertEquals(_portNumber, port.getPort());
-        assertEquals(_tcpTransports, port.getTransports());
-        assertEquals(nonAmqpProtocolSet, port.getProtocols());
+        assertNotNull(_port);
+        assertFalse("Port should not be an AMQP-specific subclass", _port instanceof AmqpPort);
+        assertEquals(_portId, _port.getId());
+        assertEquals(_portNumber, _port.getPort());
+        assertEquals(_tcpTransports, _port.getTransports());
+        assertEquals(nonAmqpProtocolSet, _port.getProtocols());
     }
 
     public void testCreateNonAmqpPortWithPartiallySetAttributes()
@@ -312,14 +329,14 @@ public class PortFactoryTest extends Qpi
         _attributes.put(Port.NAME, getName());
         _attributes.put(Port.ID, _portId);
 
-        Port<?> port = _factory.create(Port.class, _attributes, _broker);
+        _port = _factory.create(Port.class, _attributes, _broker);
 
-        assertNotNull(port);
-        assertFalse("Port not be an AMQP-specific port subclass", port instanceof AmqpPort);
-        assertEquals(_portId, port.getId());
-        assertEquals(_portNumber, port.getPort());
-        assertEquals(Collections.singleton(PortFactory.DEFAULT_TRANSPORT), port.getTransports());
-        assertEquals(nonAmqpProtocolSet, port.getProtocols());
+        assertNotNull(_port);
+        assertFalse("Port not be an AMQP-specific _port subclass", _port instanceof AmqpPort);
+        assertEquals(_portId, _port.getId());
+        assertEquals(_portNumber, _port.getPort());
+        assertEquals(Collections.singleton(PortFactory.DEFAULT_TRANSPORT), _port.getTransports());
+        assertEquals(nonAmqpProtocolSet, _port.getProtocols());
 
     }
 
@@ -330,7 +347,7 @@ public class PortFactoryTest extends Qpi
 
         try
         {
-            Port<?> port = _factory.create(Port.class, _attributes, _broker);
+            _port = _factory.create(Port.class, _attributes, _broker);
             fail("Exception not thrown");
         }
         catch (IllegalConfigurationException e)
@@ -353,7 +370,7 @@ public class PortFactoryTest extends Qpi
 
         try
         {
-            Port<?> port = _factory.create(Port.class, attributes, _broker);
+            _port = _factory.create(Port.class, attributes, _broker);
             fail("RMI port creation should fail as another one already exist");
         }
         catch(IllegalConfigurationException e)
@@ -377,7 +394,7 @@ public class PortFactoryTest extends Qpi
 
         try
         {
-            Port<?> port = _factory.create(Port.class, attributes, _broker);
+            _port = _factory.create(Port.class, attributes, _broker);
             fail("RMI port creation should fail due to requesting SSL");
         }
         catch(IllegalConfigurationException e)



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org