QPID-6126: Add ability to validate CO attributes on creation, transit COs into ERRORED state if exception occurs on recovery, allow ERRORED CO restart after remediation of configuration problem

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

7 files changed, 122 insertions, 10 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 f0edc59025..1adc6561c7 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
@@ -25,10 +25,10 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.Map;
 import java.util.Set;
-import java.util.concurrent.atomic.AtomicReference;
 
 import org.apache.log4j.Logger;
 
+import org.apache.qpid.server.configuration.IllegalConfigurationException;
 import org.apache.qpid.server.model.AbstractConfiguredObject;
 import org.apache.qpid.server.model.AccessControlProvider;
 import org.apache.qpid.server.model.Broker;
@@ -83,6 +83,29 @@ public class ACLFileAccessControlProviderImpl
     }
 
     @Override
+    protected void validateOnCreate()
+    {
+        DefaultAccessControl accessControl = null;
+        try
+        {
+            accessControl = new DefaultAccessControl(getPath(), _broker);
+            accessControl.validate();
+            accessControl.open();
+        }
+        catch(RuntimeException e)
+        {
+            throw new IllegalConfigurationException(e.getMessage(), e);
+        }
+        finally
+        {
+            if (accessControl != null)
+            {
+                accessControl.close();
+            }
+        }
+    }
+
+    @Override
     protected void onOpen()
     {
         super.onOpen();
@@ -105,6 +128,7 @@ public class ACLFileAccessControlProviderImpl
     @StateTransition(currentState = {State.UNINITIALIZED, State.QUIESCED, State.ERRORED}, desiredState = State.ACTIVE)
     private void activate()
     {
+
         if(_broker.isManagementMode())
         {
 
@@ -136,7 +160,10 @@ public class ACLFileAccessControlProviderImpl
     protected void onClose()
     {
         super.onClose();
-        _accessControl.close();
+        if (_accessControl != null)
+        {
+            _accessControl.close();
+        }
     }
 
     @StateTransition(currentState = State.UNINITIALIZED, desiredState = State.QUIESCED)
diff --git a/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/ACLFileAccessControlProviderImplTest.java b/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/ACLFileAccessControlProviderImplTest.java
new file mode 100644
index 0000000000..781e553fe5
--- /dev/null
+++ b/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/ACLFileAccessControlProviderImplTest.java
@@ -0,0 +1,79 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+package org.apache.qpid.server.security.access.plugins;
+
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.io.File;
+import java.util.HashMap;
+import java.util.Map;
+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.Broker;
+import org.apache.qpid.server.model.BrokerModel;
+import org.apache.qpid.server.model.Model;
+import org.apache.qpid.test.utils.QpidTestCase;
+
+public class ACLFileAccessControlProviderImplTest extends QpidTestCase
+{
+    private TaskExecutor _taskExecutor;
+    private Model _model;
+    private Broker _broker;
+
+    public void setUp() throws Exception
+    {
+        super.setUp();
+        _taskExecutor = CurrentThreadTaskExecutor.newStartedInstance();
+        _model = BrokerModel.getInstance();
+
+        _broker = mock(Broker.class);
+        when(_broker.getTaskExecutor()).thenReturn(_taskExecutor);
+        when(_broker.getModel()).thenReturn(_model);
+        when(_broker.getId()).thenReturn(UUID.randomUUID());
+    }
+
+    public void testValidationOnCreateWithNonExistingACLFile()
+    {
+        Map<String,Object> attributes = new HashMap<>();
+        String aclFilePath = TMP_FOLDER + File.separator + "test_" + getTestName() + System.nanoTime() + ".acl";
+        attributes.put("path", aclFilePath);
+        attributes.put(ACLFileAccessControlProvider.NAME, getTestName());
+
+
+        ACLFileAccessControlProviderImpl aclProvider = new ACLFileAccessControlProviderImpl(attributes, _broker);
+        try
+        {
+            aclProvider.create();
+            fail("Exception is expected on validation with non-existing ACL file");
+        }
+        catch (IllegalConfigurationException e)
+        {
+            assertEquals("Unexpected exception message:" + e.getMessage(), String.format("ACL file '%s' is not found", aclFilePath ), e.getMessage());
+        }
+    }
+
+}
diff --git a/qpid/java/broker-plugins/derby-store/src/main/java/org/apache/qpid/server/store/derby/DerbySystemConfigImpl.java b/qpid/java/broker-plugins/derby-store/src/main/java/org/apache/qpid/server/store/derby/DerbySystemConfigImpl.java
index 32c5bcd541..a8b9edaa12 100644
--- a/qpid/java/broker-plugins/derby-store/src/main/java/org/apache/qpid/server/store/derby/DerbySystemConfigImpl.java
+++ b/qpid/java/broker-plugins/derby-store/src/main/java/org/apache/qpid/server/store/derby/DerbySystemConfigImpl.java
@@ -26,6 +26,7 @@ import org.apache.qpid.server.logging.EventLogger;
 import org.apache.qpid.server.logging.LogRecorder;
 import org.apache.qpid.server.model.AbstractSystemConfig;
 import org.apache.qpid.server.model.Broker;
+import org.apache.qpid.server.model.BrokerShutdownProvider;
 import org.apache.qpid.server.model.ManagedAttributeField;
 import org.apache.qpid.server.model.ManagedObject;
 import org.apache.qpid.server.model.SystemConfigFactoryConstructor;
@@ -47,9 +48,10 @@ public class DerbySystemConfigImpl extends AbstractSystemConfig<DerbySystemConfi
     public DerbySystemConfigImpl(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);
     }
 
     @Override
diff --git a/qpid/java/broker-plugins/jdbc-store/src/main/java/org/apache/qpid/server/store/jdbc/JDBCSystemConfigImpl.java b/qpid/java/broker-plugins/jdbc-store/src/main/java/org/apache/qpid/server/store/jdbc/JDBCSystemConfigImpl.java
index fd1cad7de4..a552b170a0 100644
--- a/qpid/java/broker-plugins/jdbc-store/src/main/java/org/apache/qpid/server/store/jdbc/JDBCSystemConfigImpl.java
+++ b/qpid/java/broker-plugins/jdbc-store/src/main/java/org/apache/qpid/server/store/jdbc/JDBCSystemConfigImpl.java
@@ -26,6 +26,7 @@ import org.apache.qpid.server.logging.EventLogger;
 import org.apache.qpid.server.logging.LogRecorder;
 import org.apache.qpid.server.model.AbstractSystemConfig;
 import org.apache.qpid.server.model.Broker;
+import org.apache.qpid.server.model.BrokerShutdownProvider;
 import org.apache.qpid.server.model.ManagedAttributeField;
 import org.apache.qpid.server.model.ManagedObject;
 import org.apache.qpid.server.model.SystemConfigFactoryConstructor;
@@ -49,9 +50,10 @@ public class JDBCSystemConfigImpl extends AbstractSystemConfig<JDBCSystemConfigI
     public JDBCSystemConfigImpl(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);
     }
 
     @Override
diff --git a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagement.java b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagement.java
index e07705656a..936cc4789a 100644
--- a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagement.java
+++ b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagement.java
@@ -122,7 +122,7 @@ public class HttpManagement extends AbstractPluginAdapter<HttpManagement> implem
         super(attributes, broker);
     }
 
