Merge pull request #495 from Microsoft/feature/truncate-event-properties

Truncate property key and value if it is longer than limit

Author: guperrot

sdk/mobile-center-analytics/src/main/java/com/microsoft/azure/mobile/analytics/Analytics.java

@@ -49,6 +49,23 @@ public class Analytics extends AbstractMobileCenterService {
      */
     private static final String ACTIVITY_SUFFIX = "Activity";
 
+    /**
+     * Max number of properties.
+     */
+    private static final int MAX_PROPERTY_COUNT = 5;
+
+    /**
+     * Max length of event/page name.
+     */
+    @VisibleForTesting
+    static final int MAX_NAME_LENGTH = 256;
+
+    /**
+     * Max length of properties.
+     */
+    @VisibleForTesting
+    static final int MAX_PROPERTY_ITEM_LENGTH = 64;
+
     /**
      * Shared instance.
      */
@@ -184,8 +201,9 @@ protected static void trackPage(String name) {
      * Track a custom page with name and optional properties.
      * The name parameter can not be null or empty. Maximum allowed length = 256.
      * The properties parameter maximum item count = 5.
-     * The properties keys/names can not be null or empty, maximum allowed key length = 64.
+     * The properties keys can not be null or empty, maximum allowed key length = 64.
      * The properties values can not be null, maximum allowed value length = 64.
+     * Any length of name/keys/values that are longer than each limit will be truncated.
      * <p>
      * TODO the backend does not support that service yet, will be public method later.
      *
@@ -195,7 +213,8 @@ protected static void trackPage(String name) {
     @SuppressWarnings("WeakerAccess")
     protected static void trackPage(String name, Map<String, String> properties) {
         final String logType = "Page";
-        if (validateName(name, logType)) {
+        name = validateName(name, logType);
+        if (name != null) {
             Map<String, String> validatedProperties = validateProperties(properties, name, logType);
             getInstance().trackPageAsync(name, validatedProperties);
         }
@@ -215,16 +234,18 @@ public static void trackEvent(String name) {
      * Track a custom event with name and optional properties.
      * The name parameter can not be null or empty. Maximum allowed length = 256.
      * The properties parameter maximum item count = 5.
-     * The properties keys/names can not be null or empty, maximum allowed key length = 64.
+     * The properties keys can not be null or empty, maximum allowed key length = 64.
      * The properties values can not be null, maximum allowed value length = 64.
+     * Any length of name/keys/values that are longer than each limit will be truncated.
      *
      * @param name       An event name.
      * @param properties Optional properties.
      */
     @SuppressWarnings("WeakerAccess")
     public static void trackEvent(String name, Map<String, String> properties) {
         final String logType = "Event";
-        if (validateName(name, logType)) {
+        name = validateName(name, logType);
+        if (name != null) {
             Map<String, String> validatedProperties = validateProperties(properties, name, logType);
             getInstance().trackEventAsync(name, validatedProperties);
         }
@@ -250,19 +271,18 @@ private static String generatePageName(Class<?> activityClass) {
      *
      * @param name    Log name to validate.
      * @param logType Log type.
-     * @return <code>true</code> if validation succeeds, otherwise <code>false</code>.
+     * @return <code>null</code> if validation failed, otherwise a valid name within the length limit will be returned.
      */
-    private static boolean validateName(String name, String logType) {
-        final int maxNameLength = 256;
+    private static String validateName(String name, String logType) {
         if (name == null || name.isEmpty()) {
             MobileCenterLog.error(Analytics.LOG_TAG, logType + " name cannot be null or empty.");
-            return false;
+            return null;
         }
-        if (name.length() > maxNameLength) {
-            MobileCenterLog.error(Analytics.LOG_TAG, String.format("%s '%s' : name length cannot be longer than %s characters.", logType, name, maxNameLength));
-            return false;
+        if (name.length() > MAX_NAME_LENGTH) {
+            MobileCenterLog.warn(Analytics.LOG_TAG, String.format("%s '%s' : name length cannot be longer than %s characters. Name will be truncated.", logType, name, MAX_NAME_LENGTH));
+            name = name.substring(0, MAX_NAME_LENGTH);
         }
-        return true;
+        return name;
     }
 
     /**
@@ -277,36 +297,36 @@ private static Map<String, String> validateProperties(Map<String, String> proper
         if (properties == null)
             return null;
         String message;
-        final int maxPropertiesCount = 5;
-        final int maxPropertyItemLength = 64;
         Map<String, String> result = new HashMap<>();
         for (Map.Entry<String, String> property : properties.entrySet()) {
-            if (result.size() >= maxPropertiesCount) {
-                message = String.format("%s '%s' : properties cannot contain more than %s items. Skipping other properties.", logType, logName, maxPropertiesCount);
+            String key = property.getKey();
+            String value = property.getValue();
+            if (result.size() >= MAX_PROPERTY_COUNT) {
+                message = String.format("%s '%s' : properties cannot contain more than %s items. Skipping other properties.", logType, logName, MAX_PROPERTY_COUNT);
                 MobileCenterLog.warn(Analytics.LOG_TAG, message);
                 break;
             }
-            if (property.getKey() == null || property.getKey().isEmpty()) {
+            if (key == null || key.isEmpty()) {
                 message = String.format("%s '%s' : a property key cannot be null or empty. Property will be skipped.", logType, logName);
                 MobileCenterLog.warn(Analytics.LOG_TAG, message);
                 continue;
             }
-            if (property.getKey().length() > maxPropertyItemLength) {
-                message = String.format("%s '%s' : property '%s' : property key length cannot be longer than %s characters. Property '%s' will be skipped.", logType, logName, property.getKey(), maxPropertyItemLength, property.getKey());
+            if (value == null) {
+                message = String.format("%s '%s' : property '%s' : property value cannot be null. Property '%s' will be skipped.", logType, logName, key, key);
                 MobileCenterLog.warn(Analytics.LOG_TAG, message);
                 continue;
             }
-            if (property.getValue() == null) {
-                message = String.format("%s '%s' : property '%s' : property value cannot be null. Property '%s' will be skipped.", logType, logName, property.getKey(), property.getKey());
+            if (key.length() > MAX_PROPERTY_ITEM_LENGTH) {
+                message = String.format("%s '%s' : property '%s' : property key length cannot be longer than %s characters. Property key will be truncated.", logType, logName, key, MAX_PROPERTY_ITEM_LENGTH);
                 MobileCenterLog.warn(Analytics.LOG_TAG, message);
-                continue;
+                key = key.substring(0, MAX_PROPERTY_ITEM_LENGTH);
             }
-            if (property.getValue().length() > maxPropertyItemLength) {
-                message = String.format("%s '%s' : property '%s' : property value cannot be longer than %s characters. Property '%s' will be skipped.", logType, logName, property.getKey(), maxPropertyItemLength, property.getKey());
+            if (value.length() > MAX_PROPERTY_ITEM_LENGTH) {
+                message = String.format("%s '%s' : property '%s' : property value cannot be longer than %s characters. Property value will be truncated.", logType, logName, key, MAX_PROPERTY_ITEM_LENGTH);
                 MobileCenterLog.warn(Analytics.LOG_TAG, message);
-                continue;
+                value = value.substring(0, MAX_PROPERTY_ITEM_LENGTH);
             }
-            result.put(property.getKey(), property.getValue());
+            result.put(key, value);
         }
         return result;
     }

sdk/mobile-center-analytics/src/test/java/com/microsoft/azure/mobile/analytics/AnalyticsTest.java

@@ -258,18 +258,28 @@ public void testTrackEvent() {
         Analytics.trackEvent(" ", null);
         verify(channel, times(1)).enqueue(any(Log.class), anyString());
         reset(channel);
-        Analytics.trackEvent(generateString(257, '*'), null);
-        verify(channel, never()).enqueue(any(Log.class), anyString());
+        final String maxName = generateString(Analytics.MAX_NAME_LENGTH, '*');
+        Analytics.trackEvent(maxName + "*", null);
+        verify(channel, times(1)).enqueue(argThat(new ArgumentMatcher<Log>() {
+
+            @Override
+            public boolean matches(Object item) {
+                if (item instanceof EventLog) {
+                    EventLog eventLog = (EventLog) item;
+                    return eventLog.getName().equals(maxName) && eventLog.getProperties() == null;
+                }
+                return false;
+            }
+        }), anyString());
         reset(channel);
-        Analytics.trackEvent(generateString(256, '*'), null);
+        Analytics.trackEvent(maxName, null);
         verify(channel, times(1)).enqueue(any(Log.class), anyString());
         reset(channel);
         Analytics.trackEvent("eventName", new HashMap<String, String>() {{
             put(null, null);
             put("", null);
-            put(generateString(65, '*'), null);
+            put(generateString(Analytics.MAX_PROPERTY_ITEM_LENGTH + 1, '*'), null);
             put("1", null);
-            put("2", generateString(65, '*'));
         }});
         verify(channel, times(1)).enqueue(argThat(new ArgumentMatcher<Log>() {
 
@@ -300,6 +310,26 @@ public boolean matches(Object item) {
                 return false;
             }
         }), anyString());
+        reset(channel);
+        final String longerMapItem = generateString(Analytics.MAX_PROPERTY_ITEM_LENGTH + 1, '*');
+        Analytics.trackEvent("eventName", new HashMap<String, String>() {{
+            put(longerMapItem, longerMapItem);
+        }});
+        verify(channel, times(1)).enqueue(argThat(new ArgumentMatcher<Log>() {
+
+            @Override
+            public boolean matches(Object item) {
+                if (item instanceof EventLog) {
+                    EventLog eventLog = (EventLog) item;
+                    if (eventLog.getProperties().size() == 1) {
+                        Map.Entry<String, String> entry = eventLog.getProperties().entrySet().iterator().next();
+                        String truncatedMapItem = generateString(Analytics.MAX_PROPERTY_ITEM_LENGTH, '*');
+                        return entry.getKey().length() == Analytics.MAX_PROPERTY_ITEM_LENGTH && entry.getValue().length() == Analytics.MAX_PROPERTY_ITEM_LENGTH;
+                    }
+                }
+                return false;
+            }
+        }), anyString());
     }
 
     @Test
@@ -317,18 +347,28 @@ public void testTrackPage() {
         Analytics.trackPage(" ", null);
         verify(channel, times(1)).enqueue(any(Log.class), anyString());
         reset(channel);
-        Analytics.trackPage(generateString(257, '*'), null);
-        verify(channel, never()).enqueue(any(Log.class), anyString());
+        final String maxName = generateString(Analytics.MAX_NAME_LENGTH, '*');
+        Analytics.trackPage(maxName + "*", null);
+        verify(channel, times(1)).enqueue(argThat(new ArgumentMatcher<Log>() {
+
+            @Override
+            public boolean matches(Object item) {
+                if (item instanceof PageLog) {
+                    PageLog pageLog = (PageLog) item;
+                    return pageLog.getName().equals(maxName) && pageLog.getProperties() == null;
+                }
+                return false;
+            }
+        }), anyString());
         reset(channel);
-        Analytics.trackPage(generateString(256, '*'), null);
+        Analytics.trackPage(maxName, null);
         verify(channel, times(1)).enqueue(any(Log.class), anyString());
         reset(channel);
         Analytics.trackPage("pageName", new HashMap<String, String>() {{
             put(null, null);
             put("", null);
-            put(generateString(65, '*'), null);
+            put(generateString(Analytics.MAX_PROPERTY_ITEM_LENGTH + 1, '*'), null);
             put("1", null);
-            put("2", generateString(65, '*'));
         }});
         verify(channel, times(1)).enqueue(argThat(new ArgumentMatcher<Log>() {
 
@@ -359,6 +399,26 @@ public boolean matches(Object item) {
                 return false;
             }
         }), anyString());
+        reset(channel);
+        final String longerMapItem = generateString(Analytics.MAX_PROPERTY_ITEM_LENGTH + 1, '*');
+        Analytics.trackPage("pageName", new HashMap<String, String>() {{
+            put(longerMapItem, longerMapItem);
+        }});
+        verify(channel, times(1)).enqueue(argThat(new ArgumentMatcher<Log>() {
+
+            @Override
+            public boolean matches(Object item) {
+                if (item instanceof PageLog) {
+                    PageLog pageLog = (PageLog) item;
+                    if (pageLog.getProperties().size() == 1) {
+                        Map.Entry<String, String> entry = pageLog.getProperties().entrySet().iterator().next();
+                        String truncatedMapItem = generateString(Analytics.MAX_PROPERTY_ITEM_LENGTH, '*');
+                        return entry.getKey().length() == Analytics.MAX_PROPERTY_ITEM_LENGTH && entry.getValue().length() == Analytics.MAX_PROPERTY_ITEM_LENGTH;
+                    }
+                }
+                return false;
+            }
+        }), anyString());
     }
 
     @Test