You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by gg...@apache.org on 2019/02/09 22:41:22 UTC
[commons-collections] branch master updated (0ac8e7c -> b97da10)
This is an automated email from the ASF dual-hosted git repository.
ggregory pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/commons-collections.git.
from 0ac8e7c Add Oracle 11 to Travis build.
new b4ac43e Sort members.
new b97da10 [COLLECTIONS-710] Calling CompositeCollection.size() will cash if the list contains null element.
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../collection/CompositeCollection.java | 30 +-
.../collection/CompositeCollectionTest.java | 325 ++++++++++++---------
2 files changed, 202 insertions(+), 153 deletions(-)
[commons-collections] 01/02: Sort members.
Posted by gg...@apache.org.
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 b4ac43e113dfe723856fe0186ef46923ca5ba573
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Sat Feb 9 17:21:31 2019 -0500
Sort members.
---
.../collection/CompositeCollectionTest.java | 292 ++++++++++-----------
1 file changed, 146 insertions(+), 146 deletions(-)
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 855c604..b3b625f 100644
--- a/src/test/java/org/apache/commons/collections4/collection/CompositeCollectionTest.java
+++ b/src/test/java/org/apache/commons/collections4/collection/CompositeCollectionTest.java
@@ -31,11 +31,28 @@ import java.util.List;
*/
public class CompositeCollectionTest<E> extends AbstractCollectionTest<E> {
+ protected CompositeCollection<E> c;
+
+ protected Collection<E> one;
+
+ protected Collection<E> two;
+
public CompositeCollectionTest(final String name) {
super(name);
}
- //-----------------------------------------------------------------------------
+ @Override
+ public String getCompatibilityVersion() {
+ return "4";
+ }
+
+ @Override
+ @SuppressWarnings("unchecked")
+ public E[] getFullElements() {
+ return (E[]) new Object[] { "1", "2", "3", "4" };
+ }
+
+ //-----------------------------------------------------------------------------
/**
* Run stock collection tests without Mutator, so turn off add, remove
*/
@@ -49,25 +66,22 @@ public class CompositeCollectionTest<E> extends AbstractCollectionTest<E> {
return false;
}
- /**
- * Empty collection is empty composite
- */
- @Override
- public Collection<E> makeObject() {
- return new CompositeCollection<>();
- }
-
@Override
public Collection<E> makeConfirmedCollection() {
return new HashSet<>();
}
+ //--------------------------------------------------------------------------
+
+ /**
+ * Full collection should look like a collection with 4 elements
+ */
@Override
- @SuppressWarnings("unchecked")
- public E[] getFullElements() {
- return (E[]) new Object[] { "1", "2", "3", "4" };
+ public Collection<E> makeConfirmedFullCollection() {
+ final Collection<E> collection = new HashSet<>();
+ collection.addAll(Arrays.asList(getFullElements()));
+ return collection;
}
-
/**
* Full collection consists of 4 collections, each with one element
*/
@@ -82,43 +96,12 @@ public class CompositeCollectionTest<E> extends AbstractCollectionTest<E> {
}
return compositeCollection;
}
-
- /**
- * Full collection should look like a collection with 4 elements
- */
- @Override
- public Collection<E> makeConfirmedFullCollection() {
- final Collection<E> collection = new HashSet<>();
- collection.addAll(Arrays.asList(getFullElements()));
- return collection;
- }
-
/**
- * Override testUnsupportedRemove, since the default impl expects removeAll,
- * retainAll and iterator().remove to throw
+ * Empty collection is empty composite
*/
@Override
- public void testUnsupportedRemove() {
- resetFull();
- try {
- getCollection().remove(null);
- fail("remove should raise UnsupportedOperationException");
- } catch (final UnsupportedOperationException e) {
- // expected
- }
- verify();
- }
-
- //--------------------------------------------------------------------------
-
- protected CompositeCollection<E> c;
- protected Collection<E> one;
- protected Collection<E> two;
-
- protected void setUpTest() {
- c = new CompositeCollection<>();
- one = new HashSet<>();
- two = new HashSet<>();
+ public Collection<E> makeObject() {
+ return new CompositeCollection<>();
}
@SuppressWarnings("serial")
@@ -154,91 +137,10 @@ public class CompositeCollectionTest<E> extends AbstractCollectionTest<E> {
});
}
- @SuppressWarnings("unchecked")
- public void testSize() {
- setUpTest();
- final HashSet<E> set = new HashSet<>();
- set.add((E) "a");
- set.add((E) "b");
- c.addComposited(set);
- assertEquals(set.size(), c.size());
- }
-
- @SuppressWarnings("unchecked")
- public void testMultipleCollectionsSize() {
- setUpTest();
- final HashSet<E> set = new HashSet<>();
- set.add((E) "a");
- set.add((E) "b");
- c.addComposited(set);
- final HashSet<E> other = new HashSet<>();
- other.add((E) "c");
- c.addComposited(other);
- assertEquals(set.size() + other.size(), c.size());
- }
-
- @SuppressWarnings("unchecked")
- public void testIsEmpty() {
- setUpTest();
- assertTrue(c.isEmpty());
- final HashSet<E> empty = new HashSet<>();
- c.addComposited(empty);
- assertTrue(c.isEmpty());
- empty.add((E) "a");
- assertTrue(!c.isEmpty());
- }
-
-
- @SuppressWarnings("unchecked")
- public void testIterator() {
- setUpTest();
- one.add((E) "1");
- two.add((E) "2");
- c.addComposited(one);
- c.addComposited(two);
- final Iterator<E> i = c.iterator();
- E next = i.next();
- assertTrue(c.contains(next));
- assertTrue(one.contains(next));
- next = i.next();
- i.remove();
- assertTrue(!c.contains(next));
- assertTrue(!two.contains(next));
- }
-
- @SuppressWarnings("unchecked")
- public void testClear() {
- setUpTest();
- one.add((E) "1");
- two.add((E) "2");
- c.addComposited(one, two);
- c.clear();
- assertTrue(one.isEmpty());
- assertTrue(two.isEmpty());
- assertTrue(c.isEmpty());
- }
-
- @SuppressWarnings("unchecked")
- public void testContainsAll() {
- setUpTest();
- one.add((E) "1");
- two.add((E) "1");
- c.addComposited(one);
- assertTrue(c.containsAll(two));
- }
-
- @SuppressWarnings("unchecked")
- public void testRetainAll() {
- setUpTest();
- one.add((E) "1");
- one.add((E) "2");
- two.add((E) "1");
- c.addComposited(one);
- c.retainAll(two);
- assertTrue(!c.contains("2"));
- assertTrue(!one.contains("2"));
- assertTrue(c.contains("1"));
- assertTrue(one.contains("1"));
+ protected void setUpTest() {
+ c = new CompositeCollection<>();
+ one = new HashSet<>();
+ two = new HashSet<>();
}
@SuppressWarnings({ "unchecked", "serial" })
@@ -277,6 +179,18 @@ public class CompositeCollectionTest<E> extends AbstractCollectionTest<E> {
assertTrue(one.contains("foo"));
}
+ @SuppressWarnings("unchecked")
+ public void testAddAllToCollection() {
+ setUpTest();
+ one.add((E) "1");
+ two.add((E) "2");
+ c.addComposited(one, two);
+ final Collection<E> toCollection = new HashSet<>();
+ toCollection.addAll(c);
+ assertTrue(toCollection.containsAll(c));
+ assertEquals(c.size(), toCollection.size());
+ }
+
@SuppressWarnings({ "unchecked", "serial" })
public void testAddMutator() {
setUpTest();
@@ -312,29 +226,67 @@ public class CompositeCollectionTest<E> extends AbstractCollectionTest<E> {
assertTrue(one.contains("foo"));
}
+
@SuppressWarnings("unchecked")
- public void testToCollection() {
+ public void testClear() {
setUpTest();
one.add((E) "1");
two.add((E) "2");
c.addComposited(one, two);
- final Collection<E> foo = c.toCollection();
- assertTrue(foo.containsAll(c));
- assertEquals(c.size(), foo.size());
- one.add((E) "3");
- assertTrue(!foo.containsAll(c));
+ c.clear();
+ assertTrue(one.isEmpty());
+ assertTrue(two.isEmpty());
+ assertTrue(c.isEmpty());
}
@SuppressWarnings("unchecked")
- public void testAddAllToCollection() {
+ public void testContainsAll() {
+ setUpTest();
+ one.add((E) "1");
+ two.add((E) "1");
+ c.addComposited(one);
+ assertTrue(c.containsAll(two));
+ }
+
+ @SuppressWarnings("unchecked")
+ public void testIsEmpty() {
+ setUpTest();
+ assertTrue(c.isEmpty());
+ final HashSet<E> empty = new HashSet<>();
+ c.addComposited(empty);
+ assertTrue(c.isEmpty());
+ empty.add((E) "a");
+ assertTrue(!c.isEmpty());
+ }
+
+ @SuppressWarnings("unchecked")
+ public void testIterator() {
setUpTest();
one.add((E) "1");
two.add((E) "2");
- c.addComposited(one, two);
- final Collection<E> toCollection = new HashSet<>();
- toCollection.addAll(c);
- assertTrue(toCollection.containsAll(c));
- assertEquals(c.size(), toCollection.size());
+ c.addComposited(one);
+ c.addComposited(two);
+ final Iterator<E> i = c.iterator();
+ E next = i.next();
+ assertTrue(c.contains(next));
+ assertTrue(one.contains(next));
+ next = i.next();
+ i.remove();
+ assertTrue(!c.contains(next));
+ assertTrue(!two.contains(next));
+ }
+
+ @SuppressWarnings("unchecked")
+ public void testMultipleCollectionsSize() {
+ setUpTest();
+ final HashSet<E> set = new HashSet<>();
+ set.add((E) "a");
+ set.add((E) "b");
+ c.addComposited(set);
+ final HashSet<E> other = new HashSet<>();
+ other.add((E) "c");
+ c.addComposited(other);
+ assertEquals(set.size() + other.size(), c.size());
}
@SuppressWarnings("unchecked")
@@ -377,9 +329,57 @@ public class CompositeCollectionTest<E> extends AbstractCollectionTest<E> {
assertEquals(2, c.size());
}
+ @SuppressWarnings("unchecked")
+ public void testRetainAll() {
+ setUpTest();
+ one.add((E) "1");
+ one.add((E) "2");
+ two.add((E) "1");
+ c.addComposited(one);
+ c.retainAll(two);
+ assertTrue(!c.contains("2"));
+ assertTrue(!one.contains("2"));
+ assertTrue(c.contains("1"));
+ assertTrue(one.contains("1"));
+ }
+
+ @SuppressWarnings("unchecked")
+ public void testSize() {
+ setUpTest();
+ final HashSet<E> set = new HashSet<>();
+ set.add((E) "a");
+ set.add((E) "b");
+ c.addComposited(set);
+ assertEquals(set.size(), c.size());
+ }
+
+ @SuppressWarnings("unchecked")
+ public void testToCollection() {
+ setUpTest();
+ one.add((E) "1");
+ two.add((E) "2");
+ c.addComposited(one, two);
+ final Collection<E> foo = c.toCollection();
+ assertTrue(foo.containsAll(c));
+ assertEquals(c.size(), foo.size());
+ one.add((E) "3");
+ assertTrue(!foo.containsAll(c));
+ }
+
+ /**
+ * Override testUnsupportedRemove, since the default impl expects removeAll,
+ * retainAll and iterator().remove to throw
+ */
@Override
- public String getCompatibilityVersion() {
- return "4";
+ public void testUnsupportedRemove() {
+ resetFull();
+ try {
+ getCollection().remove(null);
+ fail("remove should raise UnsupportedOperationException");
+ } catch (final UnsupportedOperationException e) {
+ // expected
+ }
+ verify();
}
// public void testCreate() throws Exception {
[commons-collections] 02/02: [COLLECTIONS-710] Calling
CompositeCollection.size() will cash if the list contains null element.
Posted by gg...@apache.org.
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 <ga...@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")