You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by ta...@apache.org on 2016/11/09 21:36:27 UTC
qpid-jms git commit: QPIDJMS-207 Add some additional validation
Repository: qpid-jms
Updated Branches:
refs/heads/master 78e6f64e2 -> fdedbd246
QPIDJMS-207 Add some additional validation
Validate ranges on priority and delivery mode in the Message and
MessageProducer. Add check for async completion calling close on its
own producer. Add tests to cover these cases.
Project: http://git-wip-us.apache.org/repos/asf/qpid-jms/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-jms/commit/fdedbd24
Tree: http://git-wip-us.apache.org/repos/asf/qpid-jms/tree/fdedbd24
Diff: http://git-wip-us.apache.org/repos/asf/qpid-jms/diff/fdedbd24
Branch: refs/heads/master
Commit: fdedbd2469b73d5de87a084824419a631e0fbb18
Parents: 78e6f64
Author: Timothy Bish <ta...@gmail.com>
Authored: Wed Nov 9 16:36:18 2016 -0500
Committer: Timothy Bish <ta...@gmail.com>
Committed: Wed Nov 9 16:36:18 2016 -0500
----------------------------------------------------------------------
.../org/apache/qpid/jms/JmsMessageProducer.java | 18 ++-
.../org/apache/qpid/jms/message/JmsMessage.java | 16 ++-
.../apache/qpid/jms/message/JmsMessageTest.java | 41 +++++-
.../facade/test/JmsTestMessageFacade.java | 2 +-
.../jms/producer/JmsMessageProducerTest.java | 144 ++++++++++++++++++-
.../qpid/jms/producer/JmsProducerTest.java | 4 +-
6 files changed, 210 insertions(+), 15 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/fdedbd24/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsMessageProducer.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsMessageProducer.java b/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsMessageProducer.java
index c0b8538..63fd513 100644
--- a/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsMessageProducer.java
+++ b/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsMessageProducer.java
@@ -24,7 +24,6 @@ import javax.jms.CompletionListener;
import javax.jms.DeliveryMode;
import javax.jms.Destination;
import javax.jms.IllegalStateException;
-import javax.jms.InvalidDestinationException;
import javax.jms.JMSException;
import javax.jms.Message;
import javax.jms.MessageProducer;
@@ -89,8 +88,9 @@ public class JmsMessageProducer implements AutoCloseable, MessageProducer {
* @throws JMSException if an internal error occurs during the close operation.
*/
protected void doClose() throws JMSException {
+ session.checkIsCompletionThread();
shutdown();
- this.connection.destroyResource(producerInfo);
+ connection.destroyResource(producerInfo);
}
/**
@@ -238,7 +238,14 @@ public class JmsMessageProducer implements AutoCloseable, MessageProducer {
@Override
public void setDeliveryMode(int deliveryMode) throws JMSException {
checkClosed();
- this.deliveryMode = deliveryMode;
+ switch (deliveryMode) {
+ case DeliveryMode.PERSISTENT:
+ case DeliveryMode.NON_PERSISTENT:
+ this.deliveryMode = deliveryMode;
+ break;
+ default:
+ throw new JMSException(String.format("Invalid DeliveryMode specific: %d", deliveryMode));
+ }
}
@Override
@@ -256,6 +263,11 @@ public class JmsMessageProducer implements AutoCloseable, MessageProducer {
@Override
public void setPriority(int defaultPriority) throws JMSException {
checkClosed();
+
+ if (defaultPriority < 0 || defaultPriority > 9) {
+ throw new JMSException(String.format("Priority value given {%d} is out of range (0..9)", defaultPriority));
+ }
+
this.priority = defaultPriority;
}
http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/fdedbd24/qpid-jms-client/src/main/java/org/apache/qpid/jms/message/JmsMessage.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/main/java/org/apache/qpid/jms/message/JmsMessage.java b/qpid-jms-client/src/main/java/org/apache/qpid/jms/message/JmsMessage.java
index cddf04f..c949c58 100644
--- a/qpid-jms-client/src/main/java/org/apache/qpid/jms/message/JmsMessage.java
+++ b/qpid-jms-client/src/main/java/org/apache/qpid/jms/message/JmsMessage.java
@@ -240,7 +240,16 @@ public class JmsMessage implements javax.jms.Message {
@Override
public void setJMSDeliveryMode(int mode) throws JMSException {
checkReadOnly();
- facade.setPersistent(mode == DeliveryMode.PERSISTENT);
+ switch (mode) {
+ case DeliveryMode.PERSISTENT:
+ facade.setPersistent(true);
+ break;
+ case DeliveryMode.NON_PERSISTENT:
+ facade.setPersistent(false);
+ break;
+ default:
+ throw new JMSException(String.format("Invalid DeliveryMode specific: %d", mode));
+ }
}
@Override
@@ -284,6 +293,11 @@ public class JmsMessage implements javax.jms.Message {
@Override
public void setJMSPriority(int priority) throws JMSException {
checkReadOnly();
+
+ if (priority < 0 || priority > 9) {
+ throw new JMSException(String.format("Priority value given {%d} is out of range (0..9)", priority));
+ }
+
facade.setPriority(priority);
}
http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/fdedbd24/qpid-jms-client/src/test/java/org/apache/qpid/jms/message/JmsMessageTest.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/test/java/org/apache/qpid/jms/message/JmsMessageTest.java b/qpid-jms-client/src/test/java/org/apache/qpid/jms/message/JmsMessageTest.java
index 7b05650..427027c 100644
--- a/qpid-jms-client/src/test/java/org/apache/qpid/jms/message/JmsMessageTest.java
+++ b/qpid-jms-client/src/test/java/org/apache/qpid/jms/message/JmsMessageTest.java
@@ -314,6 +314,25 @@ public class JmsMessageTest {
}
@Test
+ public void testSetJMSDeliveryModeWithInvalidValue() throws JMSException {
+ JmsMessage msg = factory.createMessage();
+ assertEquals(Message.DEFAULT_DELIVERY_MODE, msg.getJMSDeliveryMode());
+ try {
+ msg.setJMSDeliveryMode(-1);
+ fail("Should have thrown an exception");
+ } catch (JMSException ex) {
+ LOG.debug("Caught expected exception: {}", ex.getMessage());
+ }
+ try {
+ msg.setJMSDeliveryMode(3);
+ fail("Should have thrown an exception");
+ } catch (JMSException ex) {
+ LOG.debug("Caught expected exception: {}", ex.getMessage());
+ }
+ assertEquals(Message.DEFAULT_DELIVERY_MODE, msg.getJMSDeliveryMode());
+ }
+
+ @Test
public void testGetAndSetMSRedelivered() throws JMSException {
JmsMessage msg = factory.createMessage();
msg.setJMSRedelivered(this.jmsRedelivered);
@@ -336,15 +355,23 @@ public class JmsMessageTest {
@Test
public void testGetAndSetJMSPriority() throws JMSException {
- JmsMessage msg = factory.createMessage();
- msg.setJMSPriority(this.jmsPriority);
- assertTrue(msg.getJMSPriority() == this.jmsPriority);
+ JmsMessage message = factory.createMessage();
+ assertEquals(Message.DEFAULT_PRIORITY, message.getJMSPriority());
- msg.setJMSPriority(-90);
- assertEquals(0, msg.getJMSPriority());
+ try {
+ message.setJMSPriority(-1);
+ fail("Should have thrown an exception");
+ } catch (JMSException ex) {
+ LOG.debug("Caught expected exception: {}", ex.getMessage());
+ }
+ try {
+ message.setJMSPriority(10);
+ fail("Should have thrown an exception");
+ } catch (JMSException ex) {
+ LOG.debug("Caught expected exception: {}", ex.getMessage());
+ }
- msg.setJMSPriority(90);
- assertEquals(9, msg.getJMSPriority());
+ assertEquals(Message.DEFAULT_PRIORITY, message.getJMSPriority());
}
@Test
http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/fdedbd24/qpid-jms-client/src/test/java/org/apache/qpid/jms/message/facade/test/JmsTestMessageFacade.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/test/java/org/apache/qpid/jms/message/facade/test/JmsTestMessageFacade.java b/qpid-jms-client/src/test/java/org/apache/qpid/jms/message/facade/test/JmsTestMessageFacade.java
index fa06a68..d4b1cfd 100644
--- a/qpid-jms-client/src/test/java/org/apache/qpid/jms/message/facade/test/JmsTestMessageFacade.java
+++ b/qpid-jms-client/src/test/java/org/apache/qpid/jms/message/facade/test/JmsTestMessageFacade.java
@@ -63,7 +63,7 @@ public class JmsTestMessageFacade implements JmsMessageFacade {
protected long deliveryTime;
protected long timestamp;
protected String correlationId;
- protected boolean persistent;
+ protected boolean persistent = true;
protected int redeliveryCount;
protected String type;
protected JmsDestination destination;
http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/fdedbd24/qpid-jms-client/src/test/java/org/apache/qpid/jms/producer/JmsMessageProducerTest.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/test/java/org/apache/qpid/jms/producer/JmsMessageProducerTest.java b/qpid-jms-client/src/test/java/org/apache/qpid/jms/producer/JmsMessageProducerTest.java
index fef568f..87f94c8 100644
--- a/qpid-jms-client/src/test/java/org/apache/qpid/jms/producer/JmsMessageProducerTest.java
+++ b/qpid-jms-client/src/test/java/org/apache/qpid/jms/producer/JmsMessageProducerTest.java
@@ -25,11 +25,14 @@ import static org.junit.Assert.fail;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
import javax.jms.CompletionListener;
import javax.jms.Connection;
import javax.jms.DeliveryMode;
import javax.jms.Destination;
+import javax.jms.IllegalStateException;
import javax.jms.InvalidDestinationException;
import javax.jms.JMSException;
import javax.jms.Message;
@@ -117,6 +120,25 @@ public class JmsMessageProducerTest extends JmsConnectionTestSupport {
}
@Test(timeout = 10000)
+ public void testPriorityConfigurationWithInvalidPriorityValues() throws Exception {
+ MessageProducer producer = session.createProducer(null);
+ assertEquals(Message.DEFAULT_PRIORITY, producer.getPriority());
+ try {
+ producer.setPriority(-1);
+ fail("Should have thrown an exception");
+ } catch (JMSException ex) {
+ LOG.debug("Caught expected exception: {}", ex.getMessage());
+ }
+ try {
+ producer.setPriority(10);
+ fail("Should have thrown an exception");
+ } catch (JMSException ex) {
+ LOG.debug("Caught expected exception: {}", ex.getMessage());
+ }
+ assertEquals(Message.DEFAULT_PRIORITY, producer.getPriority());
+ }
+
+ @Test(timeout = 10000)
public void testTimeToLiveConfiguration() throws Exception {
MessageProducer producer = session.createProducer(null);
assertEquals(Message.DEFAULT_TIME_TO_LIVE, producer.getTimeToLive());
@@ -133,6 +155,25 @@ public class JmsMessageProducerTest extends JmsConnectionTestSupport {
}
@Test(timeout = 10000)
+ public void testDeliveryModeConfigurationWithInvalidMode() throws Exception {
+ MessageProducer producer = session.createProducer(null);
+ assertEquals(Message.DEFAULT_DELIVERY_MODE, producer.getDeliveryMode());
+ try {
+ producer.setDeliveryMode(-1);
+ fail("Should have thrown an exception");
+ } catch (JMSException ex) {
+ LOG.debug("Caught expected exception: {}", ex.getMessage());
+ }
+ try {
+ producer.setDeliveryMode(5);
+ fail("Should have thrown an exception");
+ } catch (JMSException ex) {
+ LOG.debug("Caught expected exception: {}", ex.getMessage());
+ }
+ assertEquals(Message.DEFAULT_DELIVERY_MODE, producer.getDeliveryMode());
+ }
+
+ @Test(timeout = 10000)
public void testDeliveryDelayConfiguration() throws Exception {
MessageProducer producer = session.createProducer(null);
assertEquals(Message.DEFAULT_DELIVERY_DELAY, producer.getDeliveryDelay());
@@ -516,7 +557,108 @@ public class JmsMessageProducerTest extends JmsConnectionTestSupport {
connection.close();
}
- private void sendMessages(int count, JmsMessageProducer producer, MyCompletionListener listener) throws Exception {
+ @Test(timeout = 15000)
+ public void testCompletionListenerOnCompleteCallsProducerCloseThrowsISE() throws Exception {
+ final int MESSAGE_COUNT = 1;
+
+ final MockRemotePeer remotePoor = MockRemotePeer.INSTANCE;
+
+ JmsConnectionFactory factory = new JmsConnectionFactory(
+ "mock://localhost?mock.delayCompletionCalls=true");
+
+ Connection connection = factory.createConnection();
+ Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE);
+
+ final CountDownLatch done = new CountDownLatch(1);
+ final Destination destination = new JmsQueue("explicitDestination");
+ final JmsMessageProducer producer = (JmsMessageProducer) session.createProducer(destination);
+ final CompletionListener listener = new CompletionListener() {
+
+ @Override
+ public void onCompletion(Message message) {
+ try {
+ producer.close();
+ } catch (IllegalStateException ex) {
+ done.countDown();
+ } catch (Exception e) {
+ LOG.info("Wrong exception thrown when close called in completion: {}", e.getMessage());
+ }
+ }
+
+ @Override
+ public void onException(Message message, Exception exception) {
+ LOG.info("Unexpected exception thrown in completion: {}", exception.getMessage());
+ }
+ };
+
+ sendMessages(MESSAGE_COUNT, producer, listener);
+
+ assertTrue("Not all sends made it to the remote", Wait.waitFor(new Wait.Condition() {
+
+ @Override
+ public boolean isSatisified() throws Exception {
+ return remotePoor.getPendingCompletions(destination).size() == MESSAGE_COUNT;
+ }
+ }));
+
+ remotePoor.completeAllPendingSends(destination);
+
+ assertTrue("Completion never got expected ISE", done.await(10, TimeUnit.SECONDS));
+
+ connection.close();
+ }
+
+ @Test(timeout = 15000)
+ public void testCompletionListenerOnExceptionCallsProducerCloseThrowsISE() throws Exception {
+ final int MESSAGE_COUNT = 1;
+
+ final MockRemotePeer remotePoor = MockRemotePeer.INSTANCE;
+
+ JmsConnectionFactory factory = new JmsConnectionFactory(
+ "mock://localhost?mock.delayCompletionCalls=true");
+
+ Connection connection = factory.createConnection();
+ Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE);
+
+ final CountDownLatch done = new CountDownLatch(1);
+ final Destination destination = new JmsQueue("explicitDestination");
+ final JmsMessageProducer producer = (JmsMessageProducer) session.createProducer(destination);
+ final CompletionListener listener = new CompletionListener() {
+
+ @Override
+ public void onCompletion(Message message) {
+ }
+
+ @Override
+ public void onException(Message message, Exception exception) {
+ try {
+ producer.close();
+ } catch (IllegalStateException ex) {
+ done.countDown();
+ } catch (Exception e) {
+ LOG.info("Wrong exception thrown when close called in completion: {}", e.getMessage());
+ }
+ }
+ };
+
+ sendMessages(MESSAGE_COUNT, producer, listener);
+
+ assertTrue("Not all sends made it to the remote", Wait.waitFor(new Wait.Condition() {
+
+ @Override
+ public boolean isSatisified() throws Exception {
+ return remotePoor.getPendingCompletions(destination).size() == MESSAGE_COUNT;
+ }
+ }));
+
+ remotePoor.failAllPendingSends(destination, new JMSException("Could not send message"));
+
+ assertTrue("Completion never got expected ISE", done.await(10, TimeUnit.SECONDS));
+
+ connection.close();
+ }
+
+ private void sendMessages(int count, JmsMessageProducer producer, CompletionListener listener) throws Exception {
for (int i = 0; i < count; ++i) {
Message message = session.createMessage();
message.setIntProperty("sequence", i);
http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/fdedbd24/qpid-jms-client/src/test/java/org/apache/qpid/jms/producer/JmsProducerTest.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/test/java/org/apache/qpid/jms/producer/JmsProducerTest.java b/qpid-jms-client/src/test/java/org/apache/qpid/jms/producer/JmsProducerTest.java
index 4569e4e..eeba336 100644
--- a/qpid-jms-client/src/test/java/org/apache/qpid/jms/producer/JmsProducerTest.java
+++ b/qpid-jms-client/src/test/java/org/apache/qpid/jms/producer/JmsProducerTest.java
@@ -1288,7 +1288,7 @@ public class JmsProducerTest extends JmsConnectionTestSupport {
Mockito.doThrow(new InvalidDestinationException("ide"))
.when(mockSession)
.send(Mockito.any(JmsMessageProducer.class), Mockito.any(Destination.class), Mockito.any(Message.class),
- Mockito.any(int.class), Mockito.any(int.class), Mockito.any(long.class), Mockito.any(boolean.class),
+ Mockito.any(int.class), Mockito.any(int.class), Mockito.any(long.class), Mockito.any(boolean.class),
Mockito.any(boolean.class), Mockito.any(long.class), Mockito.any(CompletionListener.class));
JmsProducer producer = new JmsProducer(mockSession, mockMessageProducer);
@@ -1299,7 +1299,7 @@ public class JmsProducerTest extends JmsConnectionTestSupport {
} catch (InvalidDestinationRuntimeException idre) {}
Mockito.verify(mockSession).send(Mockito.any(JmsMessageProducer.class), Mockito.any(Destination.class), Mockito.any(Message.class),
- Mockito.any(int.class), Mockito.any(int.class), Mockito.any(long.class), Mockito.any(boolean.class),
+ Mockito.any(int.class), Mockito.any(int.class), Mockito.any(long.class), Mockito.any(boolean.class),
Mockito.any(boolean.class), Mockito.any(long.class), Mockito.any(CompletionListener.class));
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org