[commons-collections] branch master updated: [COLLECTIONS-710] Calling CompositeCollection.size() will cash if the list contains null element.

[ggregory]

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


The following commit(s) were added to refs/heads/master by this push:
     new 036bbf3  [COLLECTIONS-710] Calling CompositeCollection.size() will 
cash if the list contains null element.
036bbf3 is described below

commit 036bbf34d2742af78ff2c44f4223e1b268e1c12c
Author: Gary Gregory <gardgreg...@gmail.com>
AuthorDate: Sat Feb 9 17:52:29 2019 -0500

    [COLLECTIONS-710] Calling CompositeCollection.size() will cash if the
    list contains null element.
---
 .../commons/collections4/set/CompositeSet.java     | 37 +++++++++++++---------
 .../commons/collections4/set/CompositeSetTest.java | 16 ++++++++++
 2 files changed, 38 insertions(+), 15 deletions(-)

diff --git 
a/src/main/java/org/apache/commons/collections4/set/CompositeSet.java 
b/src/main/java/org/apache/commons/collections4/set/CompositeSet.java
index b8f93f8..4ba7357 100644
--- a/src/main/java/org/apache/commons/collections4/set/CompositeSet.java
+++ b/src/main/java/org/apache/commons/collections4/set/CompositeSet.java
@@ -252,6 +252,9 @@ public class CompositeSet<E> implements Set<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;
@@ -291,7 +294,7 @@ public class CompositeSet<E> implements Set<E>, 
Serializable {
      */
     @Override
     public boolean removeAll(final Collection<?> coll) {
-        if (coll.size() == 0) {
+        if (CollectionUtils.isEmpty(coll)) {
             return false;
         }
         boolean changed = false;
@@ -354,21 +357,23 @@ public class CompositeSet<E> implements Set<E>, 
Serializable {
      * @see SetMutator
      */
     public synchronized void addComposited(final Set<E> set) {
-        for (final Set<E> existingSet : getSets()) {
-            final Collection<E> intersects = 
CollectionUtils.intersection(existingSet, set);
-            if (intersects.size() > 0) {
-                if (this.mutator == null) {
-                    throw new UnsupportedOperationException(
-                        "Collision adding composited set with no SetMutator 
set");
-                }
-                getMutator().resolveCollision(this, existingSet, set, 
intersects);
-                if (CollectionUtils.intersection(existingSet, set).size() > 0) 
{
-                    throw new IllegalArgumentException(
-                        "Attempt to add illegal entry unresolved by 
SetMutator.resolveCollision()");
+        if (set != null) {
+            for (final Set<E> existingSet : getSets()) {
+                final Collection<E> intersects = 
CollectionUtils.intersection(existingSet, set);
+                if (intersects.size() > 0) {
+                    if (this.mutator == null) {
+                        throw new UnsupportedOperationException(
+                                "Collision adding composited set with no 
SetMutator set");
+                    }
+                    getMutator().resolveCollision(this, existingSet, set, 
intersects);
+                    if (CollectionUtils.intersection(existingSet, set).size() 
> 0) {
+                        throw new IllegalArgumentException(
+                                "Attempt to add illegal entry unresolved by 
SetMutator.resolveCollision()");
+                    }
                 }
             }
+            all.add(set);
         }
-        all.add(set);
     }
 
     /**
@@ -388,8 +393,10 @@ public class CompositeSet<E> implements Set<E>, 
Serializable {
      * @param sets  the Sets to be appended to the composite
      */
     public void addComposited(final Set<E>... sets) {
-        for (final Set<E> set : sets) {
-            addComposited(set);
+        if (sets != null) {
+            for (final Set<E> set : sets) {
+                addComposited(set);
+            }
         }
     }
 
diff --git 
a/src/test/java/org/apache/commons/collections4/set/CompositeSetTest.java 
b/src/test/java/org/apache/commons/collections4/set/CompositeSetTest.java
index 83618c3..1d38694 100644
--- a/src/test/java/org/apache/commons/collections4/set/CompositeSetTest.java
+++ b/src/test/java/org/apache/commons/collections4/set/CompositeSetTest.java
@@ -65,6 +65,18 @@ public class CompositeSetTest<E> extends AbstractSetTest<E> {
     }
 
     @SuppressWarnings("unchecked")
+    public void testContainsAll() {
+        final CompositeSet<E> set = new CompositeSet<>(new Set[]{ buildOne(), 
buildTwo() });
+        assertFalse(set.containsAll(null));
+    }
+
+    @SuppressWarnings("unchecked")
+    public void testRemoveAll() {
+        final CompositeSet<E> set = new CompositeSet<>(new Set[]{ buildOne(), 
buildTwo() });
+        assertFalse(set.removeAll(null));
+    }
+
+    @SuppressWarnings("unchecked")
     public void testRemoveUnderlying() {
         final Set<E> one = buildOne();
         final Set<E> two = buildTwo();
@@ -132,6 +144,10 @@ public class CompositeSetTest<E> extends 
AbstractSetTest<E> {
         final Set<E> two = buildTwo();
         final CompositeSet<E> set = new CompositeSet<>();
         set.addComposited(one, two);
+        set.addComposited((Set<E>) null);
+        set.addComposited((Set<E>[]) null);
+        set.addComposited(null, null);
+        set.addComposited(null, null, null);
         final CompositeSet<E> set2 = new CompositeSet<>(buildOne());
         set2.addComposited(buildTwo());
         assertTrue(set.equals(set2));