From d67beee61e75063f2177b587504cce710cf62297 Mon Sep 17 00:00:00 2001 From: punitda Date: Fri, 16 Nov 2018 17:41:04 +0530 Subject: [PATCH 1/3] Javadoc : Explain distinctUntilChanged requires non-mutating data type in flow(Observable) Add few tests for scenarios related to mutable data type flow in distinctUntilChanged() methods for Observable --- src/main/java/io/reactivex/Observable.java | 21 +++++ .../ObservableDistinctUntilChangedTest.java | 78 +++++++++++++++++++ 2 files changed, 99 insertions(+) diff --git a/src/main/java/io/reactivex/Observable.java b/src/main/java/io/reactivex/Observable.java index c022a0f9fa..5264a9ef57 100644 --- a/src/main/java/io/reactivex/Observable.java +++ b/src/main/java/io/reactivex/Observable.java @@ -7824,6 +7824,13 @@ public final Observable distinct(Function keySelector, Call *

* Note that the operator always retains the latest item from upstream regardless of the comparison result * and uses it in the next comparison with the next upstream item. + *

+ * Note that if element {@code T} type in the flow is mutable, the comparison of the previous and current + * item may yield unexpected results if the items are mutated externally. Common cases are mutable + * {@code CharSequence}s or {@code List}s where the objects will actually have the same + * references when they are modified and {@code distinctUntilChanged} will evaluate subsequent items as same. + * To avoid such situation, it is recommended that mutable data is converted to an immutable one, + * for example using `map(CharSequence::toString)` or `map(Collections::unmodifiableList)`. *

*
Scheduler:
*
{@code distinctUntilChanged} does not operate by default on a particular {@link Scheduler}.
@@ -7856,6 +7863,13 @@ public final Observable distinctUntilChanged() { *

* Note that the operator always retains the latest key from upstream regardless of the comparison result * and uses it in the next comparison with the next key derived from the next upstream item. + *

+ * Note that if element {@code T} type in the flow is mutable, the comparison of the previous and current + * item may yield unexpected results if the items are mutated externally. Common cases are mutable + * {@code CharSequence}s or {@code List}s where the objects will actually have the same + * references when they are modified and {@code distinctUntilChanged} will evaluate subsequent items as same. + * To avoid such situation, it is recommended that mutable data is converted to an immutable one, + * for example using `map(CharSequence::toString)` or `map(Collections::unmodifiableList)`. *

*
Scheduler:
*
{@code distinctUntilChanged} does not operate by default on a particular {@link Scheduler}.
@@ -7884,6 +7898,13 @@ public final Observable distinctUntilChanged(Function keySe *

* Note that the operator always retains the latest item from upstream regardless of the comparison result * and uses it in the next comparison with the next upstream item. + *

+ * Note that if element {@code T} type in the flow is mutable, the comparison of the previous and current + * item may yield unexpected results if the items are mutated externally. Common cases are mutable + * {@code CharSequence}s or {@code List}s where the objects will actually have the same + * references when they are modified and {@code distinctUntilChanged} will evaluate subsequent items as same. + * To avoid such situation, it is recommended that mutable data is converted to an immutable one, + * for example using `map(CharSequence::toString)` or `map(Collections::unmodifiableList)`. *

*
Scheduler:
*
{@code distinctUntilChanged} does not operate by default on a particular {@link Scheduler}.
diff --git a/src/test/java/io/reactivex/internal/operators/observable/ObservableDistinctUntilChangedTest.java b/src/test/java/io/reactivex/internal/operators/observable/ObservableDistinctUntilChangedTest.java index b0ba634354..aeb57f73c6 100644 --- a/src/test/java/io/reactivex/internal/operators/observable/ObservableDistinctUntilChangedTest.java +++ b/src/test/java/io/reactivex/internal/operators/observable/ObservableDistinctUntilChangedTest.java @@ -107,6 +107,84 @@ public void testDistinctUntilChangedOfNormalSourceWithKeySelector() { verify(w, never()).onError(any(Throwable.class)); } + @Test + public void testDistinctUntilChangedOfMutableType() { + final StringBuilder charSequence = new StringBuilder(); + Observable src = Observable.create(new ObservableOnSubscribe() { + @Override + public void subscribe(ObservableEmitter emitter) throws Exception { + charSequence.append("a"); + emitter.onNext(charSequence); + charSequence.append("b"); + emitter.onNext(charSequence); + emitter.onComplete(); + } + }); + + TestObserver testObserver = TestObserver.create(); + + src.distinctUntilChanged().subscribe(testObserver); + + testObserver.assertValue(charSequence); + testObserver.assertComplete(); + } + + @Test + public void testDistinctUntilChangedOfMutableTypeWithKeyTypeSelector() { + final StringBuilder charSequence = new StringBuilder(); + Observable src = Observable.create(new ObservableOnSubscribe() { + @Override + public void subscribe(ObservableEmitter emitter) throws Exception { + charSequence.append("a"); + emitter.onNext(charSequence); + charSequence.append("b"); + emitter.onNext(charSequence); + emitter.onComplete(); + } + }); + + TestObserver testObserver = TestObserver.create(); + + src.distinctUntilChanged(new Function() { + @Override + public String apply(CharSequence charSequence) throws Exception { + return charSequence.toString(); + } + }) + .subscribe(testObserver); + + testObserver.assertValues(charSequence, charSequence); + testObserver.assertComplete(); + } + + @Test + public void testDistinctUntilChangedOfMutableTypeWithPredicate() { + final StringBuilder charSequence = new StringBuilder(); + Observable src = Observable.create(new ObservableOnSubscribe() { + @Override + public void subscribe(ObservableEmitter emitter) throws Exception { + charSequence.append("a"); + emitter.onNext(charSequence); + charSequence.append("b"); + emitter.onNext(charSequence); + emitter.onComplete(); + } + }); + + TestObserver testObserver = TestObserver.create(); + + src.distinctUntilChanged(new BiPredicate() { + @Override + public boolean test(CharSequence seq1, CharSequence seq2) throws Exception { + return seq1.length() != seq2.length(); + } + }) + .subscribe(testObserver); + + testObserver.assertValues(charSequence, charSequence); + testObserver.assertComplete(); + } + @Test @Ignore("Null values no longer allowed") public void testDistinctUntilChangedOfSourceWithNulls() { From 377a3c3bc1771c760edcb4a693ef67e379ec4715 Mon Sep 17 00:00:00 2001 From: punitda Date: Fri, 16 Nov 2018 17:55:15 +0530 Subject: [PATCH 2/3] Javadoc : Explain distinctUntilChanged requires non-mutating data type in flow(Flowable) Add few tests for scenarios related to mutable data type flow in distinctUntilChanged() methods for Flowable --- src/main/java/io/reactivex/Flowable.java | 21 +++++ .../FlowableDistinctUntilChangedTest.java | 78 +++++++++++++++++++ 2 files changed, 99 insertions(+) diff --git a/src/main/java/io/reactivex/Flowable.java b/src/main/java/io/reactivex/Flowable.java index da4f800670..ea7e295fe7 100644 --- a/src/main/java/io/reactivex/Flowable.java +++ b/src/main/java/io/reactivex/Flowable.java @@ -8740,6 +8740,13 @@ public final Flowable distinct(Function keySelector, *

* Note that the operator always retains the latest item from upstream regardless of the comparison result * and uses it in the next comparison with the next upstream item. + *

+ * Note that if element {@code T} type in the flow is mutable, the comparison of the previous and current + * item may yield unexpected results if the items are mutated externally. Common cases are mutable + * {@code CharSequence}s or {@code List}s where the objects will actually have the same + * references when they are modified and {@code distinctUntilChanged} will evaluate subsequent items as same. + * To avoid such situation, it is recommended that mutable data is converted to an immutable one, + * for example using `map(CharSequence::toString)` or `map(Collections::unmodifiableList)`. *

*
Backpressure:
*
The operator doesn't interfere with backpressure which is determined by the source {@code Publisher}'s @@ -8776,6 +8783,13 @@ public final Flowable distinctUntilChanged() { *

* Note that the operator always retains the latest key from upstream regardless of the comparison result * and uses it in the next comparison with the next key derived from the next upstream item. + *

+ * Note that if element {@code T} type in the flow is mutable, the comparison of the previous and current + * item may yield unexpected results if the items are mutated externally. Common cases are mutable + * {@code CharSequence}s or {@code List}s where the objects will actually have the same + * references when they are modified and {@code distinctUntilChanged} will evaluate subsequent items as same. + * To avoid such situation, it is recommended that mutable data is converted to an immutable one, + * for example using `map(CharSequence::toString)` or `map(Collections::unmodifiableList)`. *

*
Backpressure:
*
The operator doesn't interfere with backpressure which is determined by the source {@code Publisher}'s @@ -8808,6 +8822,13 @@ public final Flowable distinctUntilChanged(Function keySele *

* Note that the operator always retains the latest item from upstream regardless of the comparison result * and uses it in the next comparison with the next upstream item. + *

+ * Note that if element {@code T} type in the flow is mutable, the comparison of the previous and current + * item may yield unexpected results if the items are mutated externally. Common cases are mutable + * {@code CharSequence}s or {@code List}s where the objects will actually have the same + * references when they are modified and {@code distinctUntilChanged} will evaluate subsequent items as same. + * To avoid such situation, it is recommended that mutable data is converted to an immutable one, + * for example using `map(CharSequence::toString)` or `map(Collections::unmodifiableList)`. *

*
Backpressure:
*
The operator doesn't interfere with backpressure which is determined by the source {@code Publisher}'s diff --git a/src/test/java/io/reactivex/internal/operators/flowable/FlowableDistinctUntilChangedTest.java b/src/test/java/io/reactivex/internal/operators/flowable/FlowableDistinctUntilChangedTest.java index 7c5eb993e1..0cef5612b8 100644 --- a/src/test/java/io/reactivex/internal/operators/flowable/FlowableDistinctUntilChangedTest.java +++ b/src/test/java/io/reactivex/internal/operators/flowable/FlowableDistinctUntilChangedTest.java @@ -141,6 +141,84 @@ public void testDistinctUntilChangedOfSourceWithExceptionsFromKeySelector() { inOrder.verify(w, never()).onComplete(); } + @Test + public void testDistinctUntilChangedOfMutableType() { + final StringBuilder charSequence = new StringBuilder(); + Flowable src = Flowable.create(new FlowableOnSubscribe() { + @Override + public void subscribe(FlowableEmitter emitter) throws Exception { + charSequence.append("a"); + emitter.onNext(charSequence); + charSequence.append("b"); + emitter.onNext(charSequence); + emitter.onComplete(); + } + }, BackpressureStrategy.MISSING); + + TestSubscriber testObserver = TestSubscriber.create(); + + src.distinctUntilChanged().subscribe(testObserver); + + testObserver.assertValue(charSequence); + testObserver.assertComplete(); + } + + @Test + public void testDistinctUntilChangedOfMutableTypeWithKeyTypeSelector() { + final StringBuilder charSequence = new StringBuilder(); + Flowable src = Flowable.create(new FlowableOnSubscribe() { + @Override + public void subscribe(FlowableEmitter emitter) throws Exception { + charSequence.append("a"); + emitter.onNext(charSequence); + charSequence.append("b"); + emitter.onNext(charSequence); + emitter.onComplete(); + } + }, BackpressureStrategy.MISSING); + + TestSubscriber testObserver = TestSubscriber.create(); + + src.distinctUntilChanged(new Function() { + @Override + public String apply(CharSequence charSequence) throws Exception { + return charSequence.toString(); + } + }) + .subscribe(testObserver); + + testObserver.assertValues(charSequence, charSequence); + testObserver.assertComplete(); + } + + @Test + public void testDistinctUntilChangedOfMutableTypeWithPredicate() { + final StringBuilder charSequence = new StringBuilder(); + Flowable src = Flowable.create(new FlowableOnSubscribe() { + @Override + public void subscribe(FlowableEmitter emitter) throws Exception { + charSequence.append("a"); + emitter.onNext(charSequence); + charSequence.append("b"); + emitter.onNext(charSequence); + emitter.onComplete(); + } + }, BackpressureStrategy.MISSING); + + TestSubscriber testObserver = TestSubscriber.create(); + + src.distinctUntilChanged(new BiPredicate() { + @Override + public boolean test(CharSequence seq1, CharSequence seq2) throws Exception { + return seq1.length() != seq2.length(); + } + }) + .subscribe(testObserver); + + testObserver.assertValues(charSequence, charSequence); + testObserver.assertComplete(); + } + @Test public void directComparer() { Flowable.fromArray(1, 2, 2, 3, 2, 4, 1, 1, 2) From 42b56e498412a263b31f4dc95e38e76fe8c9099a Mon Sep 17 00:00:00 2001 From: punitda Date: Fri, 16 Nov 2018 19:19:24 +0530 Subject: [PATCH 3/3] Remove tests and fix typo in javadoc --- src/main/java/io/reactivex/Flowable.java | 6 +- src/main/java/io/reactivex/Observable.java | 6 +- .../FlowableDistinctUntilChangedTest.java | 78 ------------------- .../ObservableDistinctUntilChangedTest.java | 78 ------------------- 4 files changed, 6 insertions(+), 162 deletions(-) diff --git a/src/main/java/io/reactivex/Flowable.java b/src/main/java/io/reactivex/Flowable.java index ea7e295fe7..054a5cd875 100644 --- a/src/main/java/io/reactivex/Flowable.java +++ b/src/main/java/io/reactivex/Flowable.java @@ -8741,7 +8741,7 @@ public final Flowable distinct(Function keySelector, * Note that the operator always retains the latest item from upstream regardless of the comparison result * and uses it in the next comparison with the next upstream item. *

- * Note that if element {@code T} type in the flow is mutable, the comparison of the previous and current + * Note that if element type {@code T} in the flow is mutable, the comparison of the previous and current * item may yield unexpected results if the items are mutated externally. Common cases are mutable * {@code CharSequence}s or {@code List}s where the objects will actually have the same * references when they are modified and {@code distinctUntilChanged} will evaluate subsequent items as same. @@ -8784,7 +8784,7 @@ public final Flowable distinctUntilChanged() { * Note that the operator always retains the latest key from upstream regardless of the comparison result * and uses it in the next comparison with the next key derived from the next upstream item. *

- * Note that if element {@code T} type in the flow is mutable, the comparison of the previous and current + * Note that if element type {@code T} in the flow is mutable, the comparison of the previous and current * item may yield unexpected results if the items are mutated externally. Common cases are mutable * {@code CharSequence}s or {@code List}s where the objects will actually have the same * references when they are modified and {@code distinctUntilChanged} will evaluate subsequent items as same. @@ -8823,7 +8823,7 @@ public final Flowable distinctUntilChanged(Function keySele * Note that the operator always retains the latest item from upstream regardless of the comparison result * and uses it in the next comparison with the next upstream item. *

- * Note that if element {@code T} type in the flow is mutable, the comparison of the previous and current + * Note that if element type {@code T} in the flow is mutable, the comparison of the previous and current * item may yield unexpected results if the items are mutated externally. Common cases are mutable * {@code CharSequence}s or {@code List}s where the objects will actually have the same * references when they are modified and {@code distinctUntilChanged} will evaluate subsequent items as same. diff --git a/src/main/java/io/reactivex/Observable.java b/src/main/java/io/reactivex/Observable.java index 5264a9ef57..41f10e395f 100644 --- a/src/main/java/io/reactivex/Observable.java +++ b/src/main/java/io/reactivex/Observable.java @@ -7825,7 +7825,7 @@ public final Observable distinct(Function keySelector, Call * Note that the operator always retains the latest item from upstream regardless of the comparison result * and uses it in the next comparison with the next upstream item. *

- * Note that if element {@code T} type in the flow is mutable, the comparison of the previous and current + * Note that if element type {@code T} in the flow is mutable, the comparison of the previous and current * item may yield unexpected results if the items are mutated externally. Common cases are mutable * {@code CharSequence}s or {@code List}s where the objects will actually have the same * references when they are modified and {@code distinctUntilChanged} will evaluate subsequent items as same. @@ -7864,7 +7864,7 @@ public final Observable distinctUntilChanged() { * Note that the operator always retains the latest key from upstream regardless of the comparison result * and uses it in the next comparison with the next key derived from the next upstream item. *

- * Note that if element {@code T} type in the flow is mutable, the comparison of the previous and current + * Note that if element type {@code T} in the flow is mutable, the comparison of the previous and current * item may yield unexpected results if the items are mutated externally. Common cases are mutable * {@code CharSequence}s or {@code List}s where the objects will actually have the same * references when they are modified and {@code distinctUntilChanged} will evaluate subsequent items as same. @@ -7899,7 +7899,7 @@ public final Observable distinctUntilChanged(Function keySe * Note that the operator always retains the latest item from upstream regardless of the comparison result * and uses it in the next comparison with the next upstream item. *

- * Note that if element {@code T} type in the flow is mutable, the comparison of the previous and current + * Note that if element type {@code T} in the flow is mutable, the comparison of the previous and current * item may yield unexpected results if the items are mutated externally. Common cases are mutable * {@code CharSequence}s or {@code List}s where the objects will actually have the same * references when they are modified and {@code distinctUntilChanged} will evaluate subsequent items as same. diff --git a/src/test/java/io/reactivex/internal/operators/flowable/FlowableDistinctUntilChangedTest.java b/src/test/java/io/reactivex/internal/operators/flowable/FlowableDistinctUntilChangedTest.java index 0cef5612b8..7c5eb993e1 100644 --- a/src/test/java/io/reactivex/internal/operators/flowable/FlowableDistinctUntilChangedTest.java +++ b/src/test/java/io/reactivex/internal/operators/flowable/FlowableDistinctUntilChangedTest.java @@ -141,84 +141,6 @@ public void testDistinctUntilChangedOfSourceWithExceptionsFromKeySelector() { inOrder.verify(w, never()).onComplete(); } - @Test - public void testDistinctUntilChangedOfMutableType() { - final StringBuilder charSequence = new StringBuilder(); - Flowable src = Flowable.create(new FlowableOnSubscribe() { - @Override - public void subscribe(FlowableEmitter emitter) throws Exception { - charSequence.append("a"); - emitter.onNext(charSequence); - charSequence.append("b"); - emitter.onNext(charSequence); - emitter.onComplete(); - } - }, BackpressureStrategy.MISSING); - - TestSubscriber testObserver = TestSubscriber.create(); - - src.distinctUntilChanged().subscribe(testObserver); - - testObserver.assertValue(charSequence); - testObserver.assertComplete(); - } - - @Test - public void testDistinctUntilChangedOfMutableTypeWithKeyTypeSelector() { - final StringBuilder charSequence = new StringBuilder(); - Flowable src = Flowable.create(new FlowableOnSubscribe() { - @Override - public void subscribe(FlowableEmitter emitter) throws Exception { - charSequence.append("a"); - emitter.onNext(charSequence); - charSequence.append("b"); - emitter.onNext(charSequence); - emitter.onComplete(); - } - }, BackpressureStrategy.MISSING); - - TestSubscriber testObserver = TestSubscriber.create(); - - src.distinctUntilChanged(new Function() { - @Override - public String apply(CharSequence charSequence) throws Exception { - return charSequence.toString(); - } - }) - .subscribe(testObserver); - - testObserver.assertValues(charSequence, charSequence); - testObserver.assertComplete(); - } - - @Test - public void testDistinctUntilChangedOfMutableTypeWithPredicate() { - final StringBuilder charSequence = new StringBuilder(); - Flowable src = Flowable.create(new FlowableOnSubscribe() { - @Override - public void subscribe(FlowableEmitter emitter) throws Exception { - charSequence.append("a"); - emitter.onNext(charSequence); - charSequence.append("b"); - emitter.onNext(charSequence); - emitter.onComplete(); - } - }, BackpressureStrategy.MISSING); - - TestSubscriber testObserver = TestSubscriber.create(); - - src.distinctUntilChanged(new BiPredicate() { - @Override - public boolean test(CharSequence seq1, CharSequence seq2) throws Exception { - return seq1.length() != seq2.length(); - } - }) - .subscribe(testObserver); - - testObserver.assertValues(charSequence, charSequence); - testObserver.assertComplete(); - } - @Test public void directComparer() { Flowable.fromArray(1, 2, 2, 3, 2, 4, 1, 1, 2) diff --git a/src/test/java/io/reactivex/internal/operators/observable/ObservableDistinctUntilChangedTest.java b/src/test/java/io/reactivex/internal/operators/observable/ObservableDistinctUntilChangedTest.java index aeb57f73c6..b0ba634354 100644 --- a/src/test/java/io/reactivex/internal/operators/observable/ObservableDistinctUntilChangedTest.java +++ b/src/test/java/io/reactivex/internal/operators/observable/ObservableDistinctUntilChangedTest.java @@ -107,84 +107,6 @@ public void testDistinctUntilChangedOfNormalSourceWithKeySelector() { verify(w, never()).onError(any(Throwable.class)); } - @Test - public void testDistinctUntilChangedOfMutableType() { - final StringBuilder charSequence = new StringBuilder(); - Observable src = Observable.create(new ObservableOnSubscribe() { - @Override - public void subscribe(ObservableEmitter emitter) throws Exception { - charSequence.append("a"); - emitter.onNext(charSequence); - charSequence.append("b"); - emitter.onNext(charSequence); - emitter.onComplete(); - } - }); - - TestObserver testObserver = TestObserver.create(); - - src.distinctUntilChanged().subscribe(testObserver); - - testObserver.assertValue(charSequence); - testObserver.assertComplete(); - } - - @Test - public void testDistinctUntilChangedOfMutableTypeWithKeyTypeSelector() { - final StringBuilder charSequence = new StringBuilder(); - Observable src = Observable.create(new ObservableOnSubscribe() { - @Override - public void subscribe(ObservableEmitter emitter) throws Exception { - charSequence.append("a"); - emitter.onNext(charSequence); - charSequence.append("b"); - emitter.onNext(charSequence); - emitter.onComplete(); - } - }); - - TestObserver testObserver = TestObserver.create(); - - src.distinctUntilChanged(new Function() { - @Override - public String apply(CharSequence charSequence) throws Exception { - return charSequence.toString(); - } - }) - .subscribe(testObserver); - - testObserver.assertValues(charSequence, charSequence); - testObserver.assertComplete(); - } - - @Test - public void testDistinctUntilChangedOfMutableTypeWithPredicate() { - final StringBuilder charSequence = new StringBuilder(); - Observable src = Observable.create(new ObservableOnSubscribe() { - @Override - public void subscribe(ObservableEmitter emitter) throws Exception { - charSequence.append("a"); - emitter.onNext(charSequence); - charSequence.append("b"); - emitter.onNext(charSequence); - emitter.onComplete(); - } - }); - - TestObserver testObserver = TestObserver.create(); - - src.distinctUntilChanged(new BiPredicate() { - @Override - public boolean test(CharSequence seq1, CharSequence seq2) throws Exception { - return seq1.length() != seq2.length(); - } - }) - .subscribe(testObserver); - - testObserver.assertValues(charSequence, charSequence); - testObserver.assertComplete(); - } - @Test @Ignore("Null values no longer allowed") public void testDistinctUntilChangedOfSourceWithNulls() {