Skip to content

Commit 88016d4

Browse files
rstoyanchevrstoyanchev
committed
Fix issues in RSocketMessageHandler initialization
This commit ensures getRSocketStrategies() now reflects the state of corresponding RSocketMessageHandler properties even if those change after a call to setRSocketStrategies. RSocketMessageHandler has default Encoder/Decoder initializations consistent with the recent changes to RSocketStrategies.
1 parent 2bb5105 commit 88016d4

File tree

6 files changed

+289
-37
lines changed

6 files changed

+289
-37
lines changed

‎spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/reactive/MessageMappingMessageHandler.java

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,11 @@
3636
import org.springframework.context.EmbeddedValueResolverAware;
3737
import org.springframework.core.KotlinDetector;
3838
import org.springframework.core.annotation.AnnotatedElementUtils;
39+
import org.springframework.core.codec.ByteArrayDecoder;
40+
import org.springframework.core.codec.ByteBufferDecoder;
41+
import org.springframework.core.codec.DataBufferDecoder;
3942
import org.springframework.core.codec.Decoder;
43+
import org.springframework.core.codec.StringDecoder;
4044
import org.springframework.core.convert.ConversionService;
4145
import org.springframework.format.support.DefaultFormattingConversionService;
4246
import org.springframework.lang.Nullable;
@@ -99,13 +103,18 @@ public class MessageMappingMessageHandler extends AbstractMethodMessageHandler<C
99103

100104
public MessageMappingMessageHandler() {
101105
setHandlerPredicate(type -> AnnotatedElementUtils.hasAnnotation(type, Controller.class));
106+
this.decoders.add(StringDecoder.allMimeTypes());
107+
this.decoders.add(new ByteBufferDecoder());
108+
this.decoders.add(new ByteArrayDecoder());
109+
this.decoders.add(new DataBufferDecoder());
102110
}
103111

104112

105113
/**
106114
* Configure the decoders to use for incoming payloads.
107115
*/
108116
public void setDecoders(List<? extends Decoder<?>> decoders) {
117+
this.decoders.clear();
109118
this.decoders.addAll(decoders);
110119
}
111120

@@ -178,6 +187,19 @@ public void setEmbeddedValueResolver(StringValueResolver resolver) {
178187
}
179188

180189

190+
@Override
191+
public void afterPropertiesSet() {
192+
193+
// Initialize RouteMatcher before parent initializes handler mappings
194+
if (this.routeMatcher == null) {
195+
AntPathMatcher pathMatcher = new AntPathMatcher();
196+
pathMatcher.setPathSeparator(".");
197+
this.routeMatcher = new SimpleRouteMatcher(pathMatcher);
198+
}
199+
200+
super.afterPropertiesSet();
201+
}
202+
181203
@Override
182204
protected List<? extends HandlerMethodArgumentResolver> initArgumentResolvers() {
183205
List<HandlerMethodArgumentResolver> resolvers = new ArrayList<>();
@@ -203,12 +225,6 @@ protected List<? extends HandlerMethodArgumentResolver> initArgumentResolvers()
203225
resolvers.add(new PayloadMethodArgumentResolver(
204226
getDecoders(), this.validator, getReactiveAdapterRegistry(), true));
205227

206-
if (this.routeMatcher == null) {
207-
AntPathMatcher pathMatcher = new AntPathMatcher();
208-
pathMatcher.setPathSeparator(".");
209-
this.routeMatcher = new SimpleRouteMatcher(pathMatcher);
210-
}
211-
212228
return resolvers;
213229
}
214230

‎spring-messaging/src/main/java/org/springframework/messaging/rsocket/DefaultRSocketStrategies.java

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
import org.springframework.core.io.buffer.NettyDataBufferFactory;
4040
import org.springframework.lang.Nullable;
4141
import org.springframework.util.AntPathMatcher;
42-
import org.springframework.util.Assert;
4342
import org.springframework.util.MimeTypeUtils;
4443
import org.springframework.util.RouteMatcher;
4544
import org.springframework.util.SimpleRouteMatcher;
@@ -124,6 +123,7 @@ static class DefaultRSocketStrategiesBuilder implements RSocketStrategies.Builde
124123
@Nullable
125124
private MetadataExtractor metadataExtractor;
126125

126+
@Nullable
127127
private ReactiveAdapterRegistry adapterRegistry = ReactiveAdapterRegistry.getSharedInstance();
128128

129129
@Nullable
@@ -149,6 +149,8 @@ static class DefaultRSocketStrategiesBuilder implements RSocketStrategies.Builde
149149
DefaultRSocketStrategiesBuilder(RSocketStrategies other) {
150150
this.encoders.addAll(other.encoders());
151151
this.decoders.addAll(other.decoders());
152+
this.routeMatcher = other.routeMatcher();
153+
this.metadataExtractor = other.metadataExtractor();
152154
this.adapterRegistry = other.reactiveAdapterRegistry();
153155
this.bufferFactory = other.dataBufferFactory();
154156
}
@@ -179,26 +181,25 @@ public Builder decoders(Consumer<List<Decoder<?>>> consumer) {
179181
}
180182

181183
@Override
182-
public Builder routeMatcher(RouteMatcher routeMatcher) {
184+
public Builder routeMatcher(@Nullable RouteMatcher routeMatcher) {
183185
this.routeMatcher = routeMatcher;
184186
return this;
185187
}
186188

187189
@Override
188-
public Builder metadataExtractor(MetadataExtractor metadataExtractor) {
190+
public Builder metadataExtractor(@Nullable MetadataExtractor metadataExtractor) {
189191
this.metadataExtractor = metadataExtractor;
190192
return this;
191193
}
192194

193195
@Override
194-
public Builder reactiveAdapterStrategy(ReactiveAdapterRegistry registry) {
195-
Assert.notNull(registry, "ReactiveAdapterRegistry is required");
196+
public Builder reactiveAdapterStrategy(@Nullable ReactiveAdapterRegistry registry) {
196197
this.adapterRegistry = registry;
197198
return this;
198199
}
199200

200201
@Override
201-
public Builder dataBufferFactory(DataBufferFactory bufferFactory) {
202+
public Builder dataBufferFactory(@Nullable DataBufferFactory bufferFactory) {
202203
this.bufferFactory = bufferFactory;
203204
return this;
204205
}
@@ -210,7 +211,7 @@ public RSocketStrategies build() {
210211
this.routeMatcher != null ? this.routeMatcher : initRouteMatcher(),
211212
this.metadataExtractor != null ? this.metadataExtractor : initMetadataExtractor(),
212213
this.bufferFactory != null ? this.bufferFactory : initBufferFactory(),
213-
this.adapterRegistry);
214+
this.adapterRegistry != null ? this.adapterRegistry : initReactiveAdapterRegistry());
214215
}
215216

216217
private RouteMatcher initRouteMatcher() {
@@ -228,6 +229,10 @@ private MetadataExtractor initMetadataExtractor() {
228229
private DataBufferFactory initBufferFactory() {
229230
return new NettyDataBufferFactory(PooledByteBufAllocator.DEFAULT);
230231
}
232+
233+
private ReactiveAdapterRegistry initReactiveAdapterRegistry() {
234+
return ReactiveAdapterRegistry.getSharedInstance();
235+
}
231236
}
232237

233238
}

‎spring-messaging/src/main/java/org/springframework/messaging/rsocket/RSocketStrategies.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ interface Builder {
181181
* efficiency consider using the {@code PathPatternRouteMatcher} from
182182
* {@code spring-web} instead.
183183
*/
184-
Builder routeMatcher(RouteMatcher routeMatcher);
184+
Builder routeMatcher(@Nullable RouteMatcher routeMatcher);
185185

186186
/**
187187
* Configure a {@link MetadataExtractor} to extract the route along with
@@ -191,15 +191,15 @@ interface Builder {
191191
* route from {@code "message/x.rsocket.routing.v0"} or
192192
* {@code "text/plain"} metadata entries.
193193
*/
194-
Builder metadataExtractor(MetadataExtractor metadataExtractor);
194+
Builder metadataExtractor(@Nullable MetadataExtractor metadataExtractor);
195195

196196
/**
197197
* Configure the registry for reactive type support. This can be used to
198198
* to adapt to, and/or determine the semantics of a given
199199
* {@link org.reactivestreams.Publisher Publisher}.
200200
* <p>By default this {@link ReactiveAdapterRegistry#getSharedInstance()}.
201201
*/
202-
Builder reactiveAdapterStrategy(ReactiveAdapterRegistry registry);
202+
Builder reactiveAdapterStrategy(@Nullable ReactiveAdapterRegistry registry);
203203

204204
/**
205205
* Configure the DataBufferFactory to use for allocating buffers when
@@ -216,7 +216,7 @@ interface Builder {
216216
* <p>If using {@link DefaultDataBufferFactory} instead, there is no
217217
* need for related config changes in RSocket.
218218
*/
219-
Builder dataBufferFactory(DataBufferFactory bufferFactory);
219+
Builder dataBufferFactory(@Nullable DataBufferFactory bufferFactory);
220220

221221
/**
222222
* Build the {@code RSocketStrategies} instance.

‎spring-messaging/src/main/java/org/springframework/messaging/rsocket/annotation/support/RSocketMessageHandler.java

Lines changed: 37 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@
3232
import org.springframework.beans.BeanUtils;
3333
import org.springframework.core.ReactiveAdapterRegistry;
3434
import org.springframework.core.annotation.AnnotatedElementUtils;
35+
import org.springframework.core.codec.ByteArrayEncoder;
36+
import org.springframework.core.codec.ByteBufferEncoder;
37+
import org.springframework.core.codec.CharSequenceEncoder;
38+
import org.springframework.core.codec.DataBufferEncoder;
3539
import org.springframework.core.codec.Decoder;
3640
import org.springframework.core.codec.Encoder;
3741
import org.springframework.lang.Nullable;
@@ -82,6 +86,14 @@ public class RSocketMessageHandler extends MessageMappingMessageHandler {
8286
private MimeType defaultMetadataMimeType = MetadataExtractor.COMPOSITE_METADATA;
8387

8488

89+
public RSocketMessageHandler() {
90+
this.encoders.add(CharSequenceEncoder.allMimeTypes());
91+
this.encoders.add(new ByteBufferEncoder());
92+
this.encoders.add(new ByteArrayEncoder());
93+
this.encoders.add(new DataBufferEncoder());
94+
}
95+
96+
8597
/**
8698
* {@inheritDoc}
8799
* <p>If {@link #setRSocketStrategies(RSocketStrategies) rsocketStrategies}
@@ -104,6 +116,7 @@ public void setDecoders(List<? extends Decoder<?>> decoders) {
104116
* other properties.
105117
*/
106118
public void setEncoders(List<? extends Encoder<?>> encoders) {
119+
this.encoders.clear();
107120
this.encoders.addAll(encoders);
108121
}
109122

@@ -128,22 +141,32 @@ public List<? extends Encoder<?>> getEncoders() {
128141
* </ul>
129142
* <p>By default if this is not set, it is initialized from the above.
130143
*/
131-
public void setRSocketStrategies(@Nullable RSocketStrategies rsocketStrategies) {
132-
this.rsocketStrategies = rsocketStrategies;
133-
if (rsocketStrategies != null) {
134-
setDecoders(rsocketStrategies.decoders());
135-
setEncoders(rsocketStrategies.encoders());
136-
setReactiveAdapterRegistry(rsocketStrategies.reactiveAdapterRegistry());
137-
}
144+
public void setRSocketStrategies(RSocketStrategies rsocketStrategies) {
145+
setDecoders(rsocketStrategies.decoders());
146+
setEncoders(rsocketStrategies.encoders());
147+
setRouteMatcher(rsocketStrategies.routeMatcher());
148+
setMetadataExtractor(rsocketStrategies.metadataExtractor());
149+
setReactiveAdapterRegistry(rsocketStrategies.reactiveAdapterRegistry());
138150
}
139151

140152
/**
141-
* Return the configured {@link RSocketStrategies}. This may be {@code null}
142-
* before {@link #afterPropertiesSet()} is called.
153+
* Return an {@link RSocketStrategies} instance initialized from the
154+
* corresponding properties listed under {@link #setRSocketStrategies}.
143155
*/
144-
@Nullable
145156
public RSocketStrategies getRSocketStrategies() {
146-
return this.rsocketStrategies;
157+
return this.rsocketStrategies != null ? this.rsocketStrategies : initRSocketStrategies();
158+
}
159+
160+
private RSocketStrategies initRSocketStrategies() {
161+
return RSocketStrategies.builder()
162+
.decoders(List::clear)
163+
.encoders(List::clear)
164+
.decoders(decoders -> decoders.addAll(getDecoders()))
165+
.encoders(encoders -> encoders.addAll(getEncoders()))
166+
.routeMatcher(getRouteMatcher())
167+
.metadataExtractor(getMetadataExtractor())
168+
.reactiveAdapterStrategy(getReactiveAdapterRegistry())
169+
.build();
147170
}
148171

149172
/**
@@ -208,7 +231,9 @@ public MimeType getDefaultMetadataMimeType() {
208231
@Override
209232
public void afterPropertiesSet() {
210233

234+
// Add argument resolver before parent initializes argument resolution
211235
getArgumentResolverConfigurer().addCustomResolver(new RSocketRequesterMethodArgumentResolver());
236+
212237
super.afterPropertiesSet();
213238

214239
if (getMetadataExtractor() == null) {
@@ -217,15 +242,7 @@ public void afterPropertiesSet() {
217242
setMetadataExtractor(extractor);
218243
}
219244

220-
if (this.rsocketStrategies == null) {
221-
this.rsocketStrategies = RSocketStrategies.builder()
222-
.decoder(getDecoders().toArray(new Decoder<?>[0]))
223-
.encoder(getEncoders().toArray(new Encoder<?>[0]))
224-
.routeMatcher(getRouteMatcher())
225-
.metadataExtractor(getMetadataExtractor())
226-
.reactiveAdapterStrategy(getReactiveAdapterRegistry())
227-
.build();
228-
}
245+
this.rsocketStrategies = initRSocketStrategies();
229246
}
230247

231248
@Override

‎spring-messaging/src/test/java/org/springframework/messaging/rsocket/DefaultRSocketStrategiesTests.java

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
/*
2+
* Copyright 2002-2019 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.messaging.rsocket;
17+
18+
import org.junit.Test;
19+
20+
import org.springframework.core.ReactiveAdapterRegistry;
21+
import org.springframework.core.codec.ByteArrayDecoder;
22+
import org.springframework.core.codec.ByteArrayEncoder;
23+
import org.springframework.core.codec.ByteBufferDecoder;
24+
import org.springframework.core.codec.ByteBufferEncoder;
25+
import org.springframework.core.codec.CharSequenceEncoder;
26+
import org.springframework.core.codec.DataBufferDecoder;
27+
import org.springframework.core.codec.DataBufferEncoder;
28+
import org.springframework.core.codec.StringDecoder;
29+
import org.springframework.util.AntPathMatcher;
30+
import org.springframework.util.SimpleRouteMatcher;
31+
32+
import static org.assertj.core.api.Assertions.assertThat;
33+
34+
/**
35+
* Unit tests for {@link RSocketStrategies}.
36+
* @author Rossen Stoyanchev
37+
* @since 5.2
38+
*/
39+
public class DefaultRSocketStrategiesTests {
40+
41+
@Test
42+
public void defaultSettings() {
43+
RSocketStrategies strategies = RSocketStrategies.create();
44+
45+
assertThat(strategies.encoders()).hasSize(4).hasOnlyElementsOfTypes(
46+
CharSequenceEncoder.class,
47+
ByteArrayEncoder.class,
48+
ByteBufferEncoder.class,
49+
DataBufferEncoder.class);
50+
51+
assertThat(strategies.decoders()).hasSize(4).hasOnlyElementsOfTypes(
52+
StringDecoder.class,
53+
ByteArrayDecoder.class,
54+
ByteBufferDecoder.class,
55+
DataBufferDecoder.class);
56+
57+
assertThat(strategies.routeMatcher()).isNotNull();
58+
assertThat(strategies.metadataExtractor()).isNotNull();
59+
assertThat(strategies.reactiveAdapterRegistry()).isNotNull();
60+
}
61+
62+
@Test
63+
public void explicitValues() {
64+
65+
SimpleRouteMatcher matcher = new SimpleRouteMatcher(new AntPathMatcher());
66+
DefaultMetadataExtractor extractor = new DefaultMetadataExtractor();
67+
ReactiveAdapterRegistry registry = new ReactiveAdapterRegistry();
68+
69+
RSocketStrategies strategies = RSocketStrategies.builder()
70+
.encoders(encoders -> {
71+
encoders.clear();
72+
encoders.add(new ByteArrayEncoder());
73+
})
74+
.decoders(decoders -> {
75+
decoders.clear();
76+
decoders.add(new ByteArrayDecoder());
77+
})
78+
.routeMatcher(matcher)
79+
.metadataExtractor(extractor)
80+
.reactiveAdapterStrategy(registry)
81+
.build();
82+
83+
assertThat(strategies.encoders()).hasSize(1);
84+
assertThat(strategies.decoders()).hasSize(1);
85+
assertThat(strategies.routeMatcher()).isSameAs(matcher);
86+
assertThat(strategies.metadataExtractor()).isSameAs(extractor);
87+
assertThat(strategies.reactiveAdapterRegistry()).isSameAs(registry);
88+
}
89+
90+
@Test
91+
public void copyConstructor() {
92+
RSocketStrategies strategies1 = RSocketStrategies.create();
93+
RSocketStrategies strategies2 = strategies1.mutate().build();
94+
95+
assertThat(strategies1.encoders()).hasSameElementsAs(strategies2.encoders());
96+
assertThat(strategies1.decoders()).hasSameElementsAs(strategies2.decoders());
97+
assertThat(strategies1.routeMatcher()).isSameAs(strategies2.routeMatcher());
98+
assertThat(strategies1.metadataExtractor()).isSameAs(strategies2.metadataExtractor());
99+
assertThat(strategies1.reactiveAdapterRegistry()).isSameAs(strategies2.reactiveAdapterRegistry());
100+
}
101+
102+
}

0 commit comments

Comments
 (0)