QPID-4802: In management mode set state to ERRORED for failing to activate authentication providers, group providers and acl providers in order to allow editing of attributes preventing normal startup

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1478731 13f79535-47bb-0310-9956-ffa450edef68

6 files changed, 173 insertions, 41 deletions

diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/store/ManagementModeStoreHandler.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/store/ManagementModeStoreHandler.java
index abf1537ef7..d08deee29b 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/store/ManagementModeStoreHandler.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/store/ManagementModeStoreHandler.java
@@ -13,6 +13,8 @@ import org.apache.qpid.server.configuration.ConfigurationEntry;
 import org.apache.qpid.server.configuration.ConfigurationEntryStore;
 import org.apache.qpid.server.configuration.IllegalConfigurationException;
 import org.apache.qpid.server.model.AccessControlProvider;
+import org.apache.qpid.server.model.AuthenticationProvider;
+import org.apache.qpid.server.model.GroupProvider;
 import org.apache.qpid.server.model.Port;
 import org.apache.qpid.server.model.Protocol;
 import org.apache.qpid.server.model.State;
@@ -27,9 +29,12 @@ public class ManagementModeStoreHandler implements ConfigurationEntryStore
     private static final String PORT_TYPE = Port.class.getSimpleName();
     private static final String VIRTUAL_HOST_TYPE = VirtualHost.class.getSimpleName();
     private static final String ACCESS_CONTROL_PROVIDER_TYPE = AccessControlProvider.class.getSimpleName();
+    private static final String GROUP_PROVIDER_TYPE = GroupProvider.class.getSimpleName();
+    private static final String AUTHENTICATION_PROVIDER_TYPE = AuthenticationProvider.class.getSimpleName();
     private static final String ATTRIBUTE_STATE = VirtualHost.STATE;
     private static final Object MANAGEMENT_MODE_AUTH_PROVIDER = "mm-auth";
 
+
     private final ConfigurationEntryStore _store;
     private final Map<UUID, ConfigurationEntry> _cliEntries;
     private final Map<UUID, Object> _quiescedEntries;
@@ -255,11 +260,7 @@ public class ManagementModeStoreHandler implements ConfigurationEntryStore
             {
                 quiesce = true;
             }
