QPID-4390: Introduce a configuration store in java broker allowing runtime modifications and replace existing xml file configuration with json configuration store

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

7 files changed, 139 insertions, 62 deletions

diff --git a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AbstractConfiguration.java b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AbstractConfiguration.java
index 44c48523e2..f87374ac80 100644
--- a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AbstractConfiguration.java
+++ b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AbstractConfiguration.java
@@ -22,8 +22,6 @@ package org.apache.qpid.server.security.access.config;
 
 import java.io.File;
 
-import org.apache.commons.configuration.ConfigurationException;
-
 public abstract class AbstractConfiguration implements ConfigurationFile
 {
     private File _file;
@@ -39,7 +37,7 @@ public abstract class AbstractConfiguration implements ConfigurationFile
         return _file;
     }
     
-    public RuleSet load() throws ConfigurationException
+    public RuleSet load()
     {
         _config = new RuleSet();
         return _config;
diff --git a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/ConfigurationFile.java b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/ConfigurationFile.java
index 8b1a00259b..966c32e24e 100644
--- a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/ConfigurationFile.java
+++ b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/ConfigurationFile.java
@@ -22,7 +22,7 @@ package org.apache.qpid.server.security.access.config;
 
 import java.io.File;
 
-import org.apache.commons.configuration.ConfigurationException;
+import org.apache.qpid.server.configuration.IllegalConfigurationException;
 
 public interface ConfigurationFile
 {
@@ -33,19 +33,17 @@ public interface ConfigurationFile
     
     /**
      * Load this configuration file's contents into a {@link RuleSet}.
-     * 
-     * @throws ConfigurationException if the configuration file has errors.
+     * @throws IllegalConfigurationException if the configuration file has errors.
      * @throws IllegalArgumentException if individual tokens cannot be parsed.
      */
-    RuleSet load() throws ConfigurationException;
+    RuleSet load() throws IllegalConfigurationException;
     
     /**
      * Reload this configuration file's contents.
-     * 
-     * @throws ConfigurationException if the configuration file has errors.
+     * @throws IllegalConfigurationException if the configuration file has errors.
      * @throws IllegalArgumentException if individual tokens cannot be parsed.
      */
-    RuleSet reload() throws ConfigurationException;
+    RuleSet reload() throws IllegalConfigurationException;
         
     RuleSet getConfiguration();
     
diff --git a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/PlainConfiguration.java b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/PlainConfiguration.java
index 86f8ca3217..ab309c54ce 100644
--- a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/PlainConfiguration.java
+++ b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/PlainConfiguration.java
@@ -32,9 +32,9 @@ import java.util.List;
 import java.util.Map;
 import java.util.Stack;
 
-import org.apache.commons.configuration.ConfigurationException;
 import org.apache.commons.lang.StringUtils;
 import org.apache.log4j.Logger;
+import org.apache.qpid.server.configuration.IllegalConfigurationException;
 import org.apache.qpid.server.security.access.ObjectType;
 import org.apache.qpid.server.security.access.Operation;
 import org.apache.qpid.server.security.access.Permission;
@@ -73,7 +73,7 @@ public class PlainConfiguration extends AbstractConfiguration
     }
 
     @Override
-    public RuleSet load() throws ConfigurationException
+    public RuleSet load()
     {
         RuleSet ruleSet = super.load();
 
@@ -127,7 +127,7 @@ public class PlainConfiguration extends AbstractConfiguration
                         stack.removeElementAt(0);
                         if (stack.isEmpty())
                         {
-                            throw new ConfigurationException(String.format(NOT_ENOUGH_TOKENS_MSG, getLine()));
+                            throw new IllegalConfigurationException(String.format(NOT_ENOUGH_TOKENS_MSG, getLine()));
                         }
 
                         // check for and parse optional initial number for ACL lines
@@ -148,7 +148,7 @@ public class PlainConfiguration extends AbstractConfiguration
                         {
                             if(StringUtils.equalsIgnoreCase("GROUP", first))
                             {
-                                throw new ConfigurationException(String.format("GROUP keyword not supported. Groups should defined via a Group Provider, not in the ACL file.", getLine()));
+                                throw new IllegalConfigurationException(String.format("GROUP keyword not supported. Groups should defined via a Group Provider, not in the ACL file.", getLine()));
                             }
                             else if (StringUtils.equalsIgnoreCase(CONFIG, first))
                             {
@@ -156,12 +156,12 @@ public class PlainConfiguration extends AbstractConfiguration
                             }
                             else
                             {
-                                throw new ConfigurationException(String.format(UNRECOGNISED_INITIAL_MSG, first, getLine()));
+                                throw new IllegalConfigurationException(String.format(UNRECOGNISED_INITIAL_MSG, first, getLine()));
                             }
                         }
                         else
                         {
-                            throw new ConfigurationException(String.format(NUMBER_NOT_ALLOWED_MSG, first, getLine()));
+                            throw new IllegalConfigurationException(String.format(NUMBER_NOT_ALLOWED_MSG, first, getLine()));
                         }
 
                         // reset stack, start next line
@@ -183,7 +183,7 @@ public class PlainConfiguration extends AbstractConfiguration
                             }
 
                             // invalid location for continuation character (add one to line beacuse we ate the EOL)
-                            throw new ConfigurationException(String.format(PREMATURE_CONTINUATION_MSG, getLine() + 1));
+                            throw new IllegalConfigurationException(String.format(PREMATURE_CONTINUATION_MSG, getLine() + 1));
                         }
                         else if (_st.ttype == '\'' || _st.ttype == '"')
                         {
@@ -198,20 +198,20 @@ public class PlainConfiguration extends AbstractConfiguration
 
             if (!stack.isEmpty())
             {
-                throw new ConfigurationException(String.format(PREMATURE_EOF_MSG, getLine()));
+                throw new IllegalConfigurationException(String.format(PREMATURE_EOF_MSG, getLine()));
             }
         }
         catch (IllegalArgumentException iae)
         {
-            throw new ConfigurationException(String.format(PARSE_TOKEN_FAILED_MSG, getLine()), iae);
+            throw new IllegalConfigurationException(String.format(PARSE_TOKEN_FAILED_MSG, getLine()), iae);
         }
         catch (FileNotFoundException fnfe)
         {
-            throw new ConfigurationException(String.format(CONFIG_NOT_FOUND_MSG, file.getName()), fnfe);
+            throw new IllegalConfigurationException(String.format(CONFIG_NOT_FOUND_MSG, file.getName()), fnfe);
         }
         catch (IOException ioe)
         {
-            throw new ConfigurationException(String.format(CANNOT_LOAD_MSG, file.getName()), ioe);
+            throw new IllegalConfigurationException(String.format(CANNOT_LOAD_MSG, file.getName()), ioe);
         }
         finally
         {
@@ -223,7 +223,7 @@ public class PlainConfiguration extends AbstractConfiguration
                 }
                 catch (IOException e)
                 {
-                    throw new ConfigurationException(String.format(CANNOT_CLOSE_MSG, file.getName()), e);
+                    throw new IllegalConfigurationException(String.format(CANNOT_CLOSE_MSG, file.getName()), e);
                 }
             }
         }
