From d73c1d5693ba30cd47109f060fcb4497f407b961 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Thu, 10 Jul 2014 10:17:56 -0400 Subject: [PATCH] Update WebSocket client SmartLifecycle support Before this change WebSocketConnectionManager delegated SmartLifecycle methods to the client instance it contained. After this change WebSocketClient implementations are expected to implement Lifecycle (rather than SmartLifecycle). The need for this is even more evident with SockJsClient, which is a WebSocketClient implementation and contains a WebSocketTransport that in turn contains the actual WebSocketClient. In this case WebSocketConnectionManager as the top level container is the obvious place to configure autostartup while Lifecycle events can be propagated all the way down to the root WebSocketClient. --- .../client/ConnectionManagerSupport.java | 8 +-- .../client/WebSocketConnectionManager.java | 14 ++-- .../client/jetty/JettyWebSocketClient.java | 37 ++-------- .../WebSocketConnectionManagerTests.java | 68 ++++--------------- 4 files changed, 30 insertions(+), 97 deletions(-) diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/client/ConnectionManagerSupport.java b/spring-websocket/src/main/java/org/springframework/web/socket/client/ConnectionManagerSupport.java index 5ebedbb517b..3f65e0bece8 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/client/ConnectionManagerSupport.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/client/ConnectionManagerSupport.java @@ -42,7 +42,7 @@ public abstract class ConnectionManagerSupport implements SmartLifecycle { private boolean autoStartup = false; - private boolean isRunning = false; + private boolean running = false; private int phase = Integer.MAX_VALUE; @@ -104,7 +104,7 @@ public abstract class ConnectionManagerSupport implements SmartLifecycle { @Override public boolean isRunning() { synchronized (this.lifecycleMonitor) { - return this.isRunning; + return this.running; } } @@ -125,7 +125,7 @@ public abstract class ConnectionManagerSupport implements SmartLifecycle { if (logger.isInfoEnabled()) { logger.info("Starting " + this.getClass().getSimpleName()); } - this.isRunning = true; + this.running = true; openConnection(); } } @@ -146,7 +146,7 @@ public abstract class ConnectionManagerSupport implements SmartLifecycle { logger.error("Failed to stop WebSocket connection", e); } finally { - this.isRunning = false; + this.running = false; } } } diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/client/WebSocketConnectionManager.java b/spring-websocket/src/main/java/org/springframework/web/socket/client/WebSocketConnectionManager.java index 85a90cb2ab9..0da13bd898e 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/client/WebSocketConnectionManager.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/client/WebSocketConnectionManager.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2014 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.web.socket.client; import java.util.List; +import org.springframework.context.Lifecycle; import org.springframework.context.SmartLifecycle; import org.springframework.http.HttpHeaders; import org.springframework.util.concurrent.ListenableFuture; @@ -46,8 +47,6 @@ public class WebSocketConnectionManager extends ConnectionManagerSupport { private WebSocketHttpHeaders headers = new WebSocketHttpHeaders(); - private final boolean syncClientLifecycle; - public WebSocketConnectionManager(WebSocketClient client, WebSocketHandler webSocketHandler, String uriTemplate, Object... uriVariables) { @@ -55,7 +54,6 @@ public class WebSocketConnectionManager extends ConnectionManagerSupport { super(uriTemplate, uriVariables); this.client = client; this.webSocketHandler = decorateWebSocketHandler(webSocketHandler); - this.syncClientLifecycle = ((client instanceof SmartLifecycle) && !((SmartLifecycle) client).isRunning()); } @@ -116,16 +114,16 @@ public class WebSocketConnectionManager extends ConnectionManagerSupport { @Override public void startInternal() { - if (this.syncClientLifecycle) { - ((SmartLifecycle) this.client).start(); + if (this.client instanceof Lifecycle && !((Lifecycle) client).isRunning()) { + ((Lifecycle) client).start(); } super.startInternal(); } @Override public void stopInternal() throws Exception { - if (this.syncClientLifecycle) { - ((SmartLifecycle) this.client).stop(); + if (this.client instanceof Lifecycle && ((Lifecycle) client).isRunning()) { + ((Lifecycle) client).stop(); } super.stopInternal(); } diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/client/jetty/JettyWebSocketClient.java b/spring-websocket/src/main/java/org/springframework/web/socket/client/jetty/JettyWebSocketClient.java index f9e6f8a9d1c..67aec6a3152 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/client/jetty/JettyWebSocketClient.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/client/jetty/JettyWebSocketClient.java @@ -26,7 +26,7 @@ import java.util.concurrent.Future; import org.eclipse.jetty.websocket.api.Session; import org.eclipse.jetty.websocket.client.ClientUpgradeRequest; import org.eclipse.jetty.websocket.client.WebSocketClient; -import org.springframework.context.SmartLifecycle; +import org.springframework.context.Lifecycle; import org.springframework.core.task.AsyncListenableTaskExecutor; import org.springframework.core.task.SimpleAsyncTaskExecutor; import org.springframework.core.task.TaskExecutor; @@ -47,17 +47,18 @@ import org.springframework.web.util.UriComponentsBuilder; * Initiates WebSocket requests to a WebSocket server programmatically * through the Jetty WebSocket API. * + *

As of 4.1 this class implements {@link Lifecycle} rather than + * {@link org.springframework.context.SmartLifecycle}. Use + * {@link org.springframework.web.socket.client.WebSocketConnectionManager + * WebSocketConnectionManager} instead to auto-start a WebSocket connection. + * * @author Rossen Stoyanchev * @since 4.0 */ -public class JettyWebSocketClient extends AbstractWebSocketClient implements SmartLifecycle { +public class JettyWebSocketClient extends AbstractWebSocketClient implements Lifecycle { private final org.eclipse.jetty.websocket.client.WebSocketClient client; - private boolean autoStartup = true; - - private int phase = Integer.MAX_VALUE; - private final Object lifecycleMonitor = new Object(); private AsyncListenableTaskExecutor taskExecutor = new SimpleAsyncTaskExecutor(); @@ -98,24 +99,6 @@ public class JettyWebSocketClient extends AbstractWebSocketClient implements Sma return this.taskExecutor; } - public void setAutoStartup(boolean autoStartup) { - this.autoStartup = autoStartup; - } - - @Override - public boolean isAutoStartup() { - return this.autoStartup; - } - - public void setPhase(int phase) { - this.phase = phase; - } - - @Override - public int getPhase() { - return this.phase; - } - @Override public boolean isRunning() { synchronized (this.lifecycleMonitor) { @@ -157,12 +140,6 @@ public class JettyWebSocketClient extends AbstractWebSocketClient implements Sma } } - @Override - public void stop(Runnable callback) { - this.stop(); - callback.run(); - } - @Override public ListenableFuture doHandshake(WebSocketHandler webSocketHandler, String uriTemplate, Object... uriVars) { diff --git a/spring-websocket/src/test/java/org/springframework/web/socket/client/WebSocketConnectionManagerTests.java b/spring-websocket/src/test/java/org/springframework/web/socket/client/WebSocketConnectionManagerTests.java index 2c7827a9d00..fb580a4e959 100644 --- a/spring-websocket/src/test/java/org/springframework/web/socket/client/WebSocketConnectionManagerTests.java +++ b/spring-websocket/src/test/java/org/springframework/web/socket/client/WebSocketConnectionManagerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2014 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. @@ -19,17 +19,16 @@ package org.springframework.web.socket.client; import java.net.URI; import java.util.Arrays; import java.util.List; -import java.util.concurrent.Callable; import org.junit.Test; -import org.springframework.context.SmartLifecycle; +import org.springframework.context.Lifecycle; import org.springframework.http.HttpHeaders; import org.springframework.util.concurrent.ListenableFuture; import org.springframework.util.concurrent.ListenableFutureTask; import org.springframework.web.socket.WebSocketHandler; import org.springframework.web.socket.WebSocketSession; -import org.springframework.web.socket.handler.AbstractWebSocketHandler; import org.springframework.web.socket.handler.LoggingWebSocketHandlerDecorator; +import org.springframework.web.socket.handler.TextWebSocketHandler; import org.springframework.web.socket.handler.WebSocketHandlerDecorator; import org.springframework.web.socket.WebSocketHttpHeaders; import org.springframework.web.util.UriComponentsBuilder; @@ -43,13 +42,13 @@ import static org.junit.Assert.*; */ public class WebSocketConnectionManagerTests { + @Test public void openConnection() throws Exception { List subprotocols = Arrays.asList("abc"); TestLifecycleWebSocketClient client = new TestLifecycleWebSocketClient(false); - WebSocketHandler handler = new AbstractWebSocketHandler() { - }; + WebSocketHandler handler = new TextWebSocketHandler(); WebSocketConnectionManager manager = new WebSocketConnectionManager(client, handler , "/path/{id}", "123"); manager.setSubProtocols(subprotocols); @@ -68,10 +67,9 @@ public class WebSocketConnectionManagerTests { } @Test - public void syncClientLifecycle() throws Exception { + public void clientLifecycle() throws Exception { TestLifecycleWebSocketClient client = new TestLifecycleWebSocketClient(false); - WebSocketHandler handler = new AbstractWebSocketHandler() { - }; + WebSocketHandler handler = new TextWebSocketHandler(); WebSocketConnectionManager manager = new WebSocketConnectionManager(client, handler , "/a"); manager.startInternal(); @@ -81,23 +79,8 @@ public class WebSocketConnectionManagerTests { assertFalse(client.isRunning()); } - @Test - public void dontSyncClientLifecycle() throws Exception { - TestLifecycleWebSocketClient client = new TestLifecycleWebSocketClient(true); - WebSocketHandler handler = new AbstractWebSocketHandler() { - }; - WebSocketConnectionManager manager = new WebSocketConnectionManager(client, handler , "/a"); - - manager.startInternal(); - assertTrue(client.isRunning()); - - manager.stopInternal(); - assertTrue(client.isRunning()); - } - - - private static class TestLifecycleWebSocketClient implements WebSocketClient, SmartLifecycle { + private static class TestLifecycleWebSocketClient implements WebSocketClient, Lifecycle { private boolean running; @@ -128,42 +111,17 @@ public class WebSocketConnectionManagerTests { } @Override - public int getPhase() { - return 0; - } - - @Override - public boolean isAutoStartup() { - return false; - } - - @Override - public void stop(Runnable callback) { - this.running = false; - } - - @Override - public ListenableFuture doHandshake(WebSocketHandler webSocketHandler, - String uriTemplate, Object... uriVars) { - + public ListenableFuture doHandshake(WebSocketHandler handler, String uriTemplate, Object... uriVars) { URI uri = UriComponentsBuilder.fromUriString(uriTemplate).buildAndExpand(uriVars).encode().toUri(); - return doHandshake(webSocketHandler, null, uri); + return doHandshake(handler, null, uri); } @Override - public ListenableFuture doHandshake(WebSocketHandler webSocketHandler, - WebSocketHttpHeaders headers, URI uri) { - - this.webSocketHandler = webSocketHandler; + public ListenableFuture doHandshake(WebSocketHandler handler, WebSocketHttpHeaders headers, URI uri) { + this.webSocketHandler = handler; this.headers = headers; this.uri = uri; - - return new ListenableFutureTask(new Callable() { - @Override - public WebSocketSession call() throws Exception { - return null; - } - }); + return new ListenableFutureTask<>(() -> null); } }