-            else if (ACCESS_CONTROL_PROVIDER_TYPE.equals(entryType))
-            {
-                quiesce = true;
-            }
-            else if (PORT_TYPE.equalsIgnoreCase(entryType))
+            else if (PORT_TYPE.equals(entryType))
             {
                 if (attributes == null)
                 {
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AbstractAdapter.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AbstractAdapter.java
index 51fd3a5c78..a8c3f54530 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AbstractAdapter.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AbstractAdapter.java
@@ -351,15 +351,7 @@ abstract class AbstractAdapter implements ConfiguredObject
 
     protected void changeAttributes(final Map<String, Object> attributes)
     {
-        if (attributes.containsKey(ID))
-        {
-            UUID id = getId();
-            Object idAttributeValue = attributes.get(ID);
-            if (idAttributeValue != null && !idAttributeValue.equals(id))
-            {
-                throw new IllegalConfigurationException("Cannot change existing configured object id");
-            }
-        }
+        validateChangeAttributes(attributes);
         Collection<String> names = getAttributeNames();
         for (String name : names)
         {
@@ -375,6 +367,19 @@ abstract class AbstractAdapter implements ConfiguredObject
         }
     }
 
+    protected void validateChangeAttributes(final Map<String, Object> attributes)
+    {
+        if (attributes.containsKey(ID))
+        {
+            UUID id = getId();
+            Object idAttributeValue = attributes.get(ID);
+            if (idAttributeValue != null && !idAttributeValue.equals(id.toString()))
+            {
+                throw new IllegalConfigurationException("Cannot change existing configured object id");
+            }
+        }
+    }
+
     protected void authoriseSetDesiredState(State currentState, State desiredState) throws AccessControlException
     {
         // allowed by default
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AccessControlProviderAdapter.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AccessControlProviderAdapter.java
index 75b80eb56c..a6fe191523 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AccessControlProviderAdapter.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AccessControlProviderAdapter.java
@@ -29,6 +29,7 @@ import java.util.Map;
 import java.util.UUID;
 import java.util.concurrent.atomic.AtomicReference;
 
+import org.apache.log4j.Logger;
 import org.apache.qpid.server.model.AccessControlProvider;
 import org.apache.qpid.server.model.Broker;
 import org.apache.qpid.server.model.ConfiguredObject;
@@ -43,6 +44,8 @@ import org.apache.qpid.server.util.MapValueConverter;
 
 public class AccessControlProviderAdapter extends AbstractAdapter implements AccessControlProvider
 {
+    private static final Logger LOGGER = Logger.getLogger(AccessControlProviderAdapter.class);
+
     protected AccessControl _accessControl;
     protected final Broker _broker;
 
@@ -217,8 +220,23 @@ public class AccessControlProviderAdapter extends AbstractAdapter implements Acc
         {
             if ((state == State.INITIALISING || state == State.QUIESCED) && _state.compareAndSet(state, State.ACTIVE))
             {
-                _accessControl.open();
-                return true;
+                try
+                {
+                    _accessControl.open();
+                    return true;
+                }
+                catch(RuntimeException e)
+                {
+                    _state.compareAndSet(State.ACTIVE, State.ERRORED);
+                    if (_broker.isManagementMode())
+                    {
+                        LOGGER.warn("Failed to activate ACL provider: " + getName(), e);
+                    }
+                    else
+                    {
+                        throw e;
+                    }
+                }
             }
             else
             {
@@ -235,7 +253,6 @@ public class AccessControlProviderAdapter extends AbstractAdapter implements Acc
 
             return false;
         }
-
         return false;
     }
 
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AuthenticationProviderAdapter.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AuthenticationProviderAdapter.java
index 24f4757a18..5c4c8a53cd 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AuthenticationProviderAdapter.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AuthenticationProviderAdapter.java
@@ -30,6 +30,7 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.UUID;
+import java.util.concurrent.atomic.AtomicReference;
 
 import javax.security.auth.login.AccountNotFoundException;
 
@@ -58,6 +59,7 @@ import org.apache.qpid.server.security.auth.database.PrincipalDatabase;
 import org.apache.qpid.server.security.auth.manager.AuthenticationManager;
 import org.apache.qpid.server.security.auth.manager.PrincipalDatabaseAuthenticationManager;
 import org.apache.qpid.server.security.SecurityManager;
+import org.apache.qpid.server.util.MapValueConverter;
 
 public abstract class AuthenticationProviderAdapter<T extends AuthenticationManager> extends AbstractAdapter implements AuthenticationProvider
 {
@@ -68,6 +70,7 @@ public abstract class AuthenticationProviderAdapter<T extends AuthenticationMana
 
     protected Collection<String> _supportedAttributes;
     protected Map<String, AuthenticationManagerFactory> _factories;
+    private AtomicReference<State> _state;
 
     private AuthenticationProviderAdapter(UUID id, Broker broker, final T authManager, Map<String, Object> attributes, Collection<String> attributeNames)
     {
@@ -76,6 +79,9 @@ public abstract class AuthenticationProviderAdapter<T extends AuthenticationMana
         _broker = broker;
         _supportedAttributes = createSupportedAttributes(attributeNames);
         _factories = getAuthenticationManagerFactories();
+
+        State state = MapValueConverter.getEnumAttribute(State.class, STATE, attributes, State.INITIALISING);
+        _state = new AtomicReference<State>(state);
         addParent(Broker.class, broker);
 
         // set attributes now after all attribute names are known
@@ -117,7 +123,7 @@ public abstract class AuthenticationProviderAdapter<T extends AuthenticationMana
     @Override
     public State getActualState()
     {
-        return null;
+        return _state.get();
     }
 
     @Override
@@ -191,7 +197,7 @@ public abstract class AuthenticationProviderAdapter<T extends AuthenticationMana
         }
         else if(STATE.equals(name))
         {
-            return State.ACTIVE; // TODO
+            return getActualState();
         }
         else if(TIME_TO_LIVE.equals(name))
         {
@@ -214,6 +220,7 @@ public abstract class AuthenticationProviderAdapter<T extends AuthenticationMana
     public boolean setState(State currentState, State desiredState)
             throws IllegalStateTransitionException, AccessControlException
     {
+        State state = _state.get();
         if(desiredState == State.DELETED)
         {
             String providerName = getName();
@@ -227,20 +234,66 @@ public abstract class AuthenticationProviderAdapter<T extends AuthenticationMana
                     throw new IntegrityViolationException("Authentication provider '" + providerName + "' is set on port " + port.getName());
                 }
             }
-            _authManager.close();
-            _authManager.onDelete();
-            return true;
+
+            if ((state == State.INITIALISING || state == State.ACTIVE || state == State.STOPPED || state == State.QUIESCED  || state == State.ERRORED)
+                    && _state.compareAndSet(state, State.DELETED))
+            {
+                _authManager.close();
+                _authManager.onDelete();
+                return true;
+            }
+            else
+            {
+                throw new IllegalStateException("Cannot delete authentication provider in state: " + state);
+            }
         }
         else if(desiredState == State.ACTIVE)
         {
-            _authManager.initialise();
-            return true;
+            if ((state == State.INITIALISING || state == State.QUIESCED || state == State.STOPPED) && _state.compareAndSet(state, State.ACTIVE))
+            {
+                try
+                {
+                    _authManager.initialise();
+                    return true;
+                }
+                catch(RuntimeException e)
+                {
+                    _state.compareAndSet(State.ACTIVE, State.ERRORED);
+                    if (_broker.isManagementMode())
+                    {
+                        LOGGER.warn("Failed to activate authentication provider: " + getName(), e);
+                    }
+                    else
+                    {
+                        throw e;
+                    }
+                }
+            }
+            else
+            {
+                throw new IllegalStateException("Cannot activate authentication provider in state: " + state);
+            }
+        }
+        else if (desiredState == State.QUIESCED)
+        {
+            if (state == State.INITIALISING && _state.compareAndSet(state, State.QUIESCED))
+            {
+                return true;
+            }
         }
         else if(desiredState == State.STOPPED)
         {
-            _authManager.close();
-            return true;
+            if (_state.compareAndSet(state, State.STOPPED))
+            {
+                _authManager.close();
+                return true;
+            }
+            else
+            {
+                throw new IllegalStateException("Cannot stop authentication provider in state: " + state);
+            }
         }
+
         return false;
     }
 
@@ -256,11 +309,11 @@ public abstract class AuthenticationProviderAdapter<T extends AuthenticationMana
         Map<String, Object> effectiveAttributes = super.generateEffectiveAttributes(attributes);
         AuthenticationManager manager = validateAttributes(effectiveAttributes);
         manager.initialise();
-        _authManager = (T)manager;
-        String type = (String)effectiveAttributes.get(AuthenticationManagerFactory.ATTRIBUTE_TYPE);
-        AuthenticationManagerFactory managerFactory = _factories.get(type);
-        _supportedAttributes = createSupportedAttributes(managerFactory.getAttributeNames());
         super.changeAttributes(attributes);
+        _authManager = (T)manager;
+
+        // if provider was previously in ERRORED state then set its state to ACTIVE
+        _state.compareAndSet(State.ERRORED, State.ACTIVE);
     }
 
     private Map<String, AuthenticationManagerFactory> getAuthenticationManagerFactories()
@@ -287,6 +340,8 @@ public abstract class AuthenticationProviderAdapter<T extends AuthenticationMana
 
     protected AuthenticationManager validateAttributes(Map<String, Object> attributes)
     {
+        super.validateChangeAttributes(attributes);
+
         String newName = (String)attributes.get(NAME);
         String currentName = getName();
         if (!currentName.equals(newName))
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/GroupProviderAdapter.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/GroupProviderAdapter.java
index a0e5bdb0e8..3bf62dc96b 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/GroupProviderAdapter.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/GroupProviderAdapter.java
@@ -28,7 +28,9 @@ 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;
 import org.apache.qpid.server.model.Broker;
 import org.apache.qpid.server.model.ConfiguredObject;
 import org.apache.qpid.server.model.Group;
@@ -43,13 +45,17 @@ import org.apache.qpid.server.configuration.updater.TaskExecutor;
 import org.apache.qpid.server.security.access.Operation;
 import org.apache.qpid.server.security.group.GroupManager;
 import org.apache.qpid.server.security.SecurityManager;
+import org.apache.qpid.server.util.MapValueConverter;
 
 public class GroupProviderAdapter extends AbstractAdapter implements
         GroupProvider
 {
+    private static Logger LOGGER = Logger.getLogger(GroupProviderAdapter.class);
+
     private final GroupManager _groupManager;
     private final Broker _broker;
     private Collection<String> _supportedAttributes;
+    private AtomicReference<State> _state;
 
     public GroupProviderAdapter(UUID id, Broker broker, GroupManager groupManager, Map<String, Object> attributes, Collection<String> attributeNames)
     {
@@ -62,6 +68,8 @@ public class GroupProviderAdapter extends AbstractAdapter implements
         _groupManager = groupManager;
         _broker = broker;
         _supportedAttributes = createSupportedAttributes(attributeNames);
+        State state = MapValueConverter.getEnumAttribute(State.class, STATE, attributes, State.INITIALISING);
+        _state = new AtomicReference<State>(state);
        addParent(Broker.class, broker);
 
        // set attributes now after all attribute names are known
@@ -104,7 +112,7 @@ public class GroupProviderAdapter extends AbstractAdapter implements
     @Override
     public State getActualState()
     {
-        return null;
+        return _state.get();
     }
 
     @Override
@@ -180,7 +188,7 @@ public class GroupProviderAdapter extends AbstractAdapter implements
         }
         else if (STATE.equals(name))
         {
-            return State.ACTIVE; // TODO
+            return getActualState();
         }
         else if (TIME_TO_LIVE.equals(name))
         {
@@ -252,21 +260,67 @@ public class GroupProviderAdapter extends AbstractAdapter implements
     @Override
     protected boolean setState(State currentState, State desiredState)
     {
+        State state = _state.get();
         if (desiredState == State.ACTIVE)
         {
-            _groupManager.open();
-            return true;
+            if ((state == State.INITIALISING || state == State.QUIESCED || state == State.STOPPED)
+                    && _state.compareAndSet(state, State.ACTIVE))
+            {
+                try
+                {
+                    _groupManager.open();
+                    return true;
+                }
+                catch(RuntimeException e)
+                {
+                    _state.compareAndSet(State.ACTIVE, State.ERRORED);
+                    if (_broker.isManagementMode())
+                    {
+                        LOGGER.warn("Failed to activate group provider: " + getName(), e);
+                    }
+                    else
+                    {
+                        throw e;
+                    }
+                }
+            }
+            else
+            {
+                throw new IllegalStateException("Cannot activate group provider in state: " + state);
+            }
         }
         else if (desiredState == State.STOPPED)
         {
-            _groupManager.close();
-            return true;
+            if (_state.compareAndSet(state, State.STOPPED))
+            {
+                _groupManager.close();
+                return true;
+            }
+            else
+            {
+                throw new IllegalStateException("Cannot stop group provider in state: " + state);
+            }
         }
         else if (desiredState == State.DELETED)
         {
-            _groupManager.close();
-            _groupManager.onDelete();
-            return true;
+            if ((state == State.INITIALISING || state == State.ACTIVE || state == State.STOPPED || state == State.QUIESCED || state == State.ERRORED)
+                    && _state.compareAndSet(state, State.DELETED))
+            {
+                _groupManager.close();
+                _groupManager.onDelete();
+                return true;
+            }
+            else
+            {
+                throw new IllegalStateException("Cannot delete group provider in state: " + state);
+            }
+        }
+        else if (desiredState == State.QUIESCED)
+        {
+            if (state == State.INITIALISING && _state.compareAndSet(state, State.QUIESCED))
+            {
+                return true;
+            }
         }
         return false;
     }
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/PortAdapter.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/PortAdapter.java
index 388427678e..de6ae06b94 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/PortAdapter.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/PortAdapter.java
@@ -309,7 +309,7 @@ public class PortAdapter extends AbstractAdapter implements Port
         State state = _state.get();
         if (desiredState == State.DELETED)
         {
-            if (state == State.INITIALISING || state == State.ACTIVE || state == State.STOPPED || state == State.QUIESCED)
+            if (state == State.INITIALISING || state == State.ACTIVE || state == State.STOPPED || state == State.QUIESCED  || state == State.ERRORED)
             {
                 return _state.compareAndSet(state, State.DELETED);
             }
@@ -328,7 +328,7 @@ public class PortAdapter extends AbstractAdapter implements Port
                 }
                 catch(RuntimeException e)
                 {
-                    _state.compareAndSet(State.ACTIVE, state);
+                    _state.compareAndSet(State.ACTIVE, State.ERRORED);
                     throw e;
                 }
                 return true;