@@ -232,11 +232,11 @@ public class PlainConfiguration extends AbstractConfiguration
         return ruleSet;
     }
 
-    private void parseAcl(Integer number, List<String> args) throws ConfigurationException
+    private void parseAcl(Integer number, List<String> args)
     {
         if (args.size() < 3)
         {
-            throw new ConfigurationException(String.format(NOT_ENOUGH_ACL_MSG, getLine()));
+            throw new IllegalConfigurationException(String.format(NOT_ENOUGH_ACL_MSG, getLine()));
         }
 
         Permission permission = Permission.parse(args.get(0));
@@ -245,7 +245,7 @@ public class PlainConfiguration extends AbstractConfiguration
 
         if (number != null && !getConfiguration().isValidNumber(number))
         {
-            throw new ConfigurationException(String.format(BAD_ACL_RULE_NUMBER_MSG, getLine()));
+            throw new IllegalConfigurationException(String.format(BAD_ACL_RULE_NUMBER_MSG, getLine()));
         }
 
         if (args.size() == 3)
@@ -261,11 +261,11 @@ public class PlainConfiguration extends AbstractConfiguration
         }
     }
 
-    private void parseConfig(List<String> args) throws ConfigurationException
+    private void parseConfig(List<String> args)
     {
         if (args.size() < 3)
         {
-            throw new ConfigurationException(String.format(NOT_ENOUGH_CONFIG_MSG, getLine()));
+            throw new IllegalConfigurationException(String.format(NOT_ENOUGH_CONFIG_MSG, getLine()));
         }
 
         Map<String, Boolean> properties = toPluginProperties(args);
@@ -273,7 +273,7 @@ public class PlainConfiguration extends AbstractConfiguration
         getConfiguration().configure(properties);
     }
 
