ARTEMIS-1951 Fix NPE on updateQueue with NULL user
Project: http://git-wip-us.apache.org/repos/asf/activemq-artemis/repo Commit: http://git-wip-us.apache.org/repos/asf/activemq-artemis/commit/60c586a6 Tree: http://git-wip-us.apache.org/repos/asf/activemq-artemis/tree/60c586a6 Diff: http://git-wip-us.apache.org/repos/asf/activemq-artemis/diff/60c586a6 Branch: refs/heads/master Commit: 60c586a64c760414509c8990554e4bfbc5b0845f Parents: 8d78972 Author: Francesco Nigro <[email protected]> Authored: Mon Jul 2 09:18:39 2018 +0200 Committer: Justin Bertram <[email protected]> Committed: Mon Jul 2 11:18:16 2018 -0500 ---------------------------------------------------------------------- .../core/postoffice/impl/PostOfficeImpl.java | 5 ++ .../core/server/impl/ActiveMQServerImpl.java | 2 +- .../integration/client/UpdateQueueTest.java | 71 +++++++++++++++++++- 3 files changed, 76 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/60c586a6/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java ---------------------------------------------------------------------- diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java index 2b5ba60..21a7504 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java @@ -512,6 +512,11 @@ public class PostOfficeImpl implements PostOffice, NotificationListener, Binding changed = true; queue.setExclusive(exclusive); } + if (logger.isDebugEnabled()) { + if (user == null && queue.getUser() != null) { + logger.debug("Ignoring updating Queue to a NULL user"); + } + } if (user != null && !user.equals(queue.getUser())) { changed = true; queue.setUser(user); http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/60c586a6/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java ---------------------------------------------------------------------- diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java index 882c66d..9359ecc 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java @@ -2970,7 +2970,7 @@ public class ActiveMQServerImpl implements ActiveMQServer { Boolean purgeOnNoConsumers, Boolean exclusive, String user) throws Exception { - final QueueBinding queueBinding = this.postOffice.updateQueue(new SimpleString(name), routingType, maxConsumers, purgeOnNoConsumers, exclusive, new SimpleString(user)); + final QueueBinding queueBinding = this.postOffice.updateQueue(new SimpleString(name), routingType, maxConsumers, purgeOnNoConsumers, exclusive, SimpleString.toSimpleString(user)); if (queueBinding != null) { final Queue queue = queueBinding.getQueue(); return queue; http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/60c586a6/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/client/UpdateQueueTest.java ---------------------------------------------------------------------- diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/client/UpdateQueueTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/client/UpdateQueueTest.java index 9c8da67..60a84a4 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/client/UpdateQueueTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/client/UpdateQueueTest.java @@ -21,7 +21,6 @@ import javax.jms.Connection; import javax.jms.MessageConsumer; import javax.jms.MessageProducer; import javax.jms.Session; - import java.util.EnumSet; import java.util.HashMap; import java.util.concurrent.atomic.AtomicInteger; @@ -40,6 +39,76 @@ import org.junit.Test; public class UpdateQueueTest extends ActiveMQTestBase { @Test + public void testUpdateQueueWithNullUser() throws Exception { + ActiveMQServer server = createServer(true, true); + + ActiveMQConnectionFactory factory = new ActiveMQConnectionFactory(); + + server.start(); + + SimpleString ADDRESS = SimpleString.toSimpleString("queue.0"); + + final SimpleString user = new SimpleString("newUser"); + + Queue queue = server.createQueue(ADDRESS, RoutingType.ANYCAST, ADDRESS, user, null, true, false); + + long originalID = queue.getID(); + + Assert.assertEquals(user, queue.getUser()); + + Connection conn = factory.createConnection(); + Session session = conn.createSession(false, Session.AUTO_ACKNOWLEDGE); + MessageProducer prod = session.createProducer(session.createQueue(ADDRESS.toString())); + + for (int i = 0; i < 100; i++) { + prod.send(session.createTextMessage("message " + i)); + } + + server.updateQueue(ADDRESS.toString(), RoutingType.ANYCAST, 1, false, false, null); + + conn.close(); + factory.close(); + + server.stop(); + server.start(); + + validateBindingRecords(server, JournalRecordIds.QUEUE_BINDING_RECORD, 2); + + queue = server.locateQueue(ADDRESS); + + Assert.assertNotNull("queue not found", queue); + + Assert.assertEquals("newUser", user, queue.getUser()); + + factory = new ActiveMQConnectionFactory(); + + conn = factory.createConnection(); + session = conn.createSession(false, Session.AUTO_ACKNOWLEDGE); + + MessageConsumer consumer = session.createConsumer(session.createQueue(ADDRESS.toString())); + + conn.start(); + for (int i = 0; i < 100; i++) { + Assert.assertNotNull(consumer.receive(5000)); + } + + Assert.assertNull(consumer.receiveNoWait()); + + Assert.assertEquals(1, queue.getMaxConsumers()); + + conn.close(); + + Assert.assertEquals(originalID, server.locateQueue(ADDRESS).getID()); + + // stopping, restarting to make sure the system will not create an extra record without an udpate + server.stop(); + server.start(); + validateBindingRecords(server, JournalRecordIds.QUEUE_BINDING_RECORD, 2); + server.stop(); + + } + + @Test public void testUpdateQueue() throws Exception { ActiveMQServer server = createServer(true, true);
