[GSCOLLECT-1662] Fix UnifiedSet$ChainedBucket.removeLongChain() method.

Fixes #30. git-svn-id: svn+ssh://gscollections.svn.services.gs.com/svnroot/gscollections-svn/trunk@917 d5c9223b-1aff-41ac-aadd-f810b4a99ac4
goldmansachs · Dec 16, 2015 · 4fd18e9 · 4fd18e9
1 parent 5ad0ff5
commit 4fd18e9
Show file tree

Hide file tree

Showing 4 changed files with 85 additions and 4 deletions.
diff --git a/collections/src/main/java/com/gs/collections/impl/set/mutable/UnifiedSet.java b/collections/src/main/java/com/gs/collections/impl/set/mutable/UnifiedSet.java
@@ -1725,6 +1725,12 @@ private void removeLongChain(ChainedBucket oldBucket, int i)
                         bucket.three = null;
                         return;
                     default:
+                        if (bucket.three instanceof ChainedBucket)
+                        {
+                            i -= 3;
+                            oldBucket = bucket;
+                            continue;
+                        }
                         throw new AssertionError();
                 }
             }

diff --git a/...main/java/com/gs/collections/impl/set/strategy/mutable/UnifiedSetWithHashingStrategy.java b/...main/java/com/gs/collections/impl/set/strategy/mutable/UnifiedSetWithHashingStrategy.java
@@ -1763,6 +1763,12 @@ private void removeLongChain(ChainedBucket oldBucket, int i)
                         bucket.three = null;
                         return;
                     default:
+                        if (bucket.three instanceof ChainedBucket)
+                        {
+                            i -= 3;
+                            oldBucket = bucket;
+                            continue;
+                        }
                         throw new AssertionError();
                 }
             }

diff --git a/unit-tests/src/test/java/com/gs/collections/impl/map/mutable/UnifiedMapTestCase.java b/unit-tests/src/test/java/com/gs/collections/impl/map/mutable/UnifiedMapTestCase.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2014 Goldman Sachs.
+ * Copyright 2015 Goldman Sachs.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -31,6 +31,8 @@
 import com.gs.collections.impl.block.factory.Functions;
 import com.gs.collections.impl.block.factory.Procedures;
 import com.gs.collections.impl.factory.Lists;
+import com.gs.collections.impl.factory.Sets;
+import com.gs.collections.impl.list.fixed.ArrayAdapter;
 import com.gs.collections.impl.list.mutable.FastList;
 import com.gs.collections.impl.set.mutable.UnifiedSet;
 import com.gs.collections.impl.test.SerializeTestHelper;
@@ -41,7 +43,8 @@
 import org.junit.Assert;
 import org.junit.Test;
 
-import static com.gs.collections.impl.factory.Iterables.*;
+import static com.gs.collections.impl.factory.Iterables.iMap;
+import static com.gs.collections.impl.factory.Iterables.mList;
 
 public abstract class UnifiedMapTestCase extends MutableMapTestCase
 {
@@ -59,6 +62,18 @@ public abstract class UnifiedMapTestCase extends MutableMapTestCase
             Lists.mutable.of(COLLISION_1, COLLISION_2, COLLISION_3, COLLISION_4, COLLISION_5);
     protected static final MutableList<Integer> MORE_COLLISIONS = FastList.newList(COLLISIONS)
             .with(COLLISION_6, COLLISION_7, COLLISION_8, COLLISION_9);
+    protected static final String[] FREQUENT_COLLISIONS = {
+            "\u9103\ufffe",
+            "\u9104\uffdf",
+            "\u9105\uffc0",
+            "\u9106\uffa1",
+            "\u9107\uff82",
+            "\u9108\uff63",
+            "\u9109\uff44",
+            "\u910a\uff25",
+            "\u910b\uff06",
+            "\u910c\ufee7"
+    };
 
     @Test
     public void valuesCollection_toArray()
@@ -829,6 +844,29 @@ public void equalsAndHashCode()
         Assert.assertEquals(0, this.newMapWithKeyValue(null, null).hashCode());
     }
 
+    @Test
+    public void frequentCollision()
+    {
+        String[] expected = ArrayAdapter.adapt(FREQUENT_COLLISIONS)
+                .subList(0, FREQUENT_COLLISIONS.length - 2)
+                .toArray(new String[FREQUENT_COLLISIONS.length - 2]);
+        MutableMap<String, String> map = this.newMap();
+        MutableSet<String> set = Sets.mutable.of(expected);
+
+        ArrayIterate.forEach(FREQUENT_COLLISIONS, each -> map.put(each, each));
+
+        Iterator<String> itr = map.iterator();
+        while (itr.hasNext())
+        {
+            if (!set.contains(itr.next()))
+            {
+                itr.remove();
+            }
+        }
+
+        Verify.assertArrayEquals(expected, map.keysView().toArray());
+    }
+
     private static final class NoInstanceOfInEquals
     {
         private final int value;

diff --git a/unit-tests/src/test/java/com/gs/collections/impl/set/mutable/AbstractMutableSetTestCase.java b/unit-tests/src/test/java/com/gs/collections/impl/set/mutable/AbstractMutableSetTestCase.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2014 Goldman Sachs.
+ * Copyright 2015 Goldman Sachs.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -39,13 +39,15 @@
 import com.gs.collections.impl.collection.mutable.AbstractCollectionTestCase;
 import com.gs.collections.impl.factory.Lists;
 import com.gs.collections.impl.list.Interval;
+import com.gs.collections.impl.list.fixed.ArrayAdapter;
 import com.gs.collections.impl.list.mutable.FastList;
 import com.gs.collections.impl.test.Verify;
 import com.gs.collections.impl.utility.Iterate;
 import org.junit.Assert;
 import org.junit.Test;
 
-import static com.gs.collections.impl.factory.Iterables.*;
+import static com.gs.collections.impl.factory.Iterables.iSet;
+import static com.gs.collections.impl.factory.Iterables.mList;
 
 /**
  * JUnit test for {@link AbstractMutableSet}.
@@ -67,6 +69,18 @@ public abstract class AbstractMutableSetTestCase extends AbstractCollectionTestC
     protected static final MutableList<Integer> MORE_COLLISIONS = FastList.newList(COLLISIONS)
             .with(COLLISION_6, COLLISION_7, COLLISION_8, COLLISION_9);
     protected static final int SIZE = 8;
+    protected static final String[] FREQUENT_COLLISIONS = {
+            "\u9103\ufffe",
+            "\u9104\uffdf",
+            "\u9105\uffc0",
+            "\u9106\uffa1",
+            "\u9107\uff82",
+            "\u9108\uff63",
+            "\u9109\uff44",
+            "\u910a\uff25",
+            "\u910b\uff06",
+            "\u910c\ufee7"
+    };
 
     @Override
     protected abstract <T> MutableSet<T> newWith(T... littleElements);
@@ -886,4 +900,21 @@ public void toSortedBagBy()
         MutableSortedBag<Integer> bag = integers.toSortedBagBy(String::valueOf);
         Verify.assertSortedBagsEqual(TreeBag.newBagWith(1, 2, 3, 4), bag);
     }
+
+    @Test
+    public void frequentCollisions()
+    {
+        String[] expected = ArrayAdapter.adapt(FREQUENT_COLLISIONS)
+                .subList(0, FREQUENT_COLLISIONS.length - 2)
+                .toArray(new String[FREQUENT_COLLISIONS.length - 2]);
+        MutableSet<String> set1 = this.newWith();
+        MutableSet<String> set2 = this.newWith();
+
+        Collections.addAll(set1, FREQUENT_COLLISIONS);
+        Collections.addAll(set2, expected);
+
+        set1.retainAll(set2);
+
+        Verify.assertArrayEquals(expected, set1.toArray());
+    }
 }