-    private AclRulePredicates toRulePredicates(List<String> args) throws ConfigurationException
+    private AclRulePredicates toRulePredicates(List<String> args)
     {
         AclRulePredicates predicates = new AclRulePredicates();
         Iterator<String> i = args.iterator();
@@ -282,15 +282,15 @@ public class PlainConfiguration extends AbstractConfiguration
             String key = i.next();
             if (!i.hasNext())
             {
-                throw new ConfigurationException(String.format(PROPERTY_KEY_ONLY_MSG, getLine()));
+                throw new IllegalConfigurationException(String.format(PROPERTY_KEY_ONLY_MSG, getLine()));
             }
             if (!"=".equals(i.next()))
             {
-                throw new ConfigurationException(String.format(PROPERTY_NO_EQUALS_MSG, getLine()));
+                throw new IllegalConfigurationException(String.format(PROPERTY_NO_EQUALS_MSG, getLine()));
             }
             if (!i.hasNext())
             {
-                throw new ConfigurationException(String.format(PROPERTY_NO_VALUE_MSG, getLine()));
+                throw new IllegalConfigurationException(String.format(PROPERTY_NO_VALUE_MSG, getLine()));
             }
             String value = i.next();
 
@@ -300,7 +300,7 @@ public class PlainConfiguration extends AbstractConfiguration
     }
 
     /** Converts a {@link List} of "name", "=", "value" tokens into a {@link Map}. */
