Merge pull request #16 from trello/dlew/concurrent-listeners

Concurrent listeners

navi/src/main/java/com/trello/navi/internal/BaseNaviComponent.java

@@ -14,7 +14,6 @@
 import com.trello.navi.model.ActivityResult;
 import com.trello.navi.model.BundleBundle;
 import com.trello.navi.model.RequestPermissionsResult;
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
@@ -23,6 +22,7 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.CopyOnWriteArrayList;
 
 /**
  * Base helper which contains all the actual logic
@@ -32,8 +32,6 @@
  */
 public final class BaseNaviComponent implements NaviComponent {
 
-  private static final int DEFAULT_LIST_SIZE = 3;
-
   private final Set<Event<?>> handledEvents;
 
   private final Map<Event, List<Listener>> listenerMap;
@@ -104,7 +102,7 @@ public static BaseNaviComponent createFragmentComponent() {
     }
 
     if (!listenerMap.containsKey(event)) {
-      listenerMap.put(event, new ArrayList<Listener>(DEFAULT_LIST_SIZE));
+      listenerMap.put(event, new CopyOnWriteArrayList<Listener>());
     }
 
     List<Listener> listeners = listenerMap.get(event);
@@ -131,8 +129,8 @@ private <T> void emitEvent(Event<T> event, T data) {
     }
 
     List<Listener> listeners = listenerMap.get(event);
-    for (int a = 0, size = listeners.size(); a < size; a++) {
-      listeners.get(a).call(data);
+    for (Listener listener : listeners) {
+      listener.call(data);
     }
   }
 

navi/src/test/java/com/trello/navi/ConcurrencyTest.java

@@ -0,0 +1,69 @@
+package com.trello.navi;
+
+import com.trello.navi.internal.BaseNaviComponent;
+import org.junit.Test;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyZeroInteractions;
+
+public final class ConcurrencyTest {
+
+  private final BaseNaviComponent activity = BaseNaviComponent.createActivityComponent();
+
+  // Verify that we can handle a listener removing itself due to an event occurring
+  @Test public void handleInnerRemovals() {
+    final Listener<Void> listener1 = spy(new Listener<Void>() {
+      @Override public void call(Void __) {
+        activity.removeListener(Event.RESUME, this);
+      }
+    });
+    final Listener<Void> listener2 = spy(new Listener<Void>() {
+      @Override public void call(Void __) {
+        activity.removeListener(Event.RESUME, this);
+      }
+    });
+
+    activity.addListener(Event.RESUME, listener1);
+    activity.addListener(Event.RESUME, listener2);
+    activity.onResume();
+    verify(listener1).call(null);
+    verify(listener2).call(null);
+  }
+
+  // Verify that listeners added while emitting an item do not also get the current emission
+  // (since they were not registered at the time of the event).
+  @Test public void addDuringEmit() {
+    final Listener<Void> addedDuringEmit = mock(Listener.class);
+    final Listener<Void> listener = spy(new Listener<Void>() {
+      @Override public void call(Void __) {
+        activity.addListener(Event.RESUME, addedDuringEmit);
+      }
+    });
+
+    activity.addListener(Event.RESUME, listener);
+    activity.onResume();
+
+    verify(listener).call(null);
+    verifyZeroInteractions(addedDuringEmit);
+  }
+
+  // Verify that listeners removed while emitting an event still receive it (since they were
+  // registered at the time of the event).
+  @Test public void removeDuringEmit() {
+    final Listener<Void> removedDuringEmit = mock(Listener.class);
+    final Listener<Void> listener = spy(new Listener<Void>() {
+      @Override public void call(Void __) {
+        activity.removeListener(Event.RESUME, removedDuringEmit);
+      }
+    });
+
+    activity.addListener(Event.RESUME, listener);
+    activity.addListener(Event.RESUME, removedDuringEmit);
+    activity.onResume();
+
+    verify(listener).call(null);
+    verify(removedDuringEmit).call(null);
+  }
+}