Merge pull request #44 from Microsoft/feature/fix-ConcurrentModificat…

…ionException

Serialize TelemetryItem before sending it to persistence.

applicationinsights-android/src/androidTest/java/com/microsoft/applicationinsights/library/ChannelQueueTest.java

@@ -46,8 +46,8 @@ public void testItemGetsEnqueued(){
         when(mockConfig.getMaxBatchCount()).thenReturn(3);
 
         // Test
-        sut.enqueue(new Envelope());
-        sut.enqueue(new Envelope());
+        sut.enqueue(new String());
+        sut.enqueue(new String());
 
         // Verify
         Assert.assertEquals(2, sut.list.size());
@@ -59,17 +59,17 @@ public void testQueueFlushedIfMaxBatchCountReached() {
         when(mockConfig.getMaxBatchCount()).thenReturn(3);
 
         // Test
-        sut.enqueue(new Envelope());
-        sut.enqueue(new Envelope());
+        sut.enqueue(new String());
+        sut.enqueue(new String());
 
         // Verify
         Assert.assertEquals(2, sut.list.size());
-        verify(mockPersistence,never()).persist(any(IJsonSerializable[].class), anyBoolean());
+        verify(mockPersistence,never()).persist(any(String[].class), anyBoolean());
 
-        sut.enqueue(new Envelope());
+        sut.enqueue(new String());
 
         Assert.assertEquals(0, sut.list.size());
-        verify(mockPersistence,times(1)).persist(any(IJsonSerializable[].class), anyBoolean());
+        verify(mockPersistence,times(1)).persist(any(String[].class), anyBoolean());
     }
 
     public void testQueueFlushedAfterBatchIntervalReached() {
@@ -78,12 +78,12 @@ public void testQueueFlushedAfterBatchIntervalReached() {
         when(mockConfig.getMaxBatchCount()).thenReturn(3);
 
         // Test
-        sut.enqueue(new Envelope());
+        sut.enqueue(new String());
 
         // Verify
         Assert.assertEquals(1, sut.list.size());
-        verify(mockPersistence,never()).persist(any(IJsonSerializable[].class), anyBoolean());
-        verify(mockPersistence,after(250).times(1)).persist(any(IJsonSerializable[].class), anyBoolean());
+        verify(mockPersistence,never()).persist(any(String[].class), anyBoolean());
+        verify(mockPersistence,after(250).times(1)).persist(any(String[].class), anyBoolean());
         Assert.assertEquals(0, sut.list.size());
     }
 
@@ -92,16 +92,16 @@ public void testFlushingQueueWorks() {
         when(mockConfig.getMaxBatchIntervalMs()).thenReturn(200);
         when(mockConfig.getMaxBatchCount()).thenReturn(3);
 
-        sut.enqueue(new Envelope());
+        sut.enqueue(new String());
         Assert.assertEquals(1, sut.list.size());
-        verify(mockPersistence,never()).persist(any(IJsonSerializable[].class), anyBoolean());
+        verify(mockPersistence,never()).persist(any(String[].class), anyBoolean());
 
         // Test
         sut.flush();
 
         // Verify
         Assert.assertEquals(0, sut.list.size());
-        verify(mockPersistence,times(1)).persist(any(IJsonSerializable[].class), anyBoolean());
+        verify(mockPersistence,times(1)).persist(any(String[].class), anyBoolean());
     }
 
 

applicationinsights-android/src/androidTest/java/com/microsoft/applicationinsights/library/ChannelTest.java

@@ -34,13 +34,17 @@ public void testSynchronizeFlushesQueue(){
     public void testEnqueuedItemIsAddedToQueue(){
         // Test
         Envelope testItem1 = new Envelope();
+        testItem1.setDeviceId("Test");
+        String serialized1 = sut.serializeEnvelope(testItem1);
         sut.enqueue(testItem1);
         Envelope testItem2 = new Envelope();
+        testItem2.setDeviceId("Test1");
+        String serialized2 = sut.serializeEnvelope(testItem2);
         sut.enqueue(testItem2);
 
         // Verify
-        verify(mockQueue, times(1)).enqueue(testItem1);
-        verify(mockQueue, times(1)).enqueue(testItem2);
+        verify(mockQueue, times(1)).enqueue(serialized1);
+        verify(mockQueue, times(1)).enqueue(serialized2);
     }
 
     public void testProcessUnhandledExceptionIsPersistedDirectly(){
@@ -49,19 +53,20 @@ public void testProcessUnhandledExceptionIsPersistedDirectly(){
         sut.processUnhandledException(testItem1);
 
         // Verify
-        verify(mockQueue, times(0)).enqueue(testItem1);
-        verify(mockPersistence, times(1)).persist(any(IJsonSerializable[].class), eq(true));
+        verify(mockQueue, times(0)).enqueue(new String());
+        verify(mockPersistence, times(1)).persist(any(String[].class), eq(true));
     }
 
     public void testQueueFlushesWhenProcessingCrash(){
         // Setup
         Envelope testItem1 = new Envelope();
+        String serializedString = sut.serializeEnvelope(testItem1);
 
         // Test
         sut.processUnhandledException(testItem1);
 
         // Verify
-        verify(mockQueue, times(0)).enqueue(testItem1);
+        verify(mockQueue, times(0)).enqueue(serializedString);
         verify(mockQueue, times(1)).flush();
     }
 }

applicationinsights-android/src/androidTest/java/com/microsoft/applicationinsights/library/PersistenceTest.java

@@ -29,7 +29,7 @@ public void testSaveAndGetData() throws Exception {
         Persistence persistence = Persistence.getInstance();
 
         String data = "SAVE THIS DATA";
-        persistence.persist(data, false);
+        persistence.writeToDisk(data, false);
         File file = persistence.nextAvailableFile();
         Assert.assertEquals("Data retrieved from file is equal to data saved", data, persistence.load(file));
     }

applicationinsights-android/src/main/java/com/microsoft/applicationinsights/library/Channel.java

@@ -1,10 +1,13 @@
 package com.microsoft.applicationinsights.library;
 
 import com.microsoft.applicationinsights.contracts.Envelope;
-import com.microsoft.applicationinsights.contracts.shared.IJsonSerializable;
+import com.microsoft.applicationinsights.contracts.Internal;
 import com.microsoft.applicationinsights.library.config.IQueueConfig;
 import com.microsoft.applicationinsights.logging.InternalLogging;
 
+import java.io.IOException;
+import java.io.StringWriter;
+
 /**
  * This class records telemetry for application insights.
  */
@@ -80,24 +83,40 @@ protected void synchronize() {
      * @param envelope the envelope object to record
      */
     protected void enqueue(Envelope envelope) {
+        String serializedData = this.serializeEnvelope(envelope);
+
         // enqueue to queue
-        queue.enqueue(envelope);
+        queue.enqueue(serializedData);
 
         InternalLogging.info(TAG, "enqueued telemetry", envelope.getName());
     }
 
+    protected String serializeEnvelope(Envelope envelope) {
+        try {
+            if (envelope != null) {
+                StringWriter stringWriter = new StringWriter();
+                envelope.serialize(stringWriter);
+                return stringWriter.toString();
+            }
+            InternalLogging.warn(TAG, "Envelop wasn't empty but failed to serialize anything, returning null");
+            return null;
+        } catch (IOException e) {
+            InternalLogging.warn(TAG, "Failed to save data with exception: " + e.toString());
+            return null;
+        }
+    }
+
     protected void processUnhandledException(Envelope envelope) {
         queue.isCrashing = true;
         queue.flush();
 
-        IJsonSerializable[] data = new IJsonSerializable[1];
-        data[0] = envelope;
+        String[] data = new String[1];
+        data[0] = serializeEnvelope(envelope);
 
         if (this.persistence != null) {
             InternalLogging.info(TAG, "persisting crash", envelope.toString());
             this.persistence.persist(data, true);
-        }
-        else {
+        } else {
             InternalLogging.info(TAG, "error persisting crash", envelope.toString());
         }
 

applicationinsights-android/src/main/java/com/microsoft/applicationinsights/library/ChannelQueue.java

@@ -1,6 +1,5 @@
 package com.microsoft.applicationinsights.library;
 
-import com.microsoft.applicationinsights.contracts.shared.IJsonSerializable;
 import com.microsoft.applicationinsights.library.config.IQueueConfig;
 import com.microsoft.applicationinsights.logging.InternalLogging;
 
@@ -37,7 +36,7 @@ class ChannelQueue {
     /**
      * The linked list for this queue
      */
-    protected final List<IJsonSerializable> list;
+    protected final List<String> list;
 
     /**
      * If true the app is crashing and data should be persisted instead of sent
@@ -58,7 +57,7 @@ class ChannelQueue {
      * Prevent external instantiation
      */
     protected ChannelQueue(IQueueConfig config) {
-        this.list = new LinkedList<IJsonSerializable>();
+        this.list = new LinkedList<String>();
         this.timer = new Timer("Application Insights Sender Queue", true);
         this.config = config;
         this.isCrashing = false;
@@ -68,19 +67,19 @@ protected ChannelQueue(IQueueConfig config) {
     /**
      * Adds an item to the sender queue
      *
-     * @param item a telemetry item to enqueue
+     * @param serializedItem a serialized telemetry item to enqueue
      * @return true if the item was successfully added to the queue
      */
-    protected boolean enqueue(IJsonSerializable item) {
+    protected boolean enqueue(String serializedItem) {
         // prevent invalid argument exception
-        if (item == null) {
+        if (serializedItem == null) {
             return false;
         }
 
         boolean success;
         synchronized (this.LOCK) {
             // attempt to add the item to the queue
-            success = this.list.add(item);
+            success = this.list.add(serializedItem);
 
             if (success) {
                 if ((this.list.size() >= this.config.getMaxBatchCount()) || isCrashing) {
@@ -106,10 +105,10 @@ protected void flush() {
             this.scheduledPersistenceTask.cancel();
         }
 
-        IJsonSerializable[] data;
+        String[] data;
         synchronized (this.LOCK) {
             if (!list.isEmpty()) {
-                data = new IJsonSerializable[list.size()];
+                data = new String[list.size()];
                 list.toArray(data);
                 list.clear();
 
@@ -132,7 +131,7 @@ protected void schedulePersitenceTask(){
     /**
      * Initiates persisting the content queue.
      */
-    protected void executePersistenceTask(IJsonSerializable[] data){
+    protected void executePersistenceTask(String[] data){
         if (data != null) {
             if (persistence != null) {
                 persistence.persist(data, false);

applicationinsights-android/src/main/java/com/microsoft/applicationinsights/library/Persistence.java

@@ -2,16 +2,13 @@
 
 import android.content.Context;
 
-import com.microsoft.applicationinsights.contracts.shared.IJsonSerializable;
 import com.microsoft.applicationinsights.logging.InternalLogging;
 
 import java.io.BufferedReader;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
-import java.io.IOException;
 import java.io.InputStreamReader;
-import java.io.StringWriter;
 import java.lang.ref.WeakReference;
 import java.util.ArrayList;
 import java.util.UUID;
@@ -95,11 +92,11 @@ protected static Persistence getInstance() {
     /**
      * Serializes a IJsonSerializable[] and calls:
      *
-     * @param data         the data to serialize and save to disk
+     * @param data         the data to save to disk
      * @param highPriority the priority to save the data with
-     * @see Persistence#persist(String, Boolean)
+     * @see Persistence#writeToDisk(String, Boolean)
      */
-    protected void persist(IJsonSerializable[] data, Boolean highPriority) {
+    protected void persist(String[] data, Boolean highPriority) {
         if (!this.isFreeSpaceAvailable(highPriority)) {
             InternalLogging.warn(TAG, "No free space on disk to flush data.");
             Sender.getInstance().sendNextFile();
@@ -108,28 +105,22 @@ protected void persist(IJsonSerializable[] data, Boolean highPriority) {
 
         StringBuilder buffer = new StringBuilder();
         Boolean isSuccess;
-        try {
-            buffer.append('[');
-            for (int i = 0; i < data.length; i++) {
-                if (i > 0) {
-                    buffer.append(',');
-                }
-                StringWriter stringWriter = new StringWriter();
-                data[i].serialize(stringWriter);
-                buffer.append(stringWriter.toString());
+        buffer.append('[');
+        for (int i = 0; i < data.length; i++) {
+            if (i > 0) {
+                buffer.append(',');
             }
+            buffer.append(data[i]);
+        }
 
-            buffer.append(']');
-            String serializedData = buffer.toString();
-            isSuccess = this.persist(serializedData, highPriority);
-            if (isSuccess) {
-                Sender sender = Sender.getInstance();
-                if (sender != null) {
-                    sender.sendNextFile();
-                }
+        buffer.append(']');
+        String serializedData = buffer.toString();
+        isSuccess = this.writeToDisk(serializedData, highPriority);
+        if (isSuccess) {
+            Sender sender = Sender.getInstance();
+            if (sender != null) {
+                sender.sendNextFile();
             }
-        } catch (IOException e) {
-            InternalLogging.warn(TAG, "Failed to save data with exception: " + e.toString());
         }
     }
 
@@ -140,7 +131,7 @@ protected void persist(IJsonSerializable[] data, Boolean highPriority) {
      * @param highPriority the priority we want to use for persisting the data
      * @return true if the operation was successful, false otherwise
      */
-    protected boolean persist(String data, Boolean highPriority) {
+    protected boolean writeToDisk(String data, Boolean highPriority) {
         String uuid = UUID.randomUUID().toString();
         Boolean isSuccess = false;
         Context context = this.getContext();

applicationinsights-android/src/main/java/com/microsoft/applicationinsights/library/Sender.java

@@ -130,7 +130,6 @@ protected void send(File fileToSend) {
         String persistedData = this.persistence.load(fileToSend);
         if (!persistedData.isEmpty()) {
             try {
-                InternalLogging.info(TAG, "sending persisted data", persistedData);
                 this.operationsCount.getAndIncrement();
                 this.sendRequestWithPayload(persistedData, fileToSend);
             } catch (IOException e) {
@@ -160,7 +159,7 @@ protected void sendRequestWithPayload(String payload, File fileToSend) throws IO
         connection.setUseCaches(false);
 
         try {
-            InternalLogging.info(TAG, "writing payload", payload);
+            InternalLogging.info(TAG, "Logging payload", payload);
             writer = getWriter(connection);
             writer.write(payload);
             writer.flush();