From d5b4d6f380a02626fd4e9d66a27851b9a70aa403 Mon Sep 17 00:00:00 2001 From: zsxwing Date: Fri, 12 Dec 2014 14:41:49 +0800 Subject: [PATCH] Fix NPE when the key is null in GroupBy --- .../internal/operators/OperatorGroupBy.java | 19 ++++++++---- .../operators/OperatorGroupByTest.java | 30 ++++++++++++++++++- 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/src/main/java/rx/internal/operators/OperatorGroupBy.java b/src/main/java/rx/internal/operators/OperatorGroupBy.java index f934856164..39cfae4457 100644 --- a/src/main/java/rx/internal/operators/OperatorGroupBy.java +++ b/src/main/java/rx/internal/operators/OperatorGroupBy.java @@ -102,7 +102,7 @@ public Observer getObserver() { } - private final ConcurrentHashMap> groups = new ConcurrentHashMap>(); + private final ConcurrentHashMap> groups = new ConcurrentHashMap>(); private static final NotificationLite nl = NotificationLite.instance(); @@ -166,10 +166,18 @@ void requestFromGroupedObservable(long n, GroupState group) { } } + private Object groupedKey(K key) { + return key == null ? NULL_KEY : key; + } + + private K getKey(Object groupedKey) { + return groupedKey == NULL_KEY ? null : (K) groupedKey; + } + @Override public void onNext(T t) { try { - final K key = keySelector.call(t); + final Object key = groupedKey(keySelector.call(t)); GroupState group = groups.get(key); if (group == null) { // this group doesn't exist @@ -185,10 +193,10 @@ public void onNext(T t) { } } - private GroupState createNewGroup(final K key) { + private GroupState createNewGroup(final Object key) { final GroupState groupState = new GroupState(); - GroupedObservable go = GroupedObservable.create(key, new OnSubscribe() { + GroupedObservable go = GroupedObservable.create(getKey(key), new OnSubscribe() { @Override public void call(final Subscriber o) { @@ -252,7 +260,7 @@ public void onNext(T t) { return groupState; } - private void cleanupGroup(K key) { + private void cleanupGroup(Object key) { GroupState removed; removed = groups.remove(key); if (removed != null) { @@ -357,4 +365,5 @@ public Object call(Object t) { } }; + private static final Object NULL_KEY = new Object(); } diff --git a/src/test/java/rx/internal/operators/OperatorGroupByTest.java b/src/test/java/rx/internal/operators/OperatorGroupByTest.java index 432266ea32..9bbed5d04d 100644 --- a/src/test/java/rx/internal/operators/OperatorGroupByTest.java +++ b/src/test/java/rx/internal/operators/OperatorGroupByTest.java @@ -28,6 +28,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentLinkedQueue; @@ -1357,4 +1358,31 @@ public Observable call(GroupedObservable t) { }; -} \ No newline at end of file + @Test + public void testGroupByWithNullKey() { + final String[] key = new String[]{"uninitialized"}; + final List values = new ArrayList(); + Observable.just("a", "b", "c").groupBy(new Func1() { + + @Override + public String call(String value) { + return null; + } + }).subscribe(new Action1>() { + + @Override + public void call(GroupedObservable groupedObservable) { + key[0] = groupedObservable.getKey(); + groupedObservable.subscribe(new Action1() { + + @Override + public void call(String s) { + values.add(s); + } + }); + } + }); + assertEquals(null, key[0]); + assertEquals(Arrays.asList("a", "b", "c"), values); + } +}