Merge pull request #2293 from OneSignal/fix/player-log-listener-concurrency

Player Model - Fix concurrency with modifying log listeners

Author: jkasten2

OneSignalSDK/onesignal/src/main/java/com/onesignal/OneSignal.java

@@ -73,6 +73,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.TimeZone;
+import java.util.concurrent.CopyOnWriteArrayList;
 
 import static com.onesignal.GenerateNotification.BUNDLE_KEY_ACTION_ID;
 import static com.onesignal.GenerateNotification.BUNDLE_KEY_ANDROID_NOTIFICATION_ID;
@@ -1348,7 +1349,7 @@ private static void callLogListeners(@NonNull final LOG_LEVEL level, @NonNull St
       }
    }
 
-   private static final List<OneSignalLogListener> logListeners = new ArrayList<>();
+   private static final List<OneSignalLogListener> logListeners = new CopyOnWriteArrayList<>();
 
    public static void addLogListener(@NonNull OneSignalLogListener listener) {
       logListeners.add(listener);

OneSignalSDK/unittest/src/test/java/com/onesignal/OneSignalPackagePrivateHelper.java

@@ -328,6 +328,10 @@ public static boolean OneSignal_isInForeground() {
       return OneSignal.isInForeground();
    }
 
+   public static void OneSignal_Log(@NonNull OneSignal.LOG_LEVEL level, @NonNull String message) {
+      OneSignal.Log(level,message);
+   }
+
    static public class OSSharedPreferencesWrapper extends com.onesignal.OSSharedPreferencesWrapper {}
 
    static public class RemoteOutcomeParams extends OneSignalRemoteParams.InfluenceParams {

OneSignalSDK/unittest/src/test/java/com/test/onesignal/MainOneSignalClassRunner.java

@@ -91,6 +91,8 @@
 import com.onesignal.StaticResetHelper;
 import com.onesignal.SyncJobService;
 import com.onesignal.SyncService;
+import com.onesignal.debug.OneSignalLogEvent;
+import com.onesignal.debug.OneSignalLogListener;
 import com.onesignal.example.BlankActivity;
 import com.onesignal.example.MainActivity;
 import com.onesignal.influence.data.OSTrackerFactory;
@@ -133,6 +135,7 @@
 import static com.onesignal.OneSignalPackagePrivateHelper.FCMBroadcastReceiver_processBundle;
 import static com.onesignal.OneSignalPackagePrivateHelper.NotificationBundleProcessor_Process;
 import static com.onesignal.OneSignalPackagePrivateHelper.NotificationOpenedProcessor_processFromContext;
+import static com.onesignal.OneSignalPackagePrivateHelper.OneSignal_Log;
 import static com.onesignal.OneSignalPackagePrivateHelper.OneSignal_getSessionListener;
 import static com.onesignal.OneSignalPackagePrivateHelper.OneSignal_handleNotificationOpen;
 import static com.onesignal.OneSignalPackagePrivateHelper.OneSignal_isInForeground;
@@ -4167,6 +4170,46 @@ public void testAndroidManifestConfigChangeFlags_orientationFlag() throws Except
       assertFalse(mainHasFlag);
    }
 
+   // ####### Unit test log listener ########
+   @Test
+   public void testAddLogListener() {
+      final String[] lastMessage = new String[1];
+      OneSignal.addLogListener(event -> lastMessage[0] = event.getEntry());
+
+      OneSignal_Log(OneSignal.LOG_LEVEL.DEBUG, "test");
+      assertEquals(lastMessage[0], "test");
+   }
+
+   @Test
+   public void testRemoveLogListener() {
+      final String[] lastMessage = new String[1];
+      OneSignalLogListener listener = event -> lastMessage[0] = event.getEntry();
+      OneSignal.addLogListener(listener);
+
+      OneSignal.removeLogListener(listener);
+      OneSignal_Log(OneSignal.LOG_LEVEL.DEBUG, "test");
+      assertNull(lastMessage[0]);
+   }
+
+   @Test
+   public void testNestedAddLogListenerDoesNotThrow() {
+      OneSignalLogListener listener = event -> OneSignal.addLogListener(event2 -> {});
+
+      OneSignal.addLogListener(listener);
+
+      OneSignal_Log(OneSignal.LOG_LEVEL.DEBUG, "test");
+   }
+
+   @Test
+   public void testNestedRemoveLogListenerDoesNotThrow() {
+      final OneSignalLogListener[] listener = new OneSignalLogListener[1];
+
+      listener[0] = event -> OneSignal.removeLogListener(listener[0]);
+      OneSignal.addLogListener(listener[0]);
+
+      OneSignal_Log(OneSignal.LOG_LEVEL.DEBUG, "test");
+   }
+
    // ####### Unit test helper methods ########
 
    private static OSNotification createTestOSNotification() throws Exception {