From a33855109b817d0472a6bbf7b96168102ce0b623 Mon Sep 17 00:00:00 2001 From: JJ Brown Date: Sun, 12 Apr 2020 23:45:52 -0500 Subject: [PATCH 1/2] Provides optimized matchers for IsMapContaining.hasKey() and IsMapContaining.hasEntry(), which use the map's own containsKey(K key) method to avoid an O(n) worst-case linear search for every match. Incorporated null-safety checks to account for maps that do not support null keys, per @nibsi 's recommendation --- .../hamcrest/collection/IsMapContaining.java | 80 ++++++++++++++++++- .../collection/IsMapContainingKeyTest.java | 6 +- 2 files changed, 80 insertions(+), 6 deletions(-) diff --git a/hamcrest/src/main/java/org/hamcrest/collection/IsMapContaining.java b/hamcrest/src/main/java/org/hamcrest/collection/IsMapContaining.java index 4ed45392..6bf4aadc 100644 --- a/hamcrest/src/main/java/org/hamcrest/collection/IsMapContaining.java +++ b/hamcrest/src/main/java/org/hamcrest/collection/IsMapContaining.java @@ -72,9 +72,39 @@ public static Matcher> hasEntry(Matcher Matcher> hasEntry(K key, V value) { - return new IsMapContaining<>(equalTo(key), equalTo(value)); + return new IsMapContainingEntry<>(key, value); } - + + /** + * Provides a type-safe optimization over the O(n) linear search in {@link IsMapContaining#matchesSafely(Map)}, + * by leveraging the speed of the map's own {@link Map#containsKey(Object)} check. + *

+ * It preserves the same descriptors. + */ + private static class IsMapContainingEntry extends IsMapContaining + { + private final K key; + + public IsMapContainingEntry(K key, V value) + { + super(equalTo(key), equalTo(value)); + this.key = key; + } + + @Override + public boolean matchesSafely(Map map) + { + try{ + return map.containsKey(key) && super.valueMatcher.matches(map.get(key)); + } catch (NullPointerException e){ + // some maps (like Hashtable) don't want to let you check for a null key. + // to be consistent with previous behavior checking each entry, + // we squash any error coming from that to indicate simply that there's no entry with that key. + return false; + } + } + } + /** * Creates a matcher for {@link java.util.Map}s matching when the examined {@link java.util.Map} contains * at least one key that satisfies the specified matcher. @@ -98,7 +128,51 @@ public static Matcher> hasEntry(K key, V valu * the key that satisfying maps must contain */ public static Matcher> hasKey(K key) { - return new IsMapContaining<>(equalTo(key), anything()); + return new IsMapContainingKey<>(key); + } + + /** + * Provides a type-safe optimization over the O(n) linear search in {@link IsMapContaining#matchesSafely(Map)}, + * by leveraging the speed of the map's own {@link Map#containsKey(Object)} check. + *

+ * It preserves the same descriptors. + */ + private static class IsMapContainingKey extends IsMapContaining + { + private final K key; + + public IsMapContainingKey(K key) + { + super(equalTo(key), anything()); + this.key = key; + } + + @Override + public boolean matchesSafely(Map map) + { + try + { + return map.containsKey(key); + } catch (NullPointerException e){ + // some maps (like Hashtable) don't want to let you check for a null key. + // to be consistent with previous behavior checking each entry, + // we squash any error coming from that to indicate simply that there's no entry with that key. + return false; + } + } + + @Override + public void describeMismatchSafely(Map map, Description mismatchDescription) + { + mismatchDescription.appendText("map keys were ").appendValueList("[", ", ", "]", map.keySet()); + } + + @Override + public void describeTo(Description description) + { + description.appendText("map containing key ") + .appendDescriptionOf(super.keyMatcher); + } } /** diff --git a/hamcrest/src/test/java/org/hamcrest/collection/IsMapContainingKeyTest.java b/hamcrest/src/test/java/org/hamcrest/collection/IsMapContainingKeyTest.java index 13f067c8..66876d0d 100644 --- a/hamcrest/src/test/java/org/hamcrest/collection/IsMapContainingKeyTest.java +++ b/hamcrest/src/test/java/org/hamcrest/collection/IsMapContainingKeyTest.java @@ -66,11 +66,11 @@ public void testMatchesMapContainingKeyWithNumberKeys() throws Exception { } public void testHasReadableDescription() { - assertDescription("map containing [\"a\"->ANYTHING]", hasKey("a")); + assertDescription("map containing key \"a\"", hasKey("a")); } public void testDoesNotMatchEmptyMap() { - assertMismatchDescription("map was []", hasKey("Foo"), new HashMap()); + assertMismatchDescription("map keys were []", hasKey("Foo"), new HashMap()); } public void testDoesNotMatchMapMissingKey() { @@ -79,6 +79,6 @@ public void testDoesNotMatchMapMissingKey() { map.put("b", 2); map.put("c", 3); - assertMismatchDescription("map was [, , ]", hasKey("d"), map); + assertMismatchDescription("map keys were [\"a\", \"b\", \"c\"]", hasKey("d"), map); } } From 3a772560b5527931128384daace966bc1f8b2879 Mon Sep 17 00:00:00 2001 From: JJ Brown Date: Wed, 15 Apr 2020 21:00:35 -0500 Subject: [PATCH 2/2] Added tests for null-key edge cases of IsMapContaining.hasKey() and IsMapContaining.hasEntry() --- .../collection/IsMapContainingKeyTest.java | 21 ++++++++++++++- .../collection/IsMapContainingTest.java | 26 +++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/hamcrest/src/test/java/org/hamcrest/collection/IsMapContainingKeyTest.java b/hamcrest/src/test/java/org/hamcrest/collection/IsMapContainingKeyTest.java index 66876d0d..1dc347e6 100644 --- a/hamcrest/src/test/java/org/hamcrest/collection/IsMapContainingKeyTest.java +++ b/hamcrest/src/test/java/org/hamcrest/collection/IsMapContainingKeyTest.java @@ -4,6 +4,7 @@ import org.hamcrest.Matcher; import java.util.HashMap; +import java.util.Hashtable; import java.util.Map; import java.util.TreeMap; @@ -62,7 +63,25 @@ public void testMatchesMapContainingKeyWithNumberKeys() throws Exception { assertThat(map, hasKey((Number)1)); // TODO: work out the correct sprinkling of wildcards to get this to work! -// assertThat(map, hasKey(1)); + // assertThat(map, hasKey(1)); + } + + public void test_mapContainingNullKey_returnsFalse_forMapsWithNonnullKeys() throws Exception { + Map map = new Hashtable<>(); // Hashtables cannot store null keys + map.put(1, "A"); + map.put(2, "B"); + + assertDoesNotMatch(hasKey((Number)null), map); + } + + + public void test_mapContainingNullKey_returnsTrue_forMapWithNullKey() throws Exception { + Map map = new HashMap<>(); // HashMap can store null keys + map.put(1, "A"); + map.put(2, "B"); + map.put(null, "C"); + + assertMatches(hasKey((Number)null), map); } public void testHasReadableDescription() { diff --git a/hamcrest/src/test/java/org/hamcrest/collection/IsMapContainingTest.java b/hamcrest/src/test/java/org/hamcrest/collection/IsMapContainingTest.java index 3eed3463..2d0fd937 100644 --- a/hamcrest/src/test/java/org/hamcrest/collection/IsMapContainingTest.java +++ b/hamcrest/src/test/java/org/hamcrest/collection/IsMapContainingTest.java @@ -4,6 +4,7 @@ import org.hamcrest.Matcher; import java.util.HashMap; +import java.util.Hashtable; import java.util.Map; import java.util.TreeMap; @@ -46,4 +47,29 @@ public void testDoesNotMatchNull() { public void testHasReadableDescription() { assertDescription("map containing [\"a\"-><2>]", hasEntry(equalTo("a"), (equalTo(2)))); } + + public void test_mapContainsEntryWithNullKey_returnsFalseForMapsWithoutNullKeys(){ + Map map = new Hashtable<>(); // throws exception if given null key, or if map.containsKey(null) is called + map.put("a", 1); + map.put("b", 2); + + assertDoesNotMatch(hasEntry(null, 2), map); + } + + public void test_mapContainsEntryWithNullKey_returnsTrueForMapWithNullKeyAndMatchingValue(){ + Map map = new HashMap<>(); // throws exception if given null key, or if map.containsKey(null) is called + map.put("a", 1); + map.put("b", 2); + map.put(null, 3); + + assertMatches(hasEntry(null, 3), map); + } + + public void test_mapContainsEntryWithNullKey_returnsFalseForMapWithNullKeyAndNoMatchingValue(){ + Map map = new HashMap<>(); // throws exception if given null key, or if map.containsKey(null) is called + map.put("a", 1); + map.put("b", 2); + + assertDoesNotMatch(hasEntry(null, 3), map); + } }