From 68b3bc03947fc103de438d4f2482373ae2f986a8 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Tue, 12 Jul 2016 16:40:48 +0200 Subject: [PATCH] Use Supplier support in Assert in spring-test This commit makes use of the new Supplier variants of utility methods in org.springframework.util.Assert within the spring-test module. Issue: SPR-14450 --- .../mock/jndi/SimpleNamingContextBuilder.java | 5 +-- .../mock/web/MockExpressionEvaluator.java | 11 ++---- .../mock/web/MockFilterChain.java | 7 +--- .../mock/web/MockHttpServletRequest.java | 8 +--- .../mock/web/MockHttpServletResponse.java | 30 ++++----------- .../mock/web/MockHttpSession.java | 4 +- .../mock/web/MockRequestDispatcher.java | 4 +- .../mock/web/MockServletContext.java | 5 +-- .../test/annotation/ProfileValueUtils.java | 4 +- .../test/context/BootstrapUtils.java | 5 +-- .../jdbc/SqlScriptsTestExecutionListener.java | 21 ++++------ .../junit4/SpringJUnit4ClassRunner.java | 24 +++++------- .../context/junit4/rules/SpringClassRule.java | 33 ++++++++-------- .../junit4/rules/SpringMethodRule.java | 38 +++++++++---------- .../AbstractDelegatingSmartContextLoader.java | 12 ++---- .../AbstractTestContextBootstrapper.java | 16 +++----- .../context/support/ContextLoaderUtils.java | 8 +--- .../TestContextTransactionUtils.java | 4 +- .../context/transaction/TestTransaction.java | 7 ++-- .../transaction/TransactionContext.java | 15 +++----- .../TransactionalTestExecutionListener.java | 12 ++---- .../web/AbstractGenericWebContextLoader.java | 6 +-- .../web/ServletTestExecutionListener.java | 5 +-- .../web/client/DefaultRequestExpectation.java | 4 +- .../htmlunit/HtmlUnitRequestBuilder.java | 19 ++++------ .../htmlunit/MockMvcWebConnection.java | 8 +--- .../MockHttpServletRequestBuilder.java | 6 +-- 27 files changed, 116 insertions(+), 205 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/mock/jndi/SimpleNamingContextBuilder.java b/spring-test/src/main/java/org/springframework/mock/jndi/SimpleNamingContextBuilder.java index caa45a855e..45cd2d82ed 100644 --- a/spring-test/src/main/java/org/springframework/mock/jndi/SimpleNamingContextBuilder.java +++ b/spring-test/src/main/java/org/springframework/mock/jndi/SimpleNamingContextBuilder.java @@ -26,6 +26,7 @@ import javax.naming.spi.NamingManager; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.util.Assert; import org.springframework.util.ClassUtils; /** @@ -137,12 +138,10 @@ public class SimpleNamingContextBuilder implements InitialContextFactoryBuilder logger.info("Activating simple JNDI environment"); synchronized (initializationLock) { if (!initialized) { - if (NamingManager.hasInitialContextFactoryBuilder()) { - throw new IllegalStateException( + Assert.state(!NamingManager.hasInitialContextFactoryBuilder(), "Cannot activate SimpleNamingContextBuilder: there is already a JNDI provider registered. " + "Note that JNDI is a JVM-wide service, shared at the JVM system class loader level, " + "with no reset option. As a consequence, a JNDI provider must only be registered once per JVM."); - } NamingManager.setInitialContextFactoryBuilder(this); initialized = true; } diff --git a/spring-test/src/main/java/org/springframework/mock/web/MockExpressionEvaluator.java b/spring-test/src/main/java/org/springframework/mock/web/MockExpressionEvaluator.java index 3a70b66c1e..3672b52ed9 100644 --- a/spring-test/src/main/java/org/springframework/mock/web/MockExpressionEvaluator.java +++ b/spring-test/src/main/java/org/springframework/mock/web/MockExpressionEvaluator.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. @@ -20,6 +20,7 @@ import javax.servlet.jsp.JspException; import javax.servlet.jsp.PageContext; import org.apache.taglibs.standard.lang.support.ExpressionEvaluatorManager; +import org.springframework.util.Assert; /** * Mock implementation of the JSP 2.0 {@link javax.servlet.jsp.el.ExpressionEvaluator} @@ -68,9 +69,7 @@ public class MockExpressionEvaluator extends javax.servlet.jsp.el.ExpressionEval public Object evaluate(String expression, Class expectedType, javax.servlet.jsp.el.VariableResolver variableResolver, javax.servlet.jsp.el.FunctionMapper functionMapper) throws javax.servlet.jsp.el.ELException { - if (variableResolver != null) { - throw new IllegalArgumentException("Custom VariableResolver not supported"); - } + Assert.isNull(variableResolver, "Custom VariableResolver not supported"); return doEvaluate(expression, expectedType, functionMapper); } @@ -78,9 +77,7 @@ public class MockExpressionEvaluator extends javax.servlet.jsp.el.ExpressionEval protected Object doEvaluate(String expression, Class expectedType, javax.servlet.jsp.el.FunctionMapper functionMapper) throws javax.servlet.jsp.el.ELException { - if (functionMapper != null) { - throw new IllegalArgumentException("Custom FunctionMapper not supported"); - } + Assert.isNull(functionMapper, "Custom FunctionMapper not supported"); try { return ExpressionEvaluatorManager.evaluate("JSP EL expression", expression, expectedType, this.pageContext); } diff --git a/spring-test/src/main/java/org/springframework/mock/web/MockFilterChain.java b/spring-test/src/main/java/org/springframework/mock/web/MockFilterChain.java index efa7198747..05db4a5bdd 100644 --- a/spring-test/src/main/java/org/springframework/mock/web/MockFilterChain.java +++ b/spring-test/src/main/java/org/springframework/mock/web/MockFilterChain.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 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. @@ -120,10 +120,7 @@ public class MockFilterChain implements FilterChain { public void doFilter(ServletRequest request, ServletResponse response) throws IOException, ServletException { Assert.notNull(request, "Request must not be null"); Assert.notNull(response, "Response must not be null"); - - if (this.request != null) { - throw new IllegalStateException("This FilterChain has already been called!"); - } + Assert.state(this.request == null, "This FilterChain has already been called!"); if (this.iterator == null) { this.iterator = this.filters.iterator(); diff --git a/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletRequest.java b/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletRequest.java index 6eeb849298..45897aa08d 100644 --- a/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletRequest.java +++ b/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletRequest.java @@ -328,9 +328,7 @@ public class MockHttpServletRequest implements HttpServletRequest { * throwing an IllegalStateException if not active anymore. */ protected void checkActive() throws IllegalStateException { - if (!this.active) { - throw new IllegalStateException("Request is not active anymore"); - } + Assert.state(this.active, "Request is not active anymore"); } @@ -807,9 +805,7 @@ public class MockHttpServletRequest implements HttpServletRequest { @Override public AsyncContext startAsync(ServletRequest request, ServletResponse response) { - if (!this.asyncSupported) { - throw new IllegalStateException("Async not supported"); - } + Assert.state(this.asyncSupported, "Async not supported"); this.asyncStarted = true; this.asyncContext = new MockAsyncContext(request, response); return this.asyncContext; diff --git a/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletResponse.java b/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletResponse.java index 86c7d6eeec..5d63c2f540 100644 --- a/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletResponse.java +++ b/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletResponse.java @@ -181,17 +181,13 @@ public class MockHttpServletResponse implements HttpServletResponse { @Override public ServletOutputStream getOutputStream() { - if (!this.outputStreamAccessAllowed) { - throw new IllegalStateException("OutputStream access not allowed"); - } + Assert.state(this.outputStreamAccessAllowed, "OutputStream access not allowed"); return this.outputStream; } @Override public PrintWriter getWriter() throws UnsupportedEncodingException { - if (!this.writerAccessAllowed) { - throw new IllegalStateException("Writer access not allowed"); - } + Assert.state(this.writerAccessAllowed, "Writer access not allowed"); if (this.writer == null) { Writer targetWriter = (this.characterEncoding != null ? new OutputStreamWriter(this.content, this.characterEncoding) : new OutputStreamWriter(this.content)); @@ -275,9 +271,7 @@ public class MockHttpServletResponse implements HttpServletResponse { @Override public void resetBuffer() { - if (isCommitted()) { - throw new IllegalStateException("Cannot reset buffer - response is already committed"); - } + Assert.state(!isCommitted(), "Cannot reset buffer - response is already committed"); this.content.reset(); } @@ -456,9 +450,7 @@ public class MockHttpServletResponse implements HttpServletResponse { @Override public void sendError(int status, String errorMessage) throws IOException { - if (isCommitted()) { - throw new IllegalStateException("Cannot set error status - response is already committed"); - } + Assert.state(!isCommitted(), "Cannot set error status - response is already committed"); this.status = status; this.errorMessage = errorMessage; setCommitted(true); @@ -466,18 +458,14 @@ public class MockHttpServletResponse implements HttpServletResponse { @Override public void sendError(int status) throws IOException { - if (isCommitted()) { - throw new IllegalStateException("Cannot set error status - response is already committed"); - } + Assert.state(!isCommitted(), "Cannot set error status - response is already committed"); this.status = status; setCommitted(true); } @Override public void sendRedirect(String url) throws IOException { - if (isCommitted()) { - throw new IllegalStateException("Cannot send redirect - response is already committed"); - } + Assert.state(!isCommitted(), "Cannot send redirect - response is already committed"); Assert.notNull(url, "Redirect URL must not be null"); setHeader(LOCATION_HEADER, url); setStatus(HttpServletResponse.SC_MOVED_TEMPORARILY); @@ -627,10 +615,8 @@ public class MockHttpServletResponse implements HttpServletResponse { public String getIncludedUrl() { int count = this.includedUrls.size(); - if (count > 1) { - throw new IllegalStateException( - "More than 1 URL included - check getIncludedUrls instead: " + this.includedUrls); - } + Assert.state(count <= 1, + () -> "More than 1 URL included - check getIncludedUrls instead: " + this.includedUrls); return (count == 1 ? this.includedUrls.get(0) : null); } diff --git a/spring-test/src/main/java/org/springframework/mock/web/MockHttpSession.java b/spring-test/src/main/java/org/springframework/mock/web/MockHttpSession.java index 4089c0f48d..70d55da985 100644 --- a/spring-test/src/main/java/org/springframework/mock/web/MockHttpSession.java +++ b/spring-test/src/main/java/org/springframework/mock/web/MockHttpSession.java @@ -248,9 +248,7 @@ public class MockHttpSession implements HttpSession { * @throws IllegalStateException if this session has been invalidated */ private void assertIsValid() { - if (isInvalid()) { - throw new IllegalStateException("The session has already been invalidated"); - } + Assert.state(!isInvalid(), "The session has already been invalidated"); } public void setNew(boolean value) { diff --git a/spring-test/src/main/java/org/springframework/mock/web/MockRequestDispatcher.java b/spring-test/src/main/java/org/springframework/mock/web/MockRequestDispatcher.java index e43172c198..39a1e17f50 100644 --- a/spring-test/src/main/java/org/springframework/mock/web/MockRequestDispatcher.java +++ b/spring-test/src/main/java/org/springframework/mock/web/MockRequestDispatcher.java @@ -59,9 +59,7 @@ public class MockRequestDispatcher implements RequestDispatcher { public void forward(ServletRequest request, ServletResponse response) { Assert.notNull(request, "Request must not be null"); Assert.notNull(response, "Response must not be null"); - if (response.isCommitted()) { - throw new IllegalStateException("Cannot perform forward - response is already committed"); - } + Assert.state(!response.isCommitted(), "Cannot perform forward - response is already committed"); getMockHttpServletResponse(response).setForwardedUrl(this.resource); if (logger.isDebugEnabled()) { logger.debug("MockRequestDispatcher: forwarding to [" + this.resource + "]"); diff --git a/spring-test/src/main/java/org/springframework/mock/web/MockServletContext.java b/spring-test/src/main/java/org/springframework/mock/web/MockServletContext.java index df3c669050..6861381fbc 100644 --- a/spring-test/src/main/java/org/springframework/mock/web/MockServletContext.java +++ b/spring-test/src/main/java/org/springframework/mock/web/MockServletContext.java @@ -355,9 +355,8 @@ public class MockServletContext implements ServletContext { @Override public RequestDispatcher getRequestDispatcher(String path) { - if (!path.startsWith("/")) { - throw new IllegalArgumentException("RequestDispatcher path at ServletContext level must start with '/'"); - } + Assert.isTrue(path.startsWith("/"), + () -> "RequestDispatcher path [" + path + "] at ServletContext level must start with '/'"); return new MockRequestDispatcher(path); } diff --git a/spring-test/src/main/java/org/springframework/test/annotation/ProfileValueUtils.java b/spring-test/src/main/java/org/springframework/test/annotation/ProfileValueUtils.java index 17c60cad01..ce30ca480c 100644 --- a/spring-test/src/main/java/org/springframework/test/annotation/ProfileValueUtils.java +++ b/spring-test/src/main/java/org/springframework/test/annotation/ProfileValueUtils.java @@ -184,10 +184,8 @@ public abstract class ProfileValueUtils { String environmentValue = profileValueSource.get(ifProfileValue.name()); String[] annotatedValues = ifProfileValue.values(); if (StringUtils.hasLength(ifProfileValue.value())) { - if (annotatedValues.length > 0) { - throw new IllegalArgumentException("Setting both the 'value' and 'values' attributes " + + Assert.isTrue(annotatedValues.length == 0, () -> "Setting both the 'value' and 'values' attributes " + "of @IfProfileValue is not allowed: choose one or the other."); - } annotatedValues = new String[] { ifProfileValue.value() }; } diff --git a/spring-test/src/main/java/org/springframework/test/context/BootstrapUtils.java b/spring-test/src/main/java/org/springframework/test/context/BootstrapUtils.java index 2235807144..5b81650b7d 100644 --- a/spring-test/src/main/java/org/springframework/test/context/BootstrapUtils.java +++ b/spring-test/src/main/java/org/springframework/test/context/BootstrapUtils.java @@ -25,6 +25,7 @@ import org.apache.commons.logging.LogFactory; import org.springframework.beans.BeanUtils; import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.core.annotation.AnnotationAttributes; +import org.springframework.util.Assert; import org.springframework.util.ClassUtils; /** @@ -151,11 +152,9 @@ abstract class BootstrapUtils { if (annotations.size() < 1) { return null; } - if (annotations.size() > 1) { - throw new IllegalStateException(String.format( + Assert.state(annotations.size() <= 1, () -> String.format( "Configuration error: found multiple declarations of @BootstrapWith for test class [%s]: %s", testClass.getName(), annotations)); - } return annotations.iterator().next().value(); } diff --git a/spring-test/src/main/java/org/springframework/test/context/jdbc/SqlScriptsTestExecutionListener.java b/spring-test/src/main/java/org/springframework/test/context/jdbc/SqlScriptsTestExecutionListener.java index f2400357f4..59391ad623 100644 --- a/spring-test/src/main/java/org/springframework/test/context/jdbc/SqlScriptsTestExecutionListener.java +++ b/spring-test/src/main/java/org/springframework/test/context/jdbc/SqlScriptsTestExecutionListener.java @@ -43,6 +43,7 @@ import org.springframework.transaction.TransactionDefinition; import org.springframework.transaction.interceptor.DefaultTransactionAttribute; import org.springframework.transaction.interceptor.TransactionAttribute; import org.springframework.transaction.support.TransactionTemplate; +import org.springframework.util.Assert; import org.springframework.util.ClassUtils; import org.springframework.util.ObjectUtils; import org.springframework.util.ReflectionUtils; @@ -201,16 +202,12 @@ public class SqlScriptsTestExecutionListener extends AbstractTestExecutionListen final boolean newTxRequired = mergedSqlConfig.getTransactionMode() == TransactionMode.ISOLATED; if (transactionManager == null) { - if (newTxRequired) { - throw new IllegalStateException(String.format("Failed to execute SQL scripts for test context %s: " - + "cannot execute SQL scripts using Transaction Mode " - + "[%s] without a PlatformTransactionManager.", testContext, TransactionMode.ISOLATED)); - } + Assert.state(!newTxRequired, () -> String.format("Failed to execute SQL scripts for test context %s: " + + "cannot execute SQL scripts using Transaction Mode " + + "[%s] without a PlatformTransactionManager.", testContext, TransactionMode.ISOLATED)); - if (dataSource == null) { - throw new IllegalStateException(String.format("Failed to execute SQL scripts for test context %s: " - + "supply at least a DataSource or PlatformTransactionManager.", testContext)); - } + Assert.state(dataSource != null, () -> String.format("Failed to execute SQL scripts for test context %s: " + + "supply at least a DataSource or PlatformTransactionManager.", testContext)); // Execute scripts directly against the DataSource populator.execute(dataSource); @@ -228,11 +225,9 @@ public class SqlScriptsTestExecutionListener extends AbstractTestExecutionListen if (dataSource == null) { dataSource = dataSourceFromTxMgr; - if (dataSource == null) { - throw new IllegalStateException(String.format("Failed to execute SQL scripts for test context %s: " - + "could not obtain DataSource from transaction manager [%s] (named '%s').", testContext, + Assert.state(dataSource != null, () -> String.format("Failed to execute SQL scripts for test context %s: " + + "could not obtain DataSource from transaction manager [%s] (named '%s').", testContext, transactionManager.getClass().getName(), tmName)); - } } final DataSource finalDataSource = dataSource; diff --git a/spring-test/src/main/java/org/springframework/test/context/junit4/SpringJUnit4ClassRunner.java b/spring-test/src/main/java/org/springframework/test/context/junit4/SpringJUnit4ClassRunner.java index 22fa1a136d..dc8c9350c5 100644 --- a/spring-test/src/main/java/org/springframework/test/context/junit4/SpringJUnit4ClassRunner.java +++ b/spring-test/src/main/java/org/springframework/test/context/junit4/SpringJUnit4ClassRunner.java @@ -49,6 +49,7 @@ import org.springframework.test.context.junit4.statements.RunBeforeTestExecution import org.springframework.test.context.junit4.statements.RunBeforeTestMethodCallbacks; import org.springframework.test.context.junit4.statements.SpringFailOnTimeout; import org.springframework.test.context.junit4.statements.SpringRepeat; +import org.springframework.util.Assert; import org.springframework.util.ClassUtils; import org.springframework.util.ReflectionUtils; @@ -100,15 +101,12 @@ public class SpringJUnit4ClassRunner extends BlockJUnit4ClassRunner { private static final Method withRulesMethod; static { - if (!ClassUtils.isPresent("org.junit.internal.Throwables", SpringJUnit4ClassRunner.class.getClassLoader())) { - throw new IllegalStateException("SpringJUnit4ClassRunner requires JUnit 4.12 or higher."); - } + Assert.state(ClassUtils.isPresent("org.junit.internal.Throwables", SpringJUnit4ClassRunner.class.getClassLoader()), + "SpringJUnit4ClassRunner requires JUnit 4.12 or higher."); withRulesMethod = ReflectionUtils.findMethod(SpringJUnit4ClassRunner.class, "withRules", FrameworkMethod.class, Object.class, Statement.class); - if (withRulesMethod == null) { - throw new IllegalStateException("SpringJUnit4ClassRunner requires JUnit 4.12 or higher."); - } + Assert.state(withRulesMethod != null, "SpringJUnit4ClassRunner requires JUnit 4.12 or higher."); ReflectionUtils.makeAccessible(withRulesMethod); } @@ -118,14 +116,12 @@ public class SpringJUnit4ClassRunner extends BlockJUnit4ClassRunner { private static void ensureSpringRulesAreNotPresent(Class testClass) { for (Field field : testClass.getFields()) { - if (SpringClassRule.class.isAssignableFrom(field.getType())) { - throw new IllegalStateException(String.format("Detected SpringClassRule field in test class [%s], " + - "but SpringClassRule cannot be used with the SpringJUnit4ClassRunner.", testClass.getName())); - } - if (SpringMethodRule.class.isAssignableFrom(field.getType())) { - throw new IllegalStateException(String.format("Detected SpringMethodRule field in test class [%s], " + - "but SpringMethodRule cannot be used with the SpringJUnit4ClassRunner.", testClass.getName())); - } + Assert.state(!SpringClassRule.class.isAssignableFrom(field.getType()), () -> String.format( + "Detected SpringClassRule field in test class [%s], " + + "but SpringClassRule cannot be used with the SpringJUnit4ClassRunner.", testClass.getName())); + Assert.state(!SpringMethodRule.class.isAssignableFrom(field.getType()), () -> String.format( + "Detected SpringMethodRule field in test class [%s], " + + "but SpringMethodRule cannot be used with the SpringJUnit4ClassRunner.", testClass.getName())); } } diff --git a/spring-test/src/main/java/org/springframework/test/context/junit4/rules/SpringClassRule.java b/spring-test/src/main/java/org/springframework/test/context/junit4/rules/SpringClassRule.java index 53058ccc32..6b3b0ac197 100644 --- a/spring-test/src/main/java/org/springframework/test/context/junit4/rules/SpringClassRule.java +++ b/spring-test/src/main/java/org/springframework/test/context/junit4/rules/SpringClassRule.java @@ -98,9 +98,8 @@ public class SpringClassRule implements TestRule { new ConcurrentHashMap<>(64); static { - if (!ClassUtils.isPresent("org.junit.internal.Throwables", SpringClassRule.class.getClassLoader())) { - throw new IllegalStateException("SpringClassRule requires JUnit 4.12 or higher."); - } + Assert.state(ClassUtils.isPresent("org.junit.internal.Throwables", SpringClassRule.class.getClassLoader()), + "SpringClassRule requires JUnit 4.12 or higher."); } @@ -183,28 +182,26 @@ public class SpringClassRule implements TestRule { * annotated with {@code @Rule}. */ private static void validateSpringMethodRuleConfiguration(Class testClass) { - Field ruleField = null; - - for (Field field : testClass.getFields()) { - int modifiers = field.getModifiers(); - if (!Modifier.isStatic(modifiers) && Modifier.isPublic(modifiers) && - SpringMethodRule.class.isAssignableFrom(field.getType())) { - ruleField = field; - break; - } - } + Field ruleField = findSpringMethodRuleField(testClass); - if (ruleField == null) { - throw new IllegalStateException(String.format( + Assert.state(ruleField != null, () -> String.format( "Failed to find 'public SpringMethodRule' field in test class [%s]. " + "Consult the javadoc for SpringClassRule for details.", testClass.getName())); - } - if (!ruleField.isAnnotationPresent(Rule.class)) { - throw new IllegalStateException(String.format( + Assert.state(ruleField.isAnnotationPresent(Rule.class), () -> String.format( "SpringMethodRule field [%s] must be annotated with JUnit's @Rule annotation. " + "Consult the javadoc for SpringClassRule for details.", ruleField)); + } + + private static Field findSpringMethodRuleField(Class testClass) { + for (Field field : testClass.getFields()) { + int modifiers = field.getModifiers(); + if (!Modifier.isStatic(modifiers) && Modifier.isPublic(modifiers) && + SpringMethodRule.class.isAssignableFrom(field.getType())) { + return field; + } } + return null; } /** diff --git a/spring-test/src/main/java/org/springframework/test/context/junit4/rules/SpringMethodRule.java b/spring-test/src/main/java/org/springframework/test/context/junit4/rules/SpringMethodRule.java index a8f78a0d06..e08e300603 100644 --- a/spring-test/src/main/java/org/springframework/test/context/junit4/rules/SpringMethodRule.java +++ b/spring-test/src/main/java/org/springframework/test/context/junit4/rules/SpringMethodRule.java @@ -33,6 +33,7 @@ import org.springframework.test.context.junit4.statements.RunBeforeTestMethodCal import org.springframework.test.context.junit4.statements.RunPrepareTestInstanceCallbacks; import org.springframework.test.context.junit4.statements.SpringFailOnTimeout; import org.springframework.test.context.junit4.statements.SpringRepeat; +import org.springframework.util.Assert; import org.springframework.util.ClassUtils; import org.springframework.util.ReflectionUtils; @@ -101,9 +102,8 @@ public class SpringMethodRule implements MethodRule { private static final Log logger = LogFactory.getLog(SpringMethodRule.class); static { - if (!ClassUtils.isPresent("org.junit.internal.Throwables", SpringMethodRule.class.getClassLoader())) { - throw new IllegalStateException("SpringMethodRule requires JUnit 4.12 or higher."); - } + Assert.state(ClassUtils.isPresent("org.junit.internal.Throwables", SpringMethodRule.class.getClassLoader()), + "SpringMethodRule requires JUnit 4.12 or higher."); } @@ -220,28 +220,26 @@ public class SpringMethodRule implements MethodRule { * that is annotated with {@code @ClassRule}. */ private static SpringClassRule validateSpringClassRuleConfiguration(Class testClass) { - Field ruleField = null; + Field ruleField = findSpringClassRuleField(testClass); + + Assert.state(ruleField != null, () -> String.format( + "Failed to find 'public static final SpringClassRule' field in test class [%s]. " + + "Consult the javadoc for SpringClassRule for details.", testClass.getName())); + + Assert.state(ruleField.isAnnotationPresent(ClassRule.class), () -> String.format( + "SpringClassRule field [%s] must be annotated with JUnit's @ClassRule annotation. " + + "Consult the javadoc for SpringClassRule for details.", ruleField)); + + return (SpringClassRule) ReflectionUtils.getField(ruleField, null); + } + private static Field findSpringClassRuleField(Class testClass) { for (Field field : testClass.getFields()) { if (ReflectionUtils.isPublicStaticFinal(field) && SpringClassRule.class.isAssignableFrom(field.getType())) { - ruleField = field; - break; + return field; } } - - if (ruleField == null) { - throw new IllegalStateException(String.format( - "Failed to find 'public static final SpringClassRule' field in test class [%s]. " + - "Consult the javadoc for SpringClassRule for details.", testClass.getName())); - } - - if (!ruleField.isAnnotationPresent(ClassRule.class)) { - throw new IllegalStateException(String.format( - "SpringClassRule field [%s] must be annotated with JUnit's @ClassRule annotation. " + - "Consult the javadoc for SpringClassRule for details.", ruleField)); - } - - return (SpringClassRule) ReflectionUtils.getField(ruleField, null); + return null; } } diff --git a/spring-test/src/main/java/org/springframework/test/context/support/AbstractDelegatingSmartContextLoader.java b/spring-test/src/main/java/org/springframework/test/context/support/AbstractDelegatingSmartContextLoader.java index c0ae608ed7..9ffccc876e 100644 --- a/spring-test/src/main/java/org/springframework/test/context/support/AbstractDelegatingSmartContextLoader.java +++ b/spring-test/src/main/java/org/springframework/test/context/support/AbstractDelegatingSmartContextLoader.java @@ -178,11 +178,9 @@ public abstract class AbstractDelegatingSmartContextLoader implements SmartConte } } - if (configAttributes.hasClasses()) { - throw new IllegalStateException(String.format( + Assert.state(!configAttributes.hasClasses(), () -> String.format( "%s should NOT have detected default configuration classes for context configuration %s.", name(getXmlLoader()), configAttributes)); - } // Now let the annotation config loader process the configuration. delegateProcessing(getAnnotationConfigLoader(), configAttributes); @@ -195,11 +193,9 @@ public abstract class AbstractDelegatingSmartContextLoader implements SmartConte } } - if (!xmlLoaderDetectedDefaults && configAttributes.hasLocations()) { - throw new IllegalStateException(String.format( + Assert.state(xmlLoaderDetectedDefaults || !configAttributes.hasLocations(), () -> String.format( "%s should NOT have detected default locations for context configuration %s.", name(getAnnotationConfigLoader()), configAttributes)); - } if (configAttributes.hasLocations() && configAttributes.hasClasses()) { String message = String.format( @@ -237,12 +233,10 @@ public abstract class AbstractDelegatingSmartContextLoader implements SmartConte Assert.notNull(mergedConfig, "mergedConfig must not be null"); List candidates = Arrays.asList(getXmlLoader(), getAnnotationConfigLoader()); - if (mergedConfig.hasLocations() && mergedConfig.hasClasses()) { - throw new IllegalStateException(String.format( + Assert.state(!(mergedConfig.hasLocations() && mergedConfig.hasClasses()), () -> String.format( "Neither %s nor %s supports loading an ApplicationContext from %s: " + "declare either 'locations' or 'classes' but not both.", name(getXmlLoader()), name(getAnnotationConfigLoader()), mergedConfig)); - } for (SmartContextLoader loader : candidates) { // Determine if each loader can load a context from the mergedConfig. If it diff --git a/spring-test/src/main/java/org/springframework/test/context/support/AbstractTestContextBootstrapper.java b/spring-test/src/main/java/org/springframework/test/context/support/AbstractTestContextBootstrapper.java index 19fd57bca5..4ca864b953 100644 --- a/spring-test/src/main/java/org/springframework/test/context/support/AbstractTestContextBootstrapper.java +++ b/spring-test/src/main/java/org/springframework/test/context/support/AbstractTestContextBootstrapper.java @@ -387,13 +387,11 @@ public abstract class AbstractTestContextBootstrapper implements TestContextBoot Set contextCustomizers = getContextCustomizers(testClass, Collections.unmodifiableList(configAttributesList)); - if (requireLocationsClassesOrInitializers && - areAllEmpty(locations, classes, initializers, contextCustomizers)) { - throw new IllegalStateException(String.format( - "%s was unable to detect defaults, and no ApplicationContextInitializers " + - "or ContextCustomizers were declared for context configuration attributes %s", - contextLoader.getClass().getSimpleName(), configAttributesList)); - } + Assert.state(!(requireLocationsClassesOrInitializers && + areAllEmpty(locations, classes, initializers, contextCustomizers)), () -> String.format( + "%s was unable to detect defaults, and no ApplicationContextInitializers " + + "or ContextCustomizers were declared for context configuration attributes %s", + contextLoader.getClass().getSimpleName(), configAttributesList)); MergedTestPropertySources mergedTestPropertySources = TestPropertySourceUtils.buildMergedTestPropertySources(testClass); @@ -463,9 +461,7 @@ public abstract class AbstractTestContextBootstrapper implements TestContextBoot Class contextLoaderClass = resolveExplicitContextLoaderClass(configAttributesList); if (contextLoaderClass == null) { contextLoaderClass = getDefaultContextLoaderClass(testClass); - if (contextLoaderClass == null) { - throw new IllegalStateException("getDefaultContextLoaderClass() must not return null"); - } + Assert.state(contextLoaderClass != null, "getDefaultContextLoaderClass() must not return null"); } if (logger.isTraceEnabled()) { logger.trace(String.format("Using ContextLoader class [%s] for test class [%s]", diff --git a/spring-test/src/main/java/org/springframework/test/context/support/ContextLoaderUtils.java b/spring-test/src/main/java/org/springframework/test/context/support/ContextLoaderUtils.java index 71dc4330ee..6754a5bbe8 100644 --- a/spring-test/src/main/java/org/springframework/test/context/support/ContextLoaderUtils.java +++ b/spring-test/src/main/java/org/springframework/test/context/support/ContextLoaderUtils.java @@ -101,11 +101,9 @@ abstract class ContextLoaderUtils { UntypedAnnotationDescriptor desc = findAnnotationDescriptorForTypes(testClass, contextConfigType, contextHierarchyType); - if (desc == null) { - throw new IllegalArgumentException(String.format( + Assert.notNull(desc, () -> String.format( "Could not find an 'annotation declaring class' for annotation type [%s] or [%s] and test class [%s]", contextConfigType.getName(), contextHierarchyType.getName(), testClass.getName())); - } while (desc != null) { Class rootDeclaringClass = desc.getRootDeclaringClass(); @@ -237,11 +235,9 @@ abstract class ContextLoaderUtils { Class annotationType = ContextConfiguration.class; AnnotationDescriptor descriptor = findAnnotationDescriptor(testClass, annotationType); - if (descriptor == null) { - throw new IllegalArgumentException(String.format( + Assert.notNull(descriptor, () -> String.format( "Could not find an 'annotation declaring class' for annotation type [%s] and class [%s]", annotationType.getName(), testClass.getName())); - } while (descriptor != null) { convertContextConfigToConfigAttributesAndAddToList(descriptor.synthesizeAnnotation(), diff --git a/spring-test/src/main/java/org/springframework/test/context/transaction/TestContextTransactionUtils.java b/spring-test/src/main/java/org/springframework/test/context/transaction/TestContextTransactionUtils.java index b00a995587..22a6ada5c2 100644 --- a/spring-test/src/main/java/org/springframework/test/context/transaction/TestContextTransactionUtils.java +++ b/spring-test/src/main/java/org/springframework/test/context/transaction/TestContextTransactionUtils.java @@ -196,10 +196,8 @@ public abstract class TestContextTransactionUtils { // look up single TransactionManagementConfigurer Map configurers = BeanFactoryUtils.beansOfTypeIncludingAncestors( lbf, TransactionManagementConfigurer.class); - if (configurers.size() > 1) { - throw new IllegalStateException( + Assert.state(configurers.size() <= 1, "Only one TransactionManagementConfigurer may exist in the ApplicationContext"); - } if (configurers.size() == 1) { return configurers.values().iterator().next().annotationDrivenTransactionManager(); } diff --git a/spring-test/src/main/java/org/springframework/test/context/transaction/TestTransaction.java b/spring-test/src/main/java/org/springframework/test/context/transaction/TestTransaction.java index 427e0b7d33..c0a737a5d7 100644 --- a/spring-test/src/main/java/org/springframework/test/context/transaction/TestTransaction.java +++ b/spring-test/src/main/java/org/springframework/test/context/transaction/TestTransaction.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,7 @@ package org.springframework.test.context.transaction; import org.springframework.test.context.TestExecutionListeners; import org.springframework.transaction.TransactionStatus; +import org.springframework.util.Assert; /** * {@code TestTransaction} provides a collection of static utility methods for @@ -135,9 +136,7 @@ public class TestTransaction { private static TransactionContext requireCurrentTransactionContext() { TransactionContext txContext = TransactionContextHolder.getCurrentTransactionContext(); - if (txContext == null) { - throw new IllegalStateException("TransactionContext is not active"); - } + Assert.state(txContext != null, "TransactionContext is not active"); return txContext; } diff --git a/spring-test/src/main/java/org/springframework/test/context/transaction/TransactionContext.java b/spring-test/src/main/java/org/springframework/test/context/transaction/TransactionContext.java index 58225597d4..5f6cdbe40d 100644 --- a/spring-test/src/main/java/org/springframework/test/context/transaction/TransactionContext.java +++ b/spring-test/src/main/java/org/springframework/test/context/transaction/TransactionContext.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. @@ -24,6 +24,7 @@ import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.TransactionDefinition; import org.springframework.transaction.TransactionException; import org.springframework.transaction.TransactionStatus; +import org.springframework.util.Assert; /** * Transaction context for a specific {@link TestContext}. @@ -76,10 +77,8 @@ class TransactionContext { } void setFlaggedForRollback(boolean flaggedForRollback) { - if (this.transactionStatus == null) { - throw new IllegalStateException(String.format( + Assert.state(this.transactionStatus != null, () -> String.format( "Failed to set rollback flag for test context %s: transaction does not exist.", this.testContext)); - } this.flaggedForRollback = flaggedForRollback; } @@ -90,10 +89,8 @@ class TransactionContext { * @throws TransactionException if starting the transaction fails */ void startTransaction() { - if (this.transactionStatus != null) { - throw new IllegalStateException( + Assert.state(this.transactionStatus == null, "Cannot start a new transaction without ending the existing transaction first."); - } this.flaggedForRollback = this.defaultRollback; this.transactionStatus = this.transactionManager.getTransaction(this.transactionDefinition); ++this.transactionsStarted; @@ -115,10 +112,8 @@ class TransactionContext { "Ending transaction for test context %s; transaction status [%s]; rollback [%s]", this.testContext, this.transactionStatus, flaggedForRollback)); } - if (this.transactionStatus == null) { - throw new IllegalStateException(String.format( + Assert.state(this.transactionStatus != null, () -> String.format( "Failed to end transaction for test context %s: transaction does not exist.", this.testContext)); - } try { if (flaggedForRollback) { diff --git a/spring-test/src/main/java/org/springframework/test/context/transaction/TransactionalTestExecutionListener.java b/spring-test/src/main/java/org/springframework/test/context/transaction/TransactionalTestExecutionListener.java index 497c3c1c42..ab07495021 100644 --- a/spring-test/src/main/java/org/springframework/test/context/transaction/TransactionalTestExecutionListener.java +++ b/spring-test/src/main/java/org/springframework/test/context/transaction/TransactionalTestExecutionListener.java @@ -161,9 +161,7 @@ public class TransactionalTestExecutionListener extends AbstractTestExecutionLis Assert.notNull(testMethod, "The test method of the supplied TestContext must not be null"); TransactionContext txContext = TransactionContextHolder.removeCurrentTransactionContext(); - if (txContext != null) { - throw new IllegalStateException("Cannot start a new transaction without ending the existing transaction."); - } + Assert.state(txContext == null, "Cannot start a new transaction without ending the existing transaction."); PlatformTransactionManager tm = null; TransactionAttribute transactionAttribute = this.attributeSource.getTransactionAttribute(testMethod, testClass); @@ -183,11 +181,9 @@ public class TransactionalTestExecutionListener extends AbstractTestExecutionLis tm = getTransactionManager(testContext, transactionAttribute.getQualifier()); - if (tm == null) { - throw new IllegalStateException(String.format( - "Failed to retrieve PlatformTransactionManager for @Transactional test for test context %s.", - testContext)); - } + Assert.state(tm != null, () -> String.format( + "Failed to retrieve PlatformTransactionManager for @Transactional test for test context %s.", + testContext)); } if (tm != null) { diff --git a/spring-test/src/main/java/org/springframework/test/context/web/AbstractGenericWebContextLoader.java b/spring-test/src/main/java/org/springframework/test/context/web/AbstractGenericWebContextLoader.java index a74ca6a2ca..17600ec051 100644 --- a/spring-test/src/main/java/org/springframework/test/context/web/AbstractGenericWebContextLoader.java +++ b/spring-test/src/main/java/org/springframework/test/context/web/AbstractGenericWebContextLoader.java @@ -104,12 +104,10 @@ public abstract class AbstractGenericWebContextLoader extends AbstractContextLoa */ @Override public final ConfigurableApplicationContext loadContext(MergedContextConfiguration mergedConfig) throws Exception { - - if (!(mergedConfig instanceof WebMergedContextConfiguration)) { - throw new IllegalArgumentException(String.format( + Assert.isTrue(mergedConfig instanceof WebMergedContextConfiguration, () -> String.format( "Cannot load WebApplicationContext from non-web merged context configuration %s. " + "Consider annotating your test class with @WebAppConfiguration.", mergedConfig)); - } + WebMergedContextConfiguration webMergedConfig = (WebMergedContextConfiguration) mergedConfig; if (logger.isDebugEnabled()) { diff --git a/spring-test/src/main/java/org/springframework/test/context/web/ServletTestExecutionListener.java b/spring-test/src/main/java/org/springframework/test/context/web/ServletTestExecutionListener.java index 32e79eb56b..afe5dcc719 100644 --- a/spring-test/src/main/java/org/springframework/test/context/web/ServletTestExecutionListener.java +++ b/spring-test/src/main/java/org/springframework/test/context/web/ServletTestExecutionListener.java @@ -33,6 +33,7 @@ import org.springframework.test.context.TestContext; import org.springframework.test.context.TestExecutionListener; import org.springframework.test.context.support.AbstractTestExecutionListener; import org.springframework.test.context.support.DependencyInjectionTestExecutionListener; +import org.springframework.util.Assert; import org.springframework.web.context.WebApplicationContext; import org.springframework.web.context.request.RequestAttributes; import org.springframework.web.context.request.RequestContextHolder; @@ -191,11 +192,9 @@ public class ServletTestExecutionListener extends AbstractTestExecutionListener if (context instanceof WebApplicationContext) { WebApplicationContext wac = (WebApplicationContext) context; ServletContext servletContext = wac.getServletContext(); - if (!(servletContext instanceof MockServletContext)) { - throw new IllegalStateException(String.format( + Assert.state(servletContext instanceof MockServletContext, () -> String.format( "The WebApplicationContext for test context %s must be configured with a MockServletContext.", testContext)); - } if (logger.isDebugEnabled()) { logger.debug(String.format( diff --git a/spring-test/src/main/java/org/springframework/test/web/client/DefaultRequestExpectation.java b/spring-test/src/main/java/org/springframework/test/web/client/DefaultRequestExpectation.java index 1755842ada..d1983be5d5 100644 --- a/spring-test/src/main/java/org/springframework/test/web/client/DefaultRequestExpectation.java +++ b/spring-test/src/main/java/org/springframework/test/web/client/DefaultRequestExpectation.java @@ -88,9 +88,7 @@ public class DefaultRequestExpectation implements RequestExpectation { @Override public ClientHttpResponse createResponse(ClientHttpRequest request) throws IOException { ResponseCreator responseCreator = getResponseCreator(); - if (responseCreator == null) { - throw new IllegalStateException("createResponse called before ResponseCreator was set"); - } + Assert.state(responseCreator != null, "createResponse() called before ResponseCreator was set"); getRequestCount().incrementAndValidate(); return responseCreator.createResponse(request); } diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/HtmlUnitRequestBuilder.java b/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/HtmlUnitRequestBuilder.java index d92695268c..986d945768 100644 --- a/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/HtmlUnitRequestBuilder.java +++ b/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/HtmlUnitRequestBuilder.java @@ -196,7 +196,8 @@ final class HtmlUnitRequestBuilder implements RequestBuilder, Mergeable { * {@link HttpServletRequest#getContextPath()} which states it can be * an empty string, or it must start with a "/" and not end with a "/". * @param contextPath a valid contextPath - * @throws IllegalArgumentException if the contextPath is not a valid {@link HttpServletRequest#getContextPath()} + * @throws IllegalArgumentException if the contextPath is not a valid + * {@link HttpServletRequest#getContextPath()} */ public void setContextPath(String contextPath) { MockMvcWebConnection.validateContextPath(contextPath); @@ -244,10 +245,8 @@ final class HtmlUnitRequestBuilder implements RequestBuilder, Mergeable { } } else { - if (!uriComponents.getPath().startsWith(this.contextPath)) { - throw new IllegalArgumentException(uriComponents.getPath() + " should start with contextPath " + - this.contextPath); - } + Assert.isTrue(uriComponents.getPath().startsWith(this.contextPath), + () -> uriComponents.getPath() + " should start with contextPath " + this.contextPath); request.setContextPath(this.contextPath); } } @@ -260,10 +259,8 @@ final class HtmlUnitRequestBuilder implements RequestBuilder, Mergeable { StringTokenizer tokens = new StringTokenizer(cookieHeaderValue, "=;"); while (tokens.hasMoreTokens()) { String cookieName = tokens.nextToken().trim(); - if (!tokens.hasMoreTokens()) { - throw new IllegalArgumentException("Expected value for cookie name '" + cookieName - + "'. Full cookie was " + cookieHeaderValue); - } + Assert.isTrue(tokens.hasMoreTokens(), + () -> "Expected value for cookie name '" + cookieName + "'. Full cookie was " + cookieHeaderValue); String cookieValue = tokens.nextToken().trim(); processCookie(request, cookies, new Cookie(cookieName, cookieValue)); } @@ -382,9 +379,7 @@ final class HtmlUnitRequestBuilder implements RequestBuilder, Mergeable { private Locale parseLocale(String locale) { Matcher matcher = LOCALE_PATTERN.matcher(locale); - if (!matcher.matches()) { - throw new IllegalArgumentException("Invalid locale " + locale); - } + Assert.isTrue(matcher.matches(), () -> "Invalid locale " + locale); String language = matcher.group(1); String country = matcher.group(2); if (country == null) { diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/MockMvcWebConnection.java b/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/MockMvcWebConnection.java index d6ea370759..3b44432f8c 100644 --- a/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/MockMvcWebConnection.java +++ b/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/MockMvcWebConnection.java @@ -111,12 +111,8 @@ public final class MockMvcWebConnection implements WebConnection { if (contextPath == null || "".equals(contextPath)) { return; } - if (!contextPath.startsWith("/")) { - throw new IllegalArgumentException("contextPath '" + contextPath + "' must start with '/'."); - } - if (contextPath.endsWith("/")) { - throw new IllegalArgumentException("contextPath '" + contextPath + "' must not end with '/'."); - } + Assert.isTrue(contextPath.startsWith("/"), () -> "contextPath '" + contextPath + "' must start with '/'."); + Assert.isTrue(!contextPath.endsWith("/"), () -> "contextPath '" + contextPath + "' must not end with '/'."); } diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilder.java b/spring-test/src/main/java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilder.java index b7fcd6be4d..f3003f78f9 100644 --- a/spring-test/src/main/java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilder.java +++ b/spring-test/src/main/java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilder.java @@ -751,10 +751,8 @@ public class MockHttpServletRequestBuilder public MockHttpServletRequest postProcessRequest(MockHttpServletRequest request) { for (RequestPostProcessor postProcessor : this.postProcessors) { request = postProcessor.postProcessRequest(request); - if (request == null) { - throw new IllegalStateException( - "Post-processor [" + postProcessor.getClass().getName() + "] returned null"); - } + Assert.state(request != null, + () -> "Post-processor [" + postProcessor.getClass().getName() + "] returned null"); } return request; }