Fix nextKey skipping a key and throwing for keys not in the map

src/main/java/org/apache/commons/collections4/bidimap/DualTreeBidiMap.java

@@ -25,6 +25,7 @@
 import java.util.Iterator;
 import java.util.ListIterator;
 import java.util.Map;
+import java.util.NavigableMap;
 import java.util.SortedMap;
 import java.util.TreeMap;
 
@@ -364,13 +365,16 @@ public OrderedMapIterator<K, V> mapIterator() {
 
     @Override
     public K nextKey(final K key) {
-        if (isEmpty()) {
+        if (isEmpty() || !normalMap.containsKey(key)) {
             return null;
         }
         if (normalMap instanceof OrderedMap) {
             return ((OrderedMap<K, ?>) normalMap).nextKey(key);
         }
         final SortedMap<K, V> sm = (SortedMap<K, V>) normalMap;
+        if (sm instanceof NavigableMap) {
+            return ((NavigableMap<K, V>) sm).higherKey(key);
+        }
         final Iterator<K> it = sm.tailMap(key).keySet().iterator();
         it.next();
         if (it.hasNext()) {

src/main/java/org/apache/commons/collections4/map/AbstractSortedMapDecorator.java

@@ -20,6 +20,7 @@
 import java.util.Iterator;
 import java.util.ListIterator;
 import java.util.Map;
+import java.util.NavigableMap;
 import java.util.Set;
 import java.util.SortedMap;
 
@@ -151,6 +152,13 @@ public OrderedMapIterator<K, V> mapIterator() {
 
     @Override
     public K nextKey(final K key) {
+        if (!containsKey(key)) {
+            return null;
+        }
+        final SortedMap<K, V> map = decorated();
+        if (map instanceof NavigableMap) {
+            return ((NavigableMap<K, V>) map).higherKey(key);
+        }
         final Iterator<K> it = tailMap(key).keySet().iterator();
         it.next();
         return it.hasNext() ? it.next() : null;

src/test/java/org/apache/commons/collections4/bidimap/AbstractOrderedBidiMapTest.java

@@ -157,6 +157,8 @@ void testNextKey() {
             confirmedLast = confirmedObject;
         }
         assertNull(bidi.nextKey(confirmedLast));
+        // a key that is not in the map has no next key
+        assertNull(bidi.nextKey(getOtherKeys()[0]));
 
         if (!isAllowNullKey()) {
             final OrderedBidiMap<K, V> finalBidi = bidi;

src/test/java/org/apache/commons/collections4/bidimap/DualTreeBidiMapTest.java

@@ -16,6 +16,11 @@
  */
 package org.apache.commons.collections4.bidimap;
 
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+
+import org.junit.jupiter.api.Test;
+
 /**
  * JUnit tests.
  */
@@ -29,6 +34,20 @@ public DualTreeBidiMap<K, V> makeObject() {
         return new DualTreeBidiMap<>();
     }
 
+    @Test
+    void testNextKeyAbsentKey() {
+        final DualTreeBidiMap<String, Integer> map = new DualTreeBidiMap<>();
+        map.put("a", 1);
+        map.put("c", 3);
+        map.put("e", 5);
+        // an absent key inside the key range must not return the successor
+        assertNull(map.nextKey("b"));
+        // an absent key past the last key must not throw
+        assertNull(map.nextKey("z"));
+        assertEquals("c", map.nextKey("a"));
+        assertNull(map.nextKey("e"));
+    }
+
 //    void testCreate() throws Exception {
 //        resetEmpty();
 //        writeExternalFormToDisk((java.io.Serializable) map, "src/test/resources/data/test/DualTreeBidiMap.emptyCollection.version4.obj");

src/test/java/org/apache/commons/collections4/map/FixedSizeSortedMapTest.java

@@ -18,6 +18,7 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import java.util.Collections;
@@ -68,6 +69,23 @@ public SortedMap<K, V> makeObject() {
         return FixedSizeSortedMap.fixedSizeSortedMap(new TreeMap<>());
     }
 
+    @Test
+    void testNextKey() {
+        final SortedMap<String, String> base = new TreeMap<>();
+        base.put("a", "1");
+        base.put("c", "3");
+        base.put("e", "5");
+        final FixedSizeSortedMap<String, String> fixed = FixedSizeSortedMap.fixedSizeSortedMap(base);
+        // a present key returns its successor, or null once at the end
+        assertEquals("c", fixed.nextKey("a"));
+        assertEquals("e", fixed.nextKey("c"));
+        assertNull(fixed.nextKey("e"));
+        // a key that is not in the map has no next key, whether it is in range or past the end
+        assertNull(fixed.nextKey("b"));
+        assertNull(fixed.nextKey("z"));
+        assertNull(FixedSizeSortedMap.fixedSizeSortedMap(new TreeMap<String, String>()).nextKey("a"));
+    }
+
     @Test
     void testPutAllAllowsUpdatesRejectsNewKeys() {
         final SortedMap<String, String> base = new TreeMap<>();

src/test/java/org/apache/commons/collections4/map/UnmodifiableSortedMapTest.java

@@ -16,13 +16,16 @@
  */
 package org.apache.commons.collections4.map;
 
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.util.SortedMap;
 import java.util.TreeMap;
 
+import org.apache.commons.collections4.OrderedMap;
 import org.apache.commons.collections4.Unmodifiable;
 import org.junit.jupiter.api.Test;
 
@@ -67,6 +70,19 @@ public SortedMap<K, V> makeObject() {
         return UnmodifiableSortedMap.unmodifiableSortedMap(new TreeMap<>());
     }
 
+    @Test
+    void testNextKey() {
+        final SortedMap<String, String> base = new TreeMap<>();
+        base.put("a", "1");
+        base.put("c", "3");
+        final OrderedMap<String, String> map = (OrderedMap<String, String>) UnmodifiableSortedMap.unmodifiableSortedMap(base);
+        assertEquals("c", map.nextKey("a"));
+        // an absent key has no next key, whether inside the key range or past the end
+        assertNull(map.nextKey("b"));
+        assertNull(map.nextKey("c"));
+        assertNull(map.nextKey("z"));
+    }
+
     @Test
     void testDecorateFactory() {
         final SortedMap<K, V> map = makeFullMap();