Bug fix where Map/BidiMap implementations only checked key

and not value in entrySet contains(Object) and remove(Object)


git-svn-id: https://svn.apache.org/repos/asf/jakarta/commons/proper/collections/trunk@131620 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Stephen Colebourne 2004-04-01 22:18:12 +00:00
parent f34f3d2cff
commit 199716c53f
5 changed files with 97 additions and 27 deletions

View File

@ -43,6 +43,7 @@ No interface changes, or deprecations have occurred.
<center><h3>BUG FIXES</h3></center>
<ul>
<li>Map/BidiMap implementations only checked key and not value in entry set contains(Object) and remove(Object)</li>
<li>AbstractHashedMap subclasses failed to clone() correctly [27159]</li>
<li>ExtendedProperties - Close input stream in constructor [27737]</li>
</ul>

View File

@ -36,7 +36,7 @@ import org.apache.commons.collections.keyvalue.AbstractMapEntryDecorator;
* @see DualHashBidiMap
* @see DualTreeBidiMap
* @since Commons Collections 3.0
* @version $Id: AbstractDualBidiMap.java,v 1.10 2004/02/18 00:57:39 scolebourne Exp $
* @version $Id: AbstractDualBidiMap.java,v 1.11 2004/04/01 22:18:12 scolebourne Exp $
*
* @author Matthew Hawthorne
* @author Stephen Colebourne
@ -515,10 +515,14 @@ public abstract class AbstractDualBidiMap implements BidiMap {
return false;
}
Map.Entry entry = (Map.Entry) obj;
if (parent.containsKey(entry.getKey())) {
Object value = parent.maps[0].remove(entry.getKey());
parent.maps[1].remove(value);
return true;
Object key = entry.getKey();
if (parent.containsKey(key)) {
Object value = parent.maps[0].get(key);
if (value == null ? entry.getValue() == null : value.equals(entry.getValue())) {
parent.maps[0].remove(key);
parent.maps[1].remove(value);
return true;
}
}
return false;
}

View File

@ -46,7 +46,7 @@ import org.apache.commons.collections.MapIterator;
* need for unusual subclasses is here.
*
* @since Commons Collections 3.0
* @version $Revision: 1.13 $ $Date: 2004/03/13 15:54:34 $
* @version $Revision: 1.14 $ $Date: 2004/04/01 22:18:12 $
*
* @author java util HashMap
* @author Stephen Colebourne
@ -367,12 +367,12 @@ public class AbstractHashedMap implements IterableMap {
}
/**
* Compares two keys for equals.
* This implementation uses the equals method.
* Compares two keys, in internal converted form, to see if they are equal.
* This implementation uses the equals method and assumes neither key is null.
* Subclasses can override this to match differently.
*
* @param key1 the first key to compare
* @param key2 the second key to compare
* @param key1 the first key to compare passed in from outside
* @param key2 the second key extracted from the entry via <code>entry.key</code>
* @return true if equal
*/
protected boolean isEqualKey(Object key1, Object key2) {
@ -380,12 +380,12 @@ public class AbstractHashedMap implements IterableMap {
}
/**
* Compares two values for equals.
* This implementation uses the equals method.
* Compares two values, in external form, to see if they are equal.
* This implementation uses the equals method and assumes neither key is null.
* Subclasses can override this to match differently.
*
* @param value1 the first value to compare
* @param value2 the second value to compare
* @param value1 the first value to compare passed in from outside
* @param value2 the second value extracted from the entry via <code>getValue()</code>
* @return true if equal
*/
protected boolean isEqualValue(Object value1, Object value2) {
@ -753,8 +753,6 @@ public class AbstractHashedMap implements IterableMap {
/**
* Gets the entrySet view of the map.
* Changes made to the view affect this map.
* The Map Entry is not an independent object and changes as the
* iterator progresses.
* To simply iterate through the entries, use {@link #mapIterator()}.
*
* @return the entrySet view
@ -801,7 +799,9 @@ public class AbstractHashedMap implements IterableMap {
public boolean contains(Object entry) {
if (entry instanceof Map.Entry) {
return parent.containsKey(((Map.Entry) entry).getKey());
Map.Entry e = (Map.Entry) entry;
Entry match = parent.getEntry(e.getKey());
return (match != null && match.equals(e));
}
return false;
}
@ -810,11 +810,13 @@ public class AbstractHashedMap implements IterableMap {
if (obj instanceof Map.Entry == false) {
return false;
}
if (contains(obj) == false) {
return false;
}
Map.Entry entry = (Map.Entry) obj;
Object key = entry.getKey();
boolean result = parent.containsKey(key);
parent.remove(key);
return result;
return true;
}
public Iterator iterator() {

View File

@ -35,7 +35,8 @@ import org.apache.commons.collections.keyvalue.AbstractMapEntry;
import org.apache.commons.collections.list.UnmodifiableList;
/**
* Decorates a <code>Map</code> to ensure that the order of addition is retained.
* Decorates a <code>Map</code> to ensure that the order of addition is retained
* using a <code>List</code> to maintain order.
* <p>
* The order will be used via the iterators and toArray methods on the views.
* The order is also returned by the <code>MapIterator</code>.
@ -47,7 +48,7 @@ import org.apache.commons.collections.list.UnmodifiableList;
* original position in the iteration.
*
* @since Commons Collections 3.0
* @version $Revision: 1.12 $ $Date: 2004/02/18 01:13:19 $
* @version $Revision: 1.13 $ $Date: 2004/04/01 22:18:12 $
*
* @author Henri Yandell
* @author Stephen Colebourne
@ -385,12 +386,12 @@ public class ListOrderedMap
if (obj instanceof Map.Entry == false) {
return false;
}
Object key = ((Map.Entry) obj).getKey();
if (parent.getMap().containsKey(key) == false) {
return false;
if (getEntrySet().contains(obj)) {
Object key = ((Map.Entry) obj).getKey();
parent.remove(key);
return true;
}
parent.remove(key);
return true;
return false;
}
public void clear() {

View File

@ -28,6 +28,7 @@ import java.util.Set;
import org.apache.commons.collections.AbstractTestObject;
import org.apache.commons.collections.BulkTest;
import org.apache.commons.collections.collection.AbstractTestCollection;
import org.apache.commons.collections.keyvalue.DefaultMapEntry;
import org.apache.commons.collections.set.AbstractTestSet;
/**
@ -117,7 +118,7 @@ import org.apache.commons.collections.set.AbstractTestSet;
* @author Rodney Waldhoff
* @author Paul Jack
* @author Stephen Colebourne
* @version $Revision: 1.8 $ $Date: 2004/02/18 01:20:37 $
* @version $Revision: 1.9 $ $Date: 2004/04/01 22:18:12 $
*/
public abstract class AbstractTestMap extends AbstractTestObject {
@ -1005,6 +1006,67 @@ public abstract class AbstractTestMap extends AbstractTestObject {
assertTrue(map.size() == 0);
assertTrue(entrySet.size() == 0);
}
//-----------------------------------------------------------------------
public void testEntrySetContains1() {
resetFull();
Set entrySet = map.entrySet();
Map.Entry entry = (Map.Entry) entrySet.iterator().next();
assertEquals(true, entrySet.contains(entry));
}
public void testEntrySetContains2() {
resetFull();
Set entrySet = map.entrySet();
Map.Entry entry = (Map.Entry) entrySet.iterator().next();
Map.Entry test = new DefaultMapEntry(entry);
assertEquals(true, entrySet.contains(test));
}
public void testEntrySetContains3() {
resetFull();
Set entrySet = map.entrySet();
Map.Entry entry = (Map.Entry) entrySet.iterator().next();
Map.Entry test = new DefaultMapEntry(entry.getKey(), "A VERY DIFFERENT VALUE");
assertEquals(false, entrySet.contains(test));
}
public void testEntrySetRemove1() {
if (!isRemoveSupported()) return;
resetFull();
int size = map.size();
Set entrySet = map.entrySet();
Map.Entry entry = (Map.Entry) entrySet.iterator().next();
Object key = entry.getKey();
assertEquals(true, entrySet.remove(entry));
assertEquals(false, map.containsKey(key));
assertEquals(size - 1, map.size());
}
public void testEntrySetRemove2() {
if (!isRemoveSupported()) return;
resetFull();
int size = map.size();
Set entrySet = map.entrySet();
Map.Entry entry = (Map.Entry) entrySet.iterator().next();
Object key = entry.getKey();
Map.Entry test = new DefaultMapEntry(entry);
assertEquals(true, entrySet.remove(test));
assertEquals(false, map.containsKey(key));
assertEquals(size - 1, map.size());
}
public void testEntrySetRemove3() {
if (!isRemoveSupported()) return;
resetFull();
int size = map.size();
Set entrySet = map.entrySet();
Map.Entry entry = (Map.Entry) entrySet.iterator().next();
Object key = entry.getKey();
Map.Entry test = new DefaultMapEntry(entry.getKey(), "A VERY DIFFERENT VALUE");
assertEquals(false, entrySet.remove(test));
assertEquals(true, map.containsKey(key));
assertEquals(size, map.size());
}
//-----------------------------------------------------------------------
/**