From 036bbf34d2742af78ff2c44f4223e1b268e1c12c Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Sat, 9 Feb 2019 17:52:29 -0500 Subject: [PATCH] [COLLECTIONS-710] Calling CompositeCollection.size() will cash if the list contains null element. --- .../collections4/set/CompositeSet.java | 37 +++++++++++-------- .../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 b8f93f804..4ba735708 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 implements Set, 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 implements Set, 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 implements Set, Serializable { * @see SetMutator */ public synchronized void addComposited(final Set set) { - for (final Set existingSet : getSets()) { - final Collection 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 existingSet : getSets()) { + final Collection 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 implements Set, Serializable { * @param sets the Sets to be appended to the composite */ public void addComposited(final Set... sets) { - for (final Set set : sets) { - addComposited(set); + if (sets != null) { + for (final Set 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 83618c393..1d38694f3 100644 --- a/src/test/java/org/apache/commons/collections4/set/CompositeSetTest.java +++ b/src/test/java/org/apache/commons/collections4/set/CompositeSetTest.java @@ -64,6 +64,18 @@ public class CompositeSetTest extends AbstractSetTest { assertTrue(set.contains("1")); } + @SuppressWarnings("unchecked") + public void testContainsAll() { + final CompositeSet set = new CompositeSet<>(new Set[]{ buildOne(), buildTwo() }); + assertFalse(set.containsAll(null)); + } + + @SuppressWarnings("unchecked") + public void testRemoveAll() { + final CompositeSet set = new CompositeSet<>(new Set[]{ buildOne(), buildTwo() }); + assertFalse(set.removeAll(null)); + } + @SuppressWarnings("unchecked") public void testRemoveUnderlying() { final Set one = buildOne(); @@ -132,6 +144,10 @@ public class CompositeSetTest extends AbstractSetTest { final Set two = buildTwo(); final CompositeSet set = new CompositeSet<>(); set.addComposited(one, two); + set.addComposited((Set) null); + set.addComposited((Set[]) null); + set.addComposited(null, null); + set.addComposited(null, null, null); final CompositeSet set2 = new CompositeSet<>(buildOne()); set2.addComposited(buildTwo()); assertTrue(set.equals(set2));