Skip to content

Commit e33211c

Browse files
ismailalammarsimonbasle
ismailalammar
and
simonbasle
authored
Include classname of null-returning map function in NPE msg (#2984)
This commit logs the class name of the mapper provided to FluxMap, FluxMapFuseable and FluxMapSignal operators in the NB: The later is exposed in the API as `flatMap(Function,Function,Function)`. Fixes #2982. Co-authored-by: Simon Baslé <sbasle@vmware.com>
1 parent ced9a12 commit e33211c

5 files changed

Lines changed: 164 additions & 33 deletions

File tree

‎reactor-core/src/main/java/reactor/core/publisher/FluxMap.java‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2016-2021 VMware Inc. or its affiliates, All Rights Reserved.
2+
* Copyright (c) 2016-2022 VMware Inc. or its affiliates, All Rights Reserved.
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.
@@ -103,8 +103,10 @@ public void onNext(T t) {
103103
R v;
104104

105105
try {
106-
v = Objects.requireNonNull(mapper.apply(t),
107-
"The mapper returned a null value.");
106+
v = mapper.apply(t);
107+
if (v == null) {
108+
throw new NullPointerException("The mapper [" + mapper.getClass().getName() + "] returned a null value.");
109+
}
108110
}
109111
catch (Throwable e) {
110112
Throwable e_ = Operators.onNextError(t, e, actual.currentContext(), s);
@@ -203,8 +205,10 @@ public void onNext(T t) {
203205
R v;
204206

205207
try {
206-
v = Objects.requireNonNull(mapper.apply(t),
207-
"The mapper returned a null value.");
208+
v = mapper.apply(t);
209+
if (v == null) {
210+
throw new NullPointerException("The mapper [" + mapper.getClass().getName() + "] returned a null value.");
211+
}
208212
}
209213
catch (Throwable e) {
210214
Throwable e_ = Operators.onNextError(t, e, actual.currentContext(), s);
@@ -230,8 +234,10 @@ public boolean tryOnNext(T t) {
230234
R v;
231235

232236
try {
233-
v = Objects.requireNonNull(mapper.apply(t),
234-
"The mapper returned a null value.");
237+
v = mapper.apply(t);
238+
if (v == null) {
239+
throw new NullPointerException("The mapper [" + mapper.getClass().getName() + "] returned a null value.");
240+
}
235241
return actual.tryOnNext(v);
236242
}
237243
catch (Throwable e) {

‎reactor-core/src/main/java/reactor/core/publisher/FluxMapFuseable.java‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2016-2021 VMware Inc. or its affiliates, All Rights Reserved.
2+
* Copyright (c) 2016-2022 VMware Inc. or its affiliates, All Rights Reserved.
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.
@@ -110,8 +110,10 @@ public void onNext(T t) {
110110
R v;
111111

112112
try {
113-
v = Objects.requireNonNull(mapper.apply(t),
114-
"The mapper returned a null value.");
113+
v = mapper.apply(t);
114+
if (v == null) {
115+
throw new NullPointerException("The mapper [" + mapper.getClass().getName() + "] returned a null value.");
116+
}
115117
}
116118
catch (Throwable e) {
117119
Throwable e_ = Operators.onNextError(t, e, actual.currentContext(), s);
@@ -278,8 +280,10 @@ public void onNext(T t) {
278280
R v;
279281

280282
try {
281-
v = Objects.requireNonNull(mapper.apply(t),
282-
"The mapper returned a null value.");
283+
v = mapper.apply(t);
284+
if (v == null) {
285+
throw new NullPointerException("The mapper [" + mapper.getClass().getName() + "] returned a null value.");
286+
}
283287
}
284288
catch (Throwable e) {
285289
Throwable e_ = Operators.onNextError(t, e, actual.currentContext(), s);
@@ -306,8 +310,10 @@ public boolean tryOnNext(T t) {
306310
R v;
307311

308312
try {
309-
v = Objects.requireNonNull(mapper.apply(t),
310-
"The mapper returned a null value.");
313+
v = mapper.apply(t);
314+
if (v == null) {
315+
throw new NullPointerException("The mapper [" + mapper.getClass().getName() + "] returned a null value.");
316+
}
311317
return actual.tryOnNext(v);
312318
}
313319
catch (Throwable e) {

‎reactor-core/src/main/java/reactor/core/publisher/FluxMapSignal.java‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2016-2021 VMware Inc. or its affiliates, All Rights Reserved.
2+
* Copyright (c) 2016-2022 VMware Inc. or its affiliates, All Rights Reserved.
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.
@@ -141,8 +141,10 @@ public void onNext(T t) {
141141
R v;
142142

143143
try {
144-
v = Objects.requireNonNull(mapperNext.apply(t),
145-
"The mapper returned a null value.");
144+
v = mapperNext.apply(t);
145+
if (v == null) {
146+
throw new NullPointerException("The mapper [" + mapperNext.getClass().getName() + "] returned a null value.");
147+
}
146148
}
147149
catch (Throwable e) {
148150
done = true;
@@ -171,8 +173,10 @@ public void onError(Throwable t) {
171173
R v;
172174

173175
try {
174-
v = Objects.requireNonNull(mapperError.apply(t),
175-
"The mapper returned a null value.");
176+
v = mapperError.apply(t);
177+
if (v == null) {
178+
throw new NullPointerException("The mapper [" + mapperError.getClass().getName() + "] returned a null value.");
179+
}
176180
}
177181
catch (Throwable e) {
178182
done = true;
@@ -203,8 +207,10 @@ public void onComplete() {
203207
R v;
204208

205209
try {
206-
v = Objects.requireNonNull(mapperComplete.get(),
207-
"The mapper returned a null value.");
210+
v = mapperComplete.get();
211+
if (v == null) {
212+
throw new NullPointerException("The mapper [" + mapperComplete.getClass().getName() + "] returned a null value.");
213+
}
208214
}
209215
catch (Throwable e) {
210216
done = true;

‎reactor-core/src/test/java/reactor/core/publisher/FluxMapSignalTest.java‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2015-2021 VMware Inc. or its affiliates, All Rights Reserved.
2+
* Copyright (c) 2015-2022 VMware Inc. or its affiliates, All Rights Reserved.
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.
@@ -135,6 +135,44 @@ public void flatMapSignal2() {
135135
.verifyComplete();
136136
}
137137

138+
@Test
139+
void mapOnNextRejectsNull() {
140+
FluxMapTest.NullFunction<String, Mono<Integer>> mapper = new FluxMapTest.NullFunction<>();
141+
Flux.just("test")
142+
.flatMap(mapper, null, null)
143+
.as(StepVerifier::create)
144+
.verifyErrorSatisfies(err -> assertThat(err)
145+
.isInstanceOf(NullPointerException.class)
146+
.hasMessage("The mapper [reactor.core.publisher.FluxMapTest$NullFunction] returned a null value.")
147+
);
148+
}
149+
150+
@Test
151+
void mapOnErrorRejectsNull() {
152+
final IllegalStateException originalException = new IllegalStateException("expected");
153+
FluxMapTest.NullFunction<Throwable, Mono<Integer>> mapper = new FluxMapTest.NullFunction<>();
154+
Flux.error(originalException)
155+
.flatMap(null, mapper, null)
156+
.as(StepVerifier::create)
157+
.verifyErrorSatisfies(err -> assertThat(err)
158+
.isInstanceOf(NullPointerException.class)
159+
.hasMessage("The mapper [reactor.core.publisher.FluxMapTest$NullFunction] returned a null value.")
160+
.hasSuppressedException(originalException)
161+
);
162+
}
163+
164+
@Test
165+
void mapOnCompleteRejectsNull() {
166+
FluxMapTest.NullSupplier<Mono<Integer>> mapper = new FluxMapTest.NullSupplier<>();
167+
Flux.just("test")
168+
.flatMap(null, null, mapper)
169+
.as(StepVerifier::create)
170+
.verifyErrorSatisfies(err -> assertThat(err)
171+
.isInstanceOf(NullPointerException.class)
172+
.hasMessage("The mapper [reactor.core.publisher.FluxMapTest$NullSupplier] returned a null value.")
173+
);
174+
}
175+
138176
@Test
139177
public void scanOperator(){
140178
Flux<Integer> parent = Flux.just(1);

‎reactor-core/src/test/java/reactor/core/publisher/FluxMapTest.java‎

Lines changed: 86 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2016-2021 VMware Inc. or its affiliates, All Rights Reserved.
2+
* Copyright (c) 2016-2022 VMware Inc. or its affiliates, All Rights Reserved.
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.
@@ -21,8 +21,14 @@
2121
import java.util.List;
2222
import java.util.concurrent.ConcurrentLinkedQueue;
2323
import java.util.concurrent.atomic.AtomicLong;
24+
import java.util.function.Function;
25+
import java.util.function.Supplier;
26+
import java.util.stream.Stream;
2427

28+
import org.junit.jupiter.api.DynamicTest;
29+
import org.junit.jupiter.api.Named;
2530
import org.junit.jupiter.api.Test;
31+
import org.junit.jupiter.api.TestFactory;
2632
import org.mockito.Mockito;
2733
import org.reactivestreams.Subscription;
2834

@@ -33,9 +39,11 @@
3339
import reactor.test.StepVerifier;
3440
import reactor.test.publisher.FluxOperatorTest;
3541
import reactor.test.subscriber.AssertSubscriber;
42+
import reactor.test.subscriber.ConditionalTestSubscriber;
43+
import reactor.test.subscriber.TestSubscriber;
44+
import reactor.util.annotation.Nullable;
3645

37-
import static org.assertj.core.api.Assertions.assertThat;
38-
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
46+
import static org.assertj.core.api.Assertions.*;
3947
import static reactor.core.publisher.Sinks.EmitFailureHandler.FAIL_FAST;
4048

4149
public class FluxMapTest extends FluxOperatorTest<String, String> {
@@ -125,16 +133,65 @@ public void mapperThrows() {
125133
.assertNotComplete();
126134
}
127135

128-
@Test
129-
public void mapperReturnsNull() {
130-
AssertSubscriber<Object> ts = AssertSubscriber.create();
136+
@TestFactory
137+
Stream<DynamicTest> mapperReturnsNull() {
138+
Function<Integer, Integer> mapper = new NullFunction<>();
131139

132-
just.map(v -> null)
133-
.subscribe(ts);
140+
Stream<Named<Flux<Integer>>> sources = Stream.of(
141+
Named.named("normal", Flux.just(1).hide()),
142+
Named.named("fused", Flux.just(1))
143+
);
134144

135-
ts.assertError(NullPointerException.class)
136-
.assertNoValues()
137-
.assertNotComplete();
145+
return DynamicTest.stream(sources, src -> {
146+
Flux<Integer> underTest = src.map(mapper);
147+
148+
underTest
149+
.as(StepVerifier::create)
150+
.verifyErrorSatisfies(err -> assertThat(err)
151+
.isInstanceOf(NullPointerException.class)
152+
.hasMessage("The mapper [reactor.core.publisher.FluxMapTest$NullFunction] returned a null value.")
153+
);
154+
});
155+
}
156+
157+
@TestFactory
158+
Stream<DynamicTest> mapperReturnsNullConditional() {
159+
Function<Integer, Integer> mapper = new NullFunction<>();
160+
161+
Stream<Named<Boolean>> fusionModes = Stream.of(
162+
Named.named("normal", false),
163+
Named.named("fused", true)
164+
);
165+
166+
return DynamicTest.stream(fusionModes, fused -> {
167+
//the error will terminate the downstream, so we need two pairs: normal path and tryOnNext path
168+
ConditionalTestSubscriber<Integer> testSubscriberTryPath = TestSubscriber.builder().buildConditional(i -> true);
169+
ConditionalTestSubscriber<Integer> testSubscriberNormalPath = TestSubscriber.builder().buildConditional(i -> true);
170+
Fuseable.ConditionalSubscriber<Integer> conditionalSubscriberNormalPath;
171+
Fuseable.ConditionalSubscriber<Integer> conditionalSubscriberTryPath;
172+
if (fused) {
173+
conditionalSubscriberNormalPath = new FluxMapFuseable.MapFuseableConditionalSubscriber<>(testSubscriberNormalPath, mapper);
174+
conditionalSubscriberTryPath = new FluxMapFuseable.MapFuseableConditionalSubscriber<>(testSubscriberTryPath, mapper);
175+
}
176+
else {
177+
conditionalSubscriberNormalPath = new FluxMap.MapConditionalSubscriber<>(testSubscriberNormalPath, mapper);
178+
conditionalSubscriberTryPath = new FluxMap.MapConditionalSubscriber<>(testSubscriberTryPath, mapper);
179+
}
180+
181+
//test the non-conditional path
182+
conditionalSubscriberNormalPath.onNext(1);
183+
assertThat(testSubscriberNormalPath.expectTerminalError())
184+
.as("normal path")
185+
.isInstanceOf(NullPointerException.class)
186+
.hasMessage("The mapper [reactor.core.publisher.FluxMapTest$NullFunction] returned a null value.");
187+
188+
//test the conditional path
189+
conditionalSubscriberTryPath.tryOnNext(2);
190+
assertThat(testSubscriberTryPath.expectTerminalError())
191+
.as("try path")
192+
.isInstanceOf(NullPointerException.class)
193+
.hasMessage("The mapper [reactor.core.publisher.FluxMapTest$NullFunction] returned a null value.");
194+
});
138195
}
139196

140197
@Test
@@ -514,4 +571,22 @@ public void mapFuseableTryOnNextFailureStrategyResume() {
514571
Hooks.resetOnNextError();
515572
}
516573
}
574+
575+
static class NullFunction<T, R> implements Function<T, R> {
576+
577+
@Nullable
578+
@Override
579+
public R apply(T t) {
580+
return null;
581+
}
582+
}
583+
584+
static class NullSupplier<T> implements Supplier<T> {
585+
586+
@Nullable
587+
@Override
588+
public T get() {
589+
return null;
590+
}
591+
}
517592
}

0 commit comments

Comments
 (0)