Skip to content

Commit 1a579a6

Browse files
GH-8600: Fix WebSocket removeRegistration (#8601)
* GH-8600: Fix WebSocket `removeRegistration` Fixes #8600 When we register a dynamic WebSocket endpoint and use a `WebSocketHandlerDecoratorFactory` such an endpoint is not removed on an `IntegrationFlow` destruction. The actual `WebSocketHandler` is decorated, however we still use an initial one for condition. * Refactor `IntegrationWebSocketContainer` to expose a `protected` setter for the `WebSocketHandler` which is called from the `ServerWebSocketContainer` after decoration **Cherry-pick to `6.0.x` & `5.5.x`** * Fix language in Javadocs Co-authored-by: Gary Russell <grussell@vmware.com> --------- Co-authored-by: Gary Russell <grussell@vmware.com>
1 parent 934e90a commit 1a579a6

File tree

4 files changed

+33
-10
lines changed

4 files changed

+33
-10
lines changed

spring-integration-websocket/src/main/java/org/springframework/integration/websocket/ClientWebSocketContainer.java

+2-3
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ public void stop(Runnable callback) {
208208
* <p>
209209
* Opened {@link WebSocketSession} is populated to the wrapping {@link ClientWebSocketContainer}.
210210
* <p>
211-
* The {@link #webSocketHandler} is used to handle {@link WebSocketSession} events.
211+
* The {@link #getWebSocketHandler()} is used to handle {@link WebSocketSession} events.
212212
*/
213213
private final class IntegrationWebSocketConnectionManager extends ConnectionManagerSupport {
214214

@@ -258,8 +258,7 @@ protected void openConnection() {
258258
}
259259
ClientWebSocketContainer.this.headers.setSecWebSocketProtocol(getSubProtocols());
260260
CompletableFuture<WebSocketSession> future =
261-
this.client.execute(ClientWebSocketContainer.this.webSocketHandler,
262-
ClientWebSocketContainer.this.headers, getUri());
261+
this.client.execute(getWebSocketHandler(), ClientWebSocketContainer.this.headers, getUri());
263262

264263
future.whenComplete((session, throwable) -> {
265264
if (throwable == null) {

spring-integration-websocket/src/main/java/org/springframework/integration/websocket/IntegrationWebSocketContainer.java

+12-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2014-2022 the original author or authors.
2+
* Copyright 2014-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -67,8 +67,8 @@ public abstract class IntegrationWebSocketContainer implements DisposableBean {
6767

6868
protected final Log logger = LogFactory.getLog(getClass()); // NOSONAR
6969

70-
protected final WebSocketHandler webSocketHandler = new IntegrationWebSocketHandler(); // NOSONAR
71-
70+
private WebSocketHandler webSocketHandler = new IntegrationWebSocketHandler();
71+
7272
protected final Map<String, WebSocketSession> sessions = new ConcurrentHashMap<>(); // NOSONAR
7373

7474
private final List<String> supportedProtocols = new ArrayList<>();
@@ -104,6 +104,15 @@ public void addSupportedProtocols(String... protocols) {
104104
}
105105
}
106106

107+
/**
108+
* Replace the default {@link WebSocketHandler} with the one provided here, e.g. via decoration factories.
109+
* @param handler the actual {@link WebSocketHandler} to replace.
110+
* @since 5.5.18
111+
*/
112+
protected void setWebSocketHandler(WebSocketHandler handler) {
113+
this.webSocketHandler = handler;
114+
}
115+
107116
public WebSocketHandler getWebSocketHandler() {
108117
return this.webSocketHandler;
109118
}

spring-integration-websocket/src/main/java/org/springframework/integration/websocket/ServerWebSocketContainer.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2014-2022 the original author or authors.
2+
* Copyright 2014-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -150,11 +150,12 @@ public TaskScheduler getSockJsTaskScheduler() {
150150

151151
@Override
152152
public void registerWebSocketHandlers(WebSocketHandlerRegistry registry) {
153-
WebSocketHandler webSocketHandler = this.webSocketHandler;
153+
WebSocketHandler webSocketHandler = getWebSocketHandler();
154154

155155
if (this.decoratorFactories != null) {
156156
for (WebSocketHandlerDecoratorFactory factory : this.decoratorFactories) {
157157
webSocketHandler = factory.decorate(webSocketHandler);
158+
setWebSocketHandler(webSocketHandler);
158159
}
159160
}
160161

spring-integration-websocket/src/test/java/org/springframework/integration/websocket/dsl/WebSocketDslTests.java

+16-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2021-2022 the original author or authors.
2+
* Copyright 2021-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,6 +16,8 @@
1616

1717
package org.springframework.integration.websocket.dsl;
1818

19+
import java.util.concurrent.atomic.AtomicReference;
20+
1921
import jakarta.websocket.DeploymentException;
2022
import org.junit.jupiter.api.Test;
2123

@@ -37,6 +39,7 @@
3739
import org.springframework.test.annotation.DirtiesContext;
3840
import org.springframework.test.context.junit.jupiter.SpringJUnitConfig;
3941
import org.springframework.web.context.support.AnnotationConfigWebApplicationContext;
42+
import org.springframework.web.socket.WebSocketHandler;
4043
import org.springframework.web.socket.client.WebSocketClient;
4144
import org.springframework.web.socket.client.standard.StandardWebSocketClient;
4245
import org.springframework.web.socket.server.HandshakeHandler;
@@ -46,6 +49,9 @@
4649
import static org.assertj.core.api.Assertions.assertThat;
4750
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
4851
import static org.awaitility.Awaitility.await;
52+
import static org.mockito.ArgumentMatchers.any;
53+
import static org.mockito.Mockito.spy;
54+
import static org.mockito.Mockito.verify;
4955

5056
@SpringJUnitConfig(classes = WebSocketDslTests.ClientConfig.class)
5157
@DirtiesContext
@@ -61,13 +67,19 @@ public class WebSocketDslTests {
6167
IntegrationFlowContext integrationFlowContext;
6268

6369
@Test
64-
void testDynamicServerEndpointRegistration() {
70+
void testDynamicServerEndpointRegistration() throws Exception {
6571
// Dynamic server flow
6672
AnnotationConfigWebApplicationContext serverContext = this.server.getServerContext();
6773
IntegrationFlowContext serverIntegrationFlowContext = serverContext.getBean(IntegrationFlowContext.class);
74+
AtomicReference<WebSocketHandler> decoratedHandler = new AtomicReference<>();
6875
ServerWebSocketContainer serverWebSocketContainer =
6976
new ServerWebSocketContainer("/dynamic")
7077
.setHandshakeHandler(serverContext.getBean(HandshakeHandler.class))
78+
.setDecoratorFactories(handler -> {
79+
WebSocketHandler spy = spy(handler);
80+
decoratedHandler.set(spy);
81+
return spy;
82+
})
7183
.withSockJs();
7284

7385
WebSocketInboundChannelAdapter webSocketInboundChannelAdapter =
@@ -106,6 +118,8 @@ void testDynamicServerEndpointRegistration() {
106118
.extracting(Message::getPayload)
107119
.isEqualTo("dynamic test");
108120

121+
verify(decoratedHandler.get()).handleMessage(any(), any());
122+
109123
dynamicServerFlow.destroy();
110124

111125
await() // Looks like endpoint is removed on the server side somewhat async

0 commit comments

Comments
 (0)