Merge pull request #5937 from getsentry/fix/data-collection-preserve-installation-id

fix(android): [Data Collection 19] Preserve installation ID

Author: adinauer

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

@@ -678,7 +678,7 @@ private void mergeUser(final @NotNull SentryBaseEvent event) {
     }
 
     // userId should be set even if event is Cached as the userId is static and won't change anyway.
-    if (user.getId() == null && options.getDataCollectionResolver().isUserInfoWithLegacyAlways()) {
+    if (user.getId() == null) {
       user.setId(getDeviceId());
     }
     if (user.getIpAddress() == null && options.getDataCollectionResolver().isUserInfo()) {
@@ -745,8 +745,7 @@ private void setDevice(final @NotNull SentryBaseEvent event) {
       device.setScreenDpi(displayMetrics.densityDpi);
     }
 
-    if (device.getId() == null
-        && options.getDataCollectionResolver().isUserInfoWithLegacyAlways()) {
+    if (device.getId() == null) {
       device.setId(getDeviceId());
     }
 

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

@@ -175,7 +175,7 @@ private void mergeUser(final @NotNull SentryBaseEvent event) {
     }
 
     // userId should be set even if event is Cached as the userId is static and won't change anyway.
-    if (user.getId() == null && options.getDataCollectionResolver().isUserInfoWithLegacyAlways()) {
+    if (user.getId() == null) {
       user.setId(Installation.id(context));
     }
     if (user.getIpAddress() == null && options.getDataCollectionResolver().isUserInfo()) {
@@ -374,9 +374,7 @@ private void setAppExtras(final @NotNull App app, final @NotNull Hint hint) {
    */
   public @NotNull User getDefaultUser(final @NotNull Context context) {
     final @NotNull User user = new User();
-    if (options.getDataCollectionResolver().isUserInfoWithLegacyAlways()) {
-      user.setId(Installation.id(context));
-    }
+    user.setId(Installation.id(context));
     return user;
   }
 

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

@@ -109,8 +109,7 @@ public Device collectDeviceInformation(
     device.setBootTime(getBootTime());
     device.setTimezone(getTimeZone());
 
-    if (device.getId() == null
-        && options.getDataCollectionResolver().isUserInfoWithLegacyAlways()) {
+    if (device.getId() == null) {
       device.setId(getDeviceId());
     }
 

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

@@ -100,8 +100,7 @@ public static Map<String, Object> serializeScope(
         user = new User();
         scope.setUser(user);
       }
-      if (user.getId() == null
-          && options.getDataCollectionResolver().isUserInfoWithLegacyAlways()) {
+      if (user.getId() == null) {
         try {
           user.setId(Installation.id(context));
         } catch (RuntimeException e) {

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

@@ -232,13 +232,13 @@ class ApplicationExitInfoEventProcessorTest {
   }
 
   @Test
-  fun `when user info is disabled, does not set device id`() {
+  fun `when user info is disabled, sets device id`() {
     fixture.options.dataCollection.setUserInfo(false)
     val hint = HintUtils.createWithTypeCheckHint(AbnormalExitHint())
 
     val processed = processEvent(hint)
 
-    assertNull(processed.contexts.device!!.id)
+    assertNotNull(processed.contexts.device!!.id)
   }
 
   @Test
@@ -681,7 +681,7 @@ class ApplicationExitInfoEventProcessorTest {
   }
 
   @Test
-  fun `when user info is disabled, does not set installation id for missing user id`() {
+  fun `when user info is disabled, sets installation id for missing user id`() {
     fixture.options.dataCollection.setUserInfo(false)
     val hint = HintUtils.createWithTypeCheckHint(BackfillableHint())
     val original = SentryEvent()
@@ -690,7 +690,7 @@ class ApplicationExitInfoEventProcessorTest {
 
     val processed = processor.process(original, hint)
 
-    assertNull(processed!!.user!!.id)
+    assertEquals(Installation.deviceId, processed!!.user!!.id)
   }
 
   @Test

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

@@ -285,15 +285,15 @@ class DefaultAndroidEventProcessorTest {
   }
 
   @Test
-  fun `when user info is disabled, does not set automatic user data`() {
+  fun `when user info is disabled, sets installation id but not automatic ip`() {
     fixture.options.dataCollection.setUserInfo(false)
     val sut = fixture.getSut(context, isSendDefaultPii = true)
     val event = SentryEvent().apply { user = User() }
 
     sut.process(event, Hint())
 
     assertNotNull(event.user) {
-      assertNull(it.id)
+      assertNotNull(it.id)
       assertNull(it.ipAddress)
     }
   }

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

@@ -70,7 +70,7 @@ class DeviceInfoUtilTest {
     assertNotNull(enabledDevice.id)
     assertNotNull(enabledDevice.storageSize)
     assertNotNull(enabled.operatingSystem.isRooted)
-    assertNull(disabledDevice.id)
+    assertNotNull(disabledDevice.id)
     assertNull(disabledDevice.storageSize)
     assertNull(disabled.operatingSystem.isRooted)
   }
@@ -94,12 +94,12 @@ class DeviceInfoUtilTest {
   }
 
   @Test
-  fun `does not set device id when user info is disabled`() {
+  fun `sets device id when user info is disabled`() {
     val options = SentryAndroidOptions().apply { dataCollection.setUserInfo(false) }
     val deviceInfo =
       DeviceInfoUtil.getInstance(context, options).collectDeviceInformation(false, false)
 
-    assertNull(deviceInfo.id)
+    assertNotNull(deviceInfo.id)
   }
 
   @Test

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

@@ -40,7 +40,6 @@ import java.util.concurrent.atomic.AtomicReference
 import kotlin.test.BeforeTest
 import kotlin.test.Test
 import kotlin.test.assertEquals
-import kotlin.test.assertFalse
 import kotlin.test.assertNotEquals
 import kotlin.test.assertNotNull
 import kotlin.test.assertNull
@@ -355,14 +354,14 @@ class InternalSentrySdkTest {
   }
 
   @Test
-  fun `serializeScope does not provide fallback user id when user info is disabled`() {
+  fun `serializeScope provides fallback user id when user info is disabled`() {
     val options = SentryAndroidOptions().apply { dataCollection.setUserInfo(false) }
     val scope = Scope(options)
     scope.user = null
 
     val serializedScope = InternalSentrySdk.serializeScope(context, options, scope)
 
-    assertFalse((serializedScope["user"] as Map<*, *>).containsKey("id"))
+    assertTrue((serializedScope["user"] as Map<*, *>).containsKey("id"))
   }
 
   @Test

sentry/api/sentry.api

@@ -436,7 +436,6 @@ public final class io/sentry/DataCollectionResolver {
 	public fun isOutgoingResponseBody ()Z
 	public fun isOutgoingResponseBodyWithLegacyBodyGate ()Z
 	public fun isUserInfo ()Z
-	public fun isUserInfoWithLegacyAlways ()Z
 }
 
 public final class io/sentry/DateUtils {

sentry/src/main/java/io/sentry/DataCollectionResolver.java

@@ -23,10 +23,6 @@ public boolean isUserInfo() {
     return explicitOrSendDefaultPii(options.getDataCollection().getUserInfo(), true);
   }
 
-  public boolean isUserInfoWithLegacyAlways() {
-    return explicitOrDefault(options.getDataCollection().getUserInfo(), true, true);
-  }
-
   public boolean isDatabaseQueryData() {
     return explicitOrSendDefaultPii(options.getDataCollection().getDatabaseQueryData(), true);
   }