QPID-5698 : [Java Broker] Make all configured object attributes either automated or derived (except for state) and enforce this

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

10 files changed, 52 insertions, 26 deletions

diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/binding/BindingImpl.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/binding/BindingImpl.java
index df548d651f..c1a5f92717 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/binding/BindingImpl.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/binding/BindingImpl.java
@@ -29,12 +29,14 @@ import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicLong;
 
+import org.apache.qpid.server.exchange.AbstractExchange;
 import org.apache.qpid.server.exchange.ExchangeImpl;
 import org.apache.qpid.server.logging.EventLogger;
 import org.apache.qpid.server.logging.messages.BindingMessages;
 import org.apache.qpid.server.logging.subjects.BindingLogSubject;
 import org.apache.qpid.server.model.AbstractConfiguredObject;
 import org.apache.qpid.server.model.ConfiguredObject;
+import org.apache.qpid.server.model.ManagedAttributeField;
 import org.apache.qpid.server.model.Queue;
 import org.apache.qpid.server.model.State;
 import org.apache.qpid.server.model.VirtualHost;
@@ -46,12 +48,13 @@ public class BindingImpl
         extends AbstractConfiguredObject<BindingImpl>
         implements org.apache.qpid.server.model.Binding<BindingImpl>
 {
-    private final String _bindingKey;
+    private String _bindingKey;
     private final AMQQueue _queue;
     private final ExchangeImpl _exchange;
+    @ManagedAttributeField
     private Map<String, Object> _arguments;
     private final AtomicLong _matches = new AtomicLong();
-    private final BindingLogSubject _logSubject;
+    private BindingLogSubject _logSubject;
 
     final AtomicBoolean _deleted = new AtomicBoolean();
     final CopyOnWriteArrayList<StateChangeListener<BindingImpl,State>> _stateChangeListeners =
@@ -60,27 +63,41 @@ public class BindingImpl
     public BindingImpl(Map<String, Object> attributes, AMQQueue queue, ExchangeImpl exchange)
     {
         super(parentsMap(queue,exchange),enhanceWithDurable(attributes,queue,exchange),queue.getVirtualHost().getTaskExecutor());
-        _bindingKey = (String)attributes.get(org.apache.qpid.server.model.Binding.NAME);
+        _bindingKey = getName();
         _queue = queue;
         _exchange = exchange;
-        Map<String,Object> arguments = (Map<String, Object>) attributes.get(org.apache.qpid.server.model.Binding.ARGUMENTS);
-        _arguments = arguments == null ? Collections.EMPTY_MAP : Collections.unmodifiableMap(arguments);
 
 
-        //Perform ACLs
-        queue.getVirtualHost().getSecurityManager().authoriseCreateBinding(this);
-        _logSubject = new BindingLogSubject(_bindingKey,exchange,queue);
+    }
+
+    @Override
+    protected void onOpen()
+    {
+        super.onOpen();
+        _logSubject = new BindingLogSubject(_bindingKey,_exchange,_queue);
+
         getEventLogger().message(_logSubject, BindingMessages.CREATED(String.valueOf(getArguments()),
                                                                       getArguments() != null
                                                                       && !getArguments().isEmpty()));
-
-
+        if(_exchange instanceof AbstractExchange)
+        {
+            ((AbstractExchange)_exchange).doAddBinding(this);
+        }
     }
 
     @Override
     protected void onCreate()
     {
         super.onCreate();
+        try
+        {
+            _queue.getVirtualHost().getSecurityManager().authoriseCreateBinding(this);
+        }
+        catch(AccessControlException e)
+        {
+            deleted();
+            throw e;
+        }
         if (isDurable())
         {
             _queue.getVirtualHost().getDurableConfigurationStore().create(asObjectRecord());
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/AbstractExchange.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/AbstractExchange.java
index ef57af3af0..e42c3db0b1 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/AbstractExchange.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/AbstractExchange.java
@@ -609,7 +609,7 @@ public abstract class AbstractExchange<T extends AbstractExchange<T>>
     @Override
     public boolean addBinding(String bindingKey, AMQQueue queue, Map<String, Object> arguments)
     {
-        return makeBinding(null, bindingKey, queue, arguments, false, false);
+        return makeBinding(null, bindingKey, queue, arguments, false);
     }
 
     @Override
@@ -622,7 +622,6 @@ public abstract class AbstractExchange<T extends AbstractExchange<T>>
                            bindingKey,
                            queue,
                            arguments,
-                           false,
                            true);
     }
 
@@ -674,7 +673,6 @@ public abstract class AbstractExchange<T extends AbstractExchange<T>>
                                 String bindingKey,
                                 AMQQueue queue,
                                 Map<String, Object> arguments,
-                                boolean restore,
                                 boolean force)
     {
         if (bindingKey == null)
@@ -706,9 +704,9 @@ public abstract class AbstractExchange<T extends AbstractExchange<T>>
                 attributes.put(Binding.ARGUMENTS, arguments);
 
                 BindingImpl b = new BindingImpl(attributes, queue, this);
-                b.create();
 
                 addBinding(b);
+                b.create();
 
 
                 return true;
@@ -738,7 +736,6 @@ public abstract class AbstractExchange<T extends AbstractExchange<T>>
         b.getAMQQueue().addBinding(b);
         childAdded(b);
 
-        doAddBinding(b);
     }
 
     protected abstract void onBindingUpdated(final BindingImpl binding,
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/HeadersBinding.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/HeadersBinding.java
index 5de3fa4d4e..fa4e3f21dd 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/HeadersBinding.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/HeadersBinding.java
@@ -20,20 +20,21 @@
  */
 package org.apache.qpid.server.exchange;
 
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
 import org.apache.log4j.Logger;
 
 import org.apache.qpid.server.binding.BindingImpl;
 import org.apache.qpid.server.filter.AMQInvalidArgumentException;
 import org.apache.qpid.server.filter.FilterSupport;
+import org.apache.qpid.server.filter.Filterable;
 import org.apache.qpid.server.filter.MessageFilter;
 import org.apache.qpid.server.message.AMQMessageHeader;
 
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Map;
-import java.util.Set;
-import org.apache.qpid.server.filter.Filterable;
-
 /**
  * Defines binding and matching based on a set of headers.
  */
@@ -61,7 +62,8 @@ class HeadersBinding
         _binding = binding;
         if(_binding !=null)
         {
-            _mappings = _binding.getArguments();
+            Map<String, Object> arguments = _binding.getArguments();
+            _mappings = arguments == null ? Collections.<String,Object>emptyMap() : arguments;
             initMappings();
         }
         else
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java
index 4789da7126..03b19608af 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java
@@ -1351,6 +1351,10 @@ public abstract class AbstractConfiguredObject<X extends ConfiguredObject<X>> im
                 ManagedAttribute annotation = m.getAnnotation(ManagedAttribute.class);
                 if(annotation != null)
                 {
+                    if(!(annotation.automate() || annotation.derived() || annotation.state()))
+                    {
+                        throw new ServerScopedRuntimeException("ManagedAttributes must be either automated or derived. " + m.getName() + " on "  + clazz.getSimpleName() + " does not meet this criterion.");
+                    }
                     if(!clazz.isInterface() || !ConfiguredObject.class.isAssignableFrom(clazz))
                     {
                         throw new ServerScopedRuntimeException("Can only define ManagedAttributes on interfaces which extend " + ConfiguredObject.class.getSimpleName() + ". " + clazz.getSimpleName() + " does not meet these criteria.");
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/Binding.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/Binding.java
index 17b9282f66..a4e1f47160 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/Binding.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/Binding.java
@@ -39,7 +39,7 @@ public interface Binding<X extends Binding<X>> extends ConfiguredObject<X>
     @ManagedAttribute( derived = true )
     Exchange<?> getExchange();
 
-    @ManagedAttribute
+    @ManagedAttribute( automate = true )
     Map<String,Object> getArguments();
 
     @ManagedStatistic
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/ConfiguredObject.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/ConfiguredObject.java
index 54bab071fd..1404f5e7b6 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/ConfiguredObject.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/ConfiguredObject.java
@@ -122,7 +122,7 @@ public interface ConfiguredObject<X extends ConfiguredObject<X>>
      *
      * @return the actual state of the object
      */
-    @ManagedAttribute
+    @ManagedAttribute( state = true )
     State getState();
 
 
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/ManagedAttribute.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/ManagedAttribute.java
index 20d8a3c311..4360e97c7e 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/ManagedAttribute.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/ManagedAttribute.java
@@ -30,6 +30,7 @@ public @interface ManagedAttribute
     boolean secure() default false;
     boolean derived() default false;
     boolean automate() default false;
+    boolean state() default false;
     boolean mandatory() default false;
     String defaultValue() default "";
 }
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/BindingRecoverer.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/BindingRecoverer.java
index 526760aea7..a75696e9b8 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/BindingRecoverer.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/BindingRecoverer.java
@@ -130,6 +130,7 @@ public class BindingRecoverer extends AbstractDurableConfiguredObjectRecoverer<B
                 UnresolvedConfiguredObject<? extends Binding> unresolvedConfiguredObject =
                         configuredObjectTypeFactory.recover(_record, _exchange, _queue);
                 Binding binding = (Binding<?>) unresolvedConfiguredObject.resolve();
+                binding.open();
 
             }
             return (_exchange).getBinding(_bindingName, _queue);
diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/exchange/HeadersBindingTest.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/exchange/HeadersBindingTest.java
index 3d6ab9e76d..9b1da6225b 100644
--- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/exchange/HeadersBindingTest.java
+++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/exchange/HeadersBindingTest.java
@@ -361,7 +361,9 @@ public class HeadersBindingTest extends TestCase
             attributes.put(Binding.ARGUMENTS, arguments);
         }
         attributes.put(Binding.ID, id);
-        return new BindingImpl(attributes, queue, exchange);
+        BindingImpl binding = new BindingImpl(attributes, queue, exchange);
+        binding.open();
+        return binding;
     }
 
 
diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/exchange/TopicExchangeTest.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/exchange/TopicExchangeTest.java
index 7b57143016..a6930a9acd 100644
--- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/exchange/TopicExchangeTest.java
+++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/exchange/TopicExchangeTest.java
@@ -383,7 +383,9 @@ public class TopicExchangeTest extends QpidTestCase
             attributes.put(Binding.ARGUMENTS, arguments);
         }
         attributes.put(Binding.ID, id);
-        return new BindingImpl(attributes, queue, exchange);
+        BindingImpl binding = new BindingImpl(attributes, queue, exchange);
+        binding.open();
+        return binding;
     }