From 9700f09fad1e900a6968a9fc1bcdefdf32bca365 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 2 Aug 2013 15:12:20 -0400 Subject: [PATCH] Polish async feature for ServerHttpRequest/Response ServerHttpAsyncResponseControl wraps a ServetHttpRequest and -Response pair and allows putting the processing of the request in async mode so that the response remains open until explicitly closed, either from the current or from another thread. ServletServerHttpAsyncResponseControl provides a Serlvet-based implementation. --- .../http/server/AsyncServerHttpRequest.java | 35 ---- .../server/AsyncServletServerHttpRequest.java | 154 ------------------ .../ServerHttpAsyncResponseControl.java | 57 +++++++ .../ServletServerHttpAsyncRequestControl.java | 135 +++++++++++++++ .../sockjs/SockJsHttpRequestHandler.java | 10 +- .../session/AbstractHttpSockJsSession.java | 27 +-- .../web/socket/AbstractHttpRequestTests.java | 15 +- .../support/AbstractSockJsServiceTests.java | 2 +- .../AbstractHttpSockJsSessionTests.java | 5 +- 9 files changed, 228 insertions(+), 212 deletions(-) delete mode 100644 spring-web/src/main/java/org/springframework/http/server/AsyncServerHttpRequest.java delete mode 100644 spring-web/src/main/java/org/springframework/http/server/AsyncServletServerHttpRequest.java create mode 100644 spring-web/src/main/java/org/springframework/http/server/ServerHttpAsyncResponseControl.java create mode 100644 spring-web/src/main/java/org/springframework/http/server/ServletServerHttpAsyncRequestControl.java diff --git a/spring-web/src/main/java/org/springframework/http/server/AsyncServerHttpRequest.java b/spring-web/src/main/java/org/springframework/http/server/AsyncServerHttpRequest.java deleted file mode 100644 index bbf973d5d8..0000000000 --- a/spring-web/src/main/java/org/springframework/http/server/AsyncServerHttpRequest.java +++ /dev/null @@ -1,35 +0,0 @@ -/* - * Copyright 2002-2013 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. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.http.server; - - - -/** - * TODO.. - */ -public interface AsyncServerHttpRequest extends ServerHttpRequest { - - void setTimeout(long timeout); - - void startAsync(); - - boolean isAsyncStarted(); - - void completeAsync(); - - boolean isAsyncCompleted(); - -} diff --git a/spring-web/src/main/java/org/springframework/http/server/AsyncServletServerHttpRequest.java b/spring-web/src/main/java/org/springframework/http/server/AsyncServletServerHttpRequest.java deleted file mode 100644 index ded710585a..0000000000 --- a/spring-web/src/main/java/org/springframework/http/server/AsyncServletServerHttpRequest.java +++ /dev/null @@ -1,154 +0,0 @@ -/* - * Copyright 2002-2013 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. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.http.server; - -import java.io.IOException; -import java.util.ArrayList; -import java.util.List; -import java.util.concurrent.atomic.AtomicBoolean; - -import javax.servlet.AsyncContext; -import javax.servlet.AsyncEvent; -import javax.servlet.AsyncListener; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; - -import org.springframework.util.Assert; - - -public class AsyncServletServerHttpRequest extends ServletServerHttpRequest - implements AsyncServerHttpRequest, AsyncListener { - - private Long timeout; - - private AsyncContext asyncContext; - - private AtomicBoolean asyncCompleted = new AtomicBoolean(false); - - private final List timeoutHandlers = new ArrayList(); - - private final List completionHandlers = new ArrayList(); - - private final HttpServletResponse servletResponse; - - - /** - * Create a new instance for the given request/response pair. - */ - public AsyncServletServerHttpRequest(HttpServletRequest request, HttpServletResponse response) { - super(request); - this.servletResponse = response; - } - - /** - * Timeout period begins after the container thread has exited. - */ - @Override - public void setTimeout(long timeout) { - Assert.state(!isAsyncStarted(), "Cannot change the timeout with concurrent handling in progress"); - this.timeout = timeout; - } - - public void addTimeoutHandler(Runnable timeoutHandler) { - this.timeoutHandlers.add(timeoutHandler); - } - - public void addCompletionHandler(Runnable runnable) { - this.completionHandlers.add(runnable); - } - - @Override - public boolean isAsyncStarted() { - return ((this.asyncContext != null) && getServletRequest().isAsyncStarted()); - } - - /** - * Whether async request processing has completed. - *

It is important to avoid use of request and response objects after async - * processing has completed. Servlet containers often re-use them. - */ - @Override - public boolean isAsyncCompleted() { - return this.asyncCompleted.get(); - } - - @Override - public void startAsync() { - Assert.state(getServletRequest().isAsyncSupported(), - "Async support must be enabled on a servlet and for all filters involved " + - "in async request processing. This is done in Java code using the Servlet API " + - "or by adding \"true\" to servlet and " + - "filter declarations in web.xml."); - Assert.state(!isAsyncCompleted(), "Async processing has already completed"); - if (isAsyncStarted()) { - return; - } - this.asyncContext = getServletRequest().startAsync(getServletRequest(), this.servletResponse); - this.asyncContext.addListener(this); - if (this.timeout != null) { - this.asyncContext.setTimeout(this.timeout); - } - } - - @Override - public void completeAsync() { - Assert.notNull(this.asyncContext, "Cannot dispatch without an AsyncContext"); - if (isAsyncStarted() && !isAsyncCompleted()) { - this.asyncContext.complete(); - } - } - - - // --------------------------------------------------------------------- - // Implementation of AsyncListener methods - // --------------------------------------------------------------------- - - @Override - public void onStartAsync(AsyncEvent event) throws IOException { - } - - @Override - public void onError(AsyncEvent event) throws IOException { - } - - @Override - public void onTimeout(AsyncEvent event) throws IOException { - try { - for (Runnable handler : this.timeoutHandlers) { - handler.run(); - } - } - catch (Throwable t) { - // ignore - } - } - - @Override - public void onComplete(AsyncEvent event) throws IOException { - try { - for (Runnable handler : this.completionHandlers) { - handler.run(); - } - } - catch (Throwable t) { - // ignore - } - this.asyncContext = null; - this.asyncCompleted.set(true); - } - -} diff --git a/spring-web/src/main/java/org/springframework/http/server/ServerHttpAsyncResponseControl.java b/spring-web/src/main/java/org/springframework/http/server/ServerHttpAsyncResponseControl.java new file mode 100644 index 0000000000..55120c5c75 --- /dev/null +++ b/spring-web/src/main/java/org/springframework/http/server/ServerHttpAsyncResponseControl.java @@ -0,0 +1,57 @@ +/* + * Copyright 2002-2013 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.http.server; + + +/** + * A control that can put the processing of an HTTP request in asynchronous mode during + * which the response remains open until explicitly closed. + * + * @author Rossen Stoyanchev + * @since 4.0 + */ +public interface ServerHttpAsyncResponseControl { + + /** + * Enable asynchronous processing after which the response remains open until a call + * to {@link #complete()} is made or the server times out the request. Once enabled, + * additional calls to this method are ignored. + */ + void start(); + + /** + * A variation on {@link #start()} that allows specifying a timeout value to use to + * use for asynchronous processing. If {@link #complete()} is not called within the + * specified value, the request times out. + */ + void start(long timeout); + + /** + * Whether asynchronous request processing has been started. + */ + boolean hasStarted(); + + /** + * Causes asynchronous request processing to be completed. + */ + void complete(); + + /** + * Whether asynchronous request processing has been completed. + */ + boolean isCompleted(); + +} diff --git a/spring-web/src/main/java/org/springframework/http/server/ServletServerHttpAsyncRequestControl.java b/spring-web/src/main/java/org/springframework/http/server/ServletServerHttpAsyncRequestControl.java new file mode 100644 index 0000000000..5cc5b8dd40 --- /dev/null +++ b/spring-web/src/main/java/org/springframework/http/server/ServletServerHttpAsyncRequestControl.java @@ -0,0 +1,135 @@ +/* + * Copyright 2002-2013 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.http.server; + +import java.io.IOException; +import java.util.concurrent.atomic.AtomicBoolean; + +import javax.servlet.AsyncContext; +import javax.servlet.AsyncEvent; +import javax.servlet.AsyncListener; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.springframework.util.Assert; + + +/** + * A {@link ServerHttpAsyncResponseControl} to use on Servlet containers (Servlet 3.0+). + * + * @author Rossen Stoyanchev + * @since 4.0 + */ +public class ServletServerHttpAsyncRequestControl implements ServerHttpAsyncResponseControl, AsyncListener { + + private static long NO_TIMEOUT_VALUE = Long.MIN_VALUE; + + private final ServletServerHttpRequest request; + + private final ServletServerHttpResponse response; + + private AsyncContext asyncContext; + + private AtomicBoolean asyncCompleted = new AtomicBoolean(false); + + + /** + * Constructor accepting a request and response pair that are expected to be of type + * {@link ServletServerHttpRequest} and {@link ServletServerHttpResponse} + * respectively. + */ + public ServletServerHttpAsyncRequestControl(ServerHttpRequest request, ServerHttpResponse response) { + + Assert.notNull(request, "request is required"); + Assert.notNull(response, "response is required"); + + Assert.isInstanceOf(ServletServerHttpRequest.class, request); + Assert.isInstanceOf(ServletServerHttpResponse.class, response); + + this.request = (ServletServerHttpRequest) request; + this.response = (ServletServerHttpResponse) response; + + Assert.isTrue(this.request.getServletRequest().isAsyncSupported(), + "Async support must be enabled on a servlet and for all filters involved " + + "in async request processing. This is done in Java code using the Servlet API " + + "or by adding \"true\" to servlet and " + + "filter declarations in web.xml. Also you must use a Servlet 3.0+ container"); + } + + + @Override + public boolean hasStarted() { + return ((this.asyncContext != null) && this.request.getServletRequest().isAsyncStarted()); + } + + @Override + public boolean isCompleted() { + return this.asyncCompleted.get(); + } + + @Override + public void start() { + start(NO_TIMEOUT_VALUE); + } + + @Override + public void start(long timeout) { + + Assert.state(!isCompleted(), "Async processing has already completed"); + + if (hasStarted()) { + return; + } + + HttpServletRequest servletRequest = this.request.getServletRequest(); + HttpServletResponse servletResponse = this.response.getServletResponse(); + + this.asyncContext = servletRequest.startAsync(servletRequest, servletResponse); + this.asyncContext.addListener(this); + + if (timeout != NO_TIMEOUT_VALUE) { + this.asyncContext.setTimeout(timeout); + } + } + + @Override + public void complete() { + if (hasStarted() && !isCompleted()) { + this.asyncContext.complete(); + } + } + + // --------------------------------------------------------------------- + // Implementation of AsyncListener methods + // --------------------------------------------------------------------- + + @Override + public void onComplete(AsyncEvent event) throws IOException { + this.asyncContext = null; + this.asyncCompleted.set(true); + } + + @Override + public void onStartAsync(AsyncEvent event) throws IOException { } + + @Override + public void onError(AsyncEvent event) throws IOException { } + + @Override + public void onTimeout(AsyncEvent event) throws IOException { } + +} diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/SockJsHttpRequestHandler.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/SockJsHttpRequestHandler.java index 2d152c9055..d2416fb07d 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/SockJsHttpRequestHandler.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/SockJsHttpRequestHandler.java @@ -22,9 +22,9 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.springframework.http.server.AsyncServletServerHttpRequest; import org.springframework.http.server.ServerHttpRequest; import org.springframework.http.server.ServerHttpResponse; +import org.springframework.http.server.ServletServerHttpRequest; import org.springframework.http.server.ServletServerHttpResponse; import org.springframework.util.Assert; import org.springframework.web.HttpRequestHandler; @@ -71,13 +71,13 @@ public class SockJsHttpRequestHandler implements HttpRequestHandler { } @Override - public void handleRequest(HttpServletRequest request, HttpServletResponse response) + public void handleRequest(HttpServletRequest servletRequest, HttpServletResponse servletResponse) throws ServletException, IOException { - ServerHttpRequest httpRequest = new AsyncServletServerHttpRequest(request, response); - ServerHttpResponse httpResponse = new ServletServerHttpResponse(response); + ServerHttpRequest serverRequest = new ServletServerHttpRequest(servletRequest); + ServerHttpResponse serverResponse = new ServletServerHttpResponse(servletResponse); - this.sockJsService.handleRequest(httpRequest, httpResponse, this.webSocketHandler); + this.sockJsService.handleRequest(serverRequest, serverResponse, this.webSocketHandler); } } diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/session/AbstractHttpSockJsSession.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/session/AbstractHttpSockJsSession.java index 983f2197c2..d41b06f279 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/session/AbstractHttpSockJsSession.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/session/AbstractHttpSockJsSession.java @@ -20,15 +20,16 @@ import java.io.IOException; import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.BlockingQueue; -import org.springframework.http.server.AsyncServerHttpRequest; +import org.springframework.http.server.ServerHttpAsyncResponseControl; import org.springframework.http.server.ServerHttpRequest; import org.springframework.http.server.ServerHttpResponse; +import org.springframework.http.server.ServletServerHttpAsyncRequestControl; import org.springframework.util.Assert; import org.springframework.web.socket.CloseStatus; import org.springframework.web.socket.WebSocketHandler; +import org.springframework.web.socket.sockjs.SockJsProcessingException; import org.springframework.web.socket.sockjs.support.frame.SockJsFrame; import org.springframework.web.socket.sockjs.support.frame.SockJsFrame.FrameFormat; -import org.springframework.web.socket.sockjs.SockJsProcessingException; import org.springframework.web.socket.support.ExceptionWebSocketHandlerDecorator; /** @@ -43,10 +44,12 @@ public abstract class AbstractHttpSockJsSession extends AbstractSockJsSession { private final BlockingQueue messageCache = new ArrayBlockingQueue(100); - private AsyncServerHttpRequest asyncRequest; + private ServerHttpRequest request; private ServerHttpResponse response; + private ServerHttpAsyncResponseControl asyncControl; + private String protocol; @@ -113,8 +116,7 @@ public abstract class AbstractHttpSockJsSession extends AbstractSockJsSession { return; } - this.asyncRequest.setTimeout(-1); - this.asyncRequest.startAsync(); + this.asyncControl.start(-1); scheduleHeartbeat(); tryFlushCache(); @@ -129,16 +131,16 @@ public abstract class AbstractHttpSockJsSession extends AbstractSockJsSession { Assert.notNull(request, "expected request"); Assert.notNull(response, "expected response"); Assert.notNull(frameFormat, "expected frameFormat"); - Assert.isInstanceOf(AsyncServerHttpRequest.class, request, "Expected AsyncServerHttpRequest"); - this.asyncRequest = (AsyncServerHttpRequest) request; + this.request = request; this.response = response; + this.asyncControl = new ServletServerHttpAsyncRequestControl(this.request, this.response); this.frameFormat = frameFormat; } @Override public synchronized boolean isActive() { - return ((this.asyncRequest != null) && (!this.asyncRequest.isAsyncCompleted())); + return ((this.asyncControl != null) && (!this.asyncControl.isCompleted())); } protected BlockingQueue getMessageCache() { @@ -146,7 +148,7 @@ public abstract class AbstractHttpSockJsSession extends AbstractSockJsSession { } protected ServerHttpRequest getRequest() { - return this.asyncRequest; + return this.request; } protected ServerHttpResponse getResponse() { @@ -178,17 +180,18 @@ public abstract class AbstractHttpSockJsSession extends AbstractSockJsSession { protected synchronized void resetRequest() { updateLastActiveTime(); - if (isActive() && this.asyncRequest.isAsyncStarted()) { + if (isActive() && this.asyncControl.hasStarted()) { try { logger.debug("Completing async request"); - this.asyncRequest.completeAsync(); + this.asyncControl.complete(); } catch (Throwable ex) { logger.error("Failed to complete async request: " + ex.getMessage()); } } - this.asyncRequest = null; + this.request = null; this.response = null; + this.asyncControl = null; } @Override diff --git a/spring-websocket/src/test/java/org/springframework/web/socket/AbstractHttpRequestTests.java b/spring-websocket/src/test/java/org/springframework/web/socket/AbstractHttpRequestTests.java index c33063dda6..63bd10b46c 100644 --- a/spring-websocket/src/test/java/org/springframework/web/socket/AbstractHttpRequestTests.java +++ b/spring-websocket/src/test/java/org/springframework/web/socket/AbstractHttpRequestTests.java @@ -17,14 +17,18 @@ package org.springframework.web.socket; import org.junit.Before; -import org.springframework.http.server.AsyncServletServerHttpRequest; +import org.springframework.http.server.ServerHttpAsyncResponseControl; import org.springframework.http.server.ServerHttpRequest; import org.springframework.http.server.ServerHttpResponse; +import org.springframework.http.server.ServletServerHttpAsyncRequestControl; +import org.springframework.http.server.ServletServerHttpRequest; import org.springframework.http.server.ServletServerHttpResponse; import org.springframework.mock.web.test.MockHttpServletRequest; import org.springframework.mock.web.test.MockHttpServletResponse; /** + * Base class for tests using {@link ServerHttpRequest} and {@link ServerHttpResponse}. + * * @author Rossen Stoyanchev */ public class AbstractHttpRequestTests { @@ -37,6 +41,8 @@ public class AbstractHttpRequestTests { protected MockHttpServletResponse servletResponse; + protected ServerHttpAsyncResponseControl asyncControl; + @Before public void setUp() { @@ -49,10 +55,15 @@ public class AbstractHttpRequestTests { } protected void resetRequestAndResponse() { + resetRequest(); resetResponse(); + this.asyncControl = new ServletServerHttpAsyncRequestControl(this.request, this.response); + } + + protected void resetRequest() { this.servletRequest = new MockHttpServletRequest(); this.servletRequest.setAsyncSupported(true); - this.request = new AsyncServletServerHttpRequest(this.servletRequest, this.servletResponse); + this.request = new ServletServerHttpRequest(this.servletRequest); } protected void resetResponse() { diff --git a/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/support/AbstractSockJsServiceTests.java b/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/support/AbstractSockJsServiceTests.java index 2e34aa92f7..29658cd8d6 100644 --- a/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/support/AbstractSockJsServiceTests.java +++ b/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/support/AbstractSockJsServiceTests.java @@ -146,7 +146,7 @@ public class AbstractSockJsServiceTests extends AbstractHttpRequestTests { String body = this.servletResponse.getContentAsString(); assertEquals("{\"entropy\"", body.substring(0, body.indexOf(':'))); - assertEquals(",\"origins\":[\"*:*\"],\"cookie_needed\":true,\"websocket\":true}", + assertEquals(",\"origins\":[\"*:*\"],\"cookie_needed\":false,\"websocket\":true}", body.substring(body.indexOf(','))); this.service.setDummySessionCookieEnabled(false); diff --git a/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/transport/session/AbstractHttpSockJsSessionTests.java b/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/transport/session/AbstractHttpSockJsSessionTests.java index 39c3464929..b39f4ccf2d 100644 --- a/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/transport/session/AbstractHttpSockJsSessionTests.java +++ b/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/transport/session/AbstractHttpSockJsSessionTests.java @@ -20,9 +20,9 @@ import java.io.IOException; import org.junit.Before; import org.junit.Test; -import org.springframework.http.server.AsyncServletServerHttpRequest; import org.springframework.http.server.ServerHttpRequest; import org.springframework.http.server.ServerHttpResponse; +import org.springframework.http.server.ServletServerHttpRequest; import org.springframework.http.server.ServletServerHttpResponse; import org.springframework.mock.web.test.MockHttpServletRequest; import org.springframework.mock.web.test.MockHttpServletResponse; @@ -31,7 +31,6 @@ import org.springframework.web.socket.WebSocketHandler; import org.springframework.web.socket.sockjs.support.frame.SockJsFrame; import org.springframework.web.socket.sockjs.support.frame.SockJsFrame.DefaultFrameFormat; import org.springframework.web.socket.sockjs.support.frame.SockJsFrame.FrameFormat; -import org.springframework.web.socket.sockjs.transport.session.AbstractHttpSockJsSession; import org.springframework.web.socket.sockjs.transport.session.AbstractHttpSockJsSessionTests.TestAbstractHttpSockJsSession; import static org.junit.Assert.*; @@ -67,7 +66,7 @@ public class AbstractHttpSockJsSessionTests extends BaseAbstractSockJsSessionTes this.servletRequest = new MockHttpServletRequest(); this.servletRequest.setAsyncSupported(true); - this.request = new AsyncServletServerHttpRequest(this.servletRequest, this.servletResponse); + this.request = new ServletServerHttpRequest(this.servletRequest); } @Override