From f81b165cea9623574263685b505029950f632693 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Tue, 18 Oct 2022 11:38:57 +0200 Subject: [PATCH 1/7] No longer serialize static fields; use toString if no fields were serialized --- .../JsonReflectionObjectSerializer.java | 8 +++ .../io/sentry/JsonObjectSerializerTest.kt | 58 +++++++++++++++++++ .../JsonReflectionObjectSerializerTest.kt | 20 ++++++- 3 files changed, 83 insertions(+), 3 deletions(-) diff --git a/sentry/src/main/java/io/sentry/JsonReflectionObjectSerializer.java b/sentry/src/main/java/io/sentry/JsonReflectionObjectSerializer.java index 30df2da185..6126d69385 100644 --- a/sentry/src/main/java/io/sentry/JsonReflectionObjectSerializer.java +++ b/sentry/src/main/java/io/sentry/JsonReflectionObjectSerializer.java @@ -84,6 +84,9 @@ public final class JsonReflectionObjectSerializer { if (Modifier.isTransient(field.getModifiers())) { continue; } + if (Modifier.isStatic(field.getModifiers())) { + continue; + } String fieldName = field.getName(); try { field.setAccessible(true); @@ -94,6 +97,11 @@ public final class JsonReflectionObjectSerializer { logger.log(SentryLevel.INFO, "Cannot access field " + fieldName + "."); } } + + if (map.isEmpty()) { + map.put("toString", object.toString()); + } + return map; } diff --git a/sentry/src/test/java/io/sentry/JsonObjectSerializerTest.kt b/sentry/src/test/java/io/sentry/JsonObjectSerializerTest.kt index 76acb45e90..af0bd999b5 100644 --- a/sentry/src/test/java/io/sentry/JsonObjectSerializerTest.kt +++ b/sentry/src/test/java/io/sentry/JsonObjectSerializerTest.kt @@ -1,8 +1,11 @@ package io.sentry +import com.nhaarman.mockitokotlin2.inOrder import com.nhaarman.mockitokotlin2.mock +import com.nhaarman.mockitokotlin2.times import com.nhaarman.mockitokotlin2.verify import org.junit.Test +import java.util.Locale import java.util.TimeZone internal class JsonObjectSerializerTest { @@ -166,6 +169,60 @@ internal class JsonObjectSerializerTest { verify(fixture.writer).endObject() } + @Test + fun `serialize locale`() { + val inOrder = inOrder(fixture.writer) + fixture.getSUT().serialize(fixture.writer, fixture.logger, Locale.US) + + inOrder.verify(fixture.writer).beginObject() + inOrder.verify(fixture.writer).name("toString") + inOrder.verify(fixture.writer).value("en_US") + inOrder.verify(fixture.writer).endObject() + } + + @Test + fun `serialize locale in map`() { + val map = mapOf("one" to Locale.US) + val inOrder = inOrder(fixture.writer) + fixture.getSUT().serialize(fixture.writer, fixture.logger, map) + inOrder.verify(fixture.writer).beginObject() + inOrder.verify(fixture.writer).name("one") + inOrder.verify(fixture.writer).beginObject() + inOrder.verify(fixture.writer).name("toString") + inOrder.verify(fixture.writer).value("en_US") + inOrder.verify(fixture.writer, times(2)).endObject() + } + + @Test + fun `serialize locale in list`() { + val list = listOf(Locale.US, Locale.GERMAN) + val inOrder = inOrder(fixture.writer) + fixture.getSUT().serialize(fixture.writer, fixture.logger, list) + inOrder.verify(fixture.writer).beginArray() + inOrder.verify(fixture.writer).beginObject() + inOrder.verify(fixture.writer).name("toString") + inOrder.verify(fixture.writer).value("en_US") + inOrder.verify(fixture.writer).endObject() + inOrder.verify(fixture.writer).beginObject() + inOrder.verify(fixture.writer).name("toString") + inOrder.verify(fixture.writer).value("de") + inOrder.verify(fixture.writer).endObject() + inOrder.verify(fixture.writer).endArray() + } + + @Test + fun `serialize locale in object`() { + val obj = ClassWithLocaleProperty(Locale.US) + val inOrder = inOrder(fixture.writer) + fixture.getSUT().serialize(fixture.writer, fixture.logger, obj) + inOrder.verify(fixture.writer).beginObject() + inOrder.verify(fixture.writer).name("localeProperty") + inOrder.verify(fixture.writer).beginObject() + inOrder.verify(fixture.writer).name("toString") + inOrder.verify(fixture.writer).value("en_US") + inOrder.verify(fixture.writer, times(2)).endObject() + } + class UnknownClassWithData( private val integer: Int, private val string: String @@ -173,3 +230,4 @@ internal class JsonObjectSerializerTest { } data class ClassWithEnumProperty(val enumProperty: DataCategory) +data class ClassWithLocaleProperty(val localeProperty: Locale) diff --git a/sentry/src/test/java/io/sentry/JsonReflectionObjectSerializerTest.kt b/sentry/src/test/java/io/sentry/JsonReflectionObjectSerializerTest.kt index 5c2937410b..dfa65edbd1 100644 --- a/sentry/src/test/java/io/sentry/JsonReflectionObjectSerializerTest.kt +++ b/sentry/src/test/java/io/sentry/JsonReflectionObjectSerializerTest.kt @@ -3,7 +3,10 @@ package io.sentry import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.verify import org.junit.Test +import java.util.Locale import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertTrue class JsonReflectionObjectSerializerTest { @@ -87,11 +90,16 @@ class JsonReflectionObjectSerializerTest { } @Test - fun `serialize object without fields`() { + fun `serialize object without fields using toString`() { val objectWithoutFields = ClassWithoutFields() - val expected = mapOf() + val expected = mapOf( + "toString" to "" + ) val actual = fixture.getSut().serialize(objectWithoutFields, fixture.logger) - assertEquals(expected, actual) + assertNotNull(actual) + assertTrue(actual is Map<*, *>) + assertEquals(1, actual.size) + assertEquals(objectWithoutFields.toString(), actual["toString"]) } @Test @@ -263,6 +271,12 @@ class JsonReflectionObjectSerializerTest { assertEquals("Error", actual) } + @Test + fun `locale`() { + val actual = fixture.getSut().serialize(Locale.US, fixture.logger) + assertEquals(mapOf("toString" to "en_US"), actual) + } + // Helper class ClassWithPrimitiveFields( From cabfe8f23a3e3340f165030e97f1d83e7a80086a Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Tue, 18 Oct 2022 11:45:11 +0200 Subject: [PATCH 2/7] Add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 896e619709..9957e58fbc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - Ensure potential callback exceptions are caught #2123 ([#2291](https://github.com/getsentry/sentry-java/pull/2291)) - Remove verbose FrameMetricsAggregator failure logging ([#2293](https://github.com/getsentry/sentry-java/pull/2293)) - Ignore broken regex for tracePropagationTarget ([#2288](https://github.com/getsentry/sentry-java/pull/2288)) +- No longer serialize static fields; use toString as fallback ([#2309](https://github.com/getsentry/sentry-java/pull/2309)) ### Features From 32d057495e4c39625e9fc541cc19714b8a13af22 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Wed, 19 Oct 2022 08:54:52 +0200 Subject: [PATCH 3/7] Use toString as string not map; Also add other types --- .../JsonReflectionObjectSerializer.java | 57 +++++++++++-- .../io/sentry/JsonObjectSerializerTest.kt | 23 ++---- .../JsonReflectionObjectSerializerTest.kt | 81 +++++++++++++++++-- .../vendor/gson/stream/JsonWriterTest.java | 8 ++ 4 files changed, 139 insertions(+), 30 deletions(-) diff --git a/sentry/src/main/java/io/sentry/JsonReflectionObjectSerializer.java b/sentry/src/main/java/io/sentry/JsonReflectionObjectSerializer.java index 6126d69385..b740126e79 100644 --- a/sentry/src/main/java/io/sentry/JsonReflectionObjectSerializer.java +++ b/sentry/src/main/java/io/sentry/JsonReflectionObjectSerializer.java @@ -2,13 +2,21 @@ import java.lang.reflect.Field; import java.lang.reflect.Modifier; +import java.net.InetAddress; +import java.net.URI; import java.util.ArrayList; +import java.util.Calendar; import java.util.Collection; +import java.util.Currency; import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Set; +import java.util.UUID; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicIntegerArray; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -39,6 +47,22 @@ public final class JsonReflectionObjectSerializer { return object; } else if (object instanceof String) { return object; + } else if (object instanceof Locale) { + return object.toString(); // TODO + } else if (object instanceof AtomicIntegerArray) { + return atomicIntegerArrayToList((AtomicIntegerArray) object); + } else if (object instanceof AtomicBoolean) { + return ((AtomicBoolean) object).get(); + } else if (object instanceof URI) { + return object.toString(); + } else if (object instanceof InetAddress) { + return object.toString(); + } else if (object instanceof UUID) { + return object.toString(); + } else if (object instanceof Currency) { + return object.toString(); + } else if (object instanceof Calendar) { + return calendarToMap((Calendar) object); } else if (object.getClass().isEnum()) { return object.toString(); } else { @@ -63,7 +87,12 @@ public final class JsonReflectionObjectSerializer { } else if (object instanceof Map) { serializedObject = map((Map) object, logger); } else { - serializedObject = serializeObject(object, logger); + @NotNull Map objectAsMap = serializeObject(object, logger); + if (objectAsMap.isEmpty()) { + serializedObject = object.toString(); + } else { + serializedObject = objectAsMap; + } } } catch (Exception exception) { logger.log(SentryLevel.INFO, "Not serializing object due to throwing sub-path.", exception); @@ -98,10 +127,6 @@ public final class JsonReflectionObjectSerializer { } } - if (map.isEmpty()) { - map.put("toString", object.toString()); - } - return map; } @@ -116,6 +141,15 @@ public final class JsonReflectionObjectSerializer { return list; } + private @NotNull List atomicIntegerArrayToList(@NotNull AtomicIntegerArray array) + throws Exception { + List list = new ArrayList<>(); + for (int i = 0; i < array.length(); i++) { + list.add(array.get(i)); + } + return list; + } + private @NotNull List list(@NotNull Collection collection, @NotNull ILogger logger) throws Exception { List list = new ArrayList<>(); @@ -139,4 +173,17 @@ public final class JsonReflectionObjectSerializer { } return hashMap; } + + private @NotNull Map calendarToMap(@NotNull Calendar calendar) { + Map map = new HashMap<>(); + + map.put("year", (long) calendar.get(1)); + map.put("month", (long) calendar.get(2)); + map.put("dayOfMonth", (long) calendar.get(5)); + map.put("hourOfDay", (long) calendar.get(11)); + map.put("minute", (long) calendar.get(12)); + map.put("second", (long) calendar.get(13)); + + return map; + } } diff --git a/sentry/src/test/java/io/sentry/JsonObjectSerializerTest.kt b/sentry/src/test/java/io/sentry/JsonObjectSerializerTest.kt index af0bd999b5..346d4a06cf 100644 --- a/sentry/src/test/java/io/sentry/JsonObjectSerializerTest.kt +++ b/sentry/src/test/java/io/sentry/JsonObjectSerializerTest.kt @@ -117,9 +117,9 @@ internal class JsonObjectSerializerTest { @Test fun `serialize unknown object without data`() { - fixture.getSUT().serialize(fixture.writer, fixture.logger, object {}) - verify(fixture.writer).beginObject() - verify(fixture.writer).endObject() + val value = object {} + fixture.getSUT().serialize(fixture.writer, fixture.logger, value) + verify(fixture.writer).value(value.toString()) } @Test @@ -174,10 +174,7 @@ internal class JsonObjectSerializerTest { val inOrder = inOrder(fixture.writer) fixture.getSUT().serialize(fixture.writer, fixture.logger, Locale.US) - inOrder.verify(fixture.writer).beginObject() - inOrder.verify(fixture.writer).name("toString") inOrder.verify(fixture.writer).value("en_US") - inOrder.verify(fixture.writer).endObject() } @Test @@ -187,10 +184,8 @@ internal class JsonObjectSerializerTest { fixture.getSUT().serialize(fixture.writer, fixture.logger, map) inOrder.verify(fixture.writer).beginObject() inOrder.verify(fixture.writer).name("one") - inOrder.verify(fixture.writer).beginObject() - inOrder.verify(fixture.writer).name("toString") inOrder.verify(fixture.writer).value("en_US") - inOrder.verify(fixture.writer, times(2)).endObject() + inOrder.verify(fixture.writer).endObject() } @Test @@ -199,14 +194,8 @@ internal class JsonObjectSerializerTest { val inOrder = inOrder(fixture.writer) fixture.getSUT().serialize(fixture.writer, fixture.logger, list) inOrder.verify(fixture.writer).beginArray() - inOrder.verify(fixture.writer).beginObject() - inOrder.verify(fixture.writer).name("toString") inOrder.verify(fixture.writer).value("en_US") - inOrder.verify(fixture.writer).endObject() - inOrder.verify(fixture.writer).beginObject() - inOrder.verify(fixture.writer).name("toString") inOrder.verify(fixture.writer).value("de") - inOrder.verify(fixture.writer).endObject() inOrder.verify(fixture.writer).endArray() } @@ -217,10 +206,8 @@ internal class JsonObjectSerializerTest { fixture.getSUT().serialize(fixture.writer, fixture.logger, obj) inOrder.verify(fixture.writer).beginObject() inOrder.verify(fixture.writer).name("localeProperty") - inOrder.verify(fixture.writer).beginObject() - inOrder.verify(fixture.writer).name("toString") inOrder.verify(fixture.writer).value("en_US") - inOrder.verify(fixture.writer, times(2)).endObject() + inOrder.verify(fixture.writer).endObject() } class UnknownClassWithData( diff --git a/sentry/src/test/java/io/sentry/JsonReflectionObjectSerializerTest.kt b/sentry/src/test/java/io/sentry/JsonReflectionObjectSerializerTest.kt index dfa65edbd1..a3b9ef008d 100644 --- a/sentry/src/test/java/io/sentry/JsonReflectionObjectSerializerTest.kt +++ b/sentry/src/test/java/io/sentry/JsonReflectionObjectSerializerTest.kt @@ -3,10 +3,14 @@ package io.sentry import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.verify import org.junit.Test +import java.net.URI +import java.util.Calendar +import java.util.Currency import java.util.Locale +import java.util.UUID +import java.util.concurrent.atomic.AtomicBoolean +import java.util.concurrent.atomic.AtomicIntegerArray import kotlin.test.assertEquals -import kotlin.test.assertNotNull -import kotlin.test.assertTrue class JsonReflectionObjectSerializerTest { @@ -96,10 +100,7 @@ class JsonReflectionObjectSerializerTest { "toString" to "" ) val actual = fixture.getSut().serialize(objectWithoutFields, fixture.logger) - assertNotNull(actual) - assertTrue(actual is Map<*, *>) - assertEquals(1, actual.size) - assertEquals(objectWithoutFields.toString(), actual["toString"]) + assertEquals(objectWithoutFields.toString(), actual) } @Test @@ -274,7 +275,73 @@ class JsonReflectionObjectSerializerTest { @Test fun `locale`() { val actual = fixture.getSut().serialize(Locale.US, fixture.logger) - assertEquals(mapOf("toString" to "en_US"), actual) + assertEquals("en_US", actual) + } + + @Test + fun `AtomicIntegerArray is serialized`() { + val actual = fixture.getSut().serialize(AtomicIntegerArray(arrayOf(1, 2, 3).toIntArray()), fixture.logger) + assertEquals(listOf(1, 2, 3), actual) + } + + @Test + fun `AtomicBoolean is serialized`() { + val actual = fixture.getSut().serialize(AtomicBoolean(true), fixture.logger) + assertEquals(true, actual) + } + + @Test + fun `StringBuffer is serialized`() { + val sb = StringBuffer() + sb.append("hello") + sb.append(" ") + sb.append("world") + val actual = fixture.getSut().serialize(sb, fixture.logger) + assertEquals("hello world", actual) + } + + @Test + fun `StringBuilder is serialized`() { + val sb = StringBuilder() + sb.append("hello") + sb.append(" ") + sb.append("world") + val actual = fixture.getSut().serialize(sb, fixture.logger) + assertEquals("hello world", actual) + } + + @Test + fun `URI is serialized`() { + val actual = fixture.getSut().serialize(URI("http://localhost:8081/api/product?id=99"), fixture.logger) + assertEquals("http://localhost:8081/api/product?id=99", actual) + } + + @Test + fun `UUID is serialized`() { + val actual = fixture.getSut().serialize(UUID.fromString("828900a5-15dc-413f-8c17-6ef04d74e074"), fixture.logger) + assertEquals("828900a5-15dc-413f-8c17-6ef04d74e074", actual) + } + + @Test + fun `Currency is serialized`() { + val actual = fixture.getSut().serialize(Currency.getInstance("USD"), fixture.logger) + assertEquals("USD", actual) + } + + @Test + fun `Calendar is serialized`() { + val calendar = Calendar.getInstance() + calendar.set(2022, 0, 1, 11, 59, 58) + val actual = fixture.getSut().serialize(calendar, fixture.logger) + val expected = mapOf( + "month" to 0L, + "year" to 2022L, + "dayOfMonth" to 1L, + "hourOfDay" to 11L, + "minute" to 59L, + "second" to 58L + ) + assertEquals(expected, actual) } // Helper diff --git a/sentry/src/test/java/io/sentry/vendor/gson/stream/JsonWriterTest.java b/sentry/src/test/java/io/sentry/vendor/gson/stream/JsonWriterTest.java index affb9312fa..6249fb1867 100644 --- a/sentry/src/test/java/io/sentry/vendor/gson/stream/JsonWriterTest.java +++ b/sentry/src/test/java/io/sentry/vendor/gson/stream/JsonWriterTest.java @@ -27,6 +27,7 @@ import java.io.StringWriter; import java.math.BigDecimal; import java.math.BigInteger; +import java.util.concurrent.atomic.AtomicInteger; @SuppressWarnings({"resource", "NullAway"}) // Ignore warnings to preserve original code. public final class JsonWriterTest extends TestCase { @@ -313,6 +314,13 @@ public void testBooleans() throws IOException { assertEquals("[true,false]", stringWriter.toString()); } + public void testAtomicInteger() throws IOException { + StringWriter stringWriter = new StringWriter(); + JsonWriter jsonWriter = new JsonWriter(stringWriter); + jsonWriter.value(new AtomicInteger(42)); + assertEquals("42", stringWriter.toString()); + } + public void testBoxedBooleans() throws IOException { StringWriter stringWriter = new StringWriter(); JsonWriter jsonWriter = new JsonWriter(stringWriter); From 8988eacc5120b7882d7145f803efc210713ad3cd Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Wed, 19 Oct 2022 08:59:39 +0200 Subject: [PATCH 4/7] Remove todo --- .../src/main/java/io/sentry/JsonReflectionObjectSerializer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry/src/main/java/io/sentry/JsonReflectionObjectSerializer.java b/sentry/src/main/java/io/sentry/JsonReflectionObjectSerializer.java index b740126e79..f909846294 100644 --- a/sentry/src/main/java/io/sentry/JsonReflectionObjectSerializer.java +++ b/sentry/src/main/java/io/sentry/JsonReflectionObjectSerializer.java @@ -48,7 +48,7 @@ public final class JsonReflectionObjectSerializer { } else if (object instanceof String) { return object; } else if (object instanceof Locale) { - return object.toString(); // TODO + return object.toString(); } else if (object instanceof AtomicIntegerArray) { return atomicIntegerArrayToList((AtomicIntegerArray) object); } else if (object instanceof AtomicBoolean) { From 524a3e5d965ba02d7d85f6f7a0a586a08ef891e3 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Wed, 19 Oct 2022 17:45:52 +0200 Subject: [PATCH 5/7] Address code review --- sentry/api/sentry.api | 6 ++ .../java/io/sentry/JsonObjectSerializer.java | 27 ++++++++ .../JsonReflectionObjectSerializer.java | 25 +------- .../io/sentry/JsonSerializationUtils.java | 35 +++++++++++ .../io/sentry/JsonObjectSerializerTest.kt | 61 +++++++++++++++++++ 5 files changed, 132 insertions(+), 22 deletions(-) create mode 100644 sentry/src/main/java/io/sentry/JsonSerializationUtils.java diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index c3c8acccef..b321a83ce2 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -581,6 +581,12 @@ public abstract interface class io/sentry/JsonSerializable { public abstract fun serialize (Lio/sentry/JsonObjectWriter;Lio/sentry/ILogger;)V } +public final class io/sentry/JsonSerializationUtils { + public fun ()V + public static fun atomicIntegerArrayToList (Ljava/util/concurrent/atomic/AtomicIntegerArray;)Ljava/util/List; + public static fun calendarToMap (Ljava/util/Calendar;)Ljava/util/Map; +} + public final class io/sentry/JsonSerializer : io/sentry/ISerializer { public fun (Lio/sentry/SentryOptions;)V public fun deserialize (Ljava/io/Reader;Ljava/lang/Class;)Ljava/lang/Object; diff --git a/sentry/src/main/java/io/sentry/JsonObjectSerializer.java b/sentry/src/main/java/io/sentry/JsonObjectSerializer.java index bc940a2655..cf2e5f5de6 100644 --- a/sentry/src/main/java/io/sentry/JsonObjectSerializer.java +++ b/sentry/src/main/java/io/sentry/JsonObjectSerializer.java @@ -1,11 +1,22 @@ package io.sentry; +import static io.sentry.JsonSerializationUtils.atomicIntegerArrayToList; +import static io.sentry.JsonSerializationUtils.calendarToMap; + import java.io.IOException; +import java.net.InetAddress; +import java.net.URI; import java.util.Arrays; +import java.util.Calendar; import java.util.Collection; +import java.util.Currency; import java.util.Date; +import java.util.Locale; import java.util.Map; import java.util.TimeZone; +import java.util.UUID; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicIntegerArray; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -46,6 +57,22 @@ public void serialize( serializeCollection(writer, logger, Arrays.asList((Object[]) object)); } else if (object instanceof Map) { serializeMap(writer, logger, (Map) object); + } else if (object instanceof Locale) { + writer.value(object.toString()); + } else if (object instanceof AtomicIntegerArray) { + serializeCollection(writer, logger, atomicIntegerArrayToList((AtomicIntegerArray) object)); + } else if (object instanceof AtomicBoolean) { + writer.value(((AtomicBoolean) object).get()); + } else if (object instanceof URI) { + writer.value(object.toString()); + } else if (object instanceof InetAddress) { + writer.value(object.toString()); + } else if (object instanceof UUID) { + writer.value(object.toString()); + } else if (object instanceof Currency) { + writer.value(object.toString()); + } else if (object instanceof Calendar) { + serializeMap(writer, logger, calendarToMap((Calendar) object)); } else if (object.getClass().isEnum()) { writer.value(object.toString()); } else { diff --git a/sentry/src/main/java/io/sentry/JsonReflectionObjectSerializer.java b/sentry/src/main/java/io/sentry/JsonReflectionObjectSerializer.java index f909846294..b63ee8bb4e 100644 --- a/sentry/src/main/java/io/sentry/JsonReflectionObjectSerializer.java +++ b/sentry/src/main/java/io/sentry/JsonReflectionObjectSerializer.java @@ -1,5 +1,8 @@ package io.sentry; +import static io.sentry.JsonSerializationUtils.atomicIntegerArrayToList; +import static io.sentry.JsonSerializationUtils.calendarToMap; + import java.lang.reflect.Field; import java.lang.reflect.Modifier; import java.net.InetAddress; @@ -141,15 +144,6 @@ public final class JsonReflectionObjectSerializer { return list; } - private @NotNull List atomicIntegerArrayToList(@NotNull AtomicIntegerArray array) - throws Exception { - List list = new ArrayList<>(); - for (int i = 0; i < array.length(); i++) { - list.add(array.get(i)); - } - return list; - } - private @NotNull List list(@NotNull Collection collection, @NotNull ILogger logger) throws Exception { List list = new ArrayList<>(); @@ -173,17 +167,4 @@ public final class JsonReflectionObjectSerializer { } return hashMap; } - - private @NotNull Map calendarToMap(@NotNull Calendar calendar) { - Map map = new HashMap<>(); - - map.put("year", (long) calendar.get(1)); - map.put("month", (long) calendar.get(2)); - map.put("dayOfMonth", (long) calendar.get(5)); - map.put("hourOfDay", (long) calendar.get(11)); - map.put("minute", (long) calendar.get(12)); - map.put("second", (long) calendar.get(13)); - - return map; - } } diff --git a/sentry/src/main/java/io/sentry/JsonSerializationUtils.java b/sentry/src/main/java/io/sentry/JsonSerializationUtils.java new file mode 100644 index 0000000000..ba4c5f5a05 --- /dev/null +++ b/sentry/src/main/java/io/sentry/JsonSerializationUtils.java @@ -0,0 +1,35 @@ +package io.sentry; + +import java.util.ArrayList; +import java.util.Calendar; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.AtomicIntegerArray; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; + +@ApiStatus.Internal +public final class JsonSerializationUtils { + + public static @NotNull Map calendarToMap(@NotNull Calendar calendar) { + Map map = new HashMap<>(); + + map.put("year", (long) calendar.get(Calendar.YEAR)); + map.put("month", (long) calendar.get(Calendar.MONTH)); + map.put("dayOfMonth", (long) calendar.get(Calendar.DAY_OF_MONTH)); + map.put("hourOfDay", (long) calendar.get(Calendar.HOUR_OF_DAY)); + map.put("minute", (long) calendar.get(Calendar.MINUTE)); + map.put("second", (long) calendar.get(Calendar.SECOND)); + + return map; + } + + public static @NotNull List atomicIntegerArrayToList(@NotNull AtomicIntegerArray array) { + List list = new ArrayList<>(); + for (int i = 0; i < array.length(); i++) { + list.add(array.get(i)); + } + return list; + } +} diff --git a/sentry/src/test/java/io/sentry/JsonObjectSerializerTest.kt b/sentry/src/test/java/io/sentry/JsonObjectSerializerTest.kt index 346d4a06cf..b03041060a 100644 --- a/sentry/src/test/java/io/sentry/JsonObjectSerializerTest.kt +++ b/sentry/src/test/java/io/sentry/JsonObjectSerializerTest.kt @@ -5,8 +5,14 @@ import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.times import com.nhaarman.mockitokotlin2.verify import org.junit.Test +import java.net.URI +import java.util.Calendar +import java.util.Currency import java.util.Locale import java.util.TimeZone +import java.util.UUID +import java.util.concurrent.atomic.AtomicBoolean +import java.util.concurrent.atomic.AtomicIntegerArray internal class JsonObjectSerializerTest { @@ -210,6 +216,61 @@ internal class JsonObjectSerializerTest { inOrder.verify(fixture.writer).endObject() } + @Test + fun `serializing AtomicIntegerArray`() { + fixture.getSUT().serialize(fixture.writer, fixture.logger, AtomicIntegerArray(arrayOf(1, 2, 3).toIntArray())) + verify(fixture.writer).beginArray() + verify(fixture.writer).value(1 as Number) + verify(fixture.writer).value(2 as Number) + verify(fixture.writer).value(3 as Number) + verify(fixture.writer).endArray() + } + + @Test + fun `serializing AtomicBoolean`() { + fixture.getSUT().serialize(fixture.writer, fixture.logger, AtomicBoolean(true)) + verify(fixture.writer).value(true) + } + + @Test + fun `serializing URI`() { + fixture.getSUT().serialize(fixture.writer, fixture.logger, URI("http://localhost:8081/api/product?id=99")) + verify(fixture.writer).value("http://localhost:8081/api/product?id=99") + } + + @Test + fun `serializing UUID`() { + fixture.getSUT().serialize(fixture.writer, fixture.logger, UUID.fromString("828900a5-15dc-413f-8c17-6ef04d74e074")) + verify(fixture.writer).value("828900a5-15dc-413f-8c17-6ef04d74e074") + } + + @Test + fun `serializing Currency`() { + fixture.getSUT().serialize(fixture.writer, fixture.logger, Currency.getInstance("USD")) + verify(fixture.writer).value("USD") + } + + @Test + fun `serializing Calendar`() { + val calendar = Calendar.getInstance() + calendar.set(2022, 0, 1, 11, 59, 58) + fixture.getSUT().serialize(fixture.writer, fixture.logger, calendar) + verify(fixture.writer).beginObject() + verify(fixture.writer).name("year") + verify(fixture.writer).value(2022L as java.lang.Long) + verify(fixture.writer).name("month") + verify(fixture.writer).value(0L as java.lang.Long) + verify(fixture.writer).name("dayOfMonth") + verify(fixture.writer).value(1L as java.lang.Long) + verify(fixture.writer).name("hourOfDay") + verify(fixture.writer).value(11L as java.lang.Long) + verify(fixture.writer).name("minute") + verify(fixture.writer).value(59L as java.lang.Long) + verify(fixture.writer).name("second") + verify(fixture.writer).value(58L as java.lang.Long) + verify(fixture.writer).endObject() + } + class UnknownClassWithData( private val integer: Int, private val string: String From 885b2cde91df4a71c8b1dca322be4af7c98a9261 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 20 Oct 2022 10:06:35 +0200 Subject: [PATCH 6/7] Address code review --- sentry/api/sentry.api | 12 +++---- .../java/io/sentry/JsonObjectSerializer.java | 4 +-- .../JsonReflectionObjectSerializer.java | 4 +-- .../{ => util}/JsonSerializationUtils.java | 14 ++++---- .../sentry/util/JsonSerializationUtilsTest.kt | 33 +++++++++++++++++++ 5 files changed, 51 insertions(+), 16 deletions(-) rename sentry/src/main/java/io/sentry/{ => util}/JsonSerializationUtils.java (63%) create mode 100644 sentry/src/test/java/io/sentry/util/JsonSerializationUtilsTest.kt diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index b321a83ce2..db26605694 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -581,12 +581,6 @@ public abstract interface class io/sentry/JsonSerializable { public abstract fun serialize (Lio/sentry/JsonObjectWriter;Lio/sentry/ILogger;)V } -public final class io/sentry/JsonSerializationUtils { - public fun ()V - public static fun atomicIntegerArrayToList (Ljava/util/concurrent/atomic/AtomicIntegerArray;)Ljava/util/List; - public static fun calendarToMap (Ljava/util/Calendar;)Ljava/util/Map; -} - public final class io/sentry/JsonSerializer : io/sentry/ISerializer { public fun (Lio/sentry/SentryOptions;)V public fun deserialize (Ljava/io/Reader;Ljava/lang/Class;)Ljava/lang/Object; @@ -3260,6 +3254,12 @@ public abstract interface class io/sentry/util/HintUtils$SentryNullableConsumer public abstract fun accept (Ljava/lang/Object;)V } +public final class io/sentry/util/JsonSerializationUtils { + public fun ()V + public static fun atomicIntegerArrayToList (Ljava/util/concurrent/atomic/AtomicIntegerArray;)Ljava/util/List; + public static fun calendarToMap (Ljava/util/Calendar;)Ljava/util/Map; +} + public final class io/sentry/util/LogUtils { public fun ()V public static fun logNotInstanceOf (Ljava/lang/Class;Ljava/lang/Object;Lio/sentry/ILogger;)V diff --git a/sentry/src/main/java/io/sentry/JsonObjectSerializer.java b/sentry/src/main/java/io/sentry/JsonObjectSerializer.java index cf2e5f5de6..e51cffc92c 100644 --- a/sentry/src/main/java/io/sentry/JsonObjectSerializer.java +++ b/sentry/src/main/java/io/sentry/JsonObjectSerializer.java @@ -1,7 +1,7 @@ package io.sentry; -import static io.sentry.JsonSerializationUtils.atomicIntegerArrayToList; -import static io.sentry.JsonSerializationUtils.calendarToMap; +import static io.sentry.util.JsonSerializationUtils.atomicIntegerArrayToList; +import static io.sentry.util.JsonSerializationUtils.calendarToMap; import java.io.IOException; import java.net.InetAddress; diff --git a/sentry/src/main/java/io/sentry/JsonReflectionObjectSerializer.java b/sentry/src/main/java/io/sentry/JsonReflectionObjectSerializer.java index b63ee8bb4e..97c2303104 100644 --- a/sentry/src/main/java/io/sentry/JsonReflectionObjectSerializer.java +++ b/sentry/src/main/java/io/sentry/JsonReflectionObjectSerializer.java @@ -1,7 +1,7 @@ package io.sentry; -import static io.sentry.JsonSerializationUtils.atomicIntegerArrayToList; -import static io.sentry.JsonSerializationUtils.calendarToMap; +import static io.sentry.util.JsonSerializationUtils.atomicIntegerArrayToList; +import static io.sentry.util.JsonSerializationUtils.calendarToMap; import java.lang.reflect.Field; import java.lang.reflect.Modifier; diff --git a/sentry/src/main/java/io/sentry/JsonSerializationUtils.java b/sentry/src/main/java/io/sentry/util/JsonSerializationUtils.java similarity index 63% rename from sentry/src/main/java/io/sentry/JsonSerializationUtils.java rename to sentry/src/main/java/io/sentry/util/JsonSerializationUtils.java index ba4c5f5a05..18f1e80d55 100644 --- a/sentry/src/main/java/io/sentry/JsonSerializationUtils.java +++ b/sentry/src/main/java/io/sentry/util/JsonSerializationUtils.java @@ -1,4 +1,4 @@ -package io.sentry; +package io.sentry.util; import java.util.ArrayList; import java.util.Calendar; @@ -12,8 +12,8 @@ @ApiStatus.Internal public final class JsonSerializationUtils { - public static @NotNull Map calendarToMap(@NotNull Calendar calendar) { - Map map = new HashMap<>(); + public static @NotNull Map calendarToMap(final @NotNull Calendar calendar) { + final @NotNull Map map = new HashMap<>(); map.put("year", (long) calendar.get(Calendar.YEAR)); map.put("month", (long) calendar.get(Calendar.MONTH)); @@ -25,9 +25,11 @@ public final class JsonSerializationUtils { return map; } - public static @NotNull List atomicIntegerArrayToList(@NotNull AtomicIntegerArray array) { - List list = new ArrayList<>(); - for (int i = 0; i < array.length(); i++) { + public static @NotNull List atomicIntegerArrayToList( + final @NotNull AtomicIntegerArray array) { + final int numberOfItems = array.length(); + final @NotNull List list = new ArrayList<>(numberOfItems); + for (int i = 0; i < numberOfItems; i++) { list.add(array.get(i)); } return list; diff --git a/sentry/src/test/java/io/sentry/util/JsonSerializationUtilsTest.kt b/sentry/src/test/java/io/sentry/util/JsonSerializationUtilsTest.kt new file mode 100644 index 0000000000..6bd875e22a --- /dev/null +++ b/sentry/src/test/java/io/sentry/util/JsonSerializationUtilsTest.kt @@ -0,0 +1,33 @@ +package io.sentry.util + +import java.util.Calendar +import java.util.concurrent.atomic.AtomicIntegerArray +import kotlin.test.Test +import kotlin.test.assertEquals + +class JsonSerializationUtilsTest { + + @Test + fun `serializes calendar to map`() { + val calendar = Calendar.getInstance() + calendar.set(2022, 0, 1, 11, 59, 58) + + val actual = JsonSerializationUtils.calendarToMap(calendar) + + val expected = mapOf( + "month" to 0L, + "year" to 2022L, + "dayOfMonth" to 1L, + "hourOfDay" to 11L, + "minute" to 59L, + "second" to 58L + ) + assertEquals(expected, actual) + } + + @Test + fun `serializes AtomicIntegerArray to list`() { + val actual = JsonSerializationUtils.atomicIntegerArrayToList(AtomicIntegerArray(arrayOf(1, 2, 3).toIntArray())) + assertEquals(listOf(1, 2, 3), actual) + } +} From 5c0a93d76443d191fbe278bea3f03eae21910998 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 20 Oct 2022 13:39:09 +0200 Subject: [PATCH 7/7] Remove long cast --- .../java/io/sentry/util/JsonSerializationUtils.java | 12 ++++++------ .../test/java/io/sentry/JsonObjectSerializerTest.kt | 12 ++++++------ .../io/sentry/JsonReflectionObjectSerializerTest.kt | 12 ++++++------ .../io/sentry/util/JsonSerializationUtilsTest.kt | 12 ++++++------ 4 files changed, 24 insertions(+), 24 deletions(-) diff --git a/sentry/src/main/java/io/sentry/util/JsonSerializationUtils.java b/sentry/src/main/java/io/sentry/util/JsonSerializationUtils.java index 18f1e80d55..afb8ec2e95 100644 --- a/sentry/src/main/java/io/sentry/util/JsonSerializationUtils.java +++ b/sentry/src/main/java/io/sentry/util/JsonSerializationUtils.java @@ -15,12 +15,12 @@ public final class JsonSerializationUtils { public static @NotNull Map calendarToMap(final @NotNull Calendar calendar) { final @NotNull Map map = new HashMap<>(); - map.put("year", (long) calendar.get(Calendar.YEAR)); - map.put("month", (long) calendar.get(Calendar.MONTH)); - map.put("dayOfMonth", (long) calendar.get(Calendar.DAY_OF_MONTH)); - map.put("hourOfDay", (long) calendar.get(Calendar.HOUR_OF_DAY)); - map.put("minute", (long) calendar.get(Calendar.MINUTE)); - map.put("second", (long) calendar.get(Calendar.SECOND)); + map.put("year", calendar.get(Calendar.YEAR)); + map.put("month", calendar.get(Calendar.MONTH)); + map.put("dayOfMonth", calendar.get(Calendar.DAY_OF_MONTH)); + map.put("hourOfDay", calendar.get(Calendar.HOUR_OF_DAY)); + map.put("minute", calendar.get(Calendar.MINUTE)); + map.put("second", calendar.get(Calendar.SECOND)); return map; } diff --git a/sentry/src/test/java/io/sentry/JsonObjectSerializerTest.kt b/sentry/src/test/java/io/sentry/JsonObjectSerializerTest.kt index b03041060a..2a998dcc0f 100644 --- a/sentry/src/test/java/io/sentry/JsonObjectSerializerTest.kt +++ b/sentry/src/test/java/io/sentry/JsonObjectSerializerTest.kt @@ -257,17 +257,17 @@ internal class JsonObjectSerializerTest { fixture.getSUT().serialize(fixture.writer, fixture.logger, calendar) verify(fixture.writer).beginObject() verify(fixture.writer).name("year") - verify(fixture.writer).value(2022L as java.lang.Long) + verify(fixture.writer).value(2022 as java.lang.Integer) verify(fixture.writer).name("month") - verify(fixture.writer).value(0L as java.lang.Long) + verify(fixture.writer).value(0 as java.lang.Integer) verify(fixture.writer).name("dayOfMonth") - verify(fixture.writer).value(1L as java.lang.Long) + verify(fixture.writer).value(1 as java.lang.Integer) verify(fixture.writer).name("hourOfDay") - verify(fixture.writer).value(11L as java.lang.Long) + verify(fixture.writer).value(11 as java.lang.Integer) verify(fixture.writer).name("minute") - verify(fixture.writer).value(59L as java.lang.Long) + verify(fixture.writer).value(59 as java.lang.Integer) verify(fixture.writer).name("second") - verify(fixture.writer).value(58L as java.lang.Long) + verify(fixture.writer).value(58 as java.lang.Integer) verify(fixture.writer).endObject() } diff --git a/sentry/src/test/java/io/sentry/JsonReflectionObjectSerializerTest.kt b/sentry/src/test/java/io/sentry/JsonReflectionObjectSerializerTest.kt index a3b9ef008d..84b21e78d6 100644 --- a/sentry/src/test/java/io/sentry/JsonReflectionObjectSerializerTest.kt +++ b/sentry/src/test/java/io/sentry/JsonReflectionObjectSerializerTest.kt @@ -334,12 +334,12 @@ class JsonReflectionObjectSerializerTest { calendar.set(2022, 0, 1, 11, 59, 58) val actual = fixture.getSut().serialize(calendar, fixture.logger) val expected = mapOf( - "month" to 0L, - "year" to 2022L, - "dayOfMonth" to 1L, - "hourOfDay" to 11L, - "minute" to 59L, - "second" to 58L + "month" to 0, + "year" to 2022, + "dayOfMonth" to 1, + "hourOfDay" to 11, + "minute" to 59, + "second" to 58 ) assertEquals(expected, actual) } diff --git a/sentry/src/test/java/io/sentry/util/JsonSerializationUtilsTest.kt b/sentry/src/test/java/io/sentry/util/JsonSerializationUtilsTest.kt index 6bd875e22a..c50ce8d180 100644 --- a/sentry/src/test/java/io/sentry/util/JsonSerializationUtilsTest.kt +++ b/sentry/src/test/java/io/sentry/util/JsonSerializationUtilsTest.kt @@ -15,12 +15,12 @@ class JsonSerializationUtilsTest { val actual = JsonSerializationUtils.calendarToMap(calendar) val expected = mapOf( - "month" to 0L, - "year" to 2022L, - "dayOfMonth" to 1L, - "hourOfDay" to 11L, - "minute" to 59L, - "second" to 58L + "month" to 0, + "year" to 2022, + "dayOfMonth" to 1, + "hourOfDay" to 11, + "minute" to 59, + "second" to 58 ) assertEquals(expected, actual) }