From b97da105fbccac149223327e49d29ac9024913fb Mon Sep 17 00:00:00 2001
From: Gary Gregory <garydgregory@gmail.com>
Date: Sat, 9 Feb 2019 17:41:20 -0500
Subject: [PATCH] [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 83b43029e..57541dcb7 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 b3b625fda..8cdc920c4 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")