-    protected Map<String, Boolean> toPluginProperties(List<String> args) throws ConfigurationException
+    protected Map<String, Boolean> toPluginProperties(List<String> args)
     {
         Map<String, Boolean> properties = new HashMap<String, Boolean>();
         Iterator<String> i = args.iterator();
@@ -309,15 +309,15 @@ public class PlainConfiguration extends AbstractConfiguration
             String key = i.next().toLowerCase();
             if (!i.hasNext())
             {
-                throw new ConfigurationException(String.format(PROPERTY_KEY_ONLY_MSG, getLine()));
+                throw new IllegalConfigurationException(String.format(PROPERTY_KEY_ONLY_MSG, getLine()));
             }
             if (!"=".equals(i.next()))
             {
-                throw new ConfigurationException(String.format(PROPERTY_NO_EQUALS_MSG, getLine()));
+                throw new IllegalConfigurationException(String.format(PROPERTY_NO_EQUALS_MSG, getLine()));
             }
             if (!i.hasNext())
             {
-                throw new ConfigurationException(String.format(PROPERTY_NO_VALUE_MSG, getLine()));
+                throw new IllegalConfigurationException(String.format(PROPERTY_NO_VALUE_MSG, getLine()));
             }
 
             // parse property value and save
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 19b9135ea6..6f7885da94 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
@@ -45,7 +45,7 @@ public class DefaultAccessControl implements AccessControl
 
     private RuleSet _ruleSet;
 
-    public DefaultAccessControl(String fileName) throws ConfigurationException
+    public DefaultAccessControl(String fileName)
     {
         if (_logger.isDebugEnabled())
         {
diff --git a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControlFactory.java b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControlFactory.java
index 38ea61357e..a3d7823caf 100644
--- a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControlFactory.java
+++ b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControlFactory.java
@@ -20,28 +20,40 @@
  */
 package org.apache.qpid.server.security.access.plugins;
 
-import org.apache.commons.configuration.Configuration;
-import org.apache.commons.configuration.ConfigurationException;
+import java.io.File;
+import java.util.Map;
+
+import org.apache.qpid.server.configuration.IllegalConfigurationException;
 import org.apache.qpid.server.plugin.AccessControlFactory;
 import org.apache.qpid.server.security.AccessControl;
 
 public class DefaultAccessControlFactory implements AccessControlFactory
 {
-    public AccessControl createInstance(Configuration securityConfiguration)
-    {
-        String aclConfiguration = securityConfiguration.getString("acl");
-        if(aclConfiguration == null)
-        {
-            return null;
-        }
+    public static final String ATTRIBUTE_ACL_FILE = "aclFile";
 
-        try
-        {
-            return new DefaultAccessControl(aclConfiguration);
-        }
-        catch (ConfigurationException e)
+    public AccessControl createInstance(Map<String, Object> aclConfiguration)
+    {
+        if (aclConfiguration != null)
         {
-            throw new RuntimeException("caught exception during instance creation", e);
+            Object aclFile = aclConfiguration.get(ATTRIBUTE_ACL_FILE);
+            if (aclFile != null)
+            {
+                if (aclFile instanceof String)
+                {
+                    String aclPath = (String) aclFile;
+                    if (!new File(aclPath).exists())
+                    {
+                        throw new IllegalConfigurationException("ACL file '" + aclPath + "' is not found");
+                    }
+                    return new DefaultAccessControl(aclPath);
+                }
+                else
+                {
+                    throw new IllegalConfigurationException("Expected '" + ATTRIBUTE_ACL_FILE + "' attribute value of type String but was " + aclFile.getClass()
+                            + ": " + aclFile);
+                }
+            }
         }
+        return null;
     }
 }
diff --git a/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/PlainConfigurationTest.java b/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/PlainConfigurationTest.java
index 21d8ff4400..cbfc9003c8 100644
--- a/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/PlainConfigurationTest.java
+++ b/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/PlainConfigurationTest.java
@@ -26,7 +26,7 @@ import java.util.Map;
 
 import junit.framework.TestCase;
 
-import org.apache.commons.configuration.ConfigurationException;
+import org.apache.qpid.server.configuration.IllegalConfigurationException;
 import org.apache.qpid.server.security.access.ObjectProperties;
 import org.apache.qpid.server.security.access.ObjectProperties.Property;
 import org.apache.qpid.server.security.access.ObjectType;
@@ -73,7 +73,7 @@ public class PlainConfigurationTest extends TestCase
 
             fail("fail");
         }
-        catch (ConfigurationException ce)
+        catch (IllegalConfigurationException ce)
         {
             assertEquals(String.format(PlainConfiguration.CONFIG_NOT_FOUND_MSG, "doesnotexist"), ce.getMessage());
             assertTrue(ce.getCause() instanceof FileNotFoundException);
@@ -87,7 +87,7 @@ public class PlainConfigurationTest extends TestCase
             writeACLConfig("ACL ALLOW ALL \\ ALL");
             fail("fail");
         }
-        catch (ConfigurationException ce)
+        catch (IllegalConfigurationException ce)
         {
             assertEquals(String.format(PlainConfiguration.PREMATURE_CONTINUATION_MSG, 1), ce.getMessage());
         }
@@ -100,7 +100,7 @@ public class PlainConfigurationTest extends TestCase
             writeACLConfig("ACL unparsed ALL ALL");
             fail("fail");
         }
-        catch (ConfigurationException ce)
+        catch (IllegalConfigurationException ce)
         {
             assertEquals(String.format(PlainConfiguration.PARSE_TOKEN_FAILED_MSG, 1), ce.getMessage());
             assertTrue(ce.getCause() instanceof IllegalArgumentException);
@@ -115,7 +115,7 @@ public class PlainConfigurationTest extends TestCase
             writeACLConfig("ACL ALLOW");
             fail("fail");
         }
-        catch (ConfigurationException ce)
+        catch (IllegalConfigurationException ce)
         {
             assertEquals(String.format(PlainConfiguration.NOT_ENOUGH_ACL_MSG, 1), ce.getMessage());
         }
@@ -128,7 +128,7 @@ public class PlainConfigurationTest extends TestCase
             writeACLConfig("CONFIG");
             fail("fail");
         }
-        catch (ConfigurationException ce)
+        catch (IllegalConfigurationException ce)
         {
             assertEquals(String.format(PlainConfiguration.NOT_ENOUGH_TOKENS_MSG, 1), ce.getMessage());
         }
@@ -141,7 +141,7 @@ public class PlainConfigurationTest extends TestCase
             writeACLConfig("INVALID");
             fail("fail");
         }
-        catch (ConfigurationException ce)
+        catch (IllegalConfigurationException ce)
         {
             assertEquals(String.format(PlainConfiguration.NOT_ENOUGH_TOKENS_MSG, 1), ce.getMessage());
         }
@@ -154,7 +154,7 @@ public class PlainConfigurationTest extends TestCase
             writeACLConfig("ACL ALLOW adk CREATE QUEUE name");
             fail("fail");
         }
-        catch (ConfigurationException ce)
+        catch (IllegalConfigurationException ce)
         {
             assertEquals(String.format(PlainConfiguration.PROPERTY_KEY_ONLY_MSG, 1), ce.getMessage());
         }
@@ -167,7 +167,7 @@ public class PlainConfigurationTest extends TestCase
             writeACLConfig("ACL ALLOW adk CREATE QUEUE name test");
             fail("fail");
         }
