QPID-5754 : [Java Broker] Make state change operations methods rather than calls to setDesiredState

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

3 files changed, 56 insertions, 65 deletions

diff --git a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/ACLFileAccessControlProviderImpl.java b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/ACLFileAccessControlProviderImpl.java
index 91e64e5334..0b31bdbc14 100644
--- a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/ACLFileAccessControlProviderImpl.java
+++ b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/ACLFileAccessControlProviderImpl.java
@@ -33,13 +33,12 @@ import org.apache.qpid.server.model.AbstractConfiguredObject;
 import org.apache.qpid.server.model.AccessControlProvider;
 import org.apache.qpid.server.model.Broker;
 import org.apache.qpid.server.model.ConfiguredObject;
-import org.apache.qpid.server.model.IllegalStateTransitionException;
 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.model.StateTransition;
 import org.apache.qpid.server.security.AccessControl;
 import org.apache.qpid.server.security.access.Operation;
-import org.apache.qpid.server.util.MapValueConverter;
 
 public class ACLFileAccessControlProviderImpl
         extends AbstractConfiguredObject<ACLFileAccessControlProviderImpl>
@@ -50,7 +49,7 @@ public class ACLFileAccessControlProviderImpl
     protected DefaultAccessControl _accessControl;
     protected final Broker _broker;
 
-    private AtomicReference<State> _state;
+    private AtomicReference<State> _state = new AtomicReference<>(State.UNINITIALIZED);
 
     @ManagedAttributeField
     private String _path;
@@ -63,9 +62,6 @@ public class ACLFileAccessControlProviderImpl
 
         _broker = broker;
 
-        State state = MapValueConverter.getEnumAttribute(State.class, STATE, attributes, State.INITIALISING);
-        _state = new AtomicReference<State>(state);
-
     }
 
     @Override
@@ -108,81 +104,68 @@ public class ACLFileAccessControlProviderImpl
     }
 
     @Override
-    public Object getAttribute(String name)
-    {
-        if(STATE.equals(name))
-        {
-            return getState();
-        }
-        return super.getAttribute(name);
-    }
-
-    @Override
     public <C extends ConfiguredObject> Collection<C> getChildren(Class<C> clazz)
     {
         return Collections.emptySet();
     }
 
