From 55e601c322a2c7c6da130c01aa4fda822cae80c2 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Mon, 20 May 2019 22:19:03 +0200 Subject: [PATCH 1/2] Revise system property replacement tests See gh-22959 --- .../core/io/ResourceEditorTests.java | 28 +++++++++++++------ .../ResourceArrayPropertyEditorTests.java | 14 ++++------ 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/spring-core/src/test/java/org/springframework/core/io/ResourceEditorTests.java b/spring-core/src/test/java/org/springframework/core/io/ResourceEditorTests.java index f87a0707240..66a965be73e 100644 --- a/spring-core/src/test/java/org/springframework/core/io/ResourceEditorTests.java +++ b/spring-core/src/test/java/org/springframework/core/io/ResourceEditorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2010 the original author or authors. + * Copyright 2002-2019 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. @@ -34,7 +34,7 @@ import static org.junit.Assert.*; public class ResourceEditorTests { @Test - public void sunnyDay() throws Exception { + public void sunnyDay() { PropertyEditor editor = new ResourceEditor(); editor.setAsText("classpath:org/springframework/core/io/ResourceEditorTests.class"); Resource resource = (Resource) editor.getValue(); @@ -43,19 +43,19 @@ public class ResourceEditorTests { } @Test(expected = IllegalArgumentException.class) - public void ctorWithNullCtorArgs() throws Exception { + public void ctorWithNullCtorArgs() { new ResourceEditor(null, null); } @Test - public void setAndGetAsTextWithNull() throws Exception { + public void setAndGetAsTextWithNull() { PropertyEditor editor = new ResourceEditor(); editor.setAsText(null); assertEquals("", editor.getAsText()); } @Test - public void setAndGetAsTextWithWhitespaceResource() throws Exception { + public void setAndGetAsTextWithWhitespaceResource() { PropertyEditor editor = new ResourceEditor(); editor.setAsText(" "); assertEquals("", editor.getAsText()); @@ -63,6 +63,20 @@ public class ResourceEditorTests { @Test public void testSystemPropertyReplacement() { + PropertyEditor editor = new ResourceEditor(); + System.setProperty("test.prop", "foo"); + try { + editor.setAsText("${test.prop}"); + Resource resolved = (Resource) editor.getValue(); + assertEquals("foo", resolved.getFilename()); + } + finally { + System.getProperties().remove("test.prop"); + } + } + + @Test + public void testSystemPropertyReplacementWithUnresolvablePlaceholder() { PropertyEditor editor = new ResourceEditor(); System.setProperty("test.prop", "foo"); try { @@ -76,13 +90,11 @@ public class ResourceEditorTests { } @Test(expected = IllegalArgumentException.class) - public void testStrictSystemPropertyReplacement() { + public void testStrictSystemPropertyReplacementWithUnresolvablePlaceholder() { PropertyEditor editor = new ResourceEditor(new DefaultResourceLoader(), new StandardEnvironment(), false); System.setProperty("test.prop", "foo"); try { editor.setAsText("${test.prop}-${bar}"); - Resource resolved = (Resource) editor.getValue(); - assertEquals("foo-${bar}", resolved.getFilename()); } finally { System.getProperties().remove("test.prop"); diff --git a/spring-core/src/test/java/org/springframework/core/io/support/ResourceArrayPropertyEditorTests.java b/spring-core/src/test/java/org/springframework/core/io/support/ResourceArrayPropertyEditorTests.java index 93305ce2c73..f3bcafaa815 100644 --- a/spring-core/src/test/java/org/springframework/core/io/support/ResourceArrayPropertyEditorTests.java +++ b/spring-core/src/test/java/org/springframework/core/io/support/ResourceArrayPropertyEditorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2019 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. @@ -32,7 +32,7 @@ import static org.junit.Assert.*; public class ResourceArrayPropertyEditorTests { @Test - public void testVanillaResource() throws Exception { + public void testVanillaResource() { PropertyEditor editor = new ResourceArrayPropertyEditor(); editor.setAsText("classpath:org/springframework/core/io/support/ResourceArrayPropertyEditor.class"); Resource[] resources = (Resource[]) editor.getValue(); @@ -41,7 +41,7 @@ public class ResourceArrayPropertyEditorTests { } @Test - public void testPatternResource() throws Exception { + public void testPatternResource() { // N.B. this will sometimes fail if you use classpath: instead of classpath*:. // The result depends on the classpath - if test-classes are segregated from classes // and they come first on the classpath (like in Maven) then it breaks, if classes @@ -58,9 +58,9 @@ public class ResourceArrayPropertyEditorTests { PropertyEditor editor = new ResourceArrayPropertyEditor(); System.setProperty("test.prop", "foo"); try { - editor.setAsText("${test.prop}-${bar}"); + editor.setAsText("${test.prop}"); Resource[] resources = (Resource[]) editor.getValue(); - assertEquals("foo-${bar}", resources[0].getFilename()); + assertEquals("foo", resources[0].getFilename()); } finally { System.getProperties().remove("test.prop"); @@ -68,15 +68,13 @@ public class ResourceArrayPropertyEditorTests { } @Test(expected = IllegalArgumentException.class) - public void testStrictSystemPropertyReplacement() { + public void testStrictSystemPropertyReplacementWithUnresolvablePlaceholder() { PropertyEditor editor = new ResourceArrayPropertyEditor( new PathMatchingResourcePatternResolver(), new StandardEnvironment(), false); System.setProperty("test.prop", "foo"); try { editor.setAsText("${test.prop}-${bar}"); - Resource[] resources = (Resource[]) editor.getValue(); - assertEquals("foo-${bar}", resources[0].getFilename()); } finally { System.getProperties().remove("test.prop"); From 75d751d96816f43305ff72ba326ff7acd98e7fd3 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Mon, 20 May 2019 22:19:11 +0200 Subject: [PATCH 2/2] Polishing --- .../aop/framework/JdkDynamicAopProxy.java | 6 ++--- .../beans/TypeConverterDelegate.java | 3 +-- .../context/request/async/DeferredResult.java | 26 +++++++++---------- .../sockjs/support/AbstractSockJsService.java | 4 +-- 4 files changed, 19 insertions(+), 20 deletions(-) diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/JdkDynamicAopProxy.java b/spring-aop/src/main/java/org/springframework/aop/framework/JdkDynamicAopProxy.java index 43895091f46..43223de5a2b 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/JdkDynamicAopProxy.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/JdkDynamicAopProxy.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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,6 @@ final class JdkDynamicAopProxy implements AopProxy, InvocationHandler, Serializa @Override @Nullable public Object invoke(Object proxy, Method method, Object[] args) throws Throwable { - MethodInvocation invocation; Object oldProxy = null; boolean setProxyContext = false; @@ -207,7 +206,8 @@ final class JdkDynamicAopProxy implements AopProxy, InvocationHandler, Serializa } else { // We need to create a method invocation... - invocation = new ReflectiveMethodInvocation(proxy, target, method, args, targetClass, chain); + MethodInvocation invocation = + new ReflectiveMethodInvocation(proxy, target, method, args, targetClass, chain); // Proceed to the joinpoint through the interceptor chain. retVal = invocation.proceed(); } diff --git a/spring-beans/src/main/java/org/springframework/beans/TypeConverterDelegate.java b/spring-beans/src/main/java/org/springframework/beans/TypeConverterDelegate.java index 192d08060d3..38a59ba3d68 100644 --- a/spring-beans/src/main/java/org/springframework/beans/TypeConverterDelegate.java +++ b/spring-beans/src/main/java/org/springframework/beans/TypeConverterDelegate.java @@ -522,8 +522,7 @@ class TypeConverterDelegate { return original; } - int i = 0; - for (; it.hasNext(); i++) { + for (int i = 0; it.hasNext(); i++) { Object element = it.next(); String indexedPropertyName = buildIndexedPropertyName(propertyName, i); Object convertedElement = convertIfNecessary(indexedPropertyName, null, element, diff --git a/spring-web/src/main/java/org/springframework/web/context/request/async/DeferredResult.java b/spring-web/src/main/java/org/springframework/web/context/request/async/DeferredResult.java index 6763c867815..185c2fe947c 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/async/DeferredResult.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/async/DeferredResult.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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,7 +21,6 @@ import java.util.concurrent.Callable; import java.util.function.Consumer; import java.util.function.Supplier; - import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -60,7 +59,7 @@ public class DeferredResult { @Nullable - private final Long timeout; + private final Long timeoutValue; private final Supplier timeoutResult; @@ -89,35 +88,36 @@ public class DeferredResult { *

By default not set in which case the default configured in the MVC * Java Config or the MVC namespace is used, or if that's not set, then the * timeout depends on the default of the underlying server. - * @param timeout timeout value in milliseconds + * @param timeoutValue timeout value in milliseconds */ - public DeferredResult(Long timeout) { - this(timeout, () -> RESULT_NONE); + public DeferredResult(Long timeoutValue) { + this(timeoutValue, () -> RESULT_NONE); } /** * Create a DeferredResult with a timeout value and a default result to use * in case of timeout. - * @param timeout timeout value in milliseconds (ignored if {@code null}) + * @param timeoutValue timeout value in milliseconds (ignored if {@code null}) * @param timeoutResult the result to use */ - public DeferredResult(@Nullable Long timeout, final Object timeoutResult) { + public DeferredResult(@Nullable Long timeoutValue, final Object timeoutResult) { this.timeoutResult = () -> timeoutResult; - this.timeout = timeout; + this.timeoutValue = timeoutValue; } /** * Variant of {@link #DeferredResult(Long, Object)} that accepts a dynamic * fallback value based on a {@link Supplier}. - * @param timeout timeout value in milliseconds (ignored if {@code null}) + * @param timeoutValue timeout value in milliseconds (ignored if {@code null}) * @param timeoutResult the result supplier to use * @since 5.1.1 */ - public DeferredResult(@Nullable Long timeout, Supplier timeoutResult) { + public DeferredResult(@Nullable Long timeoutValue, Supplier timeoutResult) { this.timeoutResult = timeoutResult; - this.timeout = timeout; + this.timeoutValue = timeoutValue; } + /** * Return {@code true} if this DeferredResult is no longer usable either * because it was previously set or because the underlying request expired. @@ -155,7 +155,7 @@ public class DeferredResult { */ @Nullable final Long getTimeoutValue() { - return this.timeout; + return this.timeoutValue; } /** diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/support/AbstractSockJsService.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/support/AbstractSockJsService.java index bb003bdebed..eb01ff72177 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/support/AbstractSockJsService.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/support/AbstractSockJsService.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -354,7 +354,7 @@ public abstract class AbstractSockJsService implements SockJsService, CorsConfig String requestInfo = (logger.isDebugEnabled() ? request.getMethod() + " " + request.getURI() : null); try { - if (sockJsPath.equals("") || sockJsPath.equals("/")) { + if (sockJsPath.isEmpty() || sockJsPath.equals("/")) { if (requestInfo != null) { logger.debug("Processing transport request: " + requestInfo); }