Fine-tune HTTP/RMI Invoker exception handling

Issue: SPR-15684

spring-context/src/main/java/org/springframework/remoting/rmi/JndiRmiClientInterceptor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2017 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.
@@ -35,6 +35,7 @@ import org.springframework.remoting.RemoteLookupFailureException;
 import org.springframework.remoting.support.DefaultRemoteInvocationFactory;
 import org.springframework.remoting.support.RemoteInvocation;
 import org.springframework.remoting.support.RemoteInvocationFactory;
+import org.springframework.util.Assert;
 
 /**
  * {@link org.aopalliance.intercept.MethodInterceptor} for accessing RMI services
@@ -97,9 +98,8 @@ public class JndiRmiClientInterceptor extends JndiObjectLocator implements Metho
 	 * but can also be optional if the lookup returns a typed stub.
 	 */
 	public void setServiceInterface(Class<?> serviceInterface) {
-		if (serviceInterface != null && !serviceInterface.isInterface()) {
-			throw new IllegalArgumentException("'serviceInterface' must be an interface");
-		}
+		Assert.notNull(serviceInterface, "'serviceInterface' must not be null");
+		Assert.isTrue(serviceInterface.isInterface(), "'serviceInterface' must be an interface");
 		this.serviceInterface = serviceInterface;
 	}
 

spring-context/src/main/java/org/springframework/remoting/rmi/JndiRmiProxyFactoryBean.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2017 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.
@@ -21,6 +21,7 @@ import javax.naming.NamingException;
 import org.springframework.aop.framework.ProxyFactory;
 import org.springframework.beans.factory.BeanClassLoaderAware;
 import org.springframework.beans.factory.FactoryBean;
+import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
 
 /**
@@ -77,10 +78,9 @@ public class JndiRmiProxyFactoryBean extends JndiRmiClientInterceptor
 	@Override
 	public void afterPropertiesSet() throws NamingException {
 		super.afterPropertiesSet();
-		if (getServiceInterface() == null) {
-			throw new IllegalArgumentException("Property 'serviceInterface' is required");
-		}
-		this.serviceProxy = new ProxyFactory(getServiceInterface(), this).getProxy(this.beanClassLoader);
+		Class<?> ifc = getServiceInterface();
+		Assert.notNull(ifc, "Property 'serviceInterface' is required");
+		this.serviceProxy = new ProxyFactory(ifc, this).getProxy(this.beanClassLoader);
 	}
 
 

spring-context/src/main/java/org/springframework/remoting/rmi/RemoteInvocationSerializingExporter.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2017 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.
@@ -107,7 +107,9 @@ public abstract class RemoteInvocationSerializingExporter extends RemoteInvocati
 	}
 
 	protected final Object getProxy() {
-		Assert.notNull(this.proxy, ClassUtils.getShortName(getClass()) + " has not been initialized");
+		if (this.proxy == null) {
+			throw new IllegalStateException(ClassUtils.getShortName(getClass()) + " has not been initialized");
+		}
 		return this.proxy;
 	}
 
@@ -142,7 +144,7 @@ public abstract class RemoteInvocationSerializingExporter extends RemoteInvocati
 		Object obj = ois.readObject();
 		if (!(obj instanceof RemoteInvocation)) {
 			throw new RemoteException("Deserialized object needs to be assignable to type [" +
-					RemoteInvocation.class.getName() + "]: " + obj);
+					RemoteInvocation.class.getName() + "]: " + ClassUtils.getDescriptiveType(obj));
 		}
 		return (RemoteInvocation) obj;
 	}

spring-context/src/main/java/org/springframework/remoting/rmi/RmiProxyFactoryBean.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2017 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.
@@ -19,6 +19,7 @@ package org.springframework.remoting.rmi;
 import org.springframework.aop.framework.ProxyFactory;
 import org.springframework.beans.factory.BeanClassLoaderAware;
 import org.springframework.beans.factory.FactoryBean;
+import org.springframework.util.Assert;
 
 /**
  * {@link FactoryBean} for RMI proxies, supporting both conventional RMI services
@@ -65,10 +66,9 @@ public class RmiProxyFactoryBean extends RmiClientInterceptor implements Factory
 	@Override
 	public void afterPropertiesSet() {
 		super.afterPropertiesSet();
-		if (getServiceInterface() == null) {
-			throw new IllegalArgumentException("Property 'serviceInterface' is required");
-		}
-		this.serviceProxy = new ProxyFactory(getServiceInterface(), this).getProxy(getBeanClassLoader());
+		Class<?> ifc = getServiceInterface();
+		Assert.notNull(ifc, "Property 'serviceInterface' is required");
+		this.serviceProxy = new ProxyFactory(ifc, this).getProxy(getBeanClassLoader());
 	}
 
 

spring-context/src/main/java/org/springframework/remoting/support/RemoteAccessor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2017 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.
@@ -16,6 +16,8 @@
 
 package org.springframework.remoting.support;
 
+import org.springframework.util.Assert;
+
 /**
  * Abstract base class for classes that access a remote service.
  * Provides a "serviceInterface" bean property.
@@ -46,9 +48,8 @@ public abstract class RemoteAccessor extends RemotingSupport {
 	 * but can also be optional if the lookup returns a typed proxy.
 	 */
 	public void setServiceInterface(Class<?> serviceInterface) {
-		if (serviceInterface != null && !serviceInterface.isInterface()) {
-			throw new IllegalArgumentException("'serviceInterface' must be an interface");
-		}
+		Assert.notNull(serviceInterface, "'serviceInterface' must not be null");
+		Assert.isTrue(serviceInterface.isInterface(), "'serviceInterface' must be an interface");
 		this.serviceInterface = serviceInterface;
 	}
 

