diff --git a/src/main/java/net/minecraftforge/common/util/MutableHashedLinkedMap.java b/src/main/java/net/minecraftforge/common/util/MutableHashedLinkedMap.java index eb10cf6006..3249098de4 100644 --- a/src/main/java/net/minecraftforge/common/util/MutableHashedLinkedMap.java +++ b/src/main/java/net/minecraftforge/common/util/MutableHashedLinkedMap.java @@ -5,9 +5,11 @@ package net.minecraftforge.common.util; +import java.util.ConcurrentModificationException; import java.util.Iterator; import java.util.Map; import java.util.NoSuchElementException; +import java.util.Objects; import org.jetbrains.annotations.Nullable; @@ -22,11 +24,34 @@ import it.unimi.dsi.fastutil.objects.Object2ObjectOpenCustomHashMap; */ public class MutableHashedLinkedMap implements Iterable> { + /** + * A strategy that uses {@link Objects#hashCode(Object)} and {@link Object#equals(Object)}. + */ + public static final Strategy BASIC = new BasicStrategy(); + + /** + * A strategy that uses {@link System#identityHashCode(Object)} and {@code a == b} comparisons. + */ + public static final Strategy IDENTITY = new IdentityStrategy(); + private final Strategy strategy; private final Map entries; private final MergeFunction merge; private Entry head = null; private Entry last = null; + /* + * Number of changes done to the flow of the map, we do not care about value changes. + * This is to allow the iterator to fast fail with concurrent modification exceptions if needed. + */ + private transient int changes = 0; + + /** + * Creates a new instance using the {@link #BASIC} strategy. + */ + public MutableHashedLinkedMap() + { + this(BASIC); + } /** * Creates a mutable linked map with a default new-value-selecting merge function. @@ -73,8 +98,9 @@ public class MutableHashedLinkedMap implements Iterable> return ret; } - var self = new Entry(key, value); + changes++; + var self = new Entry(key, value); var l = last; self.previous = l; if (l == null) @@ -114,15 +140,40 @@ public class MutableHashedLinkedMap implements Iterable> return new Iterator<>() { private Entry current = head; + private Entry last = null; + private int expectedChanges = changes; + @Override public boolean hasNext() { return current != null; } @Override public Map.Entry next() { + if (changes != expectedChanges) + throw new ConcurrentModificationException(); + if (!hasNext()) throw new NoSuchElementException(); - var ret = current; + + last = current; current = current.next; - return ret; + return last; + } + + @Override + public void remove() + { + if (last == null) + throw new IllegalStateException("Invalid remove() call, must call next() first"); + + if (changes != expectedChanges) + throw new ConcurrentModificationException(); + + var removed = entries.remove(last.getKey()); + if (removed != last) + throw new ConcurrentModificationException(); + + expectedChanges++; + MutableHashedLinkedMap.this.remove(last); + last = null; } }; } @@ -183,6 +234,8 @@ public class MutableHashedLinkedMap implements Iterable> entries.put(key, entry); } + changes++; + entry.previous = target; if (target.next == null) last = target; @@ -230,6 +283,8 @@ public class MutableHashedLinkedMap implements Iterable> entries.put(key, entry); } + changes++; + entry.previous = target.previous; if (target.previous == null) head = entry; @@ -244,7 +299,11 @@ public class MutableHashedLinkedMap implements Iterable> private void remove(Entry e) { - if (head == null) + changes++; + + var previous = e.previous; + + if (head == e) head = e.next; else if (e.previous != null) // Should never be null, but just in case. { @@ -253,10 +312,10 @@ public class MutableHashedLinkedMap implements Iterable> } if (last == e) - last = e.previous; + last = previous; else if (e.next != null) // Should never be null, but just in case. { - e.next.previous = e.previous; + e.next.previous = previous; e.next = null; } } @@ -314,4 +373,28 @@ public class MutableHashedLinkedMap implements Iterable> (value == null ? 0 : value.hashCode()); } } + + private static class BasicStrategy implements Strategy { + @Override + public int hashCode(Object o) { + return Objects.hashCode(o); + } + + @Override + public boolean equals(Object a, Object b) { + return Objects.equals(a, b); + } + } + + private static class IdentityStrategy implements Strategy { + @Override + public int hashCode(Object o) { + return System.identityHashCode(o); + } + + @Override + public boolean equals(Object a, Object b) { + return a == b; + } + } } diff --git a/src/test/java/net/minecraftforge/test/MutableHashedLinkedMapTest.java b/src/test/java/net/minecraftforge/test/MutableHashedLinkedMapTest.java new file mode 100644 index 0000000000..3a10f5c28b --- /dev/null +++ b/src/test/java/net/minecraftforge/test/MutableHashedLinkedMapTest.java @@ -0,0 +1,523 @@ +/* + * Copyright (c) Forge Development LLC and contributors + * SPDX-License-Identifier: LGPL-2.1-only + */ + +package net.minecraftforge.test; + +import it.unimi.dsi.fastutil.Hash.Strategy; +import net.minecraftforge.common.util.MutableHashedLinkedMap; + +import org.junit.jupiter.api.MethodOrderer; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestMethodOrder; + +import java.util.ConcurrentModificationException; +import java.util.Map; +import java.util.function.BiPredicate; +import java.util.function.ToIntFunction; +import java.util.stream.Collector; +import java.util.stream.Stream; +import java.util.stream.StreamSupport; + +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; + +@TestMethodOrder(MethodOrderer.MethodName.class) +public class MutableHashedLinkedMapTest { + private static final Strategy FIRST_CHARACTER = strat( + k -> k == null ? 0 : Character.hashCode(k.charAt(0)), + (a, b) -> a == null ? b == null : b == null ? false : a.charAt(0) == b.charAt(0) + ); + + @Test + void retainsInsertOrder() { + var map = new MutableHashedLinkedMap(); + map.put("first", 1); + map.put("second", 2); + map.put("third", 3); + + assertArrayEquals(arr("first", "second", "third"), keysArray(map)); + assertArrayEquals(arr(1, 2, 3), valuesArray(map)); + } + + @Test + void retainsInsertOrderAfterOverride() { + var map = new MutableHashedLinkedMap(); + map.put("first", 1); + map.put("second", 2); + map.put("third", 3); + + var old = map.put("second", 5); + + assertEquals(old, 2); + assertArrayEquals(arr("first", "second", "third"), keysArray(map)); + assertArrayEquals(arr(1, 5, 3), valuesArray(map)); + } + + @Test + void retainsInsertOrderAfterOverrideWithStrategy() { + var map = new MutableHashedLinkedMap(FIRST_CHARACTER); + map.put("first", 1); + map.put("second", 2); + map.put("third", 3); + + var old = map.put("should override", 5); + + assertEquals(old, 2); + assertArrayEquals(arr("first", "second", "third"), keysArray(map)); + assertArrayEquals(arr(1, 5, 3), valuesArray(map)); + } + + + @Test + void strategyCollisionDoesNotOverwriteKey() { + var map = new MutableHashedLinkedMap(FIRST_CHARACTER); + map.put("static", 1); + + var old = map.put("shouldn't change the name, but should change the value", 5); + + assertEquals(old, 1); + assertArrayEquals(arr("static"), keysArray(map)); + assertArrayEquals(arr(5), valuesArray(map)); + } + + @Test + void putFirst() { + var map = new MutableHashedLinkedMap(); + map.put("second", 2); + map.put("third", 3); + + var old = map.putFirst("first", 1); + + assertNull(old); + assertArrayEquals(arr("first", "second", "third"), keysArray(map)); + assertArrayEquals(arr(1, 2, 3), valuesArray(map)); + } + + @Test + void putAfter() { + var map = new MutableHashedLinkedMap(); + map.put("first", 1); + map.put("third", 3); + + var old = map.putAfter("first", "second", 2); + + assertNull(old); + assertArrayEquals(arr("first", "second", "third"), keysArray(map)); + assertArrayEquals(arr(1, 2, 3), valuesArray(map)); + } + + @Test + void putAfterWithStrategy() { + var map = new MutableHashedLinkedMap(FIRST_CHARACTER); + map.put("first", 1); + map.put("third", 3); + + var old = map.putAfter("first entry", "second", 2); + + assertNull(old); + assertArrayEquals(arr("first", "second", "third"), keysArray(map)); + assertArrayEquals(arr(1, 2, 3), valuesArray(map)); + } + + @Test + void putBefore() { + var map = new MutableHashedLinkedMap(); + map.put("first", 1); + map.put("third", 3); + + var old = map.putBefore("third", "second", 2); + + assertNull(old); + assertArrayEquals(arr("first", "second", "third"), keysArray(map)); + assertArrayEquals(arr(1, 2, 3), valuesArray(map)); + } + + @Test + void putBeforeWithStrategy() { + var map = new MutableHashedLinkedMap(FIRST_CHARACTER); + map.put("first", 1); + map.put("third", 3); + + var old = map.putBefore("third entry", "second", 2); + + assertNull(old); + assertArrayEquals(arr("first", "second", "third"), keysArray(map)); + assertArrayEquals(arr(1, 2, 3), valuesArray(map)); + } + + @Test + void isEmpty() { + var map = new MutableHashedLinkedMap(); + assertTrue(map.isEmpty(), "Freshly initalized map is not empty!"); + map.put("entry", 1); + assertFalse(map.isEmpty(), "Map is empty, but contains an element"); + map.remove("entry"); + assertTrue(map.isEmpty(), "Map is not empty after removing its only entry"); + } + + @Test + void get() { + var map = new MutableHashedLinkedMap(); + map.put("first", 1); + map.put("second", 2); + map.put("third", 3); + + assertEquals(1, map.get("first")); + assertEquals(2, map.get("second")); + assertEquals(3, map.get("third")); + } + + @Test + void getWithStrategy() { + var map = new MutableHashedLinkedMap(FIRST_CHARACTER); + map.put("first", 1); + map.put("second", 2); + map.put("third", 3); + + assertEquals(1, map.get("first entry")); + assertEquals(2, map.get("second entry")); + assertEquals(3, map.get("third entry")); + } + + @Test + void contains() { + var map = new MutableHashedLinkedMap(); + map.put("first", 1); + map.put("second", 2); + map.put("third", 3); + + assertTrue(map.contains("first")); + assertTrue(map.contains("second")); + assertTrue(map.contains("third")); + } + + @Test + void containsWithStrategy() { + var map = new MutableHashedLinkedMap(FIRST_CHARACTER); + map.put("first", 1); + map.put("second", 2); + map.put("third", 3); + + assertTrue(map.contains("first entry")); + assertTrue(map.contains("second entry")); + assertTrue(map.contains("third entry")); + } + + @Test + void removeHead() { + var map = new MutableHashedLinkedMap(); + map.put("first", 1); + map.put("second", 2); + map.put("third", 3); + + var old = map.remove("first"); + + assertEquals(1, old); + assertArrayEquals(arr("second", "third"), keysArray(map)); + assertArrayEquals(arr(2, 3), valuesArray(map)); + } + + @Test + void removeHeadWithStrategy() { + var map = new MutableHashedLinkedMap(FIRST_CHARACTER); + map.put("first", 1); + map.put("second", 2); + map.put("third", 3); + + var old = map.remove("first entry"); + + assertEquals(1, old); + assertArrayEquals(arr("second", "third"), keysArray(map)); + assertArrayEquals(arr(2, 3), valuesArray(map)); + } + + @Test + void removeMiddle() { + var map = new MutableHashedLinkedMap(); + map.put("first", 1); + map.put("second", 2); + map.put("third", 3); + + var old = map.remove("second"); + + assertEquals(2, old); + assertArrayEquals(arr("first", "third"), keysArray(map)); + assertArrayEquals(arr(1, 3), valuesArray(map)); + } + + @Test + void removeMiddleWithStrategy() { + var map = new MutableHashedLinkedMap(FIRST_CHARACTER); + map.put("first", 1); + map.put("second", 2); + map.put("third", 3); + + var old = map.remove("second entry"); + + assertEquals(2, old); + assertArrayEquals(arr("first", "third"), keysArray(map)); + assertArrayEquals(arr(1, 3), valuesArray(map)); + } + + @Test + void removeMultipleMiddle() { + var map = new MutableHashedLinkedMap(); + map.put("first", 1); + map.put("second", 2); + map.put("third", 3); + map.put("forth", 4); + map.put("fifth", 5); + map.put("sixth", 6); + + assertEquals(2, map.remove("second")); + assertEquals(3, map.remove("third")); + assertEquals(4, map.remove("forth")); + + assertArrayEquals(arr("first", "fifth", "sixth"), keysArray(map)); + assertArrayEquals(arr(1, 5, 6), valuesArray(map)); + } + + @Test + void removeMultipleMiddleWithStrategy() { + var map = new MutableHashedLinkedMap(FIRST_CHARACTER); + map.put("1st", 1); + map.put("2nd", 2); + map.put("3rd", 3); + map.put("4th", 4); + map.put("5th", 5); + map.put("6th", 6); + + assertEquals(2, map.remove("2nd entry")); + assertEquals(3, map.remove("3rd entry")); + assertEquals(4, map.remove("4th entry")); + + assertArrayEquals(arr("1st", "5th", "6th"), keysArray(map)); + assertArrayEquals(arr(1, 5, 6), valuesArray(map)); + } + + @Test + void removeTail() { + var map = new MutableHashedLinkedMap(); + map.put("first", 1); + map.put("second", 2); + map.put("third", 3); + + var old = map.remove("third"); + + assertEquals(3, old); + assertArrayEquals(arr("first", "second"), keysArray(map)); + assertArrayEquals(arr(1, 2), valuesArray(map)); + } + + @Test + void removeTailWithStrategy() { + var map = new MutableHashedLinkedMap(FIRST_CHARACTER); + map.put("first", 1); + map.put("second", 2); + map.put("third", 3); + + var old = map.remove("third entry"); + + assertEquals(3, old); + assertArrayEquals(arr("first", "second"), keysArray(map)); + assertArrayEquals(arr(1, 2), valuesArray(map)); + } + + @Test + void iteratorInOrder() { + var map = new MutableHashedLinkedMap(); + map.put("first", 1); + map.put("second", 2); + map.put("third", 3); + + var itr = map.iterator(); + + assertEquals("first", itr.next().getKey()); + assertEquals("second", itr.next().getKey()); + assertEquals("third", itr.next().getKey()); + assertFalse(itr.hasNext()); + } + + @Test + void iteratorRemoveHead() { + var map = new MutableHashedLinkedMap(); + map.put("first", 1); + map.put("second", 2); + map.put("third", 3); + + var itr = map.iterator(); + while (itr.hasNext()) { + var next = itr.next(); + if (next.getKey().equals("first")) + itr.remove(); + } + + assertArrayEquals(arr("second", "third"), keysArray(map)); + assertArrayEquals(arr(2, 3), valuesArray(map)); + } + + @Test + void iteratorRemoveMiddle() { + var map = new MutableHashedLinkedMap(); + map.put("first", 1); + map.put("second", 2); + map.put("third", 3); + + var itr = map.iterator(); + while (itr.hasNext()) { + var next = itr.next(); + if (next.getKey().equals("second")) + itr.remove(); + } + + assertArrayEquals(arr("first", "third"), keysArray(map)); + assertArrayEquals(arr(1, 3), valuesArray(map)); + } + + @Test + void iteratorRemoveEnd() { + var map = new MutableHashedLinkedMap(); + map.put("first", 1); + map.put("second", 2); + map.put("third", 3); + + var itr = map.iterator(); + while (itr.hasNext()) { + var next = itr.next(); + if (next.getKey().equals("third")) + itr.remove(); + } + + assertArrayEquals(arr("first", "second"), keysArray(map)); + assertArrayEquals(arr(1, 2), valuesArray(map)); + } + + @Test + void iteratorRemoveAll() { + var map = new MutableHashedLinkedMap(); + map.put("first", 1); + map.put("second", 2); + map.put("third", 3); + + var itr = map.iterator(); + while (itr.hasNext()) { + itr.next(); + itr.remove(); + } + + assertTrue(map.isEmpty()); + assertArrayEquals(arr(), keysArray(map)); + assertArrayEquals(arr(), valuesArray(map)); + } + + @Test + void iteratorRemoveRequiresNext() { + var map = new MutableHashedLinkedMap(); + map.put("first", 1); + map.put("second", 2); + map.put("third", 3); + + var itr = map.iterator(); + assertThrows(IllegalStateException.class, () -> itr.remove()); + } + + @Test + void iteratorNextThrowsOnAdded() { + var map = new MutableHashedLinkedMap(); + map.put("first", 1); + map.put("second", 2); + map.put("third", 3); + + var itr = map.iterator(); + map.put("forth", 4); + + assertThrows(ConcurrentModificationException.class, () -> itr.next()); + } + + @Test + void iteratorNextThrowsOnRemoved() { + var map = new MutableHashedLinkedMap(); + map.put("first", 1); + map.put("second", 2); + map.put("third", 3); + + var itr = map.iterator(); + map.remove("first"); + + assertThrows(ConcurrentModificationException.class, () -> itr.next()); + } + + @Test + void iteratorRemoveThrowsOnAdded() { + var map = new MutableHashedLinkedMap(); + map.put("first", 1); + map.put("second", 2); + map.put("third", 3); + + var itr = map.iterator(); + itr.next(); + map.put("forth", 4); + + assertThrows(ConcurrentModificationException.class, () -> itr.remove()); + } + + @Test + void iteratorRemoveThrowsOnRemoved() { + var map = new MutableHashedLinkedMap(); + map.put("first", 1); + map.put("second", 2); + map.put("third", 3); + + var itr = map.iterator(); + itr.next(); + map.remove("first"); + + assertThrows(ConcurrentModificationException.class, () -> itr.remove()); + } + + @Test + void paintingsTest() { + var map = new MutableHashedLinkedMap(); + for (char x = 'a'; x <= 'z'; x++) + map.put(x, (int)x); + map.remove('i'); + map.remove('j'); + map.remove('k'); + map.remove('l'); + var keys = stream(map).map(e->e.getKey()).collect(Collector.of(StringBuilder::new, StringBuilder::append, StringBuilder::append, StringBuilder::toString)); + assertEquals("abcdefghmnopqrstuvwxyz", keys); + } + + + + private static Strategy strat(ToIntFunction hash, BiPredicate equals) { + return new Strategy() { + @Override public int hashCode(K o) { return hash.applyAsInt(o); } + @Override public boolean equals(K a, K b) { return equals.test(a, b); } + }; + } + + @SafeVarargs + private static T[] arr(T... values) { + return values; + } + + private static Stream> stream(MutableHashedLinkedMap map) { + return StreamSupport.stream(map.spliterator(), false); + } + + @SuppressWarnings("unchecked") + private static K[] keysArray(MutableHashedLinkedMap map) { + return (K[])stream(map).map(e -> e.getKey()).toArray(); + } + @SuppressWarnings("unchecked") + private static V[] valuesArray(MutableHashedLinkedMap map) { + return (V[])stream(map).map(e -> e.getValue()).toArray(); + } +}