QPID-4390: Code clean up

git-svn-id: https://svn.apache.org/repos/asf/qpid/branches/java-broker-config-qpid-4390@1423438 13f79535-47bb-0310-9956-ffa450edef68

7 files changed, 436 insertions, 18 deletions

diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/VirtualHostConfiguration.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/VirtualHostConfiguration.java
index 1a21075940..bde6047a0a 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/VirtualHostConfiguration.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/VirtualHostConfiguration.java
@@ -23,7 +23,6 @@ package org.apache.qpid.server.configuration;
 import org.apache.commons.configuration.CompositeConfiguration;
 import org.apache.commons.configuration.Configuration;
 import org.apache.commons.configuration.ConfigurationException;
-import org.apache.commons.configuration.XMLConfiguration;
 
 import org.apache.qpid.server.configuration.plugins.AbstractConfiguration;
 import org.apache.qpid.server.model.Broker;
@@ -56,17 +55,17 @@ public class VirtualHostConfiguration extends AbstractConfiguration
         Configuration configuration = null;
         if (configurationFile == null)
         {
-            configuration = new XMLConfiguration();
+            throw new IllegalConfigurationException("Virtualhost configuration file must be supplied!");
         }
         else
         {
             Configuration virtualHostConfig = XmlConfigurationUtilities.parseConfig(configurationFile, null);
 
-            // check if it is old virtual host configuration file
+            // check if it is an old virtual host configuration file which has an element of the same name as virtual host
             Configuration config = virtualHostConfig.subset("virtualhost." + XmlConfigurationUtilities.escapeTagName(name));
             if (config.isEmpty())
             {
-                // try to load virtual host configuration from 'name' element
+                // assume it is a new configuration which does not have an element of the same name as the virtual host
                 configuration = virtualHostConfig;
             }
             else
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/store/JsonConfigurationEntryStore.j b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/store/JsonConfigurationEntryStore.j
new file mode 100644
index 0000000000..b77b08542f
--- /dev/null
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/store/JsonConfigurationEntryStore.j
@@ -0,0 +1,388 @@
+package org.apache.qpid.server.configuration.store;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeMap;
+import java.util.TreeSet;
+import java.util.UUID;
+
+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.Broker;
+import org.codehaus.jackson.JsonGenerationException;
+import org.codehaus.jackson.JsonNode;
+import org.codehaus.jackson.JsonProcessingException;
+import org.codehaus.jackson.map.JsonMappingException;
+import org.codehaus.jackson.map.ObjectMapper;
+import org.codehaus.jackson.map.SerializationConfig;
+
+public class JsonConfigurationEntryStore implements ConfigurationEntryStore
+{
+    private static final String ID = "id";
+    private static final String TYPE = "type";
+
+    private ObjectMapper _objectMapper;
+    private Map<UUID, ConfigurationEntry> _entries;
+    private File _storeFile;
+    private UUID _rootId;
+
+    public JsonConfigurationEntryStore(String filePath)
+    {
+        _storeFile = new File(filePath);
+        _objectMapper = new ObjectMapper();
+        _objectMapper.configure(SerializationConfig.Feature.INDENT_OUTPUT, true);
+        _entries = new HashMap<UUID, ConfigurationEntry>();
+        if (_storeFile.exists())
+        {
+            JsonNode node = load(_storeFile, _objectMapper);
+            _rootId = getId(node);
+            _entries.putAll(flattenTree(_rootId, node));
+        }
+        else
+        {
+            File parent = _storeFile.getParentFile();
+            if (!parent.exists())
+            {
+                if (!parent.mkdirs())
+                {
+                    throw new IllegalConfigurationException("Cannot create folder(s) for the store at " + _storeFile);
+                }
+            }
+            try
+            {
+                if (!_storeFile.createNewFile())
+                {
+                    throw new IllegalConfigurationException("Cannot create store file at " + _storeFile);
+                }
+            }
+            catch (IOException e)
+            {
+                throw new IllegalConfigurationException("Cannot write into file at " + _storeFile, e);
+            }
+            ConfigurationEntry brokerEntry = new ConfigurationEntry(UUID.randomUUID(), Broker.class.getSimpleName(), null,
+                    null, null);
+            _rootId = brokerEntry.getId();
+            save(brokerEntry);
+        }
+    }
+
+    @Override
+    public synchronized Collection<ConfigurationEntry> list()
+    {
+        return Collections.unmodifiableCollection(_entries.values());
+    }
+
+    @Override
+    public synchronized void save(ConfigurationEntry entry)
+    {
+        ConfigurationEntry parent = null;
+        ConfigurationEntry oldEntry = _entries.put(entry.getId(), entry);
+        if (!entry.equals(oldEntry))
+        {
+            if (oldEntry == null && parent != null)
+            {
+                Set<UUID> parentChildren =  parent.getChildrenIds();
+                parentChildren.add(entry.getId());
+                _entries.put(parentId, new ConfigurationEntry(parent.getId(), parent.getType(), parent.getAttributes(), parent.getParentId(), parentChildren));
+            }
+            saveAsTree();
+        }
+    }
+
+    @Override
+    public synchronized void remove(UUID entryId)
+    {
+        if (removeInternal(entryId))
+        {
+            saveAsTree();
+        }
+    }
+
+    @Override
+    public synchronized void save(ConfigurationEntry[] entries)
+    {
+        boolean theSame = true;
+        for (int i = 0; i < entries.length; i++)
+        {
+            ConfigurationEntry entry = entries[i];
+            UUID parentId = entry.getParentId();
+            if (parentId != null)
+            {
+                ConfigurationEntry parent = _entries.get(parentId);
+                if (parent == null)
+                {
+                    throw new IllegalConfigurationException("Unknown parentId " + entry.getParentId());
+                }
+            }
+        }
+        for (int i = 0; i < entries.length; i++)
+        {
+            ConfigurationEntry entry = entries[i];
+            ConfigurationEntry oldEntry = _entries.put(entry.getId(), entry);
+            if (!entry.equals(oldEntry))
+            {
+                theSame = false;
+                if (oldEntry == null)
+                {
+                    ConfigurationEntry parent = _entries.get(entry.getParentId());
+                    if (parent != null)
+                    {
+                        Set<UUID> parentChildren =  parent.getChildrenIds();
+                        parentChildren.add(entry.getId());
+                        _entries.put(parent.getId(), new ConfigurationEntry(parent.getId(), parent.getType(), parent.getAttributes(), parent.getParentId(), parentChildren));
+                    }
+                }
+            }
+        }
+        if (!theSame)
+        {
+            saveAsTree();
+        }
+    }
+
+    private boolean removeInternal(UUID entryId)
+    {
+        ConfigurationEntry oldEntry = _entries.remove(entryId);
+        if (oldEntry != null)
+        {
+            Set<UUID> children = oldEntry.getChildrenIds();
+            if (children != null && !children.isEmpty())
+            {
+                for (UUID childId : children)
+                {
+                    removeInternal(childId);
+                }
+            }
+            UUID parentId = oldEntry.getParentId();
+            if (parentId != null)
+            {
+                ConfigurationEntry parent = _entries.get(parentId);
+                if (parent != null)
+                {
+                    Set<UUID> parentChildren =  parent.getChildrenIds();
+                    parentChildren.remove(oldEntry.getId());
+                    _entries.put(parentId, new ConfigurationEntry(parent.getId(), parent.getType(), parent.getAttributes(), parent.getParentId(), parentChildren));
+                }
+            }
+            return true;
+        }
+        return false;
+    }
+
+    private void saveAsTree()
+    {
+        saveAsTree(_rootId, _entries, _objectMapper, _storeFile);
+    }
+
+    private void saveAsTree(UUID rootId, Map<UUID, ConfigurationEntry> entries, ObjectMapper mapper, File file)
+    {
+        Map<String, Object> tree = toTree(rootId, entries);
+        try
+        {
+            mapper.writeValue(file, tree);
+        }
+        catch (JsonGenerationException e)
+        {
+            throw new IllegalConfigurationException("Cannot generate json!", e);
+        }
+        catch (JsonMappingException e)
+        {
+            throw new IllegalConfigurationException("Cannot map objects for json serialization!", e);
+        }
+        catch (IOException e)
+        {
+            throw new IllegalConfigurationException("Cannot save configuration into " + file + "!", e);
+        }
+    }
+
+    private Map<String, Object> toTree(UUID rootId, Map<UUID, ConfigurationEntry> entries)
+    {
+        ConfigurationEntry entry = entries.get(rootId);
+        if (entry == null || !entry.getId().equals(rootId))
+        {
+            throw new IllegalConfigurationException("Cannot find entry with id " + rootId + "!");
+        }
+        Map<String, Object> tree = new TreeMap<String, Object>();
+        Map<String, Object> attributes = entry.getAttributes();
+        if (attributes != null)
+        {
+            tree.putAll(attributes);
+        }
+        tree.put(ID, entry.getId());
+        tree.put(TYPE, entry.getType());
+        Set<UUID> childrenIds = entry.getChildrenIds();
+        if (childrenIds != null && !childrenIds.isEmpty())
+        {
+            for (UUID relationship : childrenIds)
+            {
+                ConfigurationEntry child = entries.get(relationship);
+                if (child != null)
+                {
+                    String relationshipName = child.getType().toLowerCase() + "s";
+                    @SuppressWarnings("unchecked")
+                    Set<Map<String, Object>> children = (Set<Map<String, Object>>) tree.get(relationshipName);
+                    if (children == null)
+                    {
+                        children = new TreeSet<Map<String, Object>>();
+                        tree.put(relationshipName, children);
+                    }
+                    Map<String, Object> childAsMap = toTree(relationship, entries);
+                    children.add(childAsMap);
+                }
+            }
+        }
+        return tree;
+    }
+
+    private JsonNode load(File file, ObjectMapper mapper)
+    {
+        JsonNode root = null;
+        try
+        {
+            root = mapper.readTree(file);
+        }
+        catch (JsonProcessingException e)
+        {
+            throw new IllegalConfigurationException("Cannot parse file '" + file + "'!", e);
+        }
+        catch (IOException e)
+        {
+            throw new IllegalConfigurationException("Cannot read file '" + file + "'!", e);
+        }
+        return root;
+    }
+
+    private Map<UUID, ConfigurationEntry> flattenTree(UUID id, JsonNode root)
+    {
+        String type = getType(root);
+        UUID parentId = getParentId(root);
+
+        Map<String, Object> attributes = new HashMap<String, Object>();
+        Set<UUID> childrenIds = new TreeSet<UUID>();
+
+        ConfigurationEntry entry = new ConfigurationEntry(id, type, attributes, childrenIds, this);
+        Map<UUID, ConfigurationEntry> entries = new HashMap<UUID, ConfigurationEntry>();
+        entries.put(id, entry);
+        Iterator<String> fieldNames = root.getFieldNames();
+        while (fieldNames.hasNext())
+        {
+            String name = fieldNames.next();
+            JsonNode node = root.get(name);
+            if (node.isValueNode())
+            {
+                if (node.isBoolean())
+                {
+                    attributes.put(name, node.asBoolean());
+                }
+                else if (node.isDouble())
+                {
+                    attributes.put(name, node.asDouble());
+                }
+                else if (node.isInt())
+                {
+                    attributes.put(name, node.asInt());
+                }
+                else if (node.isLong())
+                {
+                    attributes.put(name, node.asLong());
+                }
+                else if (node.isNull())
+                {
+                    attributes.put(name, null);
+                }
+                else
+                {
+                    String text = node.asText();
+                    try
+                    {
+                        UUID referedId = UUID.fromString(text);
+                        attributes.put(name, referedId);
+                    }
+                    catch (Exception e)
+                    {
+                        attributes.put(name, text);
+                    }
+                }
+            }
+            else if (node.isArray())
+            {
+                Iterator<JsonNode> elements = node.getElements();
+                while (elements.hasNext())
+                {
+                    JsonNode element = elements.next();
+                    UUID elementId = getId(element);
+                    childrenIds.add(elementId);
+                    Map<UUID, ConfigurationEntry> subEntries = flattenTree(elementId, element);
+                    entries.putAll(subEntries);
+                }
+            }
+            else
+            {
+                throw new IllegalConfigurationException("Unexpected node " + root + "!");
+            }
+        }
+
+        return entries;
+    }
+
+    private String getType(JsonNode root)
+    {
+        JsonNode typeNode = root.get(TYPE);
+        if (typeNode != null)
+        {
+            return typeNode.asText();
+        }
+        return null;
+    }
+
+    private UUID getId(JsonNode node)
+    {
+        JsonNode idNode = node.get(ID);
+        if (idNode == null)
+        {
+            return UUID.randomUUID();
+        }
+        String id = idNode.asText();
+        if (id == null)
+        {
+            return UUID.randomUUID();
+        }
+        try
+        {
+            return UUID.fromString(id);
+        }
+        catch (Exception e)
+        {
+            return UUID.nameUUIDFromBytes(id.getBytes());
+        }
+    }
+
+    @Override
+    public ConfigurationEntry getRootEntry()
+    {
+        // TODO Auto-generated method stub
+        return null;
+    }
+
+    @Override
+    public ConfigurationEntry getEntry(UUID id)
+    {
+        // TODO Auto-generated method stub
+        return null;
+    }
+
+    @Override
+    public void remove(UUID... entryIds)
+    {
+        // TODO Auto-generated method stub
+        
+    }
+
+}
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AbstractKeyStoreAdapter.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AbstractKeyStoreAdapter.java
index fdd71a2062..a479de9769 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AbstractKeyStoreAdapter.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AbstractKeyStoreAdapter.java
@@ -26,8 +26,6 @@ import java.util.Collections;
 import java.util.Map;
 import java.util.UUID;
 
-import javax.net.ssl.KeyManagerFactory;
-
 import org.apache.qpid.server.model.Broker;
 import org.apache.qpid.server.model.ConfiguredObject;
 import org.apache.qpid.server.model.KeyStore;
@@ -134,7 +132,7 @@ public abstract class AbstractKeyStoreAdapter extends AbstractAdapter
         {
             return getId();
         }
-        if(KeyStore.NAME.equals(name))
+        else if(KeyStore.NAME.equals(name))
         {
             return getName();
         }
@@ -185,16 +183,16 @@ public abstract class AbstractKeyStoreAdapter extends AbstractAdapter
         _password = password;
     }
 
