Merge pull request #839 from Microsoft/feature/event-properties-concu…

…rrent-exception Avoid concurrent modification exception when doing property traversal
microsoft · Oct 11, 2018 · b279e92 · b279e92
2 parents 8c08c23 + c472185
commit b279e92
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 15 deletions.
diff --git a/sdk/appcenter-analytics/src/main/java/com/microsoft/appcenter/analytics/EventProperties.java b/sdk/appcenter-analytics/src/main/java/com/microsoft/appcenter/analytics/EventProperties.java
@@ -9,8 +9,8 @@
 import com.microsoft.appcenter.utils.AppCenterLog;
 
 import java.util.Date;
-import java.util.LinkedHashMap;
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 
 import static com.microsoft.appcenter.analytics.Analytics.LOG_TAG;
 
@@ -23,8 +23,11 @@ public class EventProperties {
 
     /**
      * Properties key/value pairs.
+     * Using a concurrent map to avoid concurrent modification exception when doing the traversal copy.
+     * We also need to traverse a snapshot of properties when doing property inheritance between targets.
+     * There is no need to block in a more global way than the snapshot traversal.
      */
-    private final Map<String, TypedProperty> mProperties = new LinkedHashMap<>();
+    private final Map<String, TypedProperty> mProperties = new ConcurrentHashMap<>();
 
     Map<String, TypedProperty> getProperties() {
         return mProperties;

diff --git a/...r-analytics/src/test/java/com/microsoft/appcenter/analytics/PropertyConfiguratorTest.java b/...r-analytics/src/test/java/com/microsoft/appcenter/analytics/PropertyConfiguratorTest.java
@@ -28,6 +28,7 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Date;
+import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -86,6 +87,10 @@ private static StringTypedProperty typedProperty(String name, String value) {
         return typedProperty;
     }
 
+    private static void assertUnorderedListEquals(List<TypedProperty> expected, List<TypedProperty> actual) {
+        assertEquals(new HashSet<>(expected), new HashSet<>(actual));
+    }
+
     @Mock
     private Channel mChannel;
 
@@ -422,7 +427,7 @@ public void trackEventWithCommonTypedProperties() {
         typedProperties.add(typedProperty("myLong", Long.MAX_VALUE));
         typedProperties.add(typedProperty("myDate", new Date(456)));
         typedProperties.add(typedProperty("myDouble", -3.14E3));
-        assertEquals(typedProperties, log.getTypedProperties());
+        assertUnorderedListEquals(typedProperties, log.getTypedProperties());
     }
 
     @Test
@@ -474,16 +479,14 @@ public void eventPropertiesCascading() {
         /* Verify properties. */
         assertNull(log.getProperties());
         List<TypedProperty> typedProperties = new ArrayList<>();
-
-        /* The order should be child -> parent -> grandparent which is in the order of override properties. */
-        typedProperties.add(typedProperty("d", "444"));
-        typedProperties.add(typedProperty("e", "555"));
-        typedProperties.add(typedProperty("f", "6666"));
         typedProperties.add(typedProperty("a", "11"));
         typedProperties.add(typedProperty("b", "22"));
         typedProperties.add(typedProperty("c", "3"));
+        typedProperties.add(typedProperty("d", "444"));
+        typedProperties.add(typedProperty("e", "555"));
+        typedProperties.add(typedProperty("f", "6666"));
         typedProperties.add(typedProperty("g", "7777"));
-        assertEquals(typedProperties, log.getTypedProperties());
+        assertUnorderedListEquals(typedProperties, log.getTypedProperties());
     }
 
     @Test
@@ -535,15 +538,13 @@ public void eventPropertiesCascadingWithTypes() {
         /* Verify properties. */
         assertNull(log.getProperties());
         List<TypedProperty> typedProperties = new ArrayList<>();
-
-        /* The order should be child -> parent -> grandparent which is in the order of override properties. */
-        typedProperties.add(typedProperty("d", true));
-        typedProperties.add(typedProperty("e", 55.5));
-        typedProperties.add(typedProperty("f", new Date(6666)));
         typedProperties.add(typedProperty("a", 11));
         typedProperties.add(typedProperty("b", "22"));
         typedProperties.add(typedProperty("c", "3"));
+        typedProperties.add(typedProperty("d", true));
+        typedProperties.add(typedProperty("e", 55.5));
+        typedProperties.add(typedProperty("f", new Date(6666)));
         typedProperties.add(typedProperty("g", "7777"));
-        assertEquals(typedProperties, log.getTypedProperties());
+        assertUnorderedListEquals(typedProperties, log.getTypedProperties());
     }
 }