Make user segment a top level property (#2257)

Author: adinauer

CHANGELOG.md

@@ -1,5 +1,12 @@
 # Changelog
 
+## Unreleased
+
+### Features
+
+- Make user segment a top level property ([#2257](https://github.com/getsentry/sentry-java/pull/2257))
+- Replace user `other` with `data` ([#2258](https://github.com/getsentry/sentry-java/pull/2258))
+
 ## 6.5.0-beta.1
 
 ### Features

sentry-spring/src/main/java/io/sentry/spring/SentryUserFilter.java

@@ -62,15 +62,15 @@ private void apply(final @NotNull User existingUser, final @Nullable User userFr
       Optional.ofNullable(userFromProvider.getId()).ifPresent(existingUser::setId);
       Optional.ofNullable(userFromProvider.getIpAddress()).ifPresent(existingUser::setIpAddress);
       Optional.ofNullable(userFromProvider.getUsername()).ifPresent(existingUser::setUsername);
-      if (userFromProvider.getOthers() != null && !userFromProvider.getOthers().isEmpty()) {
-        Map<String, String> existingUserOthers = existingUser.getOthers();
-        if (existingUserOthers == null) {
-          existingUserOthers = new ConcurrentHashMap<>();
+      if (userFromProvider.getData() != null && !userFromProvider.getData().isEmpty()) {
+        Map<String, String> existingUserData = existingUser.getData();
+        if (existingUserData == null) {
+          existingUserData = new ConcurrentHashMap<>();
         }
-        for (final Map.Entry<String, String> entry : userFromProvider.getOthers().entrySet()) {
-          existingUserOthers.put(entry.getKey(), entry.getValue());
+        for (final Map.Entry<String, String> entry : userFromProvider.getData().entrySet()) {
+          existingUserData.put(entry.getKey(), entry.getValue());
         }
-        existingUser.setOthers(existingUserOthers);
+        existingUser.setData(existingUserData);
       }
     }
   }

sentry/api/sentry.api

@@ -3017,17 +3017,21 @@ public final class io/sentry/protocol/TransactionNameSource : java/lang/Enum {
 public final class io/sentry/protocol/User : io/sentry/JsonSerializable, io/sentry/JsonUnknown {
 	public fun <init> ()V
 	public fun <init> (Lio/sentry/protocol/User;)V
+	public fun getData ()Ljava/util/Map;
 	public fun getEmail ()Ljava/lang/String;
 	public fun getId ()Ljava/lang/String;
 	public fun getIpAddress ()Ljava/lang/String;
 	public fun getOthers ()Ljava/util/Map;
+	public fun getSegment ()Ljava/lang/String;
 	public fun getUnknown ()Ljava/util/Map;
 	public fun getUsername ()Ljava/lang/String;
 	public fun serialize (Lio/sentry/JsonObjectWriter;Lio/sentry/ILogger;)V
+	public fun setData (Ljava/util/Map;)V
 	public fun setEmail (Ljava/lang/String;)V
 	public fun setId (Ljava/lang/String;)V
 	public fun setIpAddress (Ljava/lang/String;)V
 	public fun setOthers (Ljava/util/Map;)V
+	public fun setSegment (Ljava/lang/String;)V
 	public fun setUnknown (Ljava/util/Map;)V
 	public fun setUsername (Ljava/lang/String;)V
 }
@@ -3039,10 +3043,12 @@ public final class io/sentry/protocol/User$Deserializer : io/sentry/JsonDeserial
 }
 
 public final class io/sentry/protocol/User$JsonKeys {
+	public static final field DATA Ljava/lang/String;
 	public static final field EMAIL Ljava/lang/String;
 	public static final field ID Ljava/lang/String;
 	public static final field IP_ADDRESS Ljava/lang/String;
 	public static final field OTHER Ljava/lang/String;
+	public static final field SEGMENT Ljava/lang/String;
 	public static final field USERNAME Ljava/lang/String;
 	public fun <init> ()V
 }

sentry/src/main/java/io/sentry/Baggage.java

@@ -292,9 +292,13 @@ public void setValuesFromTransaction(
   }
 
   private static @Nullable String getSegment(final @NotNull User user) {
-    final Map<String, String> others = user.getOthers();
-    if (others != null) {
-      return others.get("segment");
+    if (user.getSegment() != null) {
+      return user.getSegment();
+    }
+
+    final Map<String, String> userData = user.getData();
+    if (userData != null) {
+      return userData.get("segment");
     } else {
       return null;
     }

sentry/src/main/java/io/sentry/protocol/User.java

@@ -31,14 +31,16 @@ public final class User implements JsonUnknown, JsonSerializable {
   /** Username of the user. */
   private @Nullable String username;
 
+  private @Nullable String segment;
+
   /** Remote IP address of the user. */
   private @Nullable String ipAddress;
 
   /**
    * Additional arbitrary fields, as stored in the database (and sometimes as sent by clients). All
    * data from `self.other` should end up here after store normalization.
    */
-  private @Nullable Map<String, @NotNull String> other;
+  private @Nullable Map<String, @NotNull String> data;
 
   /** unknown fields, only internal usage. */
   private @Nullable Map<String, @NotNull Object> unknown;
@@ -50,7 +52,8 @@ public User(final @NotNull User user) {
     this.username = user.username;
     this.id = user.id;
     this.ipAddress = user.ipAddress;
-    this.other = CollectionUtils.newConcurrentHashMap(user.other);
+    this.segment = user.segment;
+    this.data = CollectionUtils.newConcurrentHashMap(user.data);
     this.unknown = CollectionUtils.newConcurrentHashMap(user.unknown);
   }
 
@@ -108,6 +111,24 @@ public void setUsername(final @Nullable String username) {
     this.username = username;
   }
 
+  /**
+   * Gets the segment of the user.
+   *
+   * @return the user segment.
+   */
+  public @Nullable String getSegment() {
+    return segment;
+  }
+
+  /**
+   * Sets the segment of the user.
+   *
+   * @param segment the segment.
+   */
+  public void setSegment(final @Nullable String segment) {
+    this.segment = segment;
+  }
+
   /**
    * Gets the IP address of the user.
    *
@@ -129,19 +150,43 @@ public void setIpAddress(final @Nullable String ipAddress) {
   /**
    * Gets other user related data.
    *
+   * @deprecated use {{@link User#getData()}} instead
    * @return the other user data.
    */
+  @Deprecated
+  @SuppressWarnings("InlineMeSuggester")
   public @Nullable Map<String, @NotNull String> getOthers() {
-    return other;
+    return getData();
   }
 
   /**
    * Sets other user related data.
    *
+   * @deprecated use {{@link User#setData(Map)}} instead
    * @param other the other user related data..
    */
+  @Deprecated
+  @SuppressWarnings("InlineMeSuggester")
   public void setOthers(final @Nullable Map<String, @NotNull String> other) {
-    this.other = CollectionUtils.newConcurrentHashMap(other);
+    this.setData(other);
+  }
+
+  /**
+   * Gets additional arbitrary fields of the user.
+   *
+   * @return the other user data.
+   */
+  public @Nullable Map<String, @NotNull String> getData() {
+    return data;
+  }
+
+  /**
+   * Sets additional arbitrary fields of the user.
+   *
+   * @param data the other user related data..
+   */
+  public void setData(final @Nullable Map<String, @NotNull String> data) {
+    this.data = CollectionUtils.newConcurrentHashMap(data);
   }
 
   // region json
@@ -161,8 +206,10 @@ public static final class JsonKeys {
     public static final String EMAIL = "email";
     public static final String ID = "id";
     public static final String USERNAME = "username";
+    public static final String SEGMENT = "segment";
     public static final String IP_ADDRESS = "ip_address";
     public static final String OTHER = "other";
+    public static final String DATA = "data";
   }
 
   @Override
@@ -178,11 +225,14 @@ public void serialize(@NotNull JsonObjectWriter writer, @NotNull ILogger logger)
     if (username != null) {
       writer.name(JsonKeys.USERNAME).value(username);
     }
+    if (segment != null) {
+      writer.name(JsonKeys.SEGMENT).value(segment);
+    }
     if (ipAddress != null) {
       writer.name(JsonKeys.IP_ADDRESS).value(ipAddress);
     }
-    if (other != null) {
-      writer.name(JsonKeys.OTHER).value(logger, other);
+    if (data != null) {
+      writer.name(JsonKeys.DATA).value(logger, data);
     }
     if (unknown != null) {
       for (String key : unknown.keySet()) {
@@ -214,14 +264,25 @@ public static final class Deserializer implements JsonDeserializer<User> {
           case JsonKeys.USERNAME:
             user.username = reader.nextStringOrNull();
             break;
+          case JsonKeys.SEGMENT:
+            user.segment = reader.nextStringOrNull();
+            break;
           case JsonKeys.IP_ADDRESS:
             user.ipAddress = reader.nextStringOrNull();
             break;
-          case JsonKeys.OTHER:
-            user.other =
+          case JsonKeys.DATA:
+            user.data =
                 CollectionUtils.newConcurrentHashMap(
                     (Map<String, String>) reader.nextObjectOrNull());
             break;
+          case JsonKeys.OTHER:
+            // restore `other` from legacy JSON
+            if (user.data == null || user.data.isEmpty()) {
+              user.data =
+                  CollectionUtils.newConcurrentHashMap(
+                      (Map<String, String>) reader.nextObjectOrNull());
+            }
+            break;
           default:
             if (unknown == null) {
               unknown = new ConcurrentHashMap<>();

sentry/src/test/java/io/sentry/ScopeTest.kt

@@ -30,8 +30,8 @@ class ScopeTest {
         user.id = "123"
         user.ipAddress = "123.x"
         user.username = "userName"
-        val others = mutableMapOf(Pair("others", "others"))
-        user.others = others
+        val data = mutableMapOf(Pair("data", "data"))
+        user.data = data
 
         scope.user = user
 

sentry/src/test/java/io/sentry/protocol/UserSerializationTest.kt

@@ -43,6 +43,15 @@ class UserSerializationTest {
         assertEquals(expectedJson, actualJson)
     }
 
+    @Test
+    fun `deserialize legacy`() {
+        val inputJson = sanitizedFile("json/user_legacy.json")
+        val expectedJson = sanitizedFile("json/user.json")
+        val actual = deserialize(inputJson)
+        val actualJson = serialize(actual)
+        assertEquals(expectedJson, actualJson)
+    }
+
     // Helper
 
     private fun sanitizedFile(path: String): String {

sentry/src/test/java/io/sentry/protocol/UserTest.kt

@@ -17,7 +17,7 @@ class UserTest {
         assertNotNull(clone)
         assertNotSame(user, clone)
 
-        assertNotSame(user.others, clone.others)
+        assertNotSame(user.data, clone.data)
 
         assertNotSame(user.unknown, clone.unknown)
     }
@@ -31,7 +31,8 @@ class UserTest {
         assertEquals("123", clone.id)
         assertEquals("123.x", clone.ipAddress)
         assertEquals("userName", clone.username)
-        assertEquals("others", clone.others!!["others"])
+        assertEquals("userSegment", clone.segment)
+        assertEquals("data", clone.data!!["data"])
         assertEquals("unknown", clone.unknown!!["unknown"])
     }
 
@@ -44,36 +45,38 @@ class UserTest {
         user.id = "456"
         user.ipAddress = "456.x"
         user.username = "newUserName"
-        user.others!!["others"] = "newOthers"
-        user.others!!["anotherOne"] = "anotherOne"
+        user.segment = "newUserSegment"
+        user.data!!["data"] = "newOthers"
+        user.data!!["anotherOne"] = "anotherOne"
         val newUnknown = mapOf(Pair("unknown", "newUnknown"), Pair("otherUnknown", "otherUnknown"))
         user.setUnknown(newUnknown)
 
         assertEquals("[email protected]", clone.email)
         assertEquals("123", clone.id)
         assertEquals("123.x", clone.ipAddress)
         assertEquals("userName", clone.username)
-        assertEquals("others", clone.others!!["others"])
-        assertEquals(1, clone.others!!.size)
+        assertEquals("userSegment", clone.segment)
+        assertEquals("data", clone.data!!["data"])
+        assertEquals(1, clone.data!!.size)
         assertEquals("unknown", clone.unknown!!["unknown"])
         assertEquals(1, clone.unknown!!.size)
     }
 
     @Test
-    fun `setting null others do not crash`() {
+    fun `setting null data do not crash`() {
         val user = createUser()
-        user.others = null
+        user.data = null
 
-        assertNull(user.others)
+        assertNull(user.data)
     }
 
     @Test
-    fun `when setOther receives immutable map as an argument, its still possible to add more others to the user`() {
+    fun `when setOther receives immutable map as an argument, its still possible to add more data to the user`() {
         val user = User().apply {
-            others = Collections.unmodifiableMap(mapOf("key1" to "value1"))
-            others!!["key2"] = "value2"
+            data = Collections.unmodifiableMap(mapOf("key1" to "value1"))
+            data!!["key2"] = "value2"
         }
-        assertNotNull(user.others) {
+        assertNotNull(user.data) {
             assertEquals(mapOf("key1" to "value1", "key2" to "value2"), it)
         }
     }
@@ -84,8 +87,9 @@ class UserTest {
             id = "123"
             ipAddress = "123.x"
             username = "userName"
-            val others = mutableMapOf(Pair("others", "others"))
-            setOthers(others)
+            segment = "userSegment"
+            val data = mutableMapOf(Pair("data", "data"))
+            setData(data)
             val unknown = mapOf(Pair("unknown", "unknown"))
             setUnknown(unknown)
         }

sentry/src/test/resources/json/sentry_base_event.json

@@ -159,7 +159,7 @@
     "id": "efb2084b-1871-4b59-8897-b4bd9f196a01",
     "username": "60c05dff-7140-4d94-9a61-c9cdd9ca9b96",
     "ip_address": "51d22b77-f663-4dbe-8103-8b749d1d9a48",
-    "other":
+    "data":
     {
       "dc2813d0-0f66-4a3f-a995-71268f61a8fa": "991659ad-7c59-4dd3-bb89-0bd5c74014bd"
     }

sentry/src/test/resources/json/sentry_event.json

@@ -306,7 +306,7 @@
         "id": "efb2084b-1871-4b59-8897-b4bd9f196a01",
         "username": "60c05dff-7140-4d94-9a61-c9cdd9ca9b96",
         "ip_address": "51d22b77-f663-4dbe-8103-8b749d1d9a48",
-        "other":
+        "data":
         {
             "dc2813d0-0f66-4a3f-a995-71268f61a8fa": "991659ad-7c59-4dd3-bb89-0bd5c74014bd"
         }