Aligned JmsTemplate and DefaultMessageListenerContainer receiveTimeout values

Issue: SPR-14212

spring-jms/src/main/java/org/springframework/jms/core/JmsTemplate.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2016 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -89,18 +89,6 @@ import org.springframework.util.ReflectionUtils;
  */
 public class JmsTemplate extends JmsDestinationAccessor implements JmsOperations {
 
-	/**
-	 * Timeout value indicating that a receive operation should
-	 * check if a message is immediately available without blocking.
-	 */
-	public static final long RECEIVE_TIMEOUT_NO_WAIT = -1;
-
-	/**
-	 * Timeout value indicating a blocking receive without timeout.
-	 */
-	public static final long RECEIVE_TIMEOUT_INDEFINITE_WAIT = 0;
-
-
 	/** The JMS 2.0 MessageProducer.setDeliveryDelay method, if available */
 	private static final Method setDeliveryDelayMethod =
 			ClassUtils.getMethodIfAvailable(MessageProducer.class, "setDeliveryDelay", long.class);
@@ -315,11 +303,13 @@ public class JmsTemplate extends JmsDestinationAccessor implements JmsOperations
 	 * Set the timeout to use for receive calls (in milliseconds).
 	 * <p>The default is {@link #RECEIVE_TIMEOUT_INDEFINITE_WAIT}, which indicates
 	 * a blocking receive without timeout.
-	 * <p>Specify {@link #RECEIVE_TIMEOUT_NO_WAIT} to inidicate that a receive operation
-	 * should check if a message is immediately available without blocking.
+	 * <p>Specify {@link #RECEIVE_TIMEOUT_NO_WAIT} (or any other negative value)
+	 * to indicate that a receive operation should check if a message is
+	 * immediately available without blocking.
+	 * @see #receiveFromConsumer(MessageConsumer, long)
 	 * @see javax.jms.MessageConsumer#receive(long)
-	 * @see javax.jms.MessageConsumer#receive()
 	 * @see javax.jms.MessageConsumer#receiveNoWait()
+	 * @see javax.jms.MessageConsumer#receive()
 	 */
 	public void setReceiveTimeout(long receiveTimeout) {
 		this.receiveTimeout = receiveTimeout;
@@ -800,7 +790,7 @@ public class JmsTemplate extends JmsDestinationAccessor implements JmsOperations
 			if (resourceHolder != null && resourceHolder.hasTimeout()) {
 				timeout = Math.min(timeout, resourceHolder.getTimeToLiveInMillis());
 			}
-			Message message = doReceive(consumer, timeout);
+			Message message = receiveFromConsumer(consumer, timeout);
 			if (session.getTransacted()) {
 				// Commit necessary - but avoid commit call within a JTA transaction.
 				if (isSessionLocallyTransacted(session)) {
@@ -821,25 +811,6 @@ public class JmsTemplate extends JmsDestinationAccessor implements JmsOperations
 		}
 	}
 
-	/**
-	 * Actually receive a message from the given consumer.
-	 * @param consumer the JMS MessageConsumer to receive with
-	 * @param timeout the receive timeout
-	 * @return the JMS Message received, or {@code null} if none
-	 * @throws JMSException if thrown by JMS API methods
-	 */
-	private Message doReceive(MessageConsumer consumer, long timeout) throws JMSException {
-		if (timeout == RECEIVE_TIMEOUT_NO_WAIT) {
-			return consumer.receiveNoWait();
-		}
-		else if (timeout > 0) {
-			return consumer.receive(timeout);
-		}
-		else {
-			return consumer.receive();
-		}
-	}
-
 
 	//---------------------------------------------------------------------------------------
 	// Convenience methods for receiving auto-converted messages
@@ -952,7 +923,7 @@ public class JmsTemplate extends JmsDestinationAccessor implements JmsOperations
 				logger.debug("Sending created message: " + requestMessage);
 			}
 			doSend(producer, requestMessage);
-			return doReceive(consumer, getReceiveTimeout());
+			return receiveFromConsumer(consumer, getReceiveTimeout());
 		}
 		finally {
 			JmsUtils.closeMessageConsumer(consumer);

spring-jms/src/main/java/org/springframework/jms/listener/AbstractPollingMessageListenerContainer.java

@@ -158,9 +158,13 @@ public abstract class AbstractPollingMessageListenerContainer extends AbstractMe
 	 * The default is 1000 ms, that is, 1 second.
 	 * <p><b>NOTE:</b> This value needs to be smaller than the transaction
 	 * timeout used by the transaction manager (in the appropriate unit,
-	 * of course). -1 indicates no timeout at all; however, this is only
-	 * feasible if not running within a transaction manager.
+	 * of course). 0 indicates no timeout at all; however, this is only
+	 * feasible if not running within a transaction manager and generally
+	 * discouraged since such a listener container cannot cleanly shut down.
+	 * A negative value such as -1 indicates a no-wait receive operation.
+	 * @see #receiveFromConsumer(MessageConsumer, long)
 	 * @see javax.jms.MessageConsumer#receive(long)
+	 * @see javax.jms.MessageConsumer#receiveNoWait()
 	 * @see javax.jms.MessageConsumer#receive()
 	 * @see #setTransactionTimeout
 	 */
@@ -417,7 +421,7 @@ public abstract class AbstractPollingMessageListenerContainer extends AbstractMe
 	 * @throws JMSException if thrown by JMS methods
 	 */
 	protected Message receiveMessage(MessageConsumer consumer) throws JMSException {
-		return (this.receiveTimeout < 0 ? consumer.receive() : consumer.receive(this.receiveTimeout));
+		return receiveFromConsumer(consumer, getReceiveTimeout());
 	}
 
 	/**

spring-jms/src/main/java/org/springframework/jms/support/destination/JmsDestinationAccessor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2016 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -18,6 +18,8 @@ package org.springframework.jms.support.destination;
 
 import javax.jms.Destination;
 import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
 import javax.jms.Session;
 
 import org.springframework.jms.support.JmsAccessor;
@@ -38,6 +40,20 @@ import org.springframework.util.Assert;
  */
 public abstract class JmsDestinationAccessor extends JmsAccessor {
 
+	/**
+	 * Timeout value indicating that a receive operation should
+	 * check if a message is immediately available without blocking.
+	 * @since 4.3
+	 */
+	public static final long RECEIVE_TIMEOUT_NO_WAIT = -1;
+
+	/**
+	 * Timeout value indicating a blocking receive without timeout.
+	 * @since 4.3
+	 */
+	public static final long RECEIVE_TIMEOUT_INDEFINITE_WAIT = 0;
+
+
 	private DestinationResolver destinationResolver = new DynamicDestinationResolver();
 
 	private boolean pubSubDomain = false;
@@ -98,4 +114,27 @@ public abstract class JmsDestinationAccessor extends JmsAccessor {
 		return getDestinationResolver().resolveDestinationName(session, destinationName, isPubSubDomain());
 	}
 
+	/**
+	 * Actually receive a message from the given consumer.
+	 * @param consumer the JMS MessageConsumer to receive with
+	 * @param timeout the receive timeout (a negative value indicates
+	 * a no-wait receive; 0 indicates an indefinite wait attempt)
+	 * @return the JMS Message received, or {@code null} if none
+	 * @throws JMSException if thrown by JMS API methods
+	 * @since 4.3
+	 * @see #RECEIVE_TIMEOUT_NO_WAIT
+	 * @see #RECEIVE_TIMEOUT_INDEFINITE_WAIT
+	 */
+	protected Message receiveFromConsumer(MessageConsumer consumer, long timeout) throws JMSException {
+		if (timeout > 0) {
+			return consumer.receive(timeout);
+		}
+		else if (timeout < 0) {
+			return consumer.receiveNoWait();
+		}
+		else {
+			return consumer.receive();
+		}
+	}
+
 }