-    protected void setMandatoryAttribute(String name, Map<String, Object> attributes)
+    private void setMandatoryAttribute(String name, Map<String, Object> attributeValues)
     {
-        setAttribute(name, null, MapValueConverter.getStringAttribute(name, attributes));
+        setAttribute(name, null, MapValueConverter.getStringAttribute(name, attributeValues));
     }
 
-    protected void setOptionalAttribute(String name, Map<String, Object> attributes)
+    private void setOptionalAttribute(String name, Map<String, Object> attributeValues)
     {
-        if (attributes.containsKey(name))
+        if (attributeValues.get(name) != null)
         {
-            setAttribute(name, null, MapValueConverter.getStringAttribute(name, attributes));
+            setAttribute(name, null, MapValueConverter.getStringAttribute(name, attributeValues));
         }
     }
 }
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java
index feb802d68b..ff53e35404 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java
@@ -86,6 +86,7 @@ public class BrokerAdapter extends AbstractAdapter implements Broker, Configurat
     private final PortFactory _portFactory;
     private final SecurityManager _securityManager;
 
+    //TODO: delete these fields, instead, add the attributes into attribute map
     private long _maximumMessageAge;
     private long _maximumMessageCount;
     private long _maximumQueueDepth;
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/KeyStoreAdapter.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/KeyStoreAdapter.java
index 5b95889eeb..64601be926 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/KeyStoreAdapter.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/KeyStoreAdapter.java
@@ -33,7 +33,7 @@ public class KeyStoreAdapter extends AbstractKeyStoreAdapter implements KeyStore
     public KeyStoreAdapter(UUID id, Broker broker, Map<String, Object> attributes)
     {
         super(id, broker, attributes);
-        if (attributes.containsKey(CERTIFICATE_ALIAS))
+        if (attributes.get(CERTIFICATE_ALIAS) != null)
         {
             setAttribute(CERTIFICATE_ALIAS, null, attributes.get(CERTIFICATE_ALIAS));
         }
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/VirtualHostAdapter.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/VirtualHostAdapter.java
index 7d436d1d97..4eae659b45 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/VirtualHostAdapter.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/VirtualHostAdapter.java
@@ -96,8 +96,8 @@ public final class VirtualHostAdapter extends AbstractAdapter implements Virtual
     {
         super(id);
         _broker = broker;
-        _name = (String)attributes.get(NAME);
-        _configurationFile = (String)attributes.get(CONFIGURATION);
+        _name = MapValueConverter.getStringAttribute(NAME, attributes);
+        _configurationFile = MapValueConverter.getStringAttribute(CONFIGURATION, attributes);
         _brokerStatisticsGatherer = brokerStatisticsGatherer;
         addParent(Broker.class, broker);
     }
diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/startup/VirtualHostRecovererTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/startup/VirtualHostRecovererTest.java
index 0ce691d44d..5ac6001adf 100644
--- a/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/startup/VirtualHostRecovererTest.java
+++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/startup/VirtualHostRecovererTest.java
@@ -26,15 +26,14 @@ import static org.mockito.Mockito.when;
 import java.util.HashMap;
 import java.util.Map;
 
+import junit.framework.TestCase;
+
 import org.apache.qpid.server.configuration.ConfigurationEntry;
 import org.apache.qpid.server.model.Broker;
 import org.apache.qpid.server.model.VirtualHost;
 import org.apache.qpid.server.security.SecurityManager;
 import org.apache.qpid.server.stats.StatisticsGatherer;
 
-
-import junit.framework.TestCase;
-
 public class VirtualHostRecovererTest extends TestCase
 {
     public void testCreate()
@@ -48,6 +47,7 @@ public class VirtualHostRecovererTest extends TestCase
         VirtualHostRecoverer recoverer = new VirtualHostRecoverer(statisticsGatherer);
         Map<String, Object> attributes = new HashMap<String, Object>();
         attributes.put(VirtualHost.NAME, getName());
+        attributes.put(VirtualHost.CONFIGURATION, "/path/to/virtualhost.xml");
         when(entry.getAttributes()).thenReturn(attributes);
 
         VirtualHost host = recoverer.create(null, entry, parent);
@@ -56,4 +56,36 @@ public class VirtualHostRecovererTest extends TestCase
         assertEquals("Unexpected name", getName(), host.getName());
     }
 
+    public void testCreateWithoutMandatoryAttributesResultsInException()
+    {
+        StatisticsGatherer statisticsGatherer = mock(StatisticsGatherer.class);
+        SecurityManager securityManager = mock(SecurityManager.class);
+        ConfigurationEntry entry = mock(ConfigurationEntry.class);
+        Broker parent = mock(Broker.class);
+        when(parent.getSecurityManager()).thenReturn(securityManager);
+
+        Map<String, Object> attributes = new HashMap<String, Object>();
+        attributes.put(VirtualHost.NAME, getName());
+        attributes.put(VirtualHost.CONFIGURATION, "/path/to/virtualhost.xml");
+
+        VirtualHostRecoverer recoverer = new VirtualHostRecoverer(statisticsGatherer);
+
+        //TODO: configuration is made mandatory temporarily, it will became optional later.
+        String[] mandatoryAttributes = {VirtualHost.NAME, VirtualHost.CONFIGURATION};
+        for (String name : mandatoryAttributes)
+        {
+            Map<String, Object> copy = new HashMap<String, Object>(attributes);
+            copy.remove(name);
+            when(entry.getAttributes()).thenReturn(copy);
+            try
+            {
+                recoverer.create(null, entry, parent);
+                fail("Cannot create a virtual host without a manadatory attribute " + name);
+            }
+            catch(IllegalArgumentException e)
+            {
+                // pass
+            }
+        }
+    }
 }