-    @Override
-    public boolean setState(State desiredState)
-            throws IllegalStateTransitionException, AccessControlException
-    {
-        State state = _state.get();
 
-        if(desiredState == State.DELETED)
-        {
-            deleted();
-            return _state.compareAndSet(state, State.DELETED);
-        }
-        else if (desiredState == State.QUIESCED)
+    @StateTransition(currentState = {State.UNINITIALIZED, State.QUIESCED, State.ERRORED}, desiredState = State.ACTIVE)
+    private void activate()
+    {
+        if(_broker.isManagementMode())
         {
-            return _state.compareAndSet(state, State.QUIESCED);
+
+            _state.set(_accessControl.validate() ? State.QUIESCED : State.ERRORED);
         }
-        else if(desiredState == State.ACTIVE)
+        else
         {
-            if ((state == State.INITIALISING || state == State.QUIESCED) && _state.compareAndSet(state, State.ACTIVE))
+            try
+            {
+                _accessControl.open();
+                _state.set(State.ACTIVE);
+            }
+            catch (RuntimeException e)
             {
-                try
+                _state.set(State.ERRORED);
+                if (_broker.isManagementMode())
                 {
-                    _accessControl.open();
-                    return true;
+                    LOGGER.warn("Failed to activate ACL provider: " + getName(), e);
                 }
-                catch(RuntimeException e)
+                else
                 {
-                    _state.compareAndSet(State.ACTIVE, State.ERRORED);
-                    if (_broker.isManagementMode())
-                    {
-                        LOGGER.warn("Failed to activate ACL provider: " + getName(), e);
-                    }
-                    else
-                    {
-                        throw e;
-                    }
+                    throw e;
                 }
             }
-            else
-            {
-                throw new IllegalStateException("Can't activate access control provider in " + state + " state");
-            }
-        }
-        else if(desiredState == State.STOPPED)
-        {
-            if(_state.compareAndSet(state, State.STOPPED))
-            {
-                _accessControl.close();
-                return true;
-            }
-
-            return false;
         }
-        return false;
     }
 
+    @StateTransition(currentState = {State.ACTIVE, State.QUIESCED}, desiredState = State.STOPPED)
+    private void doStop()
+    {
+        close();
+        _state.set(State.STOPPED);
+    }
 
     @Override
-    protected void changeAttributes(Map<String, Object> attributes)
+    protected void onClose()
+    {
+        super.onClose();
+        _accessControl.close();
+    }
+
+    @StateTransition(currentState = State.UNINITIALIZED, desiredState = State.QUIESCED)
+    private void startQuiesced()
+    {
+        _state.set(State.QUIESCED);
+    }
+
+    @StateTransition(currentState = {State.ACTIVE, State.QUIESCED, State.STOPPED, State.ERRORED}, desiredState = State.DELETED)
+    private void doDelete()
     {
-        throw new UnsupportedOperationException("Changing attributes on AccessControlProvider is not supported");
+        close();
+        _state.set(State.DELETED);
+        deleted();
     }
 
     @Override
diff --git a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControl.java b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControl.java
index 5f5e12d435..c42dc88d53 100644
--- a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControl.java
+++ b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControl.java
@@ -20,9 +20,9 @@
  */
 package org.apache.qpid.server.security.access.plugins;
 
+import java.io.File;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
-import java.io.File;
 import java.net.SocketAddress;
 import java.security.AccessController;
 import java.util.Set;
@@ -31,12 +31,12 @@ import javax.security.auth.Subject;
 
 import org.apache.commons.lang.ObjectUtils;
 import org.apache.log4j.Logger;
+
 import org.apache.qpid.server.configuration.IllegalConfigurationException;
 import org.apache.qpid.server.connection.ConnectionPrincipal;
-import org.apache.qpid.server.logging.EventLogger;
 import org.apache.qpid.server.logging.EventLoggerProvider;
-import org.apache.qpid.server.security.Result;
 import org.apache.qpid.server.security.AccessControl;
+import org.apache.qpid.server.security.Result;
 import org.apache.qpid.server.security.access.ObjectProperties;
 import org.apache.qpid.server.security.access.ObjectType;
 import org.apache.qpid.server.security.access.Operation;
@@ -73,7 +73,7 @@ public class DefaultAccessControl implements AccessControl
     {
         if(_aclFile != null)
         {
-            if (!_aclFile.exists())
+            if (!validate())
             {
                 throw new IllegalConfigurationException("ACL file '" + _aclFile + "' is not found");
             }
@@ -84,6 +84,12 @@ public class DefaultAccessControl implements AccessControl
     }
 
     @Override
+    public boolean validate()
+    {
+        return _aclFile.exists();
+    }
+
+    @Override
     public void close()
     {
         //no-op
@@ -101,7 +107,7 @@ public class DefaultAccessControl implements AccessControl
         if(_aclFile != null)
         {
             //verify it exists
-            if (!_aclFile.exists())
+            if (!validate())
             {
                 throw new IllegalConfigurationException("ACL file '" + _aclFile + "' is not found");
             }
diff --git a/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/ACLFileAccessControlProviderFactoryTest.java b/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/ACLFileAccessControlProviderFactoryTest.java
index 01da01eb97..49697cf5b7 100644
--- a/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/ACLFileAccessControlProviderFactoryTest.java
+++ b/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/ACLFileAccessControlProviderFactoryTest.java
@@ -30,6 +30,7 @@ import java.util.UUID;
 import java.util.regex.Pattern;
 
 import org.apache.qpid.server.configuration.IllegalConfigurationException;
+import org.apache.qpid.server.configuration.updater.TaskExecutor;
 import org.apache.qpid.server.model.AccessControlProvider;
 import org.apache.qpid.server.model.Broker;
 import org.apache.qpid.server.model.BrokerModel;
@@ -53,6 +54,7 @@ public class ACLFileAccessControlProviderFactoryTest extends QpidTestCase
         when(_broker.getObjectFactory()).thenReturn(_objectFactory);
         when(_broker.getModel()).thenReturn(_objectFactory.getModel());
         when(_broker.getCategoryClass()).thenReturn(Broker.class);
+        when(_broker.getTaskExecutor()).thenReturn(mock(TaskExecutor.class));
     }
 
     public void testCreateInstanceWhenAclFileIsNotPresent()