You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by kw...@apache.org on 2012/07/06 16:01:07 UTC
svn commit: r1358216 - in /qpid/trunk/qpid/java/broker-plugins/jmx/src:
main/java/org/apache/qpid/server/jmx/mbeans/ExchangeMBean.java
test/java/org/apache/qpid/server/jmx/mbeans/ExchangeMBeanTest.java
Author: kwall
Date: Fri Jul 6 14:01:07 2012
New Revision: 1358216
URL: http://svn.apache.org/viewvc?rev=1358216&view=rev
Log:
QPID-4093: Prevent NullPointerException from ExchangeMBean when target queue does not exist
Added:
qpid/trunk/qpid/java/broker-plugins/jmx/src/test/java/org/apache/qpid/server/jmx/mbeans/ExchangeMBeanTest.java
Modified:
qpid/trunk/qpid/java/broker-plugins/jmx/src/main/java/org/apache/qpid/server/jmx/mbeans/ExchangeMBean.java
Modified: qpid/trunk/qpid/java/broker-plugins/jmx/src/main/java/org/apache/qpid/server/jmx/mbeans/ExchangeMBean.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-plugins/jmx/src/main/java/org/apache/qpid/server/jmx/mbeans/ExchangeMBean.java?rev=1358216&r1=1358215&r2=1358216&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker-plugins/jmx/src/main/java/org/apache/qpid/server/jmx/mbeans/ExchangeMBean.java (original)
+++ qpid/trunk/qpid/java/broker-plugins/jmx/src/main/java/org/apache/qpid/server/jmx/mbeans/ExchangeMBean.java Fri Jul 6 14:01:07 2012
@@ -22,8 +22,6 @@
package org.apache.qpid.server.jmx.mbeans;
import org.apache.qpid.management.common.mbeans.ManagedExchange;
-import org.apache.qpid.management.common.mbeans.ManagedQueue;
-import org.apache.qpid.management.common.mbeans.annotations.MBeanOperationParameter;
import org.apache.qpid.server.jmx.AMQManagedObject;
import org.apache.qpid.server.jmx.ManagedObject;
import org.apache.qpid.server.model.Binding;
@@ -35,6 +33,7 @@ import org.apache.qpid.server.model.Virt
import javax.management.JMException;
import javax.management.MalformedObjectNameException;
import javax.management.ObjectName;
+import javax.management.OperationsException;
import javax.management.openmbean.ArrayType;
import javax.management.openmbean.CompositeData;
import javax.management.openmbean.CompositeDataSupport;
@@ -56,6 +55,9 @@ import java.util.Map;
public class ExchangeMBean extends AMQManagedObject implements ManagedExchange
{
+ public static final String FANOUT_EXCHANGE_TYPE = "fanout";
+ public static final String HEADERS_EXCHANGE_TYPE = "headers";
+
private static final String[] TABULAR_UNIQUE_INDEX_ARRAY =
TABULAR_UNIQUE_INDEX.toArray(new String[TABULAR_UNIQUE_INDEX.size()]);
@@ -79,7 +81,6 @@ public class ExchangeMBean extends AMQMa
HEADERS_COMPOSITE_ITEM_DESC.toArray(new String[HEADERS_COMPOSITE_ITEM_DESC.size()]);
private static final String[] HEADERS_TABULAR_UNIQUE_INDEX_ARRAY =
HEADERS_TABULAR_UNIQUE_INDEX.toArray(new String[HEADERS_TABULAR_UNIQUE_INDEX.size()]);
- public static final String HEADERS_EXCHANGE_TYPE = "headers";
static
{
@@ -179,7 +180,6 @@ public class ExchangeMBean extends AMQMa
}
}
-
private TabularData getHeadersBindings(Collection<Binding> bindings) throws OpenDataException
{
TabularType bindinglistDataType =
@@ -234,7 +234,7 @@ public class ExchangeMBean extends AMQMa
for (Binding binding : bindings)
{
- String key = "fanout".equals(_exchange.getExchangeType()) ? "*" : binding.getName();
+ String key = FANOUT_EXCHANGE_TYPE.equals(_exchange.getExchangeType()) ? "*" : binding.getName();
List<String> queueList = bindingMap.get(key);
if(queueList == null)
{
@@ -269,7 +269,7 @@ public class ExchangeMBean extends AMQMa
final String[] keyAndValue = bindings[i].split("=");
if (keyAndValue == null || keyAndValue.length == 0 || keyAndValue.length > 2 || keyAndValue[0].length() == 0)
{
- throw new JMException("Format for headers binding should be \"<attribute1>=<value1>,<attribute2>=<value2>\" ");
+ throw new JMException("Format for headers binding should be \"<attribute1>=<value1>,<attribute2>=<value2>\"");
}
if(keyAndValue.length == 1)
@@ -284,40 +284,30 @@ public class ExchangeMBean extends AMQMa
}
}
- Queue queue = null;
- VirtualHost vhost = _exchange.getParent(VirtualHost.class);
- for(Queue aQueue : vhost.getQueues())
- {
- if(aQueue.getName().equals(queueName))
- {
- queue = aQueue;
- break;
- }
- }
+ VirtualHost virtualHost = _exchange.getParent(VirtualHost.class);
+ Queue queue = MBeanUtils.findQueueFromQueueName(virtualHost, queueName);
_exchange.createBinding(binding, queue, arguments, Collections.EMPTY_MAP);
}
public void removeBinding(String queueName, String bindingKey)
throws IOException, JMException
{
- Queue queue = null;
- VirtualHost vhost = _exchange.getParent(VirtualHost.class);
- for(Queue aQueue : vhost.getQueues())
- {
- if(aQueue.getName().equals(queueName))
- {
- queue = aQueue;
- break;
- }
- }
+ VirtualHost virtualHost = _exchange.getParent(VirtualHost.class);
+ Queue queue = MBeanUtils.findQueueFromQueueName(virtualHost, queueName);;
+ boolean deleted = false;
for(Binding binding : _exchange.getBindings())
{
if(queue.equals(binding.getParent(Queue.class)) && bindingKey.equals(binding.getName()))
{
binding.delete();
+ deleted = true;
}
}
-
+
+ if (!deleted)
+ {
+ throw new OperationsException("No such binding \"" + bindingKey + "\" on queue \"" + queueName + "\"");
+ }
}
}
Added: qpid/trunk/qpid/java/broker-plugins/jmx/src/test/java/org/apache/qpid/server/jmx/mbeans/ExchangeMBeanTest.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-plugins/jmx/src/test/java/org/apache/qpid/server/jmx/mbeans/ExchangeMBeanTest.java?rev=1358216&view=auto
==============================================================================
--- qpid/trunk/qpid/java/broker-plugins/jmx/src/test/java/org/apache/qpid/server/jmx/mbeans/ExchangeMBeanTest.java (added)
+++ qpid/trunk/qpid/java/broker-plugins/jmx/src/test/java/org/apache/qpid/server/jmx/mbeans/ExchangeMBeanTest.java Fri Jul 6 14:01:07 2012
@@ -0,0 +1,234 @@
+/*
+ * 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.
+ */
+package org.apache.qpid.server.jmx.mbeans;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.never;
+import static org.mockito.Matchers.isNull;
+import static org.mockito.Matchers.argThat;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Matchers.anyMap;
+
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.TreeMap;
+
+import javax.management.JMException;
+import javax.management.ListenerNotFoundException;
+import javax.management.Notification;
+import javax.management.NotificationListener;
+import javax.management.OperationsException;
+
+import org.apache.qpid.server.jmx.ManagedObjectRegistry;
+import org.apache.qpid.server.model.Binding;
+import org.apache.qpid.server.model.Exchange;
+import org.apache.qpid.server.model.LifetimePolicy;
+import org.apache.qpid.server.model.Queue;
+import org.apache.qpid.server.model.Statistics;
+import org.apache.qpid.server.model.VirtualHost;
+import org.apache.qpid.server.queue.NotificationCheck;
+import org.mockito.ArgumentMatcher;
+
+import junit.framework.TestCase;
+
+public class ExchangeMBeanTest extends TestCase
+{
+ private static final String EXCHANGE_NAME = "EXCHANGE_NAME";
+ private static final String EXCHANGE_TYPE = "EXCHANGE_TYPE";
+ private static final String QUEUE1_NAME = "QUEUE1_NAME";
+ private static final String QUEUE2_NAME = "QUEUE2_NAME";
+ private static final String BINDING1 = "BINDING1";
+ private static final String BINDING2 = "BINDING2";
+
+ private Exchange _mockExchange;
+ private VirtualHostMBean _mockVirtualHostMBean;
+ private ManagedObjectRegistry _mockManagedObjectRegistry;
+ private ExchangeMBean _exchangeMBean;
+ private Queue _mockQueue1;
+ private Queue _mockQueue2;
+ private Exchange _mockHeadersExchange;
+
+ @Override
+ protected void setUp() throws Exception
+ {
+ _mockExchange = mock(Exchange.class);
+ when(_mockExchange.getName()).thenReturn(EXCHANGE_NAME);
+ when(_mockExchange.getExchangeType()).thenReturn(EXCHANGE_TYPE);
+ _mockVirtualHostMBean = mock(VirtualHostMBean.class);
+
+ _mockManagedObjectRegistry = mock(ManagedObjectRegistry.class);
+ when(_mockVirtualHostMBean.getRegistry()).thenReturn(_mockManagedObjectRegistry);
+
+ _mockQueue1 = createMockQueue(QUEUE1_NAME);
+ _mockQueue2 = createMockQueue(QUEUE2_NAME);
+
+ VirtualHost mockVirtualHost = mock(VirtualHost.class);
+ when(mockVirtualHost.getQueues()).thenReturn(Arrays.asList(new Queue[] {_mockQueue1, _mockQueue2}));
+ when(_mockExchange.getParent(VirtualHost.class)).thenReturn(mockVirtualHost);
+
+ _exchangeMBean = new ExchangeMBean(_mockExchange, _mockVirtualHostMBean);
+
+ _mockHeadersExchange = mock(Exchange.class);
+ when(_mockHeadersExchange.getExchangeType()).thenReturn(ExchangeMBean.HEADERS_EXCHANGE_TYPE);
+ when(_mockHeadersExchange.getParent(VirtualHost.class)).thenReturn(mockVirtualHost);
+
+ }
+
+ public void testExchangeName()
+ {
+ assertEquals(EXCHANGE_NAME, _exchangeMBean.getName());
+ }
+
+ public void testExchangeType()
+ {
+ assertEquals(EXCHANGE_TYPE, _exchangeMBean.getExchangeType());
+ }
+
+ public void testNonHeadersExchangeCreateNewBinding() throws Exception
+ {
+ _exchangeMBean.createNewBinding(QUEUE1_NAME, BINDING1);
+ verify(_mockExchange).createBinding(BINDING1, _mockQueue1, Collections.EMPTY_MAP, Collections.EMPTY_MAP);
+ }
+
+ public void testCreateNewBindingWhereQueueIsUnknown() throws Exception
+ {
+ try
+ {
+ _exchangeMBean.createNewBinding("unknown", BINDING1);
+ }
+ catch (OperationsException oe)
+ {
+ // PASS
+ assertEquals("No such queue \"unknown\"", oe.getMessage());
+ }
+ verify(_mockExchange, never()).createBinding(anyString(), any(Queue.class), anyMap(), anyMap());
+ }
+
+ public void testRemoveBinding() throws Exception
+ {
+ Binding mockBinding1 = createBindingOnQueue(BINDING1, _mockQueue1);
+ Binding mockBinding2 = createBindingOnQueue(BINDING2, _mockQueue1);
+ when(_mockExchange.getBindings()).thenReturn(Arrays.asList(new Binding[] {mockBinding1, mockBinding2}));
+
+ _exchangeMBean.removeBinding(QUEUE1_NAME, BINDING1);
+ verify(mockBinding1).delete();
+ }
+
+ public void testRemoveBindingWhereQueueIsUnknown() throws Exception
+ {
+ Binding mockBinding1 = createBindingOnQueue(BINDING1, _mockQueue1);
+ when(_mockExchange.getBindings()).thenReturn(Arrays.asList(new Binding[] {mockBinding1}));
+
+ try
+ {
+ _exchangeMBean.removeBinding("unknown", BINDING1);
+ fail("Exception not thrown");
+ }
+ catch (OperationsException oe)
+ {
+ // PASS
+ assertEquals("No such queue \"unknown\"", oe.getMessage());
+ }
+ verify(mockBinding1, never()).delete();
+ }
+
+ public void testRemoveBindingWhereBindingNameIsUnknown() throws Exception
+ {
+ Binding mockBinding1 = createBindingOnQueue(BINDING1, _mockQueue1);
+ when(_mockExchange.getBindings()).thenReturn(Arrays.asList(new Binding[] {mockBinding1}));
+
+ try
+ {
+ _exchangeMBean.removeBinding(QUEUE1_NAME, "unknown");
+ fail("Exception not thrown");
+ }
+ catch (OperationsException oe)
+ {
+ // PASS
+ assertEquals("No such binding \"unknown\" on queue \"" + QUEUE1_NAME + "\"", oe.getMessage());
+ }
+ verify(mockBinding1, never()).delete();
+ }
+
+ public void testHeadersExchangeCreateNewBinding() throws Exception
+ {
+ String binding = "key1=binding1,key2=binding2";
+ Map<String, Object> expectedBindingMap = new HashMap<String, Object>()
+ {{
+ put("key1", "binding1");
+ put("key2", "binding2");
+ }};
+ _exchangeMBean = new ExchangeMBean(_mockHeadersExchange, _mockVirtualHostMBean);
+
+ _exchangeMBean.createNewBinding(QUEUE1_NAME, binding);
+ verify(_mockHeadersExchange).createBinding(binding, _mockQueue1, expectedBindingMap, Collections.EMPTY_MAP);
+ }
+
+ public void testHeadersExchangeCreateNewBindingWithFieldWithoutValue() throws Exception
+ {
+ String binding = "key1=binding1,key2=";
+ Map<String, Object> expectedBindingMap = new HashMap<String, Object>()
+ {{
+ put("key1", "binding1");
+ put("key2", "");
+ }};
+ _exchangeMBean = new ExchangeMBean(_mockHeadersExchange, _mockVirtualHostMBean);
+
+ _exchangeMBean.createNewBinding(QUEUE1_NAME, binding);
+ verify(_mockHeadersExchange).createBinding(binding, _mockQueue1, expectedBindingMap, Collections.EMPTY_MAP);
+ }
+
+ public void testHeadersExchangeCreateNewBindingMalformed() throws Exception
+ {
+ String binding = "=binding1,=";
+ _exchangeMBean = new ExchangeMBean(_mockHeadersExchange, _mockVirtualHostMBean);
+
+ try
+ {
+ _exchangeMBean.createNewBinding(QUEUE1_NAME, binding);
+ fail("Exception not thrown");
+ }
+ catch (JMException e)
+ {
+ assertEquals("Format for headers binding should be \"<attribute1>=<value1>,<attribute2>=<value2>\"", e.getMessage());
+ }
+ }
+
+ private Binding createBindingOnQueue(String bindingName, Queue queue)
+ {
+ Binding mockBinding = mock(Binding.class);
+ when(mockBinding.getParent(Queue.class)).thenReturn(queue);
+ when(mockBinding.getName()).thenReturn(bindingName);
+ return mockBinding;
+ }
+
+ private Queue createMockQueue(String queueName)
+ {
+ Queue mockQueue = mock(Queue.class);
+ when(mockQueue.getName()).thenReturn(queueName);
+ return mockQueue;
+ }
+
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org