More Sonar Fixes

* More

* Deprecate template.setQueue()

in favor of setDefaultReceiveQueue()

Causes confusion since it has no relevance to send methods.

* Fix deprecations; test.

* Fix test waiting on wrong latch.

* Increase 10k confirms wait time (test).

Author: garyrussell

spring-amqp/src/main/java/org/springframework/amqp/core/MessageProperties.java

@@ -531,8 +531,8 @@ public void setTargetBean(Object targetBean) {
 		}
 	}
 
-	@Override
-	public int hashCode() { // NOSONAR complexity
+	@Override // NOSONAR complexity
+	public int hashCode() {
 		final int prime = 31;
 		int result = 1;
 		result = prime * result + ((this.appId == null) ? 0 : this.appId.hashCode());
@@ -558,8 +558,8 @@ public int hashCode() { // NOSONAR complexity
 		return result;
 	}
 
-	@Override
-	public boolean equals(Object obj) { // NOSONAR complexity
+	@Override // NOSONAR complexity
+	public boolean equals(Object obj) { // NOSONAR line count
 		if (this == obj) {
 			return true;
 		}
@@ -715,8 +715,8 @@ else if (!this.userId.equals(other.userId)) {
 		return true;
 	}
 
-	@Override
-	public String toString() { // NOSONAR complexity
+	@Override // NOSONAR complexity
+	public String toString() {
 		return "MessageProperties [headers=" + this.headers
 				+ (this.timestamp == null ? "" : ", timestamp=" + this.timestamp)
 				+ (this.messageId == null ? "" : ", messageId=" + this.messageId)

spring-amqp/src/main/java/org/springframework/amqp/remoting/service/AmqpInvokerServiceExporter.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -71,7 +71,7 @@ public void onMessage(Message message) {
 		Object invocationRaw = this.messageConverter.fromMessage(message);
 
 		RemoteInvocationResult remoteInvocationResult;
-		if (invocationRaw == null || !(invocationRaw instanceof RemoteInvocation)) {
+		if (!(invocationRaw instanceof RemoteInvocation)) {
 			remoteInvocationResult =  new RemoteInvocationResult(
 					new IllegalArgumentException("The message does not contain a RemoteInvocation payload"));
 		}

spring-amqp/src/main/java/org/springframework/amqp/support/AmqpMessageHeaderAccessor.java

@@ -87,7 +87,7 @@ public Long getContentLength() {
 	@Override
 	public MimeType getContentType() {
 		Object value = getHeader(AmqpHeaders.CONTENT_TYPE);
-		if (value != null && value instanceof String) {
+		if (value instanceof String) {
 			return MimeType.valueOf((String) value);
 		}
 		else {

spring-rabbit/src/main/java/org/springframework/amqp/rabbit/config/AbstractRabbitListenerContainerFactory.java

@@ -342,7 +342,7 @@ public void setContainerConfigurer(Consumer<C> configurer) {
 		this.containerConfigurer = configurer;
 	}
 
-	@SuppressWarnings("deprecation")
+	@SuppressWarnings("deprecation") // NOSONAR complexity - mostly null checks
 	@Override
 	public C createListenerContainer(RabbitListenerEndpoint endpoint) { // NOSONAR complexity
 		C instance = createContainerInstance();

spring-rabbit/src/main/java/org/springframework/amqp/rabbit/config/ListenerContainerFactoryBean.java

@@ -404,7 +404,7 @@ public Class<?> getObjectType() {
 		return this.listenerContainer == null ? AbstractMessageListenerContainer.class : this.listenerContainer.getClass();
 	}
 
-	@SuppressWarnings("deprecation")
+	@SuppressWarnings("deprecation") // NOSONAR complexity - mostly null checks
 	@Override
 	protected AbstractMessageListenerContainer createInstance() { // NOSONAR complexity
 		if (this.listenerContainer == null) {
@@ -529,7 +529,7 @@ protected AbstractMessageListenerContainer createInstance() { // NOSONAR complex
 		return this.listenerContainer;
 	}
 
-	private AbstractMessageListenerContainer createContainer() {
+	private AbstractMessageListenerContainer createContainer() { // NOSONAR complexity - mostly null checks
 		if (this.type.equals(Type.simple)) {
 			SimpleMessageListenerContainer container = new SimpleMessageListenerContainer(this.connectionFactory);
 			if (this.concurrentConsumers != null) {

spring-rabbit/src/main/java/org/springframework/amqp/rabbit/config/SimpleRabbitListenerContainerFactory.java

@@ -134,7 +134,7 @@ protected SimpleMessageListenerContainer createContainerInstance() {
 		return new SimpleMessageListenerContainer();
 	}
 
-	@Override
+	@Override // NOSONAR complexity = mostly null checks
 	protected void initializeContainer(SimpleMessageListenerContainer instance, RabbitListenerEndpoint endpoint) {
 		super.initializeContainer(instance, endpoint);
 

spring-rabbit/src/main/java/org/springframework/amqp/rabbit/connection/CachingConnectionFactory.java

@@ -480,22 +480,11 @@ else if (protocolClassId == 10) {
 	}
 
 	private Channel getChannel(ChannelCachingConnectionProxy connection, boolean transactional) {
-		Semaphore checkoutPermits = null;
+		Semaphore permits = null;
 		if (this.channelCheckoutTimeout > 0) {
-			checkoutPermits = obtainPermit(connection);
-		}
-		LinkedList<ChannelProxy> channelList;
-		if (this.cacheMode == CacheMode.CHANNEL) {
-			channelList = transactional ? this.cachedChannelsTransactional
-					: this.cachedChannelsNonTransactional;
-		}
-		else {
-			channelList = transactional ? this.allocatedConnectionTransactionalChannels.get(connection)
-					: this.allocatedConnectionNonTransactionalChannels.get(connection);
-		}
-		if (channelList == null) {
-			throw new IllegalStateException("No channel list for connection " + connection);
+			permits = obtainPermits(connection);
 		}
+		LinkedList<ChannelProxy> channelList = determineChannelList(connection, transactional);
 		ChannelProxy channel = null;
 		if (connection.isOpen()) {
 			channel = findOpenChannel(channelList, channel);
@@ -510,11 +499,11 @@ private Channel getChannel(ChannelCachingConnectionProxy connection, boolean tra
 				channel = getCachedChannelProxy(connection, channelList, transactional);
 			}
 			catch (RuntimeException e) {
-				if (checkoutPermits != null) {
-					checkoutPermits.release();
+				if (permits != null) {
+					permits.release();
 					if (logger.isDebugEnabled()) {
 						logger.debug("Could not get channel; released permit for " + connection + ", remaining:"
-								+ checkoutPermits.availablePermits());
+								+ permits.availablePermits());
 					}
 				}
 				throw e;
@@ -523,7 +512,7 @@ private Channel getChannel(ChannelCachingConnectionProxy connection, boolean tra
 		return channel;
 	}
 
-	private Semaphore obtainPermit(ChannelCachingConnectionProxy connection) {
+	private Semaphore obtainPermits(ChannelCachingConnectionProxy connection) {
 		Semaphore permits;
 		permits = this.checkoutPermits.get(connection);
 		if (permits != null) {
@@ -548,7 +537,8 @@ private Semaphore obtainPermit(ChannelCachingConnectionProxy connection) {
 	}
 
 	private ChannelProxy findOpenChannel(LinkedList<ChannelProxy> channelList, // NOSONAR LinkedList.removeFirst()
-			ChannelProxy channel) {
+			ChannelProxy channelArg) {
+		ChannelProxy channel = channelArg;
 		synchronized (channelList) {
 			while (!channelList.isEmpty()) {
 				channel = channelList.removeFirst();
@@ -595,6 +585,23 @@ private void cleanUpClosedChannel(ChannelProxy channel) {
 		}
 	}
 
+	private LinkedList<ChannelProxy> determineChannelList(ChannelCachingConnectionProxy connection, // NOSONAR LL
+			boolean transactional) {
+		LinkedList<ChannelProxy> channelList; // NOSONAR must be LinkedList
+		if (this.cacheMode == CacheMode.CHANNEL) {
+			channelList = transactional ? this.cachedChannelsTransactional
+					: this.cachedChannelsNonTransactional;
+		}
+		else {
+			channelList = transactional ? this.allocatedConnectionTransactionalChannels.get(connection)
+					: this.allocatedConnectionNonTransactionalChannels.get(connection);
+		}
+		if (channelList == null) {
+			throw new IllegalStateException("No channel list for connection " + connection);
+		}
+		return channelList;
+	}
+
 	private ChannelProxy getCachedChannelProxy(ChannelCachingConnectionProxy connection,
 			LinkedList<ChannelProxy> channelList, boolean transactional) { //NOSONAR LinkedList for addLast()
 

spring-rabbit/src/main/java/org/springframework/amqp/rabbit/connection/LocalizedQueueConnectionFactory.java

@@ -18,14 +18,18 @@
 
 import java.net.MalformedURLException;
 import java.net.URISyntaxException;
+import java.util.AbstractMap.SimpleImmutableEntry;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
 import org.springframework.amqp.AmqpException;
+import org.springframework.amqp.rabbit.support.RabbitExceptionTranslator;
 import org.springframework.beans.factory.DisposableBean;
 import org.springframework.core.io.Resource;
 import org.springframework.lang.Nullable;
@@ -34,7 +38,6 @@
 import com.rabbitmq.http.client.Client;
 import com.rabbitmq.http.client.domain.QueueInfo;
 
-
 /**
  * A {@link RoutingConnectionFactory} that determines the node on which a queue is located and
  * returns a factory that connects directly to that node.
@@ -96,19 +99,9 @@ public class LocalizedQueueConnectionFactory implements ConnectionFactory, Routi
 	public LocalizedQueueConnectionFactory(ConnectionFactory defaultConnectionFactory,
 			Map<String, String> nodeToAddress, String[] adminUris, String vhost, String username, String password,
 			boolean useSSL, Resource sslPropertiesLocation) {
-		Assert.notNull(defaultConnectionFactory, "'defaultConnectionFactory' cannot be null");
-		this.defaultConnectionFactory = defaultConnectionFactory;
-		this.adminUris = Arrays.copyOf(adminUris, adminUris.length);
-		this.nodeToAddress.putAll(nodeToAddress);
-		this.vhost = vhost;
-		this.username = username;
-		this.password = password;
-		this.useSSL = useSSL;
-		this.sslPropertiesLocation = sslPropertiesLocation;
-		this.keyStore = null;
-		this.trustStore = null;
-		this.keyStorePassPhrase = null;
-		this.trustStorePassPhrase = null;
+
+		this(defaultConnectionFactory, adminUris, nodeToAddress, vhost, username, password, useSSL,
+				sslPropertiesLocation, null, null, null, null);
 	}
 
 	/**
@@ -129,19 +122,9 @@ public LocalizedQueueConnectionFactory(ConnectionFactory defaultConnectionFactor
 			Map<String, String> nodeToAddress, String[] adminUris, String vhost, String username, String password,
 			boolean useSSL, String keyStore, String trustStore,
 			String keyStorePassPhrase, String trustStorePassPhrase) {
-		Assert.notNull(defaultConnectionFactory, "'defaultConnectionFactory' cannot be null");
-		this.defaultConnectionFactory = defaultConnectionFactory;
-		this.adminUris = Arrays.copyOf(adminUris, adminUris.length);
-		this.nodeToAddress.putAll(nodeToAddress);
-		this.vhost = vhost;
-		this.username = username;
-		this.password = password;
-		this.useSSL = useSSL;
-		this.sslPropertiesLocation = null;
-		this.keyStore = keyStore;
-		this.trustStore = trustStore;
-		this.keyStorePassPhrase = keyStorePassPhrase;
-		this.trustStorePassPhrase = trustStorePassPhrase;
+
+		this(defaultConnectionFactory, adminUris, nodeToAddress, vhost, username, password, useSSL, null,
+				keyStore, trustStore, keyStorePassPhrase, trustStorePassPhrase);
 	}
 
 	/**
@@ -159,24 +142,10 @@ public LocalizedQueueConnectionFactory(ConnectionFactory defaultConnectionFactor
 	 */
 	public LocalizedQueueConnectionFactory(ConnectionFactory defaultConnectionFactory, String[] addresses,
 			String[] adminUris, String[] nodes, String vhost, String username, String password, boolean useSSL,
-			Resource sslPropertiesLocation) {
-		Assert.notNull(defaultConnectionFactory, "'defaultConnectionFactory' cannot be null");
-		Assert.isTrue(addresses.length == nodes.length,
-				"'addresses', 'adminAddresses', and 'nodes' properties must have equal length");
-		this.defaultConnectionFactory = defaultConnectionFactory;
-		this.adminUris = Arrays.copyOf(adminUris, adminUris.length);
-		for (int i = 0; i < addresses.length; i++) {
-			this.nodeToAddress.put(nodes[i], addresses[i]);
-		}
-		this.vhost = vhost;
-		this.username = username;
-		this.password = password;
-		this.useSSL = useSSL;
-		this.sslPropertiesLocation = sslPropertiesLocation;
-		this.keyStore = null;
-		this.trustStore = null;
-		this.keyStorePassPhrase = null;
-		this.trustStorePassPhrase = null;
+			@Nullable Resource sslPropertiesLocation) {
+
+		this(defaultConnectionFactory, adminUris, nodesAddressesToMap(nodes, addresses), vhost, username, password,
+				useSSL, sslPropertiesLocation, null, null, null, null);
 	}
 
 	/**
@@ -198,25 +167,40 @@ public LocalizedQueueConnectionFactory(ConnectionFactory defaultConnectionFactor
 			String[] addresses, String[] adminUris, String[] nodes, String vhost,
 			String username, String password, boolean useSSL, String keyStore, String trustStore,
 			String keyStorePassPhrase, String trustStorePassPhrase) {
+
+		this(defaultConnectionFactory, adminUris, nodesAddressesToMap(nodes, addresses), vhost, username, password,
+				useSSL, null, keyStore, trustStore, keyStorePassPhrase, trustStorePassPhrase);
+	}
+
+	private LocalizedQueueConnectionFactory(ConnectionFactory defaultConnectionFactory, String[] adminUris,
+			Map<String, String> nodeToAddress, String vhost, String username, String password, boolean useSSL,
+			@Nullable Resource sslPropertiesLocation, @Nullable String keyStore, @Nullable String trustStore,
+			@Nullable String keyStorePassPhrase, @Nullable String trustStorePassPhrase) {
+
 		Assert.notNull(defaultConnectionFactory, "'defaultConnectionFactory' cannot be null");
-		Assert.isTrue(addresses.length == nodes.length,
-				"'addresses', 'adminAddresses', and 'nodes' properties must have equal length");
 		this.defaultConnectionFactory = defaultConnectionFactory;
 		this.adminUris = Arrays.copyOf(adminUris, adminUris.length);
-		for (int i = 0; i < addresses.length; i++) {
-			this.nodeToAddress.put(nodes[i], addresses[i]);
-		}
+		this.nodeToAddress.putAll(nodeToAddress);
 		this.vhost = vhost;
 		this.username = username;
 		this.password = password;
 		this.useSSL = useSSL;
-		this.sslPropertiesLocation = null;
+		this.sslPropertiesLocation = sslPropertiesLocation;
 		this.keyStore = keyStore;
 		this.trustStore = trustStore;
 		this.keyStorePassPhrase = keyStorePassPhrase;
 		this.trustStorePassPhrase = trustStorePassPhrase;
 	}
 
+	private static Map<String, String> nodesAddressesToMap(String[] nodes, String[] addresses) {
+		Assert.isTrue(addresses.length == nodes.length,
+				"'addresses' and 'nodes' properties must have equal length");
+		return IntStream.range(0, addresses.length)
+			.mapToObj(i -> new SimpleImmutableEntry<>(nodes[i], addresses[i]))
+			.collect(Collectors.toMap(SimpleImmutableEntry::getKey, SimpleImmutableEntry::getValue,
+					(u, v) -> v)); // TODO in 2.2 use default throwingMerger() (to catch dups)
+	}
+
 	@Override
 	public Connection createConnection() throws AmqpException {
 		return this.defaultConnectionFactory.createConnection();
@@ -320,8 +304,7 @@ protected Client createClient(String adminUri, String username, String password)
 		return new Client(adminUri, username, password);
 	}
 
-	private synchronized ConnectionFactory nodeConnectionFactory(String queue, String node, String address)
-			throws Exception {
+	private synchronized ConnectionFactory nodeConnectionFactory(String queue, String node, String address)  {
 		if (this.logger.isInfoEnabled()) {
 			this.logger.info("Queue: " + queue + " is on node: " + node + " at: " + address);
 		}
@@ -341,9 +324,8 @@ private synchronized ConnectionFactory nodeConnectionFactory(String queue, Strin
 	 * @param address the address to which the factory should connect.
 	 * @param node  the node.
 	 * @return the connection factory.
-	 * @throws Exception if errors occur during creation.
 	 */
-	protected ConnectionFactory createConnectionFactory(String address, String node) throws Exception {
+	protected ConnectionFactory createConnectionFactory(String address, String node) {
 		RabbitConnectionFactoryBean rcfb = new RabbitConnectionFactoryBean();
 		rcfb.setUseSSL(this.useSSL);
 		rcfb.setSslPropertiesLocation(this.sslPropertiesLocation);
@@ -352,7 +334,14 @@ protected ConnectionFactory createConnectionFactory(String address, String node)
 		rcfb.setKeyStorePassphrase(this.keyStorePassPhrase);
 		rcfb.setTrustStorePassphrase(this.trustStorePassPhrase);
 		rcfb.afterPropertiesSet();
-		CachingConnectionFactory ccf = new CachingConnectionFactory(rcfb.getObject()); // NOSONAR never null
+		com.rabbitmq.client.ConnectionFactory rcf;
+		try {
+			rcf = rcfb.getObject();
+		}
+		catch (Exception e) {
+			throw RabbitExceptionTranslator.convertRabbitAccessException(e);
+		}
+		CachingConnectionFactory ccf = new CachingConnectionFactory(rcf); // NOSONAR never null
 		ccf.setAddresses(address);
 		ccf.setUsername(this.username);
 		ccf.setPassword(this.password);
@@ -362,12 +351,21 @@ protected ConnectionFactory createConnectionFactory(String address, String node)
 	}
 
 	@Override
-	public void destroy() throws Exception {
+	public void destroy() {
+		Exception lastException = null;
 		for (ConnectionFactory connectionFactory : this.nodeFactories.values()) {
 			if (connectionFactory instanceof DisposableBean) {
-				((DisposableBean) connectionFactory).destroy();
+				try {
+					((DisposableBean) connectionFactory).destroy();
+				}
+				catch (Exception e) {
+					lastException = e;
+				}
 			}
 		}
+		if (lastException != null) {
+			throw RabbitExceptionTranslator.convertRabbitAccessException(lastException);
+		}
 	}
 
 }