From 7f9baa3a09a25edacce0251bf7a370e4e6070081 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Fri, 26 Sep 2014 22:38:41 +0200 Subject: [PATCH] Polishing --- .../ScheduledExecutorFactoryBean.java | 25 ++++++++++--------- .../concurrent/ThreadPoolTaskScheduler.java | 18 +++++++------ .../annotation/SockJsServiceRegistration.java | 2 -- .../sockjs/support/SockJsServiceTests.java | 24 +++++------------- 4 files changed, 30 insertions(+), 39 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/scheduling/concurrent/ScheduledExecutorFactoryBean.java b/spring-context/src/main/java/org/springframework/scheduling/concurrent/ScheduledExecutorFactoryBean.java index e514b9cb986..df1d0e383e1 100644 --- a/spring-context/src/main/java/org/springframework/scheduling/concurrent/ScheduledExecutorFactoryBean.java +++ b/spring-context/src/main/java/org/springframework/scheduling/concurrent/ScheduledExecutorFactoryBean.java @@ -58,7 +58,7 @@ import org.springframework.util.ObjectUtils; * @author Juergen Hoeller * @since 2.0 * @see #setPoolSize - * @see #setRemoveOnCancelPolicy(boolean) + * @see #setRemoveOnCancelPolicy * @see #setThreadFactory * @see ScheduledExecutorTask * @see java.util.concurrent.ScheduledExecutorService @@ -68,17 +68,17 @@ import org.springframework.util.ObjectUtils; public class ScheduledExecutorFactoryBean extends ExecutorConfigurationSupport implements FactoryBean { - // ScheduledThreadPoolExecutor.setRemoveOnCancelPolicy(boolean) only available on JDK 1.7+ + // ScheduledThreadPoolExecutor.setRemoveOnCancelPolicy(boolean) only available on JDK 7+ private static final boolean setRemoveOnCancelPolicyAvailable = ClassUtils.hasMethod(ScheduledThreadPoolExecutor.class, "setRemoveOnCancelPolicy", boolean.class); private int poolSize = 1; - private boolean removeOnCancelPolicy; - private ScheduledExecutorTask[] scheduledExecutorTasks; + private boolean removeOnCancelPolicy = false; + private boolean continueScheduledExecutionAfterException = false; private boolean exposeUnconfigurableExecutor = false; @@ -95,14 +95,6 @@ public class ScheduledExecutorFactoryBean extends ExecutorConfigurationSupport this.poolSize = poolSize; } - /** - * Set the same property on ScheduledExecutorService (JDK 1.7+). - * There is no default. If not set, the executor property is not set. - */ - public void setRemoveOnCancelPolicy(boolean removeOnCancelPolicy) { - this.removeOnCancelPolicy = removeOnCancelPolicy; - } - /** * Register a list of ScheduledExecutorTask objects with the ScheduledExecutorService * that this FactoryBean creates. Depending on each ScheduledExecutorTask's settings, @@ -115,6 +107,15 @@ public class ScheduledExecutorFactoryBean extends ExecutorConfigurationSupport this.scheduledExecutorTasks = scheduledExecutorTasks; } + /** + * Set the remove-on-cancel mode on {@link ScheduledThreadPoolExecutor} (JDK 7+). + *

Default is {@code false}. If set to {@code true}, the target executor will be + * switched into remove-on-cancel mode (if possible, with a soft fallback otherwise). + */ + public void setRemoveOnCancelPolicy(boolean removeOnCancelPolicy) { + this.removeOnCancelPolicy = removeOnCancelPolicy; + } + /** * Specify whether to continue the execution of a scheduled task * after it threw an exception. diff --git a/spring-context/src/main/java/org/springframework/scheduling/concurrent/ThreadPoolTaskScheduler.java b/spring-context/src/main/java/org/springframework/scheduling/concurrent/ThreadPoolTaskScheduler.java index ab8caab3578..01da8e09325 100644 --- a/spring-context/src/main/java/org/springframework/scheduling/concurrent/ThreadPoolTaskScheduler.java +++ b/spring-context/src/main/java/org/springframework/scheduling/concurrent/ThreadPoolTaskScheduler.java @@ -58,14 +58,14 @@ import org.springframework.util.concurrent.ListenableFutureTask; public class ThreadPoolTaskScheduler extends ExecutorConfigurationSupport implements AsyncListenableTaskExecutor, SchedulingTaskExecutor, TaskScheduler { - // ScheduledThreadPoolExecutor.setRemoveOnCancelPolicy(boolean) only available on JDK 1.7+ + // ScheduledThreadPoolExecutor.setRemoveOnCancelPolicy(boolean) only available on JDK 7+ private static final boolean setRemoveOnCancelPolicyAvailable = ClassUtils.hasMethod(ScheduledThreadPoolExecutor.class, "setRemoveOnCancelPolicy", boolean.class); private volatile int poolSize = 1; - private volatile boolean removeOnCancelPolicy; + private volatile boolean removeOnCancelPolicy = false; private volatile ScheduledExecutorService scheduledExecutor; @@ -86,8 +86,9 @@ public class ThreadPoolTaskScheduler extends ExecutorConfigurationSupport } /** - * Set the same property on ScheduledExecutorService (JDK 1.7+). - * There is no default. If not set, the executor property is not set. + * Set the remove-on-cancel mode on {@link ScheduledThreadPoolExecutor} (JDK 7+). + *

Default is {@code false}. If set to {@code true}, the target executor will be + * switched into remove-on-cancel mode (if possible, with a soft fallback otherwise). *

This setting can be modified at runtime, for example through JMX. */ @UsesJava7 @@ -183,14 +184,17 @@ public class ThreadPoolTaskScheduler extends ExecutorConfigurationSupport } /** - * Return the current setting of removeOnCancelPolicy. - *

Requires an underlying {@link ScheduledThreadPoolExecutor} and JDK 1.7+. + * Return the current setting for the remove-on-cancel mode. + *

Requires an underlying {@link ScheduledThreadPoolExecutor}. */ @UsesJava7 public boolean isRemoveOnCancelPolicy() { + if (!setRemoveOnCancelPolicyAvailable) { + return false; + } if (this.scheduledExecutor == null) { // Not initialized yet: return our setting for the time being. - return (setRemoveOnCancelPolicyAvailable && this.removeOnCancelPolicy); + return this.removeOnCancelPolicy; } return getScheduledThreadPoolExecutor().getRemoveOnCancelPolicy(); } diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/config/annotation/SockJsServiceRegistration.java b/spring-websocket/src/main/java/org/springframework/web/socket/config/annotation/SockJsServiceRegistration.java index 356efa720b8..c37b66a3f34 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/config/annotation/SockJsServiceRegistration.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/config/annotation/SockJsServiceRegistration.java @@ -81,11 +81,9 @@ public class SockJsServiceRegistration { * iframe so that code in the iframe can run from a domain local to the SockJS * server. Since the iframe needs to load the SockJS javascript client library, * this property allows specifying where to load it from. - * *

By default this is set to point to * "https://cdn.jsdelivr.net/sockjs/0.3.4/sockjs.min.js". However it can * also be set to point to a URL served by the application. - * *

Note that it's possible to specify a relative URL in which case the URL * must be relative to the iframe URL. For example assuming a SockJS endpoint * mapped to "/sockjs", and resulting iframe URL "/sockjs/iframe.html", then the diff --git a/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/support/SockJsServiceTests.java b/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/support/SockJsServiceTests.java index b037532cfcb..a729e2cbdf8 100644 --- a/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/support/SockJsServiceTests.java +++ b/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/support/SockJsServiceTests.java @@ -17,12 +17,12 @@ package org.springframework.web.socket.sockjs.support; import java.io.IOException; - import javax.servlet.ServletOutputStream; import javax.servlet.http.HttpServletResponse; import org.junit.Before; import org.junit.Test; + import org.springframework.http.HttpStatus; import org.springframework.http.server.ServerHttpRequest; import org.springframework.http.server.ServerHttpResponse; @@ -55,6 +55,7 @@ public class SockJsServiceTests extends AbstractHttpRequestTests { this.service = new TestSockJsService(new ThreadPoolTaskScheduler()); } + @Test public void validateRequest() throws Exception { @@ -76,7 +77,6 @@ public class SockJsServiceTests extends AbstractHttpRequestTests { @Test public void handleInfoGet() throws Exception { - resetResponseAndHandleRequest("GET", "/echo/info", HttpStatus.OK); assertEquals("application/json;charset=UTF-8", this.servletResponse.getContentType()); @@ -98,9 +98,7 @@ public class SockJsServiceTests extends AbstractHttpRequestTests { body.substring(body.indexOf(','))); } - // SPR-11443 - - @Test + @Test // SPR-11443 public void handleInfoGetCorsFilter() throws Exception { // Simulate scenario where Filter would have already set CORS headers @@ -111,9 +109,7 @@ public class SockJsServiceTests extends AbstractHttpRequestTests { assertEquals("foobar:123", this.servletResponse.getHeader("Access-Control-Allow-Origin")); } - // SPR-11919 - - @Test + @Test // SPR-11919 public void handleInfoGetWildflyNPE() throws Exception { HttpServletResponse mockResponse = mock(HttpServletResponse.class); ServletOutputStream ous = mock(ServletOutputStream.class); @@ -128,9 +124,7 @@ public class SockJsServiceTests extends AbstractHttpRequestTests { @Test public void handleInfoOptions() throws Exception { - this.servletRequest.addHeader("Access-Control-Request-Headers", "Last-Modified"); - resetResponseAndHandleRequest("OPTIONS", "/echo/info", HttpStatus.NO_CONTENT); this.response.flush(); @@ -143,7 +137,6 @@ public class SockJsServiceTests extends AbstractHttpRequestTests { @Test public void handleIframeRequest() throws Exception { - resetResponseAndHandleRequest("GET", "/echo/iframe.html", HttpStatus.OK); assertEquals("text/html;charset=UTF-8", this.servletResponse.getContentType()); @@ -155,15 +148,12 @@ public class SockJsServiceTests extends AbstractHttpRequestTests { @Test public void handleIframeRequestNotModified() throws Exception { - this.servletRequest.addHeader("If-None-Match", "\"06b486b3208b085d9e3220f456a6caca4\""); - resetResponseAndHandleRequest("GET", "/echo/iframe.html", HttpStatus.NOT_MODIFIED); } @Test public void handleRawWebSocketRequest() throws Exception { - resetResponseAndHandleRequest("GET", "/echo", HttpStatus.OK); assertEquals("Welcome to SockJS!\n", this.servletResponse.getContentAsString()); @@ -174,13 +164,13 @@ public class SockJsServiceTests extends AbstractHttpRequestTests { @Test public void handleEmptyContentType() throws Exception { - - servletRequest.setContentType(""); + this.servletRequest.setContentType(""); resetResponseAndHandleRequest("GET", "/echo/info", HttpStatus.OK); assertEquals("Invalid/empty content should have been ignored", 200, this.servletResponse.getStatus()); } + private void resetResponseAndHandleRequest(String httpMethod, String uri, HttpStatus httpStatus) throws IOException { resetResponse(); handleRequest(httpMethod, uri, httpStatus); @@ -211,14 +201,12 @@ public class SockJsServiceTests extends AbstractHttpRequestTests { @Override protected void handleRawWebSocketRequest(ServerHttpRequest req, ServerHttpResponse res, WebSocketHandler handler) throws IOException { - this.handler = handler; } @Override protected void handleTransportRequest(ServerHttpRequest req, ServerHttpResponse res, WebSocketHandler handler, String sessionId, String transport) throws SockJsException { - this.sessionId = sessionId; this.transport = transport; this.handler = handler;