-    @StateTransition(currentState = State.UNINITIALIZED, desiredState = State.ACTIVE)
+    @StateTransition(currentState = {State.UNINITIALIZED,State.ERRORED}, desiredState = State.ACTIVE)
     private void doStart()
     {
         getBroker().getEventLogger().message(ManagementConsoleMessages.STARTUP(OPERATIONAL_LOGGING_NAME));
diff --git a/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/JMXManagementPluginImpl.java b/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/JMXManagementPluginImpl.java
index a38aa9e349..85916bf5f2 100644
--- a/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/JMXManagementPluginImpl.java
+++ b/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/JMXManagementPluginImpl.java
@@ -103,7 +103,7 @@ public class JMXManagementPluginImpl
         return _usePlatformMBeanServer;
     }
 
-    @StateTransition(currentState = State.UNINITIALIZED, desiredState = State.ACTIVE)
+    @StateTransition(currentState = {State.UNINITIALIZED,State.ERRORED}, desiredState = State.ACTIVE)
     private void doStart() throws JMException, IOException
     {
         _allowPortActivation = true;
diff --git a/qpid/java/broker-plugins/memory-store/src/main/java/org/apache/qpid/server/store/MemorySystemConfigImpl.java b/qpid/java/broker-plugins/memory-store/src/main/java/org/apache/qpid/server/store/MemorySystemConfigImpl.java
index f644b8f46b..3f5215219b 100644
--- a/qpid/java/broker-plugins/memory-store/src/main/java/org/apache/qpid/server/store/MemorySystemConfigImpl.java
+++ b/qpid/java/broker-plugins/memory-store/src/main/java/org/apache/qpid/server/store/MemorySystemConfigImpl.java
@@ -26,6 +26,7 @@ import org.apache.qpid.server.logging.EventLogger;
 import org.apache.qpid.server.logging.LogRecorder;
 import org.apache.qpid.server.model.AbstractSystemConfig;
 import org.apache.qpid.server.model.Broker;
+import org.apache.qpid.server.model.BrokerShutdownProvider;
 import org.apache.qpid.server.model.ManagedObject;
 import org.apache.qpid.server.model.SystemConfigFactoryConstructor;
 
@@ -39,9 +40,10 @@ public class MemorySystemConfigImpl extends AbstractSystemConfig<MemorySystemCon
     public MemorySystemConfigImpl(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);
     }
 
     @Override