Skip to content

Commit

Permalink
Merge 6187196 into 2f079a1
Browse files Browse the repository at this point in the history
  • Loading branch information
adinauer committed Sep 27, 2022
2 parents 2f079a1 + 6187196 commit 889d1d9
Show file tree
Hide file tree
Showing 15 changed files with 143 additions and 42 deletions.
7 changes: 7 additions & 0 deletions 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
Expand Down
Expand Up @@ -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);
}
}
}
Expand Down
6 changes: 6 additions & 0 deletions sentry/api/sentry.api
Expand Up @@ -2970,17 +2970,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
}
Expand All @@ -2992,10 +2996,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
}
Expand Down
10 changes: 7 additions & 3 deletions sentry/src/main/java/io/sentry/Baggage.java
Expand Up @@ -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;
}
Expand Down
77 changes: 69 additions & 8 deletions sentry/src/main/java/io/sentry/protocol/User.java
Expand Up @@ -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;
Expand All @@ -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);
}

Expand Down Expand Up @@ -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.
*
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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()) {
Expand Down Expand Up @@ -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<>();
Expand Down
4 changes: 2 additions & 2 deletions sentry/src/test/java/io/sentry/ScopeTest.kt
Expand Up @@ -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

Expand Down
Expand Up @@ -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 {
Expand Down
34 changes: 19 additions & 15 deletions sentry/src/test/java/io/sentry/protocol/UserTest.kt
Expand Up @@ -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)
}
Expand All @@ -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"])
}

Expand All @@ -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("a@a.com", 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)
}
}
Expand All @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion sentry/src/test/resources/json/sentry_base_event.json
Expand Up @@ -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"
}
Expand Down
2 changes: 1 addition & 1 deletion sentry/src/test/resources/json/sentry_event.json
Expand Up @@ -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"
}
Expand Down
2 changes: 1 addition & 1 deletion sentry/src/test/resources/json/sentry_transaction.json
Expand Up @@ -202,7 +202,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"
}
Expand Down
Expand Up @@ -202,7 +202,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"
}
Expand Down
Expand Up @@ -194,7 +194,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"
}
Expand Down

0 comments on commit 889d1d9

Please sign in to comment.