-        catch (ConfigurationException ce)
+        catch (IllegalConfigurationException ce)
         {
             assertEquals(String.format(PlainConfiguration.PROPERTY_NO_EQUALS_MSG, 1), ce.getMessage());
         }
@@ -180,7 +180,7 @@ public class PlainConfigurationTest extends TestCase
             writeACLConfig("ACL ALLOW adk CREATE QUEUE name =");
             fail("fail");
         }
-        catch (ConfigurationException ce)
+        catch (IllegalConfigurationException ce)
         {
             assertEquals(String.format(PlainConfiguration.PROPERTY_NO_VALUE_MSG, 1), ce.getMessage());
         }
@@ -432,7 +432,7 @@ public class PlainConfigurationTest extends TestCase
             writeACLConfig("GROUP group1 bob alice");
             fail("Expected exception not thrown");
         }
-        catch(ConfigurationException e)
+        catch(IllegalConfigurationException e)
         {
             assertTrue(e.getMessage().contains("GROUP keyword not supported"));
         }
diff --git a/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControlFactoryTest.java b/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControlFactoryTest.java
new file mode 100644
index 0000000000..ca1f19098f
--- /dev/null
+++ b/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControlFactoryTest.java
@@ -0,0 +1,69 @@
+package org.apache.qpid.server.security.access.plugins;
+
+import java.io.File;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.regex.Pattern;
+
+import org.apache.qpid.server.configuration.IllegalConfigurationException;
+import org.apache.qpid.server.security.AccessControl;
+import org.apache.qpid.test.utils.QpidTestCase;
+import org.apache.qpid.test.utils.TestFileUtils;
+
+public class DefaultAccessControlFactoryTest extends QpidTestCase
+{
+    public void testCreateInstanceWhenAclFileIsNotPresent()
+    {
+        DefaultAccessControlFactory factory = new DefaultAccessControlFactory();
+        Map<String, Object> attributes = new HashMap<String, Object>();
+        AccessControl acl = factory.createInstance(attributes);
+        assertNull("ACL was created without a configuration file", acl);
+    }
+
+    public void testCreateInstanceWhenAclFileIsSpecified()
+    {
+        File aclFile = TestFileUtils.createTempFile(this, ".acl", "ACL ALLOW all all");
+        DefaultAccessControlFactory factory = new DefaultAccessControlFactory();
+        Map<String, Object> attributes = new HashMap<String, Object>();
+        attributes.put(DefaultAccessControlFactory.ATTRIBUTE_ACL_FILE, aclFile.getAbsolutePath());
+        AccessControl acl = factory.createInstance(attributes);
+
+        assertNotNull("ACL was not created from acl file: " + aclFile.getAbsolutePath(), acl);
+    }
+
+    public void testCreateInstanceWhenAclFileIsSpecifiedButDoesNotExist()
+    {
+        File aclFile = new File(TMP_FOLDER, "my-non-existing-acl-" + System.currentTimeMillis());
+        assertFalse("ACL file " + aclFile.getAbsolutePath() + " actually exists but should not", aclFile.exists());
+        DefaultAccessControlFactory factory = new DefaultAccessControlFactory();
+        Map<String, Object> attributes = new HashMap<String, Object>();
+        attributes.put(DefaultAccessControlFactory.ATTRIBUTE_ACL_FILE, aclFile.getAbsolutePath());
+        try
+        {
+            factory.createInstance(attributes);
+            fail("It should not be possible to create ACL from non existing file");
+        }
+        catch (IllegalConfigurationException e)
+        {
+            assertTrue("Unexpected exception message", Pattern.matches("ACL file '.*' is not found", e.getMessage()));
+        }
+    }
+
+    public void testCreateInstanceWhenAclFileIsSpecifiedAsNonString()
+    {
+        DefaultAccessControlFactory factory = new DefaultAccessControlFactory();
+        Map<String, Object> attributes = new HashMap<String, Object>();
+        Integer aclFile = new Integer(0);
+        attributes.put(DefaultAccessControlFactory.ATTRIBUTE_ACL_FILE, aclFile);
+        try
+        {
+            factory.createInstance(attributes);
+            fail("It should not be possible to create ACL from Integer");
+        }
+        catch (IllegalConfigurationException e)
+        {
+            assertEquals("Unexpected exception message", "Expected '" + DefaultAccessControlFactory.ATTRIBUTE_ACL_FILE
+                    + "' attribute value of type String but was " + Integer.class + ": " + aclFile, e.getMessage());
+        }
+    }
+}