You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by sc...@apache.org on 2004/04/02 00:18:12 UTC
cvs commit: jakarta-commons/collections/src/java/org/apache/commons/collections/map AbstractHashedMap.java ListOrderedMap.java
scolebourne 2004/04/01 14:18:12
Modified: collections RELEASE-NOTES.html
collections/src/java/org/apache/commons/collections/bidimap
AbstractDualBidiMap.java
collections/src/test/org/apache/commons/collections/map
AbstractTestMap.java
collections/src/java/org/apache/commons/collections/map
AbstractHashedMap.java ListOrderedMap.java
Log:
Bug fix where Map/BidiMap implementations only checked key
and not value in entrySet contains(Object) and remove(Object)
Revision Changes Path
1.23 +1 -0 jakarta-commons/collections/RELEASE-NOTES.html
Index: RELEASE-NOTES.html
===================================================================
RCS file: /home/cvs/jakarta-commons/collections/RELEASE-NOTES.html,v
retrieving revision 1.22
retrieving revision 1.23
diff -u -r1.22 -r1.23
--- RELEASE-NOTES.html 1 Apr 2004 20:12:00 -0000 1.22
+++ RELEASE-NOTES.html 1 Apr 2004 22:18:11 -0000 1.23
@@ -43,6 +43,7 @@
<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>
1.11 +9 -5 jakarta-commons/collections/src/java/org/apache/commons/collections/bidimap/AbstractDualBidiMap.java
Index: AbstractDualBidiMap.java
===================================================================
RCS file: /home/cvs/jakarta-commons/collections/src/java/org/apache/commons/collections/bidimap/AbstractDualBidiMap.java,v
retrieving revision 1.10
retrieving revision 1.11
diff -u -r1.10 -r1.11
--- AbstractDualBidiMap.java 18 Feb 2004 00:57:39 -0000 1.10
+++ AbstractDualBidiMap.java 1 Apr 2004 22:18:12 -0000 1.11
@@ -515,10 +515,14 @@
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;
}
1.9 +63 -1 jakarta-commons/collections/src/test/org/apache/commons/collections/map/AbstractTestMap.java
Index: AbstractTestMap.java
===================================================================
RCS file: /home/cvs/jakarta-commons/collections/src/test/org/apache/commons/collections/map/AbstractTestMap.java,v
retrieving revision 1.8
retrieving revision 1.9
diff -u -r1.8 -r1.9
--- AbstractTestMap.java 18 Feb 2004 01:20:37 -0000 1.8
+++ AbstractTestMap.java 1 Apr 2004 22:18:12 -0000 1.9
@@ -28,6 +28,7 @@
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;
/**
@@ -1004,6 +1005,67 @@
map.clear();
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());
}
//-----------------------------------------------------------------------
1.14 +16 -14 jakarta-commons/collections/src/java/org/apache/commons/collections/map/AbstractHashedMap.java
Index: AbstractHashedMap.java
===================================================================
RCS file: /home/cvs/jakarta-commons/collections/src/java/org/apache/commons/collections/map/AbstractHashedMap.java,v
retrieving revision 1.13
retrieving revision 1.14
diff -u -r1.13 -r1.14
--- AbstractHashedMap.java 13 Mar 2004 15:54:34 -0000 1.13
+++ AbstractHashedMap.java 1 Apr 2004 22:18:12 -0000 1.14
@@ -367,12 +367,12 @@
}
/**
- * 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 @@
}
/**
- * 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 @@
/**
* 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 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 @@
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() {
1.13 +8 -7 jakarta-commons/collections/src/java/org/apache/commons/collections/map/ListOrderedMap.java
Index: ListOrderedMap.java
===================================================================
RCS file: /home/cvs/jakarta-commons/collections/src/java/org/apache/commons/collections/map/ListOrderedMap.java,v
retrieving revision 1.12
retrieving revision 1.13
diff -u -r1.12 -r1.13
--- ListOrderedMap.java 18 Feb 2004 01:13:19 -0000 1.12
+++ ListOrderedMap.java 1 Apr 2004 22:18:12 -0000 1.13
@@ -35,7 +35,8 @@
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>.
@@ -385,12 +386,12 @@
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() {
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org