Avoid hash code collisions in build memory

src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/model/BuildMemory.java

@@ -24,6 +24,7 @@
  */
 package com.sonyericsson.hudson.plugins.gerrit.trigger.gerritnotifier.model;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.infradna.tool.bridge_method_injector.WithBridgeMethods;
 import com.sonyericsson.hudson.plugins.gerrit.trigger.diagnostics.BuildMemoryReport;
 import com.sonyericsson.hudson.plugins.gerrit.trigger.gerritnotifier.model.BuildMemory.MemoryImprint.Entry;
@@ -44,11 +45,10 @@
 import javax.annotation.Nonnull;
 import java.io.IOException;
 import java.util.ArrayList;
-import java.util.Comparator;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
-import java.util.TreeMap;
+import java.util.HashMap;
 
 import static com.sonyericsson.hudson.plugins.gerrit.trigger.utils.Logic.shouldSkip;
 
@@ -58,32 +58,7 @@
  * @author Robert Sandell <robert.sandell@sonyericsson.com>
  */
 public class BuildMemory {
-
-    /**
-     * Compares GerritTriggeredEvents using the Object.hashCode() method. This ensures that every event received from
-     * Gerrit is kept track of individually.
-     *
-     * @author James E. Blair <jeblair@hp.com>
-     */
-    static class GerritTriggeredEventComparator implements Comparator<GerritTriggeredEvent> {
-        @Override
-        public int compare(GerritTriggeredEvent o1, GerritTriggeredEvent o2) {
-            if (o1 == null && o2 == null) {
-                return 0;
-            }
-            if (o1 != null && o2 == null) {
-                return 1;
-            }
-            if (o1 == null && o2 != null) {
-                return -1;
-            }
-            return Integer.valueOf(o1.hashCode()).compareTo(o2.hashCode());
-        }
-    }
-
-    private TreeMap<GerritTriggeredEvent, MemoryImprint> memory =
-            new TreeMap<GerritTriggeredEvent, MemoryImprint>(
-                    new GerritTriggeredEventComparator());
+    private Map<GerritTriggeredEvent, MemoryImprint> memory = new HashMap<GerritTriggeredEvent, MemoryImprint>();
     private static final Logger logger = LoggerFactory.getLogger(BuildMemory.class);
 
     /**
@@ -96,6 +71,15 @@ public synchronized MemoryImprint getMemoryImprint(GerritTriggeredEvent event) {
         return memory.get(event);
     }
 
+    /**
+     * Gets the entire memory representation for all events
+     * @return memory representation for all events.
+     */
+    @VisibleForTesting
+    Map<GerritTriggeredEvent, MemoryImprint> getMemory() {
+        return memory;
+    }
+
     /**
      * Tells if all triggered builds have started for a specific memory imprint.
      *

src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/model/BuildMemoryReportContainerTest.java

@@ -0,0 +1,111 @@
+package com.sonyericsson.hudson.plugins.gerrit.trigger.gerritnotifier.model;
+
+import com.sonymobile.tools.gerrit.gerritevents.dto.events.ChangeBasedEvent;
+import com.sonymobile.tools.gerrit.gerritevents.dto.events.GerritTriggeredEvent;
+import com.sonymobile.tools.gerrit.gerritevents.dto.events.PatchsetCreated;
+import hudson.model.AbstractProject;
+import hudson.model.ItemGroup;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.mockito.Mockito;
+
+import java.util.Arrays;
+import java.util.UUID;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * Tests {@link BuildMemory} internal container.
+ * The main requirement is not merging non-equal events with same hash code.
+ *
+ * @author Robert Sandell <rsandell@cloudbees.com>.
+ */
+@RunWith(Parameterized.class)
+public class BuildMemoryReportContainerTest {
+
+    private final GerritTriggeredEvent o1;
+    private final GerritTriggeredEvent o2;
+    private final int expected;
+    private final BuildMemory memory;
+
+    static class PatchsetCreatedWithPredefinedHash extends PatchsetCreated {
+        private final int hashCode;
+        private final UUID uuid;
+
+        private PatchsetCreatedWithPredefinedHash(int hashCode) {
+            this.hashCode = hashCode;
+            this.uuid = UUID.randomUUID();
+        }
+
+        @Override
+        public boolean equals(Object o) {
+            return o instanceof PatchsetCreatedWithPredefinedHash &&
+                    uuid.equals(((PatchsetCreatedWithPredefinedHash) o).uuid);
+
+        }
+
+        @Override
+        public int hashCode() {
+            return hashCode;
+        }
+
+        @Override
+        public String toString() {
+            return "PatchsetCreatedWithPredefinedHash{" +
+                    "uuid=" + uuid +
+                    '}';
+        }
+    }
+
+    /**
+     * The different scenarios.
+     *
+     * @return a list of parameters to the constructor.
+     * @see Parameterized
+     */
+    @Parameterized.Parameters(name = "{index}: {0}, {1} == {2}")
+    public static Iterable<Object[]> permutations() {
+        ChangeBasedEvent event1 = new PatchsetCreatedWithPredefinedHash(777);
+        ChangeBasedEvent event2 = new PatchsetCreatedWithPredefinedHash(777);
+        return Arrays.asList(new Object[][]{
+                {null, null, 1},
+                {null, event1, 2},
+                {event1, null, 2},
+                {event1, event1, 1},
+                {event2, event2, 1},
+                {event1, event2, 2},
+                {event2, event1, 2},
+        });
+    }
+
+    /**
+     * Constructor.
+     * @param o1 first argument
+     * @param o2 second argument
+     * @param expected the expected result
+     */
+    public BuildMemoryReportContainerTest(GerritTriggeredEvent o1, GerritTriggeredEvent o2, int expected) {
+        this.o1 = o1;
+        this.o2 = o2;
+        this.expected = expected;
+        memory = new BuildMemory();
+    }
+
+    /**
+     * Tests the scenario on {@link BuildMemory}.
+     *
+     * @throws Exception if so
+     */
+    @Test
+    public void testCompare() throws Exception {
+        AbstractProject mock = Mockito.mock(AbstractProject.class);
+        ItemGroup itemGroup = Mockito.mock(ItemGroup.class);
+        Mockito.when(itemGroup.getFullName()).thenReturn("Parent");
+        Mockito.when(mock.getParent()).thenReturn(itemGroup);
+        memory.triggered(o1, mock);
+        memory.triggered(o2, mock);
+
+        assertEquals(expected, memory.getMemory().size());
+    }
+}