spring-context/src/main/java/org/springframework/remoting/support/RemoteExporter.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2017 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.
@@ -19,6 +19,7 @@ package org.springframework.remoting.support;
 import org.springframework.aop.framework.ProxyFactory;
 import org.springframework.aop.framework.adapter.AdvisorAdapterRegistry;
 import org.springframework.aop.framework.adapter.GlobalAdvisorAdapterRegistry;
+import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
 
 /**
@@ -63,9 +64,8 @@ public abstract class RemoteExporter extends RemotingSupport {
 	 * The interface must be suitable for the particular service and remoting strategy.
 	 */
 	public void setServiceInterface(Class<?> serviceInterface) {
-		if (serviceInterface != null && !serviceInterface.isInterface()) {
-			throw new IllegalArgumentException("'serviceInterface' must be an interface");
-		}
+		Assert.notNull(serviceInterface, "'serviceInterface' must not be null");
+		Assert.isTrue(serviceInterface.isInterface(), "'serviceInterface' must be an interface");
 		this.serviceInterface = serviceInterface;
 	}
 
@@ -89,7 +89,7 @@ public abstract class RemoteExporter extends RemotingSupport {
 	 * @see RemoteInvocationTraceInterceptor
 	 */
 	public void setRegisterTraceInterceptor(boolean registerTraceInterceptor) {
-		this.registerTraceInterceptor = Boolean.valueOf(registerTraceInterceptor);
+		this.registerTraceInterceptor = registerTraceInterceptor;
 	}
 
 	/**
@@ -110,9 +110,7 @@ public abstract class RemoteExporter extends RemotingSupport {
 	 * @see #setService
 	 */
 	protected void checkService() throws IllegalArgumentException {
-		if (getService() == null) {
-			throw new IllegalArgumentException("Property 'service' is required");
-		}
+		Assert.notNull(getService(), "Property 'service' is required");
 	}
 
 	/**
@@ -123,10 +121,9 @@ public abstract class RemoteExporter extends RemotingSupport {
 	 */
 	protected void checkServiceInterface() throws IllegalArgumentException {
 		Class<?> serviceInterface = getServiceInterface();
+		Assert.notNull(serviceInterface, "Property 'serviceInterface' is required");
+
 		Object service = getService();
-		if (serviceInterface == null) {
-			throw new IllegalArgumentException("Property 'serviceInterface' is required");
-		}
 		if (service instanceof String) {
 			throw new IllegalArgumentException("Service [" + service + "] is a String " +
 					"rather than an actual service reference: Have you accidentally specified " +
@@ -153,20 +150,23 @@ public abstract class RemoteExporter extends RemotingSupport {
 	protected Object getProxyForService() {
 		checkService();
 		checkServiceInterface();
+
 		ProxyFactory proxyFactory = new ProxyFactory();
 		proxyFactory.addInterface(getServiceInterface());
-		if (this.registerTraceInterceptor != null ?
-				this.registerTraceInterceptor.booleanValue() : this.interceptors == null) {
+
+		if (this.registerTraceInterceptor != null ? this.registerTraceInterceptor : this.interceptors == null) {
 			proxyFactory.addAdvice(new RemoteInvocationTraceInterceptor(getExporterName()));
 		}
 		if (this.interceptors != null) {
 			AdvisorAdapterRegistry adapterRegistry = GlobalAdvisorAdapterRegistry.getInstance();
-			for (int i = 0; i < this.interceptors.length; i++) {
-				proxyFactory.addAdvisor(adapterRegistry.wrap(this.interceptors[i]));
+			for (Object interceptor : this.interceptors) {
+				proxyFactory.addAdvisor(adapterRegistry.wrap(interceptor));
 			}
 		}
+
 		proxyFactory.setTarget(getService());
 		proxyFactory.setOpaque(true);
+
 		return proxyFactory.getProxy(getBeanClassLoader());
 	}
 

spring-jms/src/main/java/org/springframework/jms/remoting/JmsInvokerProxyFactoryBean.java

@@ -59,9 +59,7 @@ public class JmsInvokerProxyFactoryBean extends JmsInvokerClientInterceptor
 	 */
 	public void setServiceInterface(Class<?> serviceInterface) {
 		Assert.notNull(serviceInterface, "'serviceInterface' must not be null");
-		if (!serviceInterface.isInterface()) {
-			throw new IllegalArgumentException("'serviceInterface' must be an interface");
-		}
+		Assert.isTrue(serviceInterface.isInterface(), "'serviceInterface' must be an interface");
 		this.serviceInterface = serviceInterface;
 	}
 
@@ -73,9 +71,7 @@ public class JmsInvokerProxyFactoryBean extends JmsInvokerClientInterceptor
 	@Override
 	public void afterPropertiesSet() {
 		super.afterPropertiesSet();
-		if (this.serviceInterface == null) {
-			throw new IllegalArgumentException("Property 'serviceInterface' is required");
-		}
+		Assert.notNull(this.serviceInterface, "Property 'serviceInterface' is required");
 		this.serviceProxy = new ProxyFactory(this.serviceInterface, this).getProxy(this.beanClassLoader);
 	}
 

spring-web/src/main/java/org/springframework/remoting/httpinvoker/AbstractHttpInvokerRequestExecutor.java

@@ -33,6 +33,7 @@ import org.springframework.remoting.rmi.CodebaseAwareObjectInputStream;
 import org.springframework.remoting.support.RemoteInvocation;
 import org.springframework.remoting.support.RemoteInvocationResult;
 import org.springframework.util.Assert;
+import org.springframework.util.ClassUtils;
 
 /**
  * Abstract base implementation of the HttpInvokerRequestExecutor interface.
@@ -291,7 +292,7 @@ public abstract class AbstractHttpInvokerRequestExecutor implements HttpInvokerR
 		Object obj = ois.readObject();
 		if (!(obj instanceof RemoteInvocationResult)) {
 			throw new RemoteException("Deserialized object needs to be assignable to type [" +
-					RemoteInvocationResult.class.getName() + "]: " + obj);
+					RemoteInvocationResult.class.getName() + "]: " + ClassUtils.getDescriptiveType(obj));
 		}
 		return (RemoteInvocationResult) obj;
 	}

spring-web/src/main/java/org/springframework/remoting/httpinvoker/HttpComponentsHttpInvokerRequestExecutor.java

@@ -212,10 +212,12 @@ public class HttpComponentsHttpInvokerRequestExecutor extends AbstractHttpInvoke
 	 */
 	protected HttpPost createHttpPost(HttpInvokerClientConfiguration config) throws IOException {
 		HttpPost httpPost = new HttpPost(config.getServiceUrl());
+
 		RequestConfig requestConfig = createRequestConfig(config);
 		if (requestConfig != null) {
 			httpPost.setConfig(requestConfig);
 		}
+
 		LocaleContext localeContext = LocaleContextHolder.getLocaleContext();
 		if (localeContext != null) {
 			Locale locale = localeContext.getLocale();
@@ -223,9 +225,11 @@ public class HttpComponentsHttpInvokerRequestExecutor extends AbstractHttpInvoke
 				httpPost.addHeader(HTTP_HEADER_ACCEPT_LANGUAGE, StringUtils.toLanguageTag(locale));
 			}
 		}
+
 		if (isAcceptGzipEncoding()) {
 			httpPost.addHeader(HTTP_HEADER_ACCEPT_ENCODING, ENCODING_GZIP);
 		}
+
 		return httpPost;
 	}
 
@@ -250,9 +254,10 @@ public class HttpComponentsHttpInvokerRequestExecutor extends AbstractHttpInvoke
 	}
 
 	private RequestConfig mergeRequestConfig(RequestConfig defaultRequestConfig) {
-		if (this.requestConfig == null) { // nothing to merge
+		if (this.requestConfig == null) {  // nothing to merge
 			return defaultRequestConfig;
 		}
+
 		RequestConfig.Builder builder = RequestConfig.copy(defaultRequestConfig);
 		int connectTimeout = this.requestConfig.getConnectTimeout();
 		if (connectTimeout >= 0) {

spring-web/src/main/java/org/springframework/remoting/httpinvoker/HttpInvokerProxyFactoryBean.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2017 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,7 @@ package org.springframework.remoting.httpinvoker;
 
 import org.springframework.aop.framework.ProxyFactory;
 import org.springframework.beans.factory.FactoryBean;
+import org.springframework.util.Assert;
 
 /**
  * {@link FactoryBean} for HTTP invoker proxies. Exposes the proxied service
@@ -60,10 +61,9 @@ public class HttpInvokerProxyFactoryBean extends HttpInvokerClientInterceptor im
 	@Override
 	public void afterPropertiesSet() {
 		super.afterPropertiesSet();
-		if (getServiceInterface() == null) {
-			throw new IllegalArgumentException("Property 'serviceInterface' is required");
-		}
-		this.serviceProxy = new ProxyFactory(getServiceInterface(), this).getProxy(getBeanClassLoader());
+		Class<?> ifc = getServiceInterface();
+		Assert.notNull(ifc, "Property 'serviceInterface' is required");
+		this.serviceProxy = new ProxyFactory(ifc, this).getProxy(getBeanClassLoader());
 	}
 
 

spring-web/src/main/java/org/springframework/remoting/httpinvoker/SimpleHttpInvokerRequestExecutor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2017 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.
@@ -107,7 +107,8 @@ public class SimpleHttpInvokerRequestExecutor extends AbstractHttpInvokerRequest
 	protected HttpURLConnection openConnection(HttpInvokerClientConfiguration config) throws IOException {
 		URLConnection con = new URL(config.getServiceUrl()).openConnection();
 		if (!(con instanceof HttpURLConnection)) {
-			throw new IOException("Service URL [" + config.getServiceUrl() + "] is not an HTTP URL");
+			throw new IOException(
+					"Service URL [" + config.getServiceUrl() + "] does not resolve to an HTTP connection");
 		}
 		return (HttpURLConnection) con;
 	}
@@ -130,6 +131,7 @@ public class SimpleHttpInvokerRequestExecutor extends AbstractHttpInvokerRequest
 		if (this.readTimeout >= 0) {
 			connection.setReadTimeout(this.readTimeout);
 		}
+
 		connection.setDoOutput(true);
 		connection.setRequestMethod(HTTP_METHOD_POST);
 		connection.setRequestProperty(HTTP_HEADER_CONTENT_TYPE, getContentType());
@@ -142,6 +144,7 @@ public class SimpleHttpInvokerRequestExecutor extends AbstractHttpInvokerRequest
 				connection.setRequestProperty(HTTP_HEADER_ACCEPT_LANGUAGE, StringUtils.toLanguageTag(locale));
 			}
 		}
+
 		if (isAcceptGzipEncoding()) {
 			connection.setRequestProperty(HTTP_HEADER_ACCEPT_ENCODING, ENCODING_GZIP);
 		}

spring-web/src/main/java/org/springframework/remoting/jaxws/JaxWsPortClientInterceptor.java

@@ -278,16 +278,14 @@ public class JaxWsPortClientInterceptor extends LocalJaxWsServiceFactory
 	 * Set the interface of the service that this factory should create a proxy for.
 	 */
 	public void setServiceInterface(Class<?> serviceInterface) {
-		if (!serviceInterface.isInterface()) {
-			throw new IllegalArgumentException("'serviceInterface' must be an interface");
-		}
+		Assert.notNull(serviceInterface, "'serviceInterface' must not be null");
+		Assert.isTrue(serviceInterface.isInterface(), "'serviceInterface' must be an interface");
 		this.serviceInterface = serviceInterface;
 	}
 
 	/**
 	 * Return the interface of the service that this factory should create a proxy for.
 	 */
-	@Nullable
 	public Class<?> getServiceInterface() {
 		return this.serviceInterface;
 	}