Split event processor in Device/App data (#180)

sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java

@@ -1,6 +1,7 @@
 package io.sentry.android.core;
 
 import static android.content.Context.ACTIVITY_SERVICE;
+import static io.sentry.core.ILogger.logIfNotNull;
 
 import android.Manifest;
 import android.app.ActivityManager;
@@ -27,6 +28,7 @@
 import io.sentry.core.SentryEvent;
 import io.sentry.core.SentryLevel;
 import io.sentry.core.SentryOptions;
+import io.sentry.core.hints.Cached;
 import io.sentry.core.protocol.App;
 import io.sentry.core.protocol.DebugImage;
 import io.sentry.core.protocol.DebugMeta;
@@ -72,14 +74,39 @@ public DefaultAndroidEventProcessor(Context context, SentryOptions options) {
 
   @Override
   public SentryEvent process(SentryEvent event, @Nullable Object hint) {
-    // TODO: Split by data to apply to cached events and
-    if (event.getSdk() == null) {
-      event.setSdk(getSdkVersion());
+    if (!(hint instanceof Cached)) {
+      processNonCachedEvent(event);
+    } else {
+      logIfNotNull(
+          options.getLogger(),
+          SentryLevel.DEBUG,
+          "Event was cached so not applying data relevant to the current app execution/version: %s",
+          event.getEventId());
+    }
+
+    if (event.getContexts().getDevice() == null) {
+      event.getContexts().setDevice(getDevice());
+    }
+    if (event.getContexts().getOperatingSystem() == null) {
+      event.getContexts().setOperatingSystem(getOperatingSystem());
     }
 
+    return event;
+  }
+
+  // Data to be applied to events that was created in the running process
+  private void processNonCachedEvent(SentryEvent event) {
     if (event.getUser() == null) {
       event.setUser(getUser());
     }
+    setAppExtras(event);
+
+    if (event.getDebugMeta() == null) {
+      event.setDebugMeta(getDebugMeta());
+    }
+    if (event.getSdk() == null) {
+      event.setSdk(getSdkVersion());
+    }
 
     PackageInfo packageInfo = getPackageInfo();
     if (packageInfo != null) {
@@ -93,20 +120,6 @@ public SentryEvent process(SentryEvent event, @Nullable Object hint) {
         event.getContexts().setApp(getApp(packageInfo));
       }
     }
-    setAppExtras(event);
-
-    if (event.getDebugMeta() == null) {
-      event.setDebugMeta(getDebugMeta());
-    }
-
-    if (event.getContexts().getDevice() == null) {
-      event.getContexts().setDevice(getDevice());
-    }
-    if (event.getContexts().getOperatingSystem() == null) {
-      event.getContexts().setOperatingSystem(getOperatingSystem());
-    }
-
-    return event;
   }
 
   private List<DebugImage> getDebugImages() {

sentry-android-core/src/test/java/io/sentry/android/core/CachedEvent.kt

@@ -0,0 +1,5 @@
+package io.sentry.android.core
+
+import io.sentry.core.hints.Cached
+
+class CachedEvent : Cached

sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt

@@ -6,11 +6,14 @@ import androidx.test.ext.junit.runners.AndroidJUnit4
 import com.nhaarman.mockitokotlin2.doReturn
 import com.nhaarman.mockitokotlin2.mock
 import io.sentry.core.ILogger
+import io.sentry.core.SentryEvent
 import io.sentry.core.SentryOptions
 import kotlin.test.BeforeTest
 import kotlin.test.Test
 import kotlin.test.assertEquals
 import kotlin.test.assertFailsWith
+import kotlin.test.assertNotNull
+import kotlin.test.assertNull
 import org.junit.runner.RunWith
 
 @RunWith(AndroidJUnit4::class)
@@ -55,4 +58,34 @@ class DefaultAndroidEventProcessorTest {
     fun `when null options is provided, invalid argument is thrown`() {
         assertFailsWith<IllegalArgumentException> { DefaultAndroidEventProcessor(context, null) }
     }
+
+    @Test
+    fun `When hint is not Cached, data should be applied`() {
+        val processor = DefaultAndroidEventProcessor(context, fixture.options)
+        var event = SentryEvent().apply {
+        }
+        // refactor and mock data later on
+        event = processor.process(event, null)
+        assertNotNull(event.user)
+        assertNotNull(event.contexts.app)
+//        assertNotNull(event.debugMeta) file doesnt exist
+        assertNotNull(event.sdk)
+        assertNotNull(event.release)
+        assertNotNull(event.dist)
+    }
+
+    @Test
+    fun `When hint is Cached, data should not be applied`() {
+        val processor = DefaultAndroidEventProcessor(context, fixture.options)
+        var event = SentryEvent().apply {
+        }
+        // refactor and mock data later on
+        event = processor.process(event, CachedEvent())
+        assertNull(event.user)
+        assertNull(event.contexts.app)
+        assertNull(event.debugMeta)
+        assertNull(event.sdk)
+        assertNull(event.release)
+        assertNull(event.dist)
+    }
 }

sentry-core/src/main/java/io/sentry/core/MainEventProcessor.java

@@ -37,13 +37,6 @@ public final class MainEventProcessor implements EventProcessor {
 
   @Override
   public SentryEvent process(SentryEvent event, @Nullable Object hint) {
-    if (event.getRelease() == null) {
-      event.setRelease(options.getRelease());
-    }
-    if (event.getEnvironment() == null) {
-      event.setEnvironment(options.getEnvironment());
-    }
-
     if (event.getPlatform() == null) {
       // this actually means JVM related.
       event.setPlatform("java");
@@ -54,18 +47,29 @@ public SentryEvent process(SentryEvent event, @Nullable Object hint) {
       event.setExceptions(sentryExceptionFactory.getSentryExceptions(throwable));
     }
 
-    if (event.getThreads() == null) {
-      if (!(hint instanceof Cached)) {
-        event.setThreads(sentryThreadFactory.getCurrentThreads());
-      } else {
-        logIfNotNull(
-            options.getLogger(),
-            SentryLevel.DEBUG,
-            "Event was cached so not applying threads: %s",
-            event.getEventId());
-      }
+    if (!(hint instanceof Cached)) {
+      processNonCachedEvent(event);
+    } else {
+      logIfNotNull(
+          options.getLogger(),
+          SentryLevel.DEBUG,
+          "Event was cached so not applying data relevant to the current app execution/version: %s",
+          event.getEventId());
     }
 
     return event;
   }
+
+  private void processNonCachedEvent(SentryEvent event) {
+    if (event.getRelease() == null) {
+      event.setRelease(options.getRelease());
+    }
+    if (event.getEnvironment() == null) {
+      event.setEnvironment(options.getEnvironment());
+    }
+
+    if (event.getThreads() == null) {
+      event.setThreads(sentryThreadFactory.getCurrentThreads());
+    }
+  }
 }

sentry-core/src/test/java/io/sentry/core/CachedEvent.kt

@@ -0,0 +1,5 @@
+package io.sentry.core
+
+import io.sentry.core.hints.Cached
+
+class CachedEvent : Cached

sentry-core/src/test/java/io/sentry/core/MainEventProcessorTest.kt

@@ -2,14 +2,18 @@ package io.sentry.core
 
 import com.nhaarman.mockitokotlin2.mock
 import kotlin.test.Test
+import kotlin.test.assertEquals
 import kotlin.test.assertFalse
+import kotlin.test.assertNull
 import kotlin.test.assertSame
 import kotlin.test.assertTrue
 
 class MainEventProcessorTest {
     class Fixture {
-        private var sentryOptions: SentryOptions = SentryOptions().apply {
+        private val sentryOptions: SentryOptions = SentryOptions().apply {
             dsn = dsnString
+            release = "release"
+            environment = "environment"
         }
         fun getSut() = MainEventProcessor(sentryOptions)
     }
@@ -21,13 +25,41 @@ class MainEventProcessorTest {
         val sut = fixture.getSut()
 
         val crashedThread = Thread.currentThread()
-        val mockThrowable = mock<Throwable>()
-        val actualThrowable = UncaughtExceptionHandlerIntegration.getUnhandledThrowable(crashedThread, mockThrowable)
-        var event = SentryEvent().apply { throwable = actualThrowable }
+        var event = generateCrashedEvent(crashedThread)
         event = sut.process(event, null)
 
         assertSame(crashedThread.id, event.exceptions.first().threadId)
         assertTrue(event.threads.first { t -> t.id == crashedThread.id }.isCrashed)
         assertFalse(event.exceptions.first().mechanism.isHandled)
     }
+
+    @Test
+    fun `When hint is not Cached, data should be applied`() {
+        val sut = fixture.getSut()
+        val crashedThread = Thread.currentThread()
+        var event = generateCrashedEvent(crashedThread)
+        event = sut.process(event, null)
+
+        assertEquals("release", event.release)
+        assertEquals("environment", event.environment)
+        assertTrue(event.threads.first { t -> t.id == crashedThread.id }.isCrashed)
+    }
+
+    @Test
+    fun `When hint is Cached, data should not be applied`() {
+        val sut = fixture.getSut()
+        val crashedThread = Thread.currentThread()
+        var event = generateCrashedEvent(crashedThread)
+        event = sut.process(event, CachedEvent())
+
+        assertNull(event.release)
+        assertNull(event.environment)
+        assertNull(event.threads)
+    }
+
+    private fun generateCrashedEvent(crashedThread: Thread = Thread.currentThread()) = SentryEvent().apply {
+        val mockThrowable = mock<Throwable>()
+        val actualThrowable = UncaughtExceptionHandlerIntegration.getUnhandledThrowable(crashedThread, mockThrowable)
+        throwable = actualThrowable
+    }
 }