[COLLECTIONS-796] SetUniqueList.createSetBasedOnList doesn't add list elements to return value

- Request in Comment: https://issues.apache.org/jira/browse/COLLECTIONS-796?focusedCommentId=17419058&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17419058
- Adressed typo in method name.
- Method is public therefore it must be backwards compatible.
- The method 'umodifiableListIterator' was not removed but annotated with @Deprecated and calls the actual method 'unmodifiableListIterator'.
- Test was simplified, with appropriate assert-method-call
This commit is contained in:
Clemens Kurz 2021-09-23 14:27:24 +02:00 committed by Bruno P. Kinoshita
parent 14291172fe
commit ffd2a02d85
7 changed files with 59 additions and 37 deletions

View File

@ -469,7 +469,7 @@ public class IteratorUtils {
* @return an immutable version of the iterator
*/
public static <E> ListIterator<E> unmodifiableListIterator(final ListIterator<E> listIterator) {
return UnmodifiableListIterator.umodifiableListIterator(listIterator);
return UnmodifiableListIterator.unmodifiableListIterator(listIterator);
}
/**

View File

@ -43,7 +43,7 @@ public final class UnmodifiableListIterator<E> implements ListIterator<E>, Unmod
* @return a new unmodifiable list iterator
* @throws NullPointerException if the iterator is null
*/
public static <E> ListIterator<E> umodifiableListIterator(final ListIterator<? extends E> iterator) {
public static <E> ListIterator<E> unmodifiableListIterator(final ListIterator<? extends E> iterator) {
Objects.requireNonNull(iterator, "iterator");
if (iterator instanceof Unmodifiable) {
@SuppressWarnings("unchecked") // safe to upcast
@ -53,6 +53,14 @@ public final class UnmodifiableListIterator<E> implements ListIterator<E>, Unmod
return new UnmodifiableListIterator<>(iterator);
}
/**
* @deprecated method name has typo in it. Use {@link org.apache.commons.collections4.iterators.UnmodifiableListIterator#unmodifiableListIterator(ListIterator)} instead.
*/
@Deprecated
public static <E> ListIterator<E> umodifiableListIterator(final ListIterator<? extends E> iterator) {
return unmodifiableListIterator(iterator);
}
/**
* Constructor.
*

View File

@ -352,6 +352,7 @@ public class SetUniqueList<E> extends AbstractSerializableListDecorator<E> {
subSet = new HashSet<>();
}
}
subSet.addAll(list);
return subSet;
}

View File

@ -118,12 +118,12 @@ public final class UnmodifiableList<E>
@Override
public ListIterator<E> listIterator() {
return UnmodifiableListIterator.umodifiableListIterator(decorated().listIterator());
return UnmodifiableListIterator.unmodifiableListIterator(decorated().listIterator());
}
@Override
public ListIterator<E> listIterator(final int index) {
return UnmodifiableListIterator.umodifiableListIterator(decorated().listIterator(index));
return UnmodifiableListIterator.unmodifiableListIterator(decorated().listIterator(index));
}
@Override

View File

@ -307,12 +307,12 @@ public class LinkedMap<K, V> extends AbstractLinkedMap<K, V> implements Serializ
@Override
public ListIterator<K> listIterator() {
return UnmodifiableListIterator.umodifiableListIterator(super.listIterator());
return UnmodifiableListIterator.unmodifiableListIterator(super.listIterator());
}
@Override
public ListIterator<K> listIterator(final int fromIndex) {
return UnmodifiableListIterator.umodifiableListIterator(super.listIterator(fromIndex));
return UnmodifiableListIterator.unmodifiableListIterator(super.listIterator(fromIndex));
}
@Override

View File

@ -24,9 +24,10 @@ import java.util.ListIterator;
import org.apache.commons.collections4.Unmodifiable;
import static org.junit.jupiter.api.Assertions.assertThrows;
/**
* Tests the UnmodifiableListIterator.
*
*/
public class UnmodifiableListIteratorTest<E> extends AbstractListIteratorTest<E> {
@ -49,12 +50,12 @@ public class UnmodifiableListIteratorTest<E> extends AbstractListIteratorTest<E>
@Override
public ListIterator<E> makeEmptyIterator() {
return UnmodifiableListIterator.umodifiableListIterator(Collections.<E>emptyList().listIterator());
return UnmodifiableListIterator.unmodifiableListIterator(Collections.<E>emptyList().listIterator());
}
@Override
public ListIterator<E> makeObject() {
return UnmodifiableListIterator.umodifiableListIterator(testList.listIterator());
return UnmodifiableListIterator.unmodifiableListIterator(testList.listIterator());
}
@Override
@ -78,15 +79,12 @@ public class UnmodifiableListIteratorTest<E> extends AbstractListIteratorTest<E>
public void testDecorateFactory() {
ListIterator<E> it = makeObject();
assertSame(it, UnmodifiableListIterator.umodifiableListIterator(it));
assertSame(it, UnmodifiableListIterator.unmodifiableListIterator(it));
it = testList.listIterator();
assertTrue(it != UnmodifiableListIterator.umodifiableListIterator(it));
assertNotSame(it, UnmodifiableListIterator.unmodifiableListIterator(it));
try {
UnmodifiableListIterator.umodifiableListIterator(null);
fail();
} catch (final NullPointerException ex) {}
assertThrows(NullPointerException.class, () -> UnmodifiableListIterator.unmodifiableListIterator(null));
}
}

View File

@ -16,14 +16,13 @@
*/
package org.apache.commons.collections4.list;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.ListIterator;
import java.util.Set;
import org.apache.commons.collections4.set.UnmodifiableSet;
import org.junit.Assert;
import org.junit.jupiter.api.Assertions;
import java.util.*;
import static org.junit.jupiter.api.Assertions.assertThrows;
/**
* JUnit tests.
@ -194,7 +193,7 @@ public class SetUniqueListTest<E> extends AbstractListTest<E> {
// repeat the test with a different class than HashSet;
// which means subclassing SetUniqueList below
list = new ArrayList<>();
uniqueList = new SetUniqueList307(list, new java.util.TreeSet<E>());
uniqueList = new SetUniqueList307(list, new TreeSet<E>());
uniqueList.add((E) hello);
uniqueList.add((E) world);
@ -558,12 +557,8 @@ public class SetUniqueListTest<E> extends AbstractListTest<E> {
public void testSubListIsUnmodifiable() {
resetFull();
final List<E> subList = getCollection().subList(1, 3);
try {
subList.remove(0);
fail("subList should be unmodifiable");
} catch (final UnsupportedOperationException e) {
// expected
}
assertEquals(2, subList.size());
assertThrows(UnsupportedOperationException.class, () -> subList.remove(0));
}
@SuppressWarnings("unchecked")
@ -616,11 +611,31 @@ public class SetUniqueListTest<E> extends AbstractListTest<E> {
}
}
// public void testCreate() throws Exception {
// resetEmpty();
// writeExternalFormToDisk((java.io.Serializable) getCollection(), "src/test/resources/data/test/SetUniqueList.emptyCollection.version4.obj");
// resetFull();
// writeExternalFormToDisk((java.io.Serializable) getCollection(), "src/test/resources/data/test/SetUniqueList.fullCollection.version4.obj");
// }
@SuppressWarnings("unchecked")
public void testCreateSetBasedOnList() {
final List<String> list = new ArrayList<>();
list.add("One");
list.add("Two");
@SuppressWarnings("rawtypes") final SetUniqueList setUniqueList = (SetUniqueList) makeObject();
// Standard case with HashSet
final Set<String> setBasedOnList = setUniqueList.createSetBasedOnList(new HashSet<>(), list);
assertEquals(list.size(), setBasedOnList.size());
list.forEach(item -> assertTrue(setBasedOnList.contains(item)));
// Use different Set than HashSet
final Set<String> setBasedOnList1 = setUniqueList.createSetBasedOnList(new TreeSet<>(), list);
assertEquals(list.size(), setBasedOnList1.size());
list.forEach(item -> assertTrue(setBasedOnList1.contains(item)));
// throws internally NoSuchMethodException --> results in HashSet
final Set<String> setBasedOnList2 = setUniqueList.createSetBasedOnList(UnmodifiableSet.unmodifiableSet(new HashSet<>()), list);
assertEquals(list.size(), setBasedOnList2.size());
list.forEach(item -> assertTrue(setBasedOnList2.contains(item)));
// provide null values as Parameter
assertThrows(NullPointerException.class, () -> setUniqueList.createSetBasedOnList(null, list));
assertThrows(NullPointerException.class, () -> setUniqueList.createSetBasedOnList(new HashSet<>(), null));
}
}