Polishing

spring-core/src/main/java/org/springframework/util/concurrent/ListenableFuture.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.
@@ -22,6 +22,7 @@ import java.util.concurrent.Future;
  * Extend {@link Future} with the capability to accept completion callbacks.
  * If the future has completed when the callback is added, the callback is
  * triggered immediately.
+ *
  * <p>Inspired by {@code com.google.common.util.concurrent.ListenableFuture}.
  *
  * @author Arjen Poutsma

spring-core/src/test/java/org/springframework/util/concurrent/SettableListenableFutureTests.java

@@ -32,7 +32,6 @@ import static org.mockito.Mockito.*;
  * @author Mattias Severson
  * @author Juergen Hoeller
  */
-@SuppressWarnings({ "rawtypes", "unchecked" })
 public class SettableListenableFutureTests {
 
 	private final SettableListenableFuture<String> settableListenableFuture = new SettableListenableFuture<>();
@@ -266,20 +265,20 @@ public class SettableListenableFutureTests {
 
 	@Test
 	public void cancelWithMayInterruptIfRunningTrueCallsOverriddenMethod() {
-		InterruptableSettableListenableFuture tested = new InterruptableSettableListenableFuture();
+		InterruptibleSettableListenableFuture interruptibleFuture = new InterruptibleSettableListenableFuture();
-		assertTrue(tested.cancel(true));
+		assertTrue(interruptibleFuture.cancel(true));
-		assertTrue(tested.calledInterruptTask());
+		assertTrue(interruptibleFuture.calledInterruptTask());
-		assertTrue(tested.isCancelled());
+		assertTrue(interruptibleFuture.isCancelled());
-		assertTrue(tested.isDone());
+		assertTrue(interruptibleFuture.isDone());
 	}
 
 	@Test
 	public void cancelWithMayInterruptIfRunningFalseDoesNotCallOverriddenMethod() {
-		InterruptableSettableListenableFuture tested = new InterruptableSettableListenableFuture();
+		InterruptibleSettableListenableFuture interruptibleFuture = new InterruptibleSettableListenableFuture();
-		assertTrue(tested.cancel(false));
+		assertTrue(interruptibleFuture.cancel(false));
-		assertFalse(tested.calledInterruptTask());
+		assertFalse(interruptibleFuture.calledInterruptTask());
-		assertTrue(tested.isCancelled());
+		assertTrue(interruptibleFuture.isCancelled());
-		assertTrue(tested.isDone());
+		assertTrue(interruptibleFuture.isDone());
 	}
 
 	@Test
@@ -350,6 +349,7 @@ public class SettableListenableFutureTests {
 	}
 
 	@Test
+	@SuppressWarnings({"rawtypes", "unchecked"})
 	public void cancelDoesNotNotifyCallbacksOnSet() {
 		ListenableFutureCallback callback = mock(ListenableFutureCallback.class);
 		settableListenableFuture.addCallback(callback);
@@ -366,6 +366,7 @@ public class SettableListenableFutureTests {
 	}
 
 	@Test
+	@SuppressWarnings({"rawtypes", "unchecked"})
 	public void cancelDoesNotNotifyCallbacksOnSetException() {
 		ListenableFutureCallback callback = mock(ListenableFutureCallback.class);
 		settableListenableFuture.addCallback(callback);
@@ -382,7 +383,7 @@ public class SettableListenableFutureTests {
 	}
 
 
-	private static class InterruptableSettableListenableFuture extends SettableListenableFuture<String> {
+	private static class InterruptibleSettableListenableFuture extends SettableListenableFuture<String> {
 
 		private boolean interrupted = false;
 

spring-jdbc/src/main/java/org/springframework/jdbc/datasource/DataSourceUtils.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.
@@ -154,7 +154,7 @@ public abstract class DataSourceUtils {
 				}
 				con.setReadOnly(true);
 			}
-			catch (SQLException ex) {
+			catch (SQLException | RuntimeException ex) {
 				Throwable exToCheck = ex;
 				while (exToCheck != null) {
 					if (exToCheck.getClass().getSimpleName().contains("Timeout")) {
@@ -166,18 +166,6 @@ public abstract class DataSourceUtils {
 				// "read-only not supported" SQLException -> ignore, it's just a hint anyway
 				logger.debug("Could not set JDBC Connection read-only", ex);
 			}
-			catch (RuntimeException ex) {
-				Throwable exToCheck = ex;
-				while (exToCheck != null) {
-					if (exToCheck.getClass().getSimpleName().contains("Timeout")) {
-						// Assume it's a connection timeout that would otherwise get lost: e.g. from Hibernate
-						throw ex;
-					}
-					exToCheck = exToCheck.getCause();
-				}
-				// "read-only not supported" UnsupportedOperationException -> ignore, it's just a hint anyway
-				logger.debug("Could not set JDBC Connection read-only", ex);
-			}
 		}
 
 		// Apply specific isolation level, if any.

spring-jms/src/main/java/org/springframework/jms/connection/JmsTransactionManager.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.
@@ -226,8 +226,7 @@ public class JmsTransactionManager extends AbstractPlatformTransactionManager
 
 	@Override
 	protected void doResume(Object transaction, Object suspendedResources) {
-		JmsResourceHolder conHolder = (JmsResourceHolder) suspendedResources;
+		TransactionSynchronizationManager.bindResource(getConnectionFactory(), suspendedResources);
-		TransactionSynchronizationManager.bindResource(getConnectionFactory(), conHolder);
 	}
 
 	@Override

spring-orm/src/main/java/org/springframework/orm/hibernate5/support/OpenSessionInViewInterceptor.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.
@@ -144,7 +144,6 @@ public class OpenSessionInViewInterceptor implements AsyncWebRequestInterceptor
 					(SessionHolder) TransactionSynchronizationManager.unbindResource(getSessionFactory());
 			logger.debug("Closing Hibernate Session in OpenSessionInViewInterceptor");
 			SessionFactoryUtils.closeSession(sessionHolder.getSession());
-
 		}
 	}
 

spring-tx/src/main/java/org/springframework/transaction/support/AbstractPlatformTransactionManager.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 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.
@@ -374,14 +374,10 @@ public abstract class AbstractPlatformTransactionManager implements PlatformTran
 				prepareSynchronization(status, definition);
 				return status;
 			}
-			catch (RuntimeException ex) {
+			catch (RuntimeException | Error ex) {
 				resume(null, suspendedResources);
 				throw ex;
 			}
-			catch (Error err) {
-				resume(null, suspendedResources);
-				throw err;
-			}
 		}
 		else {
 			// Create "empty" transaction: no actual transaction, but potentially synchronization.
@@ -430,14 +426,10 @@ public abstract class AbstractPlatformTransactionManager implements PlatformTran
 				prepareSynchronization(status, definition);
 				return status;
 			}
-			catch (RuntimeException beginEx) {
+			catch (RuntimeException | Error beginEx) {
 				resumeAfterBeginException(transaction, suspendedResources, beginEx);
 				throw beginEx;
 			}
-			catch (Error beginErr) {
-				resumeAfterBeginException(transaction, suspendedResources, beginErr);
-				throw beginErr;
-			}
 		}
 
 		if (definition.getPropagationBehavior() == TransactionDefinition.PROPAGATION_NESTED) {
@@ -589,16 +581,11 @@ public abstract class AbstractPlatformTransactionManager implements PlatformTran
 				return new SuspendedResourcesHolder(
 						suspendedResources, suspendedSynchronizations, name, readOnly, isolationLevel, wasActive);
 			}
-			catch (RuntimeException ex) {
+			catch (RuntimeException | Error ex) {
 				// doSuspend failed - original transaction is still active...
 				doResumeSynchronization(suspendedSynchronizations);
 				throw ex;
 			}
-			catch (Error err) {
-				// doSuspend failed - original transaction is still active...
-				doResumeSynchronization(suspendedSynchronizations);
-				throw err;
-			}
 		}
 		else if (transaction != null) {
 			// Transaction active but no synchronization active.
@@ -650,14 +637,10 @@ public abstract class AbstractPlatformTransactionManager implements PlatformTran
 		try {
 			resume(transaction, suspendedResources);
 		}
-		catch (RuntimeException resumeEx) {
+		catch (RuntimeException | Error resumeEx) {
 			logger.error(exMessage, beginEx);
 			throw resumeEx;
 		}
-		catch (Error resumeErr) {
-			logger.error(exMessage, beginEx);
-			throw resumeErr;
-		}
 	}
 
 	/**
@@ -782,20 +765,13 @@ public abstract class AbstractPlatformTransactionManager implements PlatformTran
 				}
 				throw ex;
 			}
-			catch (RuntimeException ex) {
+			catch (RuntimeException | Error ex) {
 				if (!beforeCompletionInvoked) {
 					triggerBeforeCompletion(status);
 				}
 				doRollbackOnCommitException(status, ex);
 				throw ex;
 			}
-			catch (Error err) {
-				if (!beforeCompletionInvoked) {
-					triggerBeforeCompletion(status);
-				}
-				doRollbackOnCommitException(status, err);
-				throw err;
-			}
 
 			// Trigger afterCommit callbacks, with an exception thrown there
 			// propagated to callers but the transaction still considered as committed.
@@ -869,14 +845,10 @@ public abstract class AbstractPlatformTransactionManager implements PlatformTran
 					logger.debug("Should roll back transaction but cannot - no transaction available");
 				}
 			}
-			catch (RuntimeException ex) {
+			catch (RuntimeException | Error ex) {
 				triggerAfterCompletion(status, TransactionSynchronization.STATUS_UNKNOWN);
 				throw ex;
 			}
-			catch (Error err) {
-				triggerAfterCompletion(status, TransactionSynchronization.STATUS_UNKNOWN);
-				throw err;
-			}
 			triggerAfterCompletion(status, TransactionSynchronization.STATUS_ROLLED_BACK);
 		}
 		finally {
@@ -906,16 +878,11 @@ public abstract class AbstractPlatformTransactionManager implements PlatformTran
 				doSetRollbackOnly(status);
 			}
 		}
-		catch (RuntimeException rbex) {
+		catch (RuntimeException | Error rbex) {
 			logger.error("Commit exception overridden by rollback exception", ex);
 			triggerAfterCompletion(status, TransactionSynchronization.STATUS_UNKNOWN);
 			throw rbex;
 		}
-		catch (Error rberr) {
-			logger.error("Commit exception overridden by rollback exception", ex);
-			triggerAfterCompletion(status, TransactionSynchronization.STATUS_UNKNOWN);
-			throw rberr;
-		}
 		triggerAfterCompletion(status, TransactionSynchronization.STATUS_ROLLED_BACK);
 	}
 

spring-web/src/main/java/org/springframework/web/accept/ContentNegotiationManager.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.
@@ -118,9 +118,7 @@ public class ContentNegotiationManager implements ContentNegotiationStrategy, Me
 	}
 
 	@Override
-	public List<MediaType> resolveMediaTypes(NativeWebRequest request)
+	public List<MediaType> resolveMediaTypes(NativeWebRequest request) throws HttpMediaTypeNotAcceptableException {
-			throws HttpMediaTypeNotAcceptableException {
-
 		for (ContentNegotiationStrategy strategy : this.strategies) {
 			List<MediaType> mediaTypes = strategy.resolveMediaTypes(request);
 			if (mediaTypes.isEmpty() || mediaTypes.equals(MEDIA_TYPE_ALL)) {

spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseEntityExceptionHandler.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.
@@ -454,10 +454,13 @@ public abstract class ResponseEntityExceptionHandler {
 			HttpServletRequest request = servletRequest.getNativeRequest(HttpServletRequest.class);
 			HttpServletResponse response = servletRequest.getNativeResponse(HttpServletResponse.class);
 			if (response.isCommitted()) {
-				logger.error("Async timeout for " + request.getMethod() + " [" +  request.getRequestURI() + "]");
+				if (logger.isErrorEnabled()) {
+					logger.error("Async timeout for " + request.getMethod() + " [" + request.getRequestURI() + "]");
+				}
 				return null;
 			}
 		}
+
 		return handleExceptionInternal(ex, null, headers, status, webRequest);
 	}
 

spring-webmvc/src/test/java/org/springframework/web/servlet/ComplexWebApplicationContext.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.
@@ -87,7 +87,7 @@ public class ComplexWebApplicationContext extends StaticWebApplicationContext {
 		ThemeChangeInterceptor interceptor4 = new ThemeChangeInterceptor();
 		interceptor4.setParamName("theme2");
 		UserRoleAuthorizationInterceptor interceptor5 = new UserRoleAuthorizationInterceptor();
-		interceptor5.setAuthorizedRoles(new String[] {"role1", "role2"});
+		interceptor5.setAuthorizedRoles("role1", "role2");
 
 		List<Object> interceptors = new ArrayList<>();
 		interceptors.add(interceptor5);
@@ -100,8 +100,7 @@ public class ComplexWebApplicationContext extends StaticWebApplicationContext {
 		interceptors.add(new MyWebRequestInterceptor());
 
 		MutablePropertyValues pvs = new MutablePropertyValues();
-		pvs.add(
+		pvs.add("mappings", "/view.do=viewHandler\n/locale.do=localeHandler\nloc.do=anotherLocaleHandler");
-				"mappings", "/view.do=viewHandler\n/locale.do=localeHandler\nloc.do=anotherLocaleHandler");
 		pvs.add("interceptors", interceptors);
 		registerSingleton("myUrlMapping1", SimpleUrlHandlerMapping.class, pvs);
 
@@ -124,7 +123,7 @@ public class ComplexWebApplicationContext extends StaticWebApplicationContext {
 		registerSingleton("noviewController", NoViewController.class);
 
 		pvs = new MutablePropertyValues();
-		pvs.add("order", new Integer(0));
+		pvs.add("order", 0);
 		pvs.add("basename", "org.springframework.web.servlet.complexviews");
 		registerSingleton("viewResolver", ResourceBundleViewResolver.class, pvs);
 
@@ -150,8 +149,8 @@ public class ComplexWebApplicationContext extends StaticWebApplicationContext {
 		pvs = new MutablePropertyValues();
 		pvs.add("order", "1");
 		pvs.add("exceptionMappings",
-			"java.lang.IllegalAccessException=failed2\n" +
+				"java.lang.IllegalAccessException=failed2\n" +
-			"ServletRequestBindingException=failed3");
+				"ServletRequestBindingException=failed3");
 		pvs.add("defaultErrorView", "failed0");
 		registerSingleton("exceptionResolver1", SimpleMappingExceptionResolver.class, pvs);
 
@@ -237,11 +236,11 @@ public class ComplexWebApplicationContext extends StaticWebApplicationContext {
 	}
 
 
-	public static interface MyHandler {
+	public interface MyHandler {
 
-		public void doSomething(HttpServletRequest request) throws ServletException, IllegalAccessException;
+		void doSomething(HttpServletRequest request) throws ServletException, IllegalAccessException;
 
-		public long lastModified();
+		long lastModified();
 	}
 
 
@@ -259,7 +258,8 @@ public class ComplexWebApplicationContext extends StaticWebApplicationContext {
 
 		@Override
 		public ModelAndView handle(HttpServletRequest request, HttpServletResponse response, Object delegate)
-			throws ServletException, IllegalAccessException {
+				throws ServletException, IllegalAccessException {
+
 			((MyHandler) delegate).doSomething(request);
 			return null;
 		}
@@ -295,7 +295,8 @@ public class ComplexWebApplicationContext extends StaticWebApplicationContext {
 
 		@Override
 		public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler)
-			throws ServletException {
+				throws ServletException {
+
 			if (request.getAttribute("test2") != null) {
 				throw new ServletException("Wrong interceptor order");
 			}
@@ -309,6 +310,7 @@ public class ComplexWebApplicationContext extends StaticWebApplicationContext {
 		public void postHandle(
 				HttpServletRequest request, HttpServletResponse response, Object handler, ModelAndView modelAndView)
 				throws ServletException {
+
 			if (request.getAttribute("test2x") != null) {
 				throw new ServletException("Wrong interceptor order");
 			}
@@ -322,9 +324,13 @@ public class ComplexWebApplicationContext extends StaticWebApplicationContext {
 		public void afterCompletion(
 				HttpServletRequest request, HttpServletResponse response, Object handler, Exception ex)
 				throws ServletException {
+
 			if (request.getAttribute("test2y") != null) {
 				throw new ServletException("Wrong interceptor order");
 			}
+			if (request.getAttribute("test1y") == null) {
+				throw new ServletException("afterCompletion invoked twice");
+			}
 			request.removeAttribute("test1y");
 		}
 	}
@@ -334,7 +340,8 @@ public class ComplexWebApplicationContext extends StaticWebApplicationContext {
 
 		@Override
 		public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler)
-			throws ServletException {
+				throws ServletException {
+
 			if (request.getAttribute("test1x") == null) {
 				throw new ServletException("Wrong interceptor order");
 			}
@@ -351,6 +358,7 @@ public class ComplexWebApplicationContext extends StaticWebApplicationContext {
 		public void postHandle(
 				HttpServletRequest request, HttpServletResponse response, Object handler, ModelAndView modelAndView)
 				throws ServletException {
+
 			if (request.getParameter("noView") != null) {
 				modelAndView.clear();
 			}
@@ -367,9 +375,13 @@ public class ComplexWebApplicationContext extends StaticWebApplicationContext {
 		public void afterCompletion(
 				HttpServletRequest request, HttpServletResponse response, Object handler, Exception ex)
 				throws Exception {
+
 			if (request.getAttribute("test1y") == null) {
 				throw new ServletException("Wrong interceptor order");
 			}
+			if (request.getAttribute("test2y") == null) {
+				throw new ServletException("afterCompletion invoked twice");
+			}
 			request.removeAttribute("test2y");
 		}
 	}

spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletRequestMethodArgumentResolverTests.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.
@@ -60,6 +60,7 @@ public class ServletRequestMethodArgumentResolverTests {
 
 	private MockHttpServletRequest servletRequest;
 
+
 	@Before
 	public void setUp() throws Exception {
 		method = getClass().getMethod("supportedParams", ServletRequest.class, MultipartRequest.class,
@@ -70,6 +71,7 @@ public class ServletRequestMethodArgumentResolverTests {
 		webRequest = new ServletWebRequest(servletRequest, new MockHttpServletResponse());
 	}
 
+
 	@Test
 	public void servletRequest() throws Exception {
 		MethodParameter servletRequestParameter = new MethodParameter(method, 0);