QPID-5591 : Address review comments from Robbie Gemmell

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

11 files changed, 81 insertions, 21 deletions

diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/ManagedAttributeField.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/ManagedAttributeField.java
new file mode 100644
index 0000000000..9369ef296a
--- /dev/null
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/ManagedAttributeField.java
@@ -0,0 +1,28 @@
+package org.apache.qpid.server.model;/*
+ *
+ * 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.
+ *
+ */
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+
+@Retention(RetentionPolicy.RUNTIME)
+public @interface ManagedAttributeField
+{
+}
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/AbstractConfiguredObject.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/AbstractConfiguredObject.java
index af71da3775..66dcd1ab3e 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/AbstractConfiguredObject.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/AbstractConfiguredObject.java
@@ -23,6 +23,7 @@ package org.apache.qpid.server.model.adapter;
 import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
 import java.security.AccessControlException;
 import java.security.AccessController;
 import java.util.*;
@@ -78,12 +79,15 @@ public abstract class AbstractConfiguredObject<X extends ConfiguredObject<X>> im
     private final Collection<ConfigurationChangeListener> _changeListeners =
             new ArrayList<ConfigurationChangeListener>();
 
+    @ManagedAttributeField
     private final UUID _id;
+
     private final Map<String, Object> _defaultAttributes = new HashMap<String, Object>();
     private final TaskExecutor _taskExecutor;
     private final long _createdTime;
     private final String _createdBy;
 
+    @ManagedAttributeField
     private String _name;
 
     private final Map<String, Attribute<?,?>> _attributeTypes;
@@ -348,10 +352,10 @@ public abstract class AbstractConfiguredObject<X extends ConfiguredObject<X>> im
     @Override
     public Object getAttribute(String name)
     {
-        Attribute<?,?> attr = _attributeTypes.get(name);
+        Attribute<X,?> attr = (Attribute<X, ?>) _attributeTypes.get(name);
         if(attr != null && attr.getAnnotation().automate())
         {
-            Object value = attr.get(this);
+            Object value = attr.getValue((X)this);
             if(value != null && attr.getAnnotation().secure() &&
                !SecurityManager.SYSTEM.equals(Subject.getSubject(AccessController.getContext())))
             {
@@ -759,21 +763,6 @@ public abstract class AbstractConfiguredObject<X extends ConfiguredObject<X>> im
 
         }
 
-        public T get(final AbstractConfiguredObject<?> object)
-        {
-            try
-            {
-                return (T) _getter.invoke(object);
-            }
-            catch (IllegalAccessException e)
-            {
-                throw new ServerScopedRuntimeException("Unable to access attribute " + getName() + " on configuredObject type " + object.getClass().getName(), e);
-            }
-            catch (InvocationTargetException e)
-            {
-                throw new ServerScopedRuntimeException("Unable to access attribute " + getName() + " on configuredObject type " + object.getClass().getName(), e);
-            }
-        }
     }
 
     private static final class Statistic<C extends ConfiguredObject, T extends Number> extends AttributeOrStatistic<C,T>
@@ -1347,13 +1336,14 @@ public abstract class AbstractConfiguredObject<X extends ConfiguredObject<X>> im
         }
     }
 
-    private static Field findField(final Attribute<?, ?> attr, Class<?> clazz)
+    private static Field findField(final Attribute<?, ?> attr, Class<?> objClass)
     {
+        Class<?> clazz = objClass;
         while(clazz != null)
         {
             for(Field field : clazz.getDeclaredFields())
             {
-                if(field.getName().equals("_" + attr.getName()))
+                if(field.getAnnotation(ManagedAttributeField.class) != null && field.getName().equals("_" + attr.getName().replace('.','_')))
                 {
                     field.setAccessible(true);
                     return field;
@@ -1361,7 +1351,11 @@ public abstract class AbstractConfiguredObject<X extends ConfiguredObject<X>> im
             }
             clazz = clazz.getSuperclass();
         }
-        return null;
+        if(objClass.isInterface() || Modifier.isAbstract(objClass.getModifiers()))
+        {
+            return null;
+        }
+        throw new ServerScopedRuntimeException("Unable to find field definition for automated field " + attr.getName() + " in class " + objClass.getName());
     }
 
     private static String getName(final Method m, final Class<?> type)
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/AbstractKeyStoreAdapter.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/AbstractKeyStoreAdapter.java
index 794687b9c7..efc86a2594 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/AbstractKeyStoreAdapter.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/AbstractKeyStoreAdapter.java
@@ -32,6 +32,7 @@ import org.apache.qpid.server.model.Broker;
 import org.apache.qpid.server.model.ConfiguredObject;
 import org.apache.qpid.server.model.KeyStore;
 import org.apache.qpid.server.model.LifetimePolicy;
+import org.apache.qpid.server.model.ManagedAttributeField;
 import org.apache.qpid.server.model.State;
 import org.apache.qpid.server.model.TrustStore;
 import org.apache.qpid.server.security.SecurityManager;
@@ -44,6 +45,7 @@ public abstract class AbstractKeyStoreAdapter<X extends ConfiguredObject<X>> ext
     public static final String DUMMY_PASSWORD_MASK = "********";
     public static final String DEFAULT_KEYSTORE_TYPE = java.security.KeyStore.getDefaultType();
 
+    @ManagedAttributeField
     private String _password;
 
 
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/ConnectionAdapter.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/ConnectionAdapter.java
index 16b1d0092b..89d5622121 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/ConnectionAdapter.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/ConnectionAdapter.java
@@ -38,14 +38,23 @@ final class ConnectionAdapter extends AbstractConfiguredObject<ConnectionAdapter
     private final Map<AMQSessionModel, SessionAdapter> _sessionAdapters =
             new HashMap<AMQSessionModel, SessionAdapter>();
 
+    @ManagedAttributeField
     private String _remoteAddress;
+    @ManagedAttributeField
     private String _localAddress;
+    @ManagedAttributeField
     private String _clientId;
+    @ManagedAttributeField
     private String _clientVersion;
+    @ManagedAttributeField
     private boolean _incoming;
+    @ManagedAttributeField
     private Transport _transport;
+    @ManagedAttributeField
     private Port _port;
+    @ManagedAttributeField
     private String _remoteProcessName;
+    @ManagedAttributeField
     private String _remoteProcessPid;
 
     public ConnectionAdapter(final AMQConnectionModel conn, TaskExecutor taskExecutor)
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/KeyStoreAdapter.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/KeyStoreAdapter.java
index cdffe2f36b..06ca0d2a24 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/KeyStoreAdapter.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/KeyStoreAdapter.java
@@ -43,6 +43,7 @@ import org.apache.qpid.server.configuration.IllegalConfigurationException;
 import org.apache.qpid.server.model.Broker;
 import org.apache.qpid.server.model.IntegrityViolationException;
 import org.apache.qpid.server.model.KeyStore;
+import org.apache.qpid.server.model.ManagedAttributeField;
 import org.apache.qpid.server.model.Port;
 import org.apache.qpid.server.model.State;
 import org.apache.qpid.server.security.SecurityManager;
@@ -64,9 +65,13 @@ public class KeyStoreAdapter extends AbstractKeyStoreAdapter<KeyStoreAdapter> im
         put(KEY_MANAGER_FACTORY_ALGORITHM, String.class);
     }});
 
+    @ManagedAttributeField
     private String _keyStoreType;
+    @ManagedAttributeField
     private String _certificateAlias;
+    @ManagedAttributeField
     private String _keyManagerFactoryAlgorithm;
+    @ManagedAttributeField
     private String _path;
 
     @SuppressWarnings("serial")
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/SessionAdapter.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/SessionAdapter.java
index 669bf19874..95e408ddf5 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/SessionAdapter.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/SessionAdapter.java
@@ -43,6 +43,7 @@ final class SessionAdapter extends AbstractConfiguredObject<SessionAdapter> impl
     private AMQSessionModel _session;
     private Map<Consumer, QueueConsumer> _consumerAdapters = new HashMap<Consumer, QueueConsumer>();
 
+    @ManagedAttributeField
     private int _channelId;
 
     public SessionAdapter(final AMQSessionModel session, TaskExecutor taskExecutor)
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/TrustStoreAdapter.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/TrustStoreAdapter.java
index 03fac7d54d..88f40a83aa 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/TrustStoreAdapter.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/TrustStoreAdapter.java
@@ -44,6 +44,7 @@ import org.apache.qpid.server.configuration.IllegalConfigurationException;
 import org.apache.qpid.server.model.AuthenticationProvider;
 import org.apache.qpid.server.model.Broker;
 import org.apache.qpid.server.model.IntegrityViolationException;
+import org.apache.qpid.server.model.ManagedAttributeField;
 import org.apache.qpid.server.model.Port;
 import org.apache.qpid.server.model.State;
 import org.apache.qpid.server.model.TrustStore;
@@ -74,10 +75,13 @@ public class TrustStoreAdapter extends AbstractKeyStoreAdapter<TrustStoreAdapter
         put(TrustStore.TRUST_MANAGER_FACTORY_ALGORITHM, TrustManagerFactory.getDefaultAlgorithm());
     }});
 
-
+    @ManagedAttributeField
     private String _trustStoreType;
+    @ManagedAttributeField
     private String _trustManagerFactoryAlgorithm;
+    @ManagedAttributeField
     private String _path;
+    @ManagedAttributeField
     private boolean _peersOnly;
 
 
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/QueueConsumerImpl.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/QueueConsumerImpl.java
index 57e4eff230..8c03e8f882 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/QueueConsumerImpl.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/QueueConsumerImpl.java
@@ -36,6 +36,7 @@ import org.apache.qpid.server.message.ServerMessage;
 import org.apache.qpid.server.model.ConfiguredObject;
 import org.apache.qpid.server.model.Consumer;
 import org.apache.qpid.server.model.LifetimePolicy;
+import org.apache.qpid.server.model.ManagedAttributeField;
 import org.apache.qpid.server.model.State;
 import org.apache.qpid.server.model.adapter.AbstractConfiguredObject;
 import org.apache.qpid.server.protocol.AMQSessionModel;
@@ -99,11 +100,17 @@ class QueueConsumerImpl
             CurrentActor.get().message(SubscriptionMessages.STATE(newState.toString()));
         }
     };
+    @ManagedAttributeField
     private boolean _durable;
+    @ManagedAttributeField
     private boolean _exclusive;
+    @ManagedAttributeField
     private boolean _noLocal;
+    @ManagedAttributeField
     private String _distributionMode;
+    @ManagedAttributeField
     private String _settlementMode;
+    @ManagedAttributeField
     private String _selector;
 
     QueueConsumerImpl(final AbstractQueue queue,
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/ExternalAuthenticationManager.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/ExternalAuthenticationManager.java
index e4b166114d..5ff9a3ddac 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/ExternalAuthenticationManager.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/ExternalAuthenticationManager.java
@@ -27,6 +27,7 @@ import javax.security.sasl.SaslServer;
 import org.apache.qpid.server.configuration.updater.TaskExecutor;
 import org.apache.qpid.server.model.Broker;
 import org.apache.qpid.server.model.ManagedAttribute;
+import org.apache.qpid.server.model.ManagedAttributeField;
 import org.apache.qpid.server.security.auth.AuthenticationResult;
 import org.apache.qpid.server.security.auth.UsernamePrincipal;
 import org.apache.qpid.server.security.auth.sasl.external.ExternalSaslServer;
@@ -35,6 +36,7 @@ public class ExternalAuthenticationManager extends AbstractAuthenticationManager
 {
     private static final String EXTERNAL = "EXTERNAL";
 
+    @ManagedAttributeField
     private boolean _useFullDN;
 
     protected ExternalAuthenticationManager(final Broker broker,
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/PrincipalDatabaseAuthenticationManager.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/PrincipalDatabaseAuthenticationManager.java
index dd975cc0c7..7b9f850536 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/PrincipalDatabaseAuthenticationManager.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/PrincipalDatabaseAuthenticationManager.java
@@ -56,6 +56,7 @@ public abstract class PrincipalDatabaseAuthenticationManager<T extends Principal
 
 
     private final PrincipalDatabase _principalDatabase;
+    @ManagedAttributeField
     private String _path;
 
     protected PrincipalDatabaseAuthenticationManager(final Broker broker,
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/SimpleLDAPAuthenticationManager.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/SimpleLDAPAuthenticationManager.java
index 5352e36d63..6e80e13dfd 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/SimpleLDAPAuthenticationManager.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/SimpleLDAPAuthenticationManager.java
@@ -49,6 +49,7 @@ import org.apache.log4j.Logger;
 import org.apache.qpid.server.configuration.updater.TaskExecutor;
 import org.apache.qpid.server.model.Broker;
 import org.apache.qpid.server.model.ManagedAttribute;
+import org.apache.qpid.server.model.ManagedAttributeField;
 import org.apache.qpid.server.model.TrustStore;
 import org.apache.qpid.server.security.auth.AuthenticationResult;
 import org.apache.qpid.server.security.auth.AuthenticationResult.AuthenticationStatus;
@@ -70,10 +71,15 @@ public class SimpleLDAPAuthenticationManager extends AbstractAuthenticationManag
      */
     private static final String JAVA_NAMING_LDAP_FACTORY_SOCKET = "java.naming.ldap.factory.socket";
 
+    @ManagedAttributeField
     private String _providerUrl;
+    @ManagedAttributeField
     private String _providerAuthUrl;
+    @ManagedAttributeField
     private String _searchContext;
+    @ManagedAttributeField
     private String _searchFilter;
+    @ManagedAttributeField
     private String _ldapContextFactory;
 
 
@@ -81,6 +87,7 @@ public class SimpleLDAPAuthenticationManager extends AbstractAuthenticationManag
      * Trust store - typically used when the Directory has been secured with a certificate signed by a
      * private CA (or self-signed certificate).
      */
+    @ManagedAttributeField
     private TrustStore _trustStore;
 
     /**