Fix incorrect session handledCount when notifying in quick succession

CHANGELOG.md

@@ -7,6 +7,9 @@
 * Prevent overwriting config.projectPackages if already set
   [#428](https://github.com/bugsnag/bugsnag-android/pull/428)
 
+* Fix incorrect session handledCount when notifying in quick succession
+  [#434](https://github.com/bugsnag/bugsnag-android/pull/434)
+
 ## 4.11.0 (2019-01-22)
 
 ### Enhancements

sdk/src/androidTest/java/com/bugsnag/android/BeforeNotifyTest.kt

@@ -4,6 +4,7 @@ import org.junit.Assert.assertEquals
 
 import android.support.test.filters.SmallTest
 import android.support.test.runner.AndroidJUnit4
+import com.bugsnag.android.BugsnagTestUtils.generateSessionTracker
 
 import org.junit.Test
 import org.junit.runner.RunWith
@@ -23,7 +24,7 @@ class BeforeNotifyTest {
             false
         }
 
-        val error = Error.Builder(config, RuntimeException("Test"), null,
+        val error = Error.Builder(config, RuntimeException("Test"), generateSessionTracker(),
             Thread.currentThread(), false).build()
         beforeNotify.run(error)
         assertEquals(context, error.context)

sdk/src/androidTest/java/com/bugsnag/android/ErrorStoreTest.java

@@ -72,7 +72,7 @@ public void testWrite() throws Exception {
     @NonNull
     private Error writeErrorToStore() {
         Error error = new Error.Builder(config, new RuntimeException(),
-            null, Thread.currentThread(), false).build();
+            BugsnagTestUtils.generateSessionTracker(), Thread.currentThread(), false).build();
         errorStore.write(error);
         return error;
     }

sdk/src/androidTest/java/com/bugsnag/android/ErrorTest.java

@@ -1,7 +1,7 @@
 package com.bugsnag.android;
 
 import static com.bugsnag.android.BugsnagTestUtils.generateClient;
-import static com.bugsnag.android.BugsnagTestUtils.generateSession;
+import static com.bugsnag.android.BugsnagTestUtils.generateSessionTracker;
 import static com.bugsnag.android.BugsnagTestUtils.streamableToJson;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -23,6 +23,7 @@
 import org.junit.runner.RunWith;
 
 import java.io.IOException;
+import java.util.Date;
 import java.util.Map;
 
 @RunWith(AndroidJUnit4.class)
@@ -41,7 +42,8 @@ public class ErrorTest {
     public void setUp() throws Exception {
         config = new Configuration("api-key");
         RuntimeException exception = new RuntimeException("Example message");
-        error = new Error.Builder(config, exception, null, Thread.currentThread(), false).build();
+        error = new Error.Builder(config, exception, generateSessionTracker(),
+            Thread.currentThread(), false).build();
     }
 
     @Test
@@ -51,13 +53,13 @@ public void testShouldIgnoreClass() {
         // Shouldn't ignore classes not in ignoreClasses
         RuntimeException runtimeException = new RuntimeException("Test");
         Error error = new Error.Builder(config,
-            runtimeException, null, Thread.currentThread(), false).build();
+            runtimeException, generateSessionTracker(), Thread.currentThread(), false).build();
         assertFalse(error.shouldIgnoreClass());
 
         // Should ignore errors in ignoreClasses
         IOException ioException = new IOException("Test");
         error = new Error.Builder(config,
-            ioException, null, Thread.currentThread(), false).build();
+            ioException, generateSessionTracker(), Thread.currentThread(), false).build();
         assertTrue(error.shouldIgnoreClass());
     }
 
@@ -89,7 +91,7 @@ public void testBasicSerialization() throws JSONException, IOException {
     @Test
     public void testHandledSerialisation() throws Exception {
         Error err = new Error.Builder(config,
-            new RuntimeException(), null, Thread.currentThread(), false)
+            new RuntimeException(), generateSessionTracker(), Thread.currentThread(), false)
             .severityReasonType(HandledState.REASON_HANDLED_EXCEPTION)
             .build();
 
@@ -107,7 +109,7 @@ public void testHandledSerialisation() throws Exception {
     @Test
     public void testUnhandledSerialisation() throws Exception {
         Error err = new Error.Builder(config,
-            new RuntimeException(), null, Thread.currentThread(), false)
+            new RuntimeException(), generateSessionTracker(), Thread.currentThread(), false)
             .severityReasonType(HandledState.REASON_UNHANDLED_EXCEPTION)
             .severity(Severity.ERROR)
             .build();
@@ -126,7 +128,7 @@ public void testUnhandledSerialisation() throws Exception {
     @Test
     public void testPromiseRejectionSerialisation() throws Exception {
         Error err = new Error.Builder(config,
-            new RuntimeException(), null, Thread.currentThread(), false)
+            new RuntimeException(), generateSessionTracker(), Thread.currentThread(), false)
             .severityReasonType(HandledState.REASON_PROMISE_REJECTION)
             .severity(Severity.ERROR)
             .build();
@@ -145,7 +147,7 @@ public void testPromiseRejectionSerialisation() throws Exception {
     @Test
     public void testLogSerialisation() throws Exception {
         Error err = new Error.Builder(config,
-            new RuntimeException(), null, Thread.currentThread(), false)
+            new RuntimeException(), generateSessionTracker(), Thread.currentThread(), false)
             .severityReasonType(HandledState.REASON_LOG)
             .severity(Severity.WARNING)
             .attributeValue("warning")
@@ -181,7 +183,7 @@ public void testUserSpecifiedSerialisation() throws Exception {
     @Test
     public void testStrictModeSerialisation() throws Exception {
         Error err = new Error.Builder(config,
-            new RuntimeException(), null, Thread.currentThread(), false)
+            new RuntimeException(), generateSessionTracker(), Thread.currentThread(), false)
             .severityReasonType(HandledState.REASON_STRICT_MODE)
             .attributeValue("Test")
             .build();
@@ -245,9 +247,10 @@ public void testSetSeverity() throws JSONException, IOException {
 
     @Test
     public void testSessionIncluded() throws Exception {
-        Session session = generateSession();
+        SessionTracker sessionTracker = generateSessionTracker();
+        final Session session = sessionTracker.startNewSession(new Date(), new User(), false);
         Error err = new Error.Builder(config,
-            new RuntimeException(), session, Thread.currentThread(), false).build();
+            new RuntimeException(), sessionTracker, Thread.currentThread(), false).build();
 
         JSONObject errorJson = streamableToJson(err);
         assertNotNull(errorJson);
@@ -262,13 +265,14 @@ public void testSessionIncluded() throws Exception {
         JSONObject eventsNode = sessionNode.getJSONObject("events");
         assertNotNull(eventsNode);
         assertEquals(2, eventsNode.length());
-        assertEquals(0, eventsNode.get("handled"));
+        assertEquals(1, eventsNode.get("handled"));
     }
 
     @Test(expected = JSONException.class)
     public void testSessionExcluded() throws Exception {
         Error err = new Error.Builder(config,
-            new RuntimeException(), null, Thread.currentThread(), false).build();
+            new RuntimeException(), generateSessionTracker(),
+            Thread.currentThread(), false).build();
 
         JSONObject errorJson = streamableToJson(err);
         assertNotNull(errorJson);
@@ -279,11 +283,13 @@ public void testSessionExcluded() throws Exception {
     public void checkExceptionMessageNullity() throws Exception {
         String msg = "Foo";
         Error err = new Error.Builder(config,
-            new RuntimeException(msg), null, Thread.currentThread(), false).build();
+            new RuntimeException(msg), generateSessionTracker(),
+            Thread.currentThread(), false).build();
         assertEquals(msg, err.getExceptionMessage());
 
         err = new Error.Builder(config,
-            new RuntimeException(), null, Thread.currentThread(), false).build();
+            new RuntimeException(), generateSessionTracker(),
+            Thread.currentThread(), false).build();
         assertEquals("", err.getExceptionMessage());
     }
 
@@ -305,7 +311,7 @@ public void testBugsnagExceptionName() throws Exception {
         BugsnagException exception = new BugsnagException("Busgang", "exceptional",
             new StackTraceElement[]{});
         Error err = new Error.Builder(config,
-            exception, null, Thread.currentThread(), false).build();
+            exception, generateSessionTracker(), Thread.currentThread(), false).build();
         assertEquals("Busgang", err.getExceptionName());
     }
 
@@ -350,7 +356,8 @@ public void testSetUser() throws Exception {
     public void testBuilderMetaData() {
         Configuration config = new Configuration("api-key");
         Error.Builder builder = new Error.Builder(config,
-            new RuntimeException("foo"), null, Thread.currentThread(), false);
+            new RuntimeException("foo"), generateSessionTracker(),
+            Thread.currentThread(), false);
 
         assertNotNull(builder.metaData(new MetaData()).build());
 
@@ -392,9 +399,9 @@ public void testBuilderNullSession() throws Throwable {
         config.setAutoCaptureSessions(false);
         RuntimeException exception = new RuntimeException("foo");
 
-        Session session = generateSession();
-        session.setAutoCaptured(true);
-        error = new Error.Builder(config, exception, session,
+        SessionTracker sessionTracker = generateSessionTracker();
+        sessionTracker.startNewSession(new Date(), new User(), true);
+        error = new Error.Builder(config, exception, sessionTracker,
             Thread.currentThread(), false).build();
 
         JSONObject errorJson = streamableToJson(error);

sdk/src/androidTest/java/com/bugsnag/android/ExceptionsTest.java

@@ -66,7 +66,8 @@ public void testNamedException() throws JSONException, IOException {
         StackTraceElement element = new StackTraceElement("Class", "method", "Class.java", 123);
         StackTraceElement[] frames = new StackTraceElement[]{element};
         Error error = new Error.Builder(config, "RuntimeException",
-            "Example message", frames, null, Thread.currentThread()).build();
+            "Example message", frames, BugsnagTestUtils.generateSessionTracker(),
+            Thread.currentThread()).build();
         Exceptions exceptions = new Exceptions(config, error.getException());
 
         JSONObject exceptionJson = streamableToJsonArray(exceptions).getJSONObject(0);

sdk/src/androidTest/java/com/bugsnag/android/NullMetadataTest.java

@@ -1,5 +1,7 @@
 package com.bugsnag.android;
 
+import static com.bugsnag.android.BugsnagTestUtils.generateClient;
+import static com.bugsnag.android.BugsnagTestUtils.generateSessionTracker;
 import static junit.framework.Assert.assertEquals;
 import static junit.framework.Assert.assertNotNull;
 
@@ -31,13 +33,13 @@ public class NullMetadataTest {
     @Before
     public void setUp() throws Exception {
         config = new Configuration("api-key");
-        BugsnagTestUtils.generateClient();
+        generateClient();
         throwable = new RuntimeException("Test");
     }
 
     @Test
     public void testErrorDefaultMetaData() throws Exception {
-        Error error = new Error.Builder(config, throwable, null,
+        Error error = new Error.Builder(config, throwable, generateSessionTracker(),
             Thread.currentThread(), false).build();
         validateDefaultMetadata(error.getMetaData());
     }
@@ -46,13 +48,14 @@ public void testErrorDefaultMetaData() throws Exception {
     public void testSecondErrorDefaultMetaData() throws Exception {
         Error error = new Error.Builder(config, "RuntimeException",
             "Something broke", new StackTraceElement[]{},
-            null, Thread.currentThread()).build();
+            generateSessionTracker(), Thread.currentThread()).build();
         validateDefaultMetadata(error.getMetaData());
     }
 
     @Test
     public void testErrorSetMetadataRef() throws Exception {
-        Error error = new Error.Builder(config, throwable, null,
+        Error error = new Error.Builder(config, throwable,
+            generateSessionTracker(),
             Thread.currentThread(), false).build();
         MetaData metaData = new MetaData();
         metaData.addToTab(TAB_KEY, "test", "data");
@@ -62,7 +65,8 @@ public void testErrorSetMetadataRef() throws Exception {
 
     @Test
     public void testErrorSetNullMetadata() throws Exception {
-        Error error = new Error.Builder(config, throwable, null,
+        Error error = new Error.Builder(config, throwable,
+            generateSessionTracker(),
             Thread.currentThread(), false).build();
         error.setMetaData(null);
         validateDefaultMetadata(error.getMetaData());
@@ -97,7 +101,7 @@ public boolean run(Error error) {
             }
         });
         Error error = new Error.Builder(config, new Throwable(),
-            null, Thread.currentThread(), false).build();
+            generateSessionTracker(), Thread.currentThread(), false).build();
         Client client = Bugsnag.getClient();
         client.notify(error, DeliveryStyle.SAME_THREAD, null);
     }

sdk/src/androidTest/java/com/bugsnag/android/ReportTest.java

@@ -31,7 +31,8 @@ public class ReportTest {
     public void setUp() throws Exception {
         Configuration config = new Configuration("example-api-key");
         RuntimeException exception = new RuntimeException("Something broke");
-        Error error = new Error.Builder(config, exception, null,
+        Error error = new Error.Builder(config, exception,
+            BugsnagTestUtils.generateSessionTracker(),
             Thread.currentThread(), false).build();
         report = new Report("api-key", error);
     }

sdk/src/androidTest/java/com/bugsnag/android/SessionTest.kt

@@ -0,0 +1,47 @@
+package com.bugsnag.android
+
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNotEquals
+import org.junit.Test
+import java.util.Date
+
+class SessionTest {
+
+    private val session = Session("123", Date(), User(), true)
+
+    /**
+     * Verifies that all the fields in session are copied into a new object correctly
+     */
+    @Test
+    fun copySession() {
+        val copy = Session.copySession(session)
+        assertNotEquals(session, copy)
+        validateSessionCopied(copy)
+    }
+
+    @Test
+    fun handledIncrementCopiesSession() {
+        val copy = session.incrementHandledAndCopy()
+        assertNotEquals(session, copy)
+        validateSessionCopied(copy)
+    }
+
+    @Test
+    fun unhandledIncrementCopiesSession() {
+        val copy = session.incrementUnhandledAndCopy()
+        assertNotEquals(session, copy)
+        validateSessionCopied(copy)
+    }
+
+    private fun validateSessionCopied(copy: Session) {
+        with(session) {
+            assertEquals(id, copy.id)
+            assertEquals(startedAt, copy.startedAt)
+            assertEquals(user, copy.user) // make a shallow copy
+            assertEquals(isAutoCaptured, copy.isAutoCaptured)
+            assertEquals(isTracked.get(), copy.isTracked.get())
+            assertEquals(session.unhandledCount, copy.unhandledCount)
+            assertEquals(session.handledCount, copy.handledCount)
+        }
+    }
+}

sdk/src/androidTest/java/com/bugsnag/android/SessionTrackerTest.java

@@ -2,15 +2,13 @@
 
 import static com.bugsnag.android.BugsnagTestUtils.generateClient;
 import static com.bugsnag.android.BugsnagTestUtils.generateSessionStore;
-import static com.bugsnag.android.BugsnagTestUtils.generateSessionTrackingApiClient;
 import static junit.framework.Assert.assertEquals;
 import static junit.framework.Assert.assertFalse;
 import static junit.framework.Assert.assertNotNull;
 import static junit.framework.Assert.assertNull;
 import static junit.framework.Assert.assertTrue;
 import static org.junit.Assert.assertNotEquals;
 
-import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -80,11 +78,11 @@ public void testUniqueSessionIds() throws Exception {
     @Test
     public void testIncrementCounts() throws Exception {
         sessionTracker.startNewSession(new Date(), user, false);
-        sessionTracker.incrementHandledError();
-        sessionTracker.incrementHandledError();
-        sessionTracker.incrementUnhandledError();
-        sessionTracker.incrementUnhandledError();
-        sessionTracker.incrementUnhandledError();
+        sessionTracker.incrementHandledAndCopy();
+        sessionTracker.incrementHandledAndCopy();
+        sessionTracker.incrementUnhandledAndCopy();
+        sessionTracker.incrementUnhandledAndCopy();
+        sessionTracker.incrementUnhandledAndCopy();
 
         Session session = sessionTracker.getCurrentSession();
         assertNotNull(session);