This is an automated email from the ASF dual-hosted git repository. ggregory pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-collections.git
commit b97da105fbccac149223327e49d29ac9024913fb Author: Gary Gregory <garydgreg...@gmail.com> AuthorDate: Sat Feb 9 17:41:20 2019 -0500 [COLLECTIONS-710] Calling CompositeCollection.size() will cash if the list contains null element. --- .../collection/CompositeCollection.java | 30 +++++++++++++++----- .../collection/CompositeCollectionTest.java | 33 ++++++++++++++++++++++ 2 files changed, 56 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/apache/commons/collections4/collection/CompositeCollection.java b/src/main/java/org/apache/commons/collections4/collection/CompositeCollection.java index 83b4302..57541dc 100644 --- a/src/main/java/org/apache/commons/collections4/collection/CompositeCollection.java +++ b/src/main/java/org/apache/commons/collections4/collection/CompositeCollection.java @@ -24,6 +24,7 @@ import java.util.Collection; import java.util.Iterator; import java.util.List; +import org.apache.commons.collections4.CollectionUtils; import org.apache.commons.collections4.iterators.EmptyIterator; import org.apache.commons.collections4.iterators.IteratorChain; import org.apache.commons.collections4.list.UnmodifiableList; @@ -260,6 +261,9 @@ public class CompositeCollection<E> implements Collection<E>, Serializable { */ @Override public boolean containsAll(final Collection<?> coll) { + if (coll == null) { + return false; + } for (final Object item : coll) { if (contains(item) == false) { return false; @@ -300,7 +304,7 @@ public class CompositeCollection<E> implements Collection<E>, Serializable { */ @Override public boolean removeAll(final Collection<?> coll) { - if (coll.size() == 0) { + if (CollectionUtils.isEmpty(coll)) { return false; } boolean changed = false; @@ -323,8 +327,10 @@ public class CompositeCollection<E> implements Collection<E>, Serializable { @Override public boolean retainAll(final Collection<?> coll) { boolean changed = false; - for (final Collection<E> item : all) { - changed |= item.retainAll(coll); + if (coll != null) { + for (final Collection<E> item : all) { + changed |= item.retainAll(coll); + } } return changed; } @@ -359,7 +365,9 @@ public class CompositeCollection<E> implements Collection<E>, Serializable { * @param compositeCollection the Collection to be appended to the composite */ public void addComposited(final Collection<E> compositeCollection) { - all.add(compositeCollection); + if (compositeCollection != null) { + all.add(compositeCollection); + } } /** @@ -370,8 +378,12 @@ public class CompositeCollection<E> implements Collection<E>, Serializable { */ public void addComposited(final Collection<E> compositeCollection1, final Collection<E> compositeCollection2) { - all.add(compositeCollection1); - all.add(compositeCollection2); + if (compositeCollection1 != null) { + all.add(compositeCollection1); + } + if (compositeCollection2 != null) { + all.add(compositeCollection2); + } } /** @@ -380,7 +392,11 @@ public class CompositeCollection<E> implements Collection<E>, Serializable { * @param compositeCollections the Collections to be appended to the composite */ public void addComposited(final Collection<E>... compositeCollections) { - all.addAll(Arrays.asList(compositeCollections)); + for (Collection<E> compositeCollection : compositeCollections) { + if (compositeCollection != null) { + all.add(compositeCollection); + } + } } /** diff --git a/src/test/java/org/apache/commons/collections4/collection/CompositeCollectionTest.java b/src/test/java/org/apache/commons/collections4/collection/CompositeCollectionTest.java index b3b625f..8cdc920 100644 --- a/src/test/java/org/apache/commons/collections4/collection/CompositeCollectionTest.java +++ b/src/test/java/org/apache/commons/collections4/collection/CompositeCollectionTest.java @@ -23,6 +23,8 @@ import java.util.HashSet; import java.util.Iterator; import java.util.List; +import org.junit.Assert; + /** * Extension of {@link AbstractCollectionTest} for exercising the * {@link CompositeCollection} implementation. @@ -246,8 +248,30 @@ public class CompositeCollectionTest<E> extends AbstractCollectionTest<E> { two.add((E) "1"); c.addComposited(one); assertTrue(c.containsAll(two)); + assertFalse(c.containsAll(null)); } + public void testAddNullList() { + ArrayList<String> nullList = null; + CompositeCollection<String> cc = new CompositeCollection<>(); + cc.addComposited(nullList); + Assert.assertEquals(0, cc.size()); + } + + public void testAddNullLists2Args() { + ArrayList<String> nullList = null; + CompositeCollection<String> cc = new CompositeCollection<>(); + cc.addComposited(nullList, nullList); + Assert.assertEquals(0, cc.size()); + } + + public void testAddNullListsVarArgs() { + ArrayList<String> nullList = null; + CompositeCollection<String> cc = new CompositeCollection<>(); + cc.addComposited(nullList, nullList, nullList); + Assert.assertEquals(0, cc.size()); + } + @SuppressWarnings("unchecked") public void testIsEmpty() { setUpTest(); @@ -315,6 +339,10 @@ public class CompositeCollectionTest<E> extends AbstractCollectionTest<E> { assertTrue(!c.contains("1")); assertTrue(!one.contains("1")); assertTrue(!two.contains("1")); + c.removeAll(null); + assertTrue(!c.contains("1")); + assertTrue(!one.contains("1")); + assertTrue(!two.contains("1")); } @SuppressWarnings("unchecked") @@ -341,6 +369,11 @@ public class CompositeCollectionTest<E> extends AbstractCollectionTest<E> { assertTrue(!one.contains("2")); assertTrue(c.contains("1")); assertTrue(one.contains("1")); + c.retainAll(null); + assertTrue(!c.contains("2")); + assertTrue(!one.contains("2")); + assertTrue(c.contains("1")); + assertTrue(one.contains("1")); } @SuppressWarnings("unchecked")