Polish SockJS exception handling and javadoc

See javadoc in SockJsService for details.

Also remove ReadOnlyMultiValueMap, CollectionUtils has a method for
that already.

spring-websocket/src/test/java/org/springframework/web/socket/sockjs/support/AbstractSockJsServiceTests.java

@@ -27,7 +27,7 @@ import org.springframework.scheduling.TaskScheduler;
 import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler;
 import org.springframework.web.socket.AbstractHttpRequestTests;
 import org.springframework.web.socket.WebSocketHandler;
-import org.springframework.web.socket.sockjs.SockJsProcessingException;
+import org.springframework.web.socket.sockjs.SockJsException;
 import org.springframework.web.socket.sockjs.transport.TransportType;
 
 import static org.junit.Assert.*;
@@ -246,7 +246,7 @@ public class AbstractSockJsServiceTests extends AbstractHttpRequestTests {
 
 		@Override
 		protected void handleTransportRequest(ServerHttpRequest req, ServerHttpResponse res, WebSocketHandler handler,
-				String sessionId, String transport) throws IOException, SockJsProcessingException {
+				String sessionId, String transport) throws IOException, SockJsException {
 
 			this.sessionId = sessionId;
 			this.transport = transport;

spring-websocket/src/test/java/org/springframework/web/socket/sockjs/transport/handler/HttpReceivingTransportHandlerTests.java

@@ -20,13 +20,12 @@ import org.junit.Before;
 import org.junit.Test;
 import org.springframework.http.MediaType;
 import org.springframework.web.socket.AbstractHttpRequestTests;
-import org.springframework.web.socket.CloseStatus;
 import org.springframework.web.socket.TextMessage;
 import org.springframework.web.socket.WebSocketHandler;
-import org.springframework.web.socket.sockjs.SockJsProcessingException;
+import org.springframework.web.socket.sockjs.SockJsMessageDeliveryException;
 import org.springframework.web.socket.sockjs.transport.session.AbstractSockJsSession;
 import org.springframework.web.socket.sockjs.transport.session.StubSockJsServiceConfig;
-import org.springframework.web.socket.sockjs.transport.session.TestSockJsSession;
+import org.springframework.web.socket.sockjs.transport.session.TestHttpSockJsSession;
 
 import static org.junit.Assert.*;
 import static org.mockito.Mockito.*;
@@ -107,48 +106,47 @@ public class HttpReceivingTransportHandlerTests  extends AbstractHttpRequestTest
 
 		this.servletRequest.setContent("[\"x\"]".getBytes("UTF-8"));
 
-		WebSocketHandler webSocketHandler = mock(WebSocketHandler.class);
-		TestSockJsSession session = new TestSockJsSession("1", sockJsConfig, webSocketHandler);
+		WebSocketHandler wsHandler = mock(WebSocketHandler.class);
+		TestHttpSockJsSession session = new TestHttpSockJsSession("1", sockJsConfig, wsHandler);
 		session.delegateConnectionEstablished();
 
-		doThrow(new Exception()).when(webSocketHandler).handleMessage(session, new TextMessage("x"));
+		doThrow(new Exception()).when(wsHandler).handleMessage(session, new TextMessage("x"));
 
 		try {
 			XhrTransportHandler transportHandler = new XhrTransportHandler();
 			transportHandler.setSockJsServiceConfiguration(sockJsConfig);
-			transportHandler.handleRequest(this.request, this.response, webSocketHandler, session);
+			transportHandler.handleRequest(this.request, this.response, wsHandler, session);
 			fail("Expected exception");
 		}
-		catch (SockJsProcessingException ex) {
-			assertEquals(CloseStatus.SERVER_ERROR, session.getStatus());
+		catch (SockJsMessageDeliveryException ex) {
+			assertNull(session.getCloseStatus());
 		}
 	}
 
 
-	private void handleRequest(AbstractHttpReceivingTransportHandler transportHandler)
-			throws Exception {
+	private void handleRequest(AbstractHttpReceivingTransportHandler transportHandler) throws Exception {
 
-		WebSocketHandler webSocketHandler = mock(WebSocketHandler.class);
-		AbstractSockJsSession session = new TestSockJsSession("1", new StubSockJsServiceConfig(), webSocketHandler);
+		WebSocketHandler wsHandler = mock(WebSocketHandler.class);
+		AbstractSockJsSession session = new TestHttpSockJsSession("1", new StubSockJsServiceConfig(), wsHandler);
 
 		transportHandler.setSockJsServiceConfiguration(new StubSockJsServiceConfig());
-		transportHandler.handleRequest(this.request, this.response, webSocketHandler, session);
+		transportHandler.handleRequest(this.request, this.response, wsHandler, session);
 
 		assertEquals("text/plain;charset=UTF-8", this.response.getHeaders().getContentType().toString());
-		verify(webSocketHandler).handleMessage(session, new TextMessage("x"));
+		verify(wsHandler).handleMessage(session, new TextMessage("x"));
 	}
 
 	private void handleRequestAndExpectFailure() throws Exception {
 
 		resetResponse();
 
-		WebSocketHandler webSocketHandler = mock(WebSocketHandler.class);
-		AbstractSockJsSession session = new TestSockJsSession("1", new StubSockJsServiceConfig(), webSocketHandler);
+		WebSocketHandler wsHandler = mock(WebSocketHandler.class);
+		AbstractSockJsSession session = new TestHttpSockJsSession("1", new StubSockJsServiceConfig(), wsHandler);
 
-		new XhrTransportHandler().handleRequest(this.request, this.response, webSocketHandler, session);
+		new XhrTransportHandler().handleRequest(this.request, this.response, wsHandler, session);
 
 		assertEquals(500, this.servletResponse.getStatus());
-		verifyNoMoreInteractions(webSocketHandler);
+		verifyNoMoreInteractions(wsHandler);
 	}
 
 }

spring-websocket/src/test/java/org/springframework/web/socket/sockjs/transport/session/AbstractHttpSockJsSessionTests.java

@@ -151,7 +151,7 @@ public class AbstractHttpSockJsSessionTests extends BaseAbstractSockJsSessionTes
 		}
 
 		@Override
-		protected void flushCache() throws IOException {
+		protected void flushCache() {
 			this.cacheFlushed = true;
 		}
 

spring-websocket/src/test/java/org/springframework/web/socket/sockjs/transport/session/AbstractSockJsSessionTests.java

@@ -18,15 +18,18 @@ package org.springframework.web.socket.sockjs.transport.session;
 
 import java.io.IOException;
 import java.sql.Date;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.concurrent.ScheduledFuture;
 
 import org.junit.Test;
 import org.springframework.web.socket.CloseStatus;
 import org.springframework.web.socket.TextMessage;
-import org.springframework.web.socket.sockjs.SockJsProcessingException;
+import org.springframework.web.socket.WebSocketHandler;
+import org.springframework.web.socket.sockjs.SockJsMessageDeliveryException;
+import org.springframework.web.socket.sockjs.SockJsTransportFailureException;
 import org.springframework.web.socket.sockjs.support.frame.SockJsFrame;
-import org.springframework.web.socket.sockjs.transport.session.AbstractSockJsSession;
+import org.springframework.web.socket.support.ExceptionWebSocketHandlerDecorator;
 
 import static org.junit.Assert.*;
 import static org.mockito.Matchers.*;
@@ -95,6 +98,33 @@ public class AbstractSockJsSessionTests extends BaseAbstractSockJsSessionTests<T
 		verifyNoMoreInteractions(this.webSocketHandler);
 	}
 
+	@Test
+	public void delegateMessagesWithErrorAndConnectionClosing() throws Exception {
+
+		WebSocketHandler wsHandler = new ExceptionWebSocketHandlerDecorator(this.webSocketHandler);
+		TestSockJsSession sockJsSession = new TestSockJsSession("1", this.sockJsConfig, wsHandler);
+
+		String msg1 = "message 1";
+		String msg2 = "message 2";
+		String msg3 = "message 3";
+
+		doThrow(new IOException()).when(this.webSocketHandler).handleMessage(sockJsSession, new TextMessage(msg2));
+
+		sockJsSession.delegateConnectionEstablished();
+		try {
+			sockJsSession.delegateMessages(new String[] { msg1, msg2, msg3 });
+			fail("expected exception");
+		}
+		catch (SockJsMessageDeliveryException ex) {
+			assertEquals(Arrays.asList(msg3), ex.getUndeliveredMessages());
+			verify(this.webSocketHandler).afterConnectionEstablished(sockJsSession);
+			verify(this.webSocketHandler).handleMessage(sockJsSession, new TextMessage(msg1));
+			verify(this.webSocketHandler).handleMessage(sockJsSession, new TextMessage(msg2));
+			verify(this.webSocketHandler).afterConnectionClosed(sockJsSession, CloseStatus.SERVER_ERROR);
+			verifyNoMoreInteractions(this.webSocketHandler);
+		}
+	}
+
 	@Test
 	public void delegateConnectionClosed() throws Exception {
 		this.session.delegateConnectionEstablished();
@@ -112,17 +142,17 @@ public class AbstractSockJsSessionTests extends BaseAbstractSockJsSessionTests<T
 		assertNew();
 
 		this.session.close();
-		assertNull("Close not ignored for a new session", this.session.getStatus());
+		assertNull("Close not ignored for a new session", this.session.getCloseStatus());
 
 		this.session.delegateConnectionEstablished();
 		assertOpen();
 
 		this.session.close();
 		assertClosed();
-		assertEquals(3000, this.session.getStatus().getCode());
+		assertEquals(3000, this.session.getCloseStatus().getCode());
 
 		this.session.close(CloseStatus.SERVER_ERROR);
-		assertEquals("Close should be ignored if already closed", 3000, this.session.getStatus().getCode());
+		assertEquals("Close should be ignored if already closed", 3000, this.session.getCloseStatus().getCode());
 	}
 
 	@Test
@@ -152,7 +182,7 @@ public class AbstractSockJsSessionTests extends BaseAbstractSockJsSessionTests<T
 		assertEquals(1, this.session.getNumberOfLastActiveTimeUpdates());
 		assertTrue(this.session.didCancelHeartbeat());
 
-		assertEquals(new CloseStatus(3000, "Go away!"), this.session.getStatus());
+		assertEquals(new CloseStatus(3000, "Go away!"), this.session.getCloseStatus());
 		assertClosed();
 		verify(this.webSocketHandler).afterConnectionClosed(this.session, new CloseStatus(3000, "Go away!"));
 	}
@@ -160,13 +190,13 @@ public class AbstractSockJsSessionTests extends BaseAbstractSockJsSessionTests<T
 	@Test
 	public void closeWithWriteFrameExceptions() throws Exception {
 
-		this.session.setExceptionOnWriteFrame(new IOException());
+		this.session.setExceptionOnWrite(new IOException());
 
 		this.session.delegateConnectionEstablished();
 		this.session.setActive(true);
 		this.session.close();
 
-		assertEquals(new CloseStatus(3000, "Go away!"), this.session.getStatus());
+		assertEquals(new CloseStatus(3000, "Go away!"), this.session.getCloseStatus());
 		assertClosed();
 	}
 
@@ -179,7 +209,7 @@ public class AbstractSockJsSessionTests extends BaseAbstractSockJsSessionTests<T
 		this.session.setActive(true);
 		this.session.close(CloseStatus.NORMAL);
 
-		assertEquals(CloseStatus.NORMAL, this.session.getStatus());
+		assertEquals(CloseStatus.NORMAL, this.session.getCloseStatus());
 		assertClosed();
 	}
 
@@ -193,28 +223,14 @@ public class AbstractSockJsSessionTests extends BaseAbstractSockJsSessionTests<T
 
 	@Test
 	public void writeFrameIoException() throws Exception {
-		this.session.setExceptionOnWriteFrame(new IOException());
+		this.session.setExceptionOnWrite(new IOException());
 		this.session.delegateConnectionEstablished();
 		try {
 			this.session.writeFrame(SockJsFrame.openFrame());
 			fail("expected exception");
 		}
-		catch (IOException ex) {
-			assertEquals(CloseStatus.SERVER_ERROR, this.session.getStatus());
-			verify(this.webSocketHandler).afterConnectionClosed(this.session, CloseStatus.SERVER_ERROR);
-		}
-	}
-
-	@Test
-	public void writeFrameThrowable() throws Exception {
-		this.session.setExceptionOnWriteFrame(new NullPointerException());
-		this.session.delegateConnectionEstablished();
-		try {
-			this.session.writeFrame(SockJsFrame.openFrame());
-			fail("expected exception");
-		}
-		catch (SockJsProcessingException ex) {
-			assertEquals(CloseStatus.SERVER_ERROR, this.session.getStatus());
+		catch (SockJsTransportFailureException ex) {
+			assertEquals(CloseStatus.SERVER_ERROR, this.session.getCloseStatus());
 			verify(this.webSocketHandler).afterConnectionClosed(this.session, CloseStatus.SERVER_ERROR);
 		}
 	}

spring-websocket/src/test/java/org/springframework/web/socket/sockjs/transport/session/TestHttpSockJsSession.java

@@ -0,0 +1,120 @@
+/*
+ * 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.web.socket.sockjs.transport.session;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+
+import org.springframework.web.socket.CloseStatus;
+import org.springframework.web.socket.WebSocketHandler;
+import org.springframework.web.socket.sockjs.SockJsTransportFailureException;
+import org.springframework.web.socket.sockjs.support.frame.SockJsFrame;
+
+/**
+ * @author Rossen Stoyanchev
+ */
+public class TestHttpSockJsSession extends AbstractHttpSockJsSession {
+
+	private boolean active;
+
+	private final List<SockJsFrame> sockJsFrames = new ArrayList<>();
+
+	private CloseStatus closeStatus;
+
+	private IOException exceptionOnWrite;
+
+	private int numberOfLastActiveTimeUpdates;
+
+	private boolean cancelledHeartbeat;
+
+	private String subProtocol;
+
+
+	public TestHttpSockJsSession(String sessionId, SockJsServiceConfig config, WebSocketHandler handler) {
+		super(sessionId, config, handler);
+	}
+
+	@Override
+	public String getAcceptedProtocol() {
+		return this.subProtocol;
+	}
+
+	@Override
+	public void setAcceptedProtocol(String protocol) {
+		this.subProtocol = protocol;
+	}
+
+	public CloseStatus getCloseStatus() {
+		return this.closeStatus;
+	}
+
+	@Override
+	public boolean isActive() {
+		return this.active;
+	}
+
+	public void setActive(boolean active) {
+		this.active = active;
+	}
+
+	public List<SockJsFrame> getSockJsFramesWritten() {
+		return this.sockJsFrames;
+	}
+
+	public void setExceptionOnWrite(IOException exceptionOnWrite) {
+		this.exceptionOnWrite = exceptionOnWrite;
+	}
+
+	public int getNumberOfLastActiveTimeUpdates() {
+		return this.numberOfLastActiveTimeUpdates;
+	}
+
+	public boolean didCancelHeartbeat() {
+		return this.cancelledHeartbeat;
+	}
+
+	@Override
+	protected void updateLastActiveTime() {
+		this.numberOfLastActiveTimeUpdates++;
+		super.updateLastActiveTime();
+	}
+
+	@Override
+	protected void cancelHeartbeat() {
+		this.cancelledHeartbeat = true;
+		super.cancelHeartbeat();
+	}
+
+	@Override
+	protected void writeFrameInternal(SockJsFrame frame) throws IOException {
+		this.sockJsFrames.add(frame);
+		if (this.exceptionOnWrite != null) {
+			throw this.exceptionOnWrite;
+		}
+	}
+
+	@Override
+	protected void disconnect(CloseStatus status) {
+		this.closeStatus = status;
+	}
+
+	@Override
+	protected void flushCache() throws SockJsTransportFailureException {
+	}
+
+}

spring-websocket/src/test/java/org/springframework/web/socket/sockjs/transport/session/TestSockJsSession.java

@@ -23,7 +23,6 @@ import java.util.List;
 import org.springframework.web.socket.CloseStatus;
 import org.springframework.web.socket.WebSocketHandler;
 import org.springframework.web.socket.sockjs.support.frame.SockJsFrame;
-import org.springframework.web.socket.sockjs.transport.session.AbstractSockJsSession;
 
 /**
  * @author Rossen Stoyanchev
@@ -32,11 +31,11 @@ public class TestSockJsSession extends AbstractSockJsSession {
 
 	private boolean active;
 
-	private final List<SockJsFrame> sockJsFramesWritten = new ArrayList<>();
+	private final List<SockJsFrame> sockJsFrames = new ArrayList<>();
 
-	private CloseStatus status;
+	private CloseStatus closeStatus;
 
-	private Exception exceptionOnWriteFrame;
+	private IOException exceptionOnWrite;
 
 	private int numberOfLastActiveTimeUpdates;
 
@@ -59,8 +58,8 @@ public class TestSockJsSession extends AbstractSockJsSession {
 		this.subProtocol = protocol;
 	}
 
-	public CloseStatus getStatus() {
-		return this.status;
+	public CloseStatus getCloseStatus() {
+		return this.closeStatus;
 	}
 
 	@Override
@@ -73,11 +72,11 @@ public class TestSockJsSession extends AbstractSockJsSession {
 	}
 
 	public List<SockJsFrame> getSockJsFramesWritten() {
-		return this.sockJsFramesWritten;
+		return this.sockJsFrames;
 	}
 
-	public void setExceptionOnWriteFrame(Exception exceptionOnWriteFrame) {
-		this.exceptionOnWriteFrame = exceptionOnWriteFrame;
+	public void setExceptionOnWrite(IOException exceptionOnWrite) {
+		this.exceptionOnWrite = exceptionOnWrite;
 	}
 
 	public int getNumberOfLastActiveTimeUpdates() {
@@ -101,20 +100,20 @@ public class TestSockJsSession extends AbstractSockJsSession {
 	}
 
 	@Override
-	protected void sendMessageInternal(String message) throws IOException {
+	protected void sendMessageInternal(String message) {
 	}
 
 	@Override
-	protected void writeFrameInternal(SockJsFrame frame) throws Exception {
-		this.sockJsFramesWritten.add(frame);
-		if (this.exceptionOnWriteFrame != null) {
-			throw this.exceptionOnWriteFrame;
+	protected void writeFrameInternal(SockJsFrame frame) throws IOException {
+		this.sockJsFrames.add(frame);
+		if (this.exceptionOnWrite != null) {
+			throw this.exceptionOnWrite;
 		}
 	}
 
 	@Override
 	protected void disconnect(CloseStatus status) throws IOException {
-		this.status = status;
+		this.closeStatus = status;
 	}
 
 }