QPID-5578 : [Java Broker] Tidy up - minimize use of MapValueConverter, make constructors consistent across ConfiguredObjects

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

7 files changed, 29 insertions, 51 deletions

diff --git a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/ACLFileAccessControlProviderFactory.java b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/ACLFileAccessControlProviderFactory.java
index f20d468a14..9ba4618b29 100644
--- a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/ACLFileAccessControlProviderFactory.java
+++ b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/ACLFileAccessControlProviderFactory.java
@@ -47,7 +47,7 @@ public class ACLFileAccessControlProviderFactory extends AbstractConfiguredObjec
     public ACLFileAccessControlProviderImpl createInstance(final Map<String, Object> attributes,
                                                        final ConfiguredObject<?>... parents)
     {
-        return new ACLFileAccessControlProviderImpl(getParent(Broker.class,parents), attributes);
+        return new ACLFileAccessControlProviderImpl(attributes, getParent(Broker.class,parents));
     }
 
 }
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 fa52f33ece..be9642795b 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
@@ -56,8 +56,7 @@ public class ACLFileAccessControlProviderImpl
     @ManagedAttributeField
     private String _path;
 
-    public ACLFileAccessControlProviderImpl(Broker broker,
-                                            Map<String, Object> attributes)
+    public ACLFileAccessControlProviderImpl(Map<String, Object> attributes, Broker broker)
     {
         super(parentsMap(broker),
               attributes, broker.getTaskExecutor());
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 37e85831a6..b72ecb950d 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
@@ -30,7 +30,7 @@ import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
-import java.util.UUID;
+import java.util.Set;
 
 import javax.net.ssl.KeyManager;
 import javax.net.ssl.SSLContext;
@@ -70,7 +70,6 @@ import org.apache.qpid.server.management.plugin.servlet.rest.UserPreferencesServ
 import org.apache.qpid.server.model.*;
 import org.apache.qpid.server.model.adapter.AbstractPluginAdapter;
 import org.apache.qpid.server.model.port.AbstractPortWithAuthProvider;
-import org.apache.qpid.server.util.MapValueConverter;
 import org.apache.qpid.server.util.ServerScopedRuntimeException;
 import org.apache.qpid.transport.network.security.ssl.QpidMultipleTrustManager;
 
@@ -122,9 +121,9 @@ public class HttpManagement extends AbstractPluginAdapter<HttpManagement> implem
     @ManagedAttributeField
     private int _sessionTimeout;
 
-    public HttpManagement(UUID id, Broker broker, Map<String, Object> attributes)
+    public HttpManagement(Map<String, Object> attributes, Broker broker)
     {
-        super(id, attributes, broker);
+        super(attributes, broker);
     }
 
     @Override
@@ -389,7 +388,7 @@ public class HttpManagement extends AbstractPluginAdapter<HttpManagement> implem
         for (Connector connector : connectors)
         {
             getBroker().getEventLogger().message(ManagementConsoleMessages.LISTENING(stringifyConnectorScheme(connector),
-                                                                  connector.getPort()));
+                                                                                     connector.getPort()));
             if (connector instanceof SslSocketConnector)
             {
                 SslContextFactory sslContextFactory = ((SslSocketConnector)connector).getSslContextFactory();
@@ -460,28 +459,22 @@ public class HttpManagement extends AbstractPluginAdapter<HttpManagement> implem
     }
 
     @Override
-    protected void changeAttributes(Map<String, Object> attributes)
+    protected void validateChange(final ConfiguredObject<?> proxyForValidation, final Set<String> changedAttributes)
     {
-        Map<String, Object> convertedAttributes = MapValueConverter.convert(attributes, ATTRIBUTE_TYPES);
-        validateAttributes(convertedAttributes);
+        super.validateChange(proxyForValidation, changedAttributes);
 
-        super.changeAttributes(convertedAttributes);
-    }
-
-    private void validateAttributes(Map<String, Object> convertedAttributes)
-    {
-        if(convertedAttributes.containsKey(HttpManagement.NAME))
+        HttpManagementConfiguration<?> updated = (HttpManagementConfiguration<?>)proxyForValidation;
+        if(changedAttributes.contains(HttpManagement.NAME))
         {
-            String newName = (String) convertedAttributes.get(HttpManagement.NAME);
-            if(!getName().equals(newName))
+            if(!getName().equals(updated.getName()))
             {
                 throw new IllegalConfigurationException("Changing the name of http management plugin is not allowed");
             }
         }
-        if (convertedAttributes.containsKey(TIME_OUT))
+        if (changedAttributes.contains(TIME_OUT))
         {
-            Number value = (Number) convertedAttributes.get(TIME_OUT);
-            if (value == null || value.longValue() < 0)
+            int value = updated.getSessionTimeout();
+            if (value < 0)
             {
                 throw new IllegalConfigurationException("Only positive integer value can be specified for the session time out attribute");
             }
diff --git a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagementFactory.java b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagementFactory.java
index 0431954879..518788a3d3 100644
--- a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagementFactory.java
+++ b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagementFactory.java
@@ -18,14 +18,13 @@
  */
 package org.apache.qpid.server.management.plugin;
 
+import java.util.HashMap;
+import java.util.Map;
+
 import org.apache.qpid.server.model.AbstractConfiguredObjectTypeFactory;
 import org.apache.qpid.server.model.Broker;
 import org.apache.qpid.server.model.ConfiguredObject;
 
-import java.util.HashMap;
-import java.util.Map;
-import java.util.UUID;
-
 public class HttpManagementFactory extends AbstractConfiguredObjectTypeFactory<HttpManagement>
 {
 
@@ -38,9 +37,7 @@ public class HttpManagementFactory extends AbstractConfiguredObjectTypeFactory<H
     public HttpManagement createInstance(final Map<String, Object> attributes, final ConfiguredObject<?>... parents)
     {
         Map<String,Object> attributesWithoutId = new HashMap<String, Object>(attributes);
-        Object idObj = attributesWithoutId.remove(ConfiguredObject.ID);
-        UUID id = idObj == null ? UUID.randomUUID() : idObj instanceof UUID ? (UUID) idObj : UUID.fromString(idObj.toString());
-        return new HttpManagement(id, getParent(Broker.class,parents), attributes);
+        return new HttpManagement(attributes, getParent(Broker.class,parents));
     }
 
 }
diff --git a/qpid/java/broker-plugins/management-http/src/test/java/org/apache/qpid/server/management/plugin/HttpManagementTest.java b/qpid/java/broker-plugins/management-http/src/test/java/org/apache/qpid/server/management/plugin/HttpManagementTest.java
index ff96a4223e..56dc947482 100644
--- a/qpid/java/broker-plugins/management-http/src/test/java/org/apache/qpid/server/management/plugin/HttpManagementTest.java
+++ b/qpid/java/broker-plugins/management-http/src/test/java/org/apache/qpid/server/management/plugin/HttpManagementTest.java
@@ -31,6 +31,7 @@ import java.util.UUID;
 
 import org.apache.qpid.server.model.AuthenticationProvider;
 import org.apache.qpid.server.model.Broker;
+import org.apache.qpid.server.model.ConfiguredObject;
 import org.apache.qpid.test.utils.QpidTestCase;
 
 public class HttpManagementTest extends QpidTestCase
@@ -52,7 +53,8 @@ public class HttpManagementTest extends QpidTestCase
         attributes.put(HttpManagement.HTTPS_SASL_AUTHENTICATION_ENABLED, true);
         attributes.put(HttpManagement.NAME, getTestName());
         attributes.put(HttpManagement.TIME_OUT, 10000l);
-        _management = new HttpManagement(_id, _broker, attributes);
+        attributes.put(ConfiguredObject.ID, _id);
+        _management = new HttpManagement(attributes, _broker);
         _management.open();
     }
 
diff --git a/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/JMXManagementFactory.java b/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/JMXManagementFactory.java
index 0d8b36cdd9..335e4fe50f 100644
--- a/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/JMXManagementFactory.java
+++ b/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/JMXManagementFactory.java
@@ -18,9 +18,7 @@
  */
 package org.apache.qpid.server.jmx;
 
-import java.util.HashMap;
 import java.util.Map;
-import java.util.UUID;
 
 import org.apache.qpid.server.model.AbstractConfiguredObjectTypeFactory;
 import org.apache.qpid.server.model.Broker;
@@ -36,9 +34,6 @@ public class JMXManagementFactory extends AbstractConfiguredObjectTypeFactory<JM
     @Override
     public JMXManagementPluginImpl createInstance(final Map<String, Object> attributes, final ConfiguredObject<?>... parents)
     {
-        Map<String,Object> attributesWithoutId = new HashMap<String, Object>(attributes);
-        Object idObj = attributesWithoutId.remove(ConfiguredObject.ID);
-        UUID id = idObj == null ? UUID.randomUUID() : idObj instanceof UUID ? (UUID) idObj : UUID.fromString(idObj.toString());
-        return new JMXManagementPluginImpl(id, getParent(Broker.class,parents),attributes);
+        return new JMXManagementPluginImpl(attributes,getParent(Broker.class,parents));
     }
 }
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 3a04ca46fb..9ece703973 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
@@ -26,7 +26,7 @@ import java.lang.reflect.Type;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.UUID;
+import java.util.Set;
 
 import javax.management.JMException;
 
@@ -51,7 +51,6 @@ import org.apache.qpid.server.model.State;
 import org.apache.qpid.server.model.VirtualHost;
 import org.apache.qpid.server.model.adapter.AbstractPluginAdapter;
 import org.apache.qpid.server.plugin.QpidServiceLoader;
-import org.apache.qpid.server.util.MapValueConverter;
 import org.apache.qpid.server.util.ServerScopedRuntimeException;
 
 public class JMXManagementPluginImpl
@@ -80,9 +79,9 @@ public class JMXManagementPluginImpl
     @ManagedAttributeField
     private boolean _usePlatformMBeanServer;
 
-    public JMXManagementPluginImpl(UUID id, Broker broker, Map<String, Object> attributes)
+    public JMXManagementPluginImpl(Map<String, Object> attributes, Broker broker)
     {
-        super(id, attributes, broker);
+        super(attributes, broker);
     }
 
     @Override
@@ -313,19 +312,12 @@ public class JMXManagementPluginImpl
     }
 
     @Override
-    protected void changeAttributes(Map<String, Object> attributes)
+    protected void validateChange(final ConfiguredObject<?> proxyForValidation, final Set<String> changedAttributes)
     {
-        Map<String, Object> convertedAttributes = MapValueConverter.convert(attributes, ATTRIBUTE_TYPES);
-        validateAttributes(convertedAttributes);
-
-        super.changeAttributes(convertedAttributes);
-    }
-
-    private void validateAttributes(Map<String, Object> convertedAttributes)
-    {
-        if(convertedAttributes.containsKey(JMXManagementPluginImpl.NAME))
+        super.validateChange(proxyForValidation, changedAttributes);
+        if(changedAttributes.contains(NAME))
         {
-            String newName = (String) convertedAttributes.get(JMXManagementPluginImpl.NAME);
+            String newName = proxyForValidation.getName();
             if(!getName().equals(newName))
             {
                 throw new IllegalConfigurationException("Changing the name of jmx management plugin is not allowed");