diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidSerializerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidSerializerTest.kt index c70305bcd..715728f7c 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidSerializerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidSerializerTest.kt @@ -10,6 +10,7 @@ import io.sentry.core.protocol.Contexts import io.sentry.core.protocol.Device import java.io.StringReader import java.io.StringWriter +import java.util.Date import java.util.TimeZone import java.util.UUID import kotlin.test.Test @@ -28,8 +29,7 @@ class AndroidSerializerTest { @Test fun `when serializing SentryEvent-SentryId object, it should become a event_id json without dashes`() { - val sentryEvent = generateEmptySentryEvent() - sentryEvent.timestamp = null + val sentryEvent = generateEmptySentryEvent(null) val actual = serializeToString(sentryEvent) @@ -50,10 +50,9 @@ class AndroidSerializerTest { @Test fun `when serializing SentryEvent-Date, it should become a timestamp json ISO format`() { - val sentryEvent = generateEmptySentryEvent() val dateIsoFormat = "2000-12-31T23:59:58Z" + val sentryEvent = generateEmptySentryEvent(DateUtils.getDateTime(dateIsoFormat)) sentryEvent.eventId = null - sentryEvent.timestamp = DateUtils.getDateTime(dateIsoFormat) val expected = "{\"timestamp\":\"$dateIsoFormat\"}" @@ -64,11 +63,8 @@ class AndroidSerializerTest { @Test fun `when deserializing timestamp, it should become a SentryEvent-Date`() { - val sentryEvent = generateEmptySentryEvent() val dateIsoFormat = "2000-12-31T23:59:58Z" - sentryEvent.eventId = null val expected = DateUtils.getDateTime(dateIsoFormat) - sentryEvent.timestamp = expected val jsonEvent = "{\"timestamp\":\"$dateIsoFormat\"}" @@ -81,7 +77,6 @@ class AndroidSerializerTest { fun `when deserializing unknown properties, it should be added to unknown field`() { val sentryEvent = generateEmptySentryEvent() sentryEvent.eventId = null - sentryEvent.timestamp = null val jsonEvent = "{\"string\":\"test\",\"int\":1,\"boolean\":true}" @@ -96,7 +91,6 @@ class AndroidSerializerTest { fun `when deserializing unknown properties with nested objects, it should be added to unknown field`() { val sentryEvent = generateEmptySentryEvent() sentryEvent.eventId = null - sentryEvent.timestamp = null val objects = hashMapOf() objects["int"] = 1 @@ -118,9 +112,8 @@ class AndroidSerializerTest { @Test fun `when serializing unknown field, it should become unknown as json format`() { - val sentryEvent = generateEmptySentryEvent() + val sentryEvent = generateEmptySentryEvent(null) sentryEvent.eventId = null - sentryEvent.timestamp = null val objects = hashMapOf() objects["int"] = 1 @@ -140,9 +133,8 @@ class AndroidSerializerTest { @Test fun `when serializing a TimeZone, it should become a timezone ID string`() { - val sentryEvent = generateEmptySentryEvent() + val sentryEvent = generateEmptySentryEvent(null) sentryEvent.eventId = null - sentryEvent.timestamp = null val device = Device() device.timezone = TimeZone.getTimeZone("Europe/Vienna") val contexts = Contexts() @@ -160,7 +152,6 @@ class AndroidSerializerTest { fun `when deserializing a timezone ID string, it should become a Device-TimeZone`() { val sentryEvent = generateEmptySentryEvent() sentryEvent.eventId = null - sentryEvent.timestamp = null val jsonEvent = "{\"contexts\":{\"device\":{\"timezone\":\"Europe/Vienna\"}}}" @@ -171,9 +162,8 @@ class AndroidSerializerTest { @Test fun `when serializing a DeviceOrientation, it should become an orientation string`() { - val sentryEvent = generateEmptySentryEvent() + val sentryEvent = generateEmptySentryEvent(null) sentryEvent.eventId = null - sentryEvent.timestamp = null val device = Device() device.orientation = Device.DeviceOrientation.LANDSCAPE val contexts = Contexts() @@ -191,7 +181,6 @@ class AndroidSerializerTest { fun `when deserializing an orientation string, it should become a DeviceOrientation`() { val sentryEvent = generateEmptySentryEvent() sentryEvent.eventId = null - sentryEvent.timestamp = null val jsonEvent = "{\"contexts\":{\"device\":{\"orientation\":\"landscape\"}}}" @@ -202,9 +191,8 @@ class AndroidSerializerTest { @Test fun `when serializing a SentryLevel, it should become a sentry level string`() { - val sentryEvent = generateEmptySentryEvent() + val sentryEvent = generateEmptySentryEvent(null) sentryEvent.eventId = null - sentryEvent.timestamp = null sentryEvent.level = SentryLevel.DEBUG val expected = "{\"level\":\"debug\"}" @@ -218,7 +206,6 @@ class AndroidSerializerTest { fun `when deserializing a sentry level string, it should become a SentryLevel`() { val sentryEvent = generateEmptySentryEvent() sentryEvent.eventId = null - sentryEvent.timestamp = null val jsonEvent = "{\"level\":\"debug\"}" @@ -234,8 +221,8 @@ class AndroidSerializerTest { assertNull(event.user) } - private fun generateEmptySentryEvent(): SentryEvent { - return SentryEvent().apply { + private fun generateEmptySentryEvent(date: Date? = null): SentryEvent { + return SentryEvent(date).apply { contexts = null } } diff --git a/sentry-core/src/main/java/io/sentry/core/Breadcrumb.java b/sentry-core/src/main/java/io/sentry/core/Breadcrumb.java index 919d2b176..f0313caaf 100644 --- a/sentry-core/src/main/java/io/sentry/core/Breadcrumb.java +++ b/sentry-core/src/main/java/io/sentry/core/Breadcrumb.java @@ -6,19 +6,20 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.TestOnly; public final class Breadcrumb implements Cloneable, IUnknownPropertiesConsumer { - private Date timestamp; + private final Date timestamp; private String message; private String type; - private Map data; + private Map data = new ConcurrentHashMap<>(); private String category; private SentryLevel level; private Map unknown; - Breadcrumb(Date timestamp) { + Breadcrumb(final Date timestamp) { this.timestamp = timestamp; } @@ -35,10 +36,6 @@ public Date getTimestamp() { return (Date) timestamp.clone(); } - void setTimestamp(Date timestamp) { - this.timestamp = timestamp; - } - public String getMessage() { return message; } @@ -55,12 +52,16 @@ public void setType(String type) { this.type = type; } - public Map getData() { + Map getData() { return data; } - public void setData(Map data) { - this.data = data; + public void setData(@NotNull String key, @NotNull String value) { + data.put(key, value); + } + + public void removeData(@NotNull String key) { + data.remove(key); } public String getCategory() { @@ -92,14 +93,13 @@ Map getUnknown() { @Override public Breadcrumb clone() throws CloneNotSupportedException { - Breadcrumb clone = (Breadcrumb) super.clone(); - - clone.timestamp = timestamp != null ? (Date) timestamp.clone() : null; + final Breadcrumb clone = (Breadcrumb) super.clone(); - if (data != null) { - Map dataClone = new ConcurrentHashMap<>(); + final Map dataRef = data; + if (dataRef != null) { + final Map dataClone = new ConcurrentHashMap<>(); - for (Map.Entry item : data.entrySet()) { + for (Map.Entry item : dataRef.entrySet()) { if (item != null) { dataClone.put(item.getKey(), item.getValue()); } @@ -110,10 +110,11 @@ public Breadcrumb clone() throws CloneNotSupportedException { clone.data = null; } - if (unknown != null) { - Map unknownClone = new HashMap<>(); + final Map unknownRef = unknown; + if (unknownRef != null) { + final Map unknownClone = new HashMap<>(); - for (Map.Entry item : unknown.entrySet()) { + for (Map.Entry item : unknownRef.entrySet()) { if (item != null) { unknownClone.put(item.getKey(), item.getValue()); // shallow copy } @@ -124,7 +125,9 @@ public Breadcrumb clone() throws CloneNotSupportedException { clone.unknown = null; } - clone.level = level != null ? SentryLevel.valueOf(level.name().toUpperCase(Locale.ROOT)) : null; + final SentryLevel levelRef = level; + clone.level = + levelRef != null ? SentryLevel.valueOf(levelRef.name().toUpperCase(Locale.ROOT)) : null; return clone; } diff --git a/sentry-core/src/main/java/io/sentry/core/Scope.java b/sentry-core/src/main/java/io/sentry/core/Scope.java index 47785aa96..288491fd4 100644 --- a/sentry-core/src/main/java/io/sentry/core/Scope.java +++ b/sentry-core/src/main/java/io/sentry/core/Scope.java @@ -8,6 +8,7 @@ import java.util.Map; import java.util.Queue; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CopyOnWriteArrayList; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -19,7 +20,7 @@ public final class Scope implements Cloneable { private @NotNull Queue breadcrumbs; private @NotNull Map tags = new ConcurrentHashMap<>(); private @NotNull Map extra = new ConcurrentHashMap<>(); - private @NotNull List eventProcessors = new ArrayList<>(); + private @NotNull List eventProcessors = new CopyOnWriteArrayList<>(); private final @NotNull SentryOptions options; public Scope(final @NotNull SentryOptions options) { @@ -79,12 +80,7 @@ Queue getBreadcrumbs() { "The BeforeBreadcrumbCallback callback threw an exception. It will be added as breadcrumb and continue.", e); - Map data = breadcrumb.getData(); - if (breadcrumb.getData() == null) { - data = new HashMap<>(); - } - data.put("sentry:message", e.getMessage()); - breadcrumb.setData(data); + breadcrumb.setData("sentry:message", e.getMessage()); } return breadcrumb; } @@ -156,52 +152,52 @@ public void removeExtra(@NotNull String key) { @Override public Scope clone() throws CloneNotSupportedException { - Scope clone = (Scope) super.clone(); - clone.level = level != null ? SentryLevel.valueOf(level.name().toUpperCase(Locale.ROOT)) : null; - clone.user = user != null ? user.clone() : null; - clone.fingerprint = fingerprint != null ? new ArrayList<>(fingerprint) : null; - clone.eventProcessors = eventProcessors != null ? new ArrayList<>(eventProcessors) : null; - - if (breadcrumbs != null) { - Queue breadcrumbsClone = createBreadcrumbsList(options.getMaxBreadcrumbs()); - - for (Breadcrumb item : breadcrumbs) { - Breadcrumb breadcrumbClone = item.clone(); - breadcrumbsClone.add(breadcrumbClone); - } - clone.breadcrumbs = breadcrumbsClone; - } else { - clone.breadcrumbs = null; - } + final Scope clone = (Scope) super.clone(); - if (tags != null) { - Map tagsClone = new ConcurrentHashMap<>(); + final SentryLevel levelRef = level; + clone.level = + levelRef != null ? SentryLevel.valueOf(levelRef.name().toUpperCase(Locale.ROOT)) : null; - for (Map.Entry item : tags.entrySet()) { - if (item != null) { - tagsClone.put(item.getKey(), item.getValue()); - } - } + final User userRef = user; + clone.user = userRef != null ? userRef.clone() : null; - clone.tags = tagsClone; - } else { - clone.tags = null; + clone.fingerprint = new ArrayList<>(fingerprint); + clone.eventProcessors = new CopyOnWriteArrayList<>(eventProcessors); + + final Queue breadcrumbsRef = breadcrumbs; + + Queue breadcrumbsClone = createBreadcrumbsList(options.getMaxBreadcrumbs()); + + for (Breadcrumb item : breadcrumbsRef) { + final Breadcrumb breadcrumbClone = item.clone(); + breadcrumbsClone.add(breadcrumbClone); } + clone.breadcrumbs = breadcrumbsClone; - if (extra != null) { - Map extraClone = new HashMap<>(); + final Map tagsRef = tags; - for (Map.Entry item : extra.entrySet()) { - if (item != null) { - extraClone.put(item.getKey(), item.getValue()); - } + final Map tagsClone = new ConcurrentHashMap<>(); + + for (Map.Entry item : tagsRef.entrySet()) { + if (item != null) { + tagsClone.put(item.getKey(), item.getValue()); } + } - clone.extra = extraClone; - } else { - clone.extra = null; + clone.tags = tagsClone; + + final Map extraRef = extra; + + Map extraClone = new HashMap<>(); + + for (Map.Entry item : extraRef.entrySet()) { + if (item != null) { + extraClone.put(item.getKey(), item.getValue()); + } } + clone.extra = extraClone; + return clone; } diff --git a/sentry-core/src/main/java/io/sentry/core/SentryClient.java b/sentry-core/src/main/java/io/sentry/core/SentryClient.java index 109afe2ae..88070349a 100644 --- a/sentry-core/src/main/java/io/sentry/core/SentryClient.java +++ b/sentry-core/src/main/java/io/sentry/core/SentryClient.java @@ -195,10 +195,8 @@ private SentryEvent executeBeforeSend(SentryEvent event, @Nullable Object hint) Breadcrumb breadcrumb = new Breadcrumb(); breadcrumb.setMessage("BeforeSend callback failed."); breadcrumb.setCategory("SentryClient"); - Map data = new HashMap<>(); - data.put("sentry:message", e.getMessage()); breadcrumb.setLevel(SentryLevel.ERROR); - breadcrumb.setData(data); + breadcrumb.setData("sentry:message", e.getMessage()); event.addBreadcrumb(breadcrumb); } } diff --git a/sentry-core/src/main/java/io/sentry/core/SentryEvent.java b/sentry-core/src/main/java/io/sentry/core/SentryEvent.java index 3b17de9cc..de13fd4ec 100644 --- a/sentry-core/src/main/java/io/sentry/core/SentryEvent.java +++ b/sentry-core/src/main/java/io/sentry/core/SentryEvent.java @@ -12,7 +12,7 @@ public final class SentryEvent implements IUnknownPropertiesConsumer { private SentryId eventId; - private Date timestamp; + private final Date timestamp; private transient Throwable throwable; private Message message; private String serverName; @@ -37,7 +37,7 @@ public final class SentryEvent implements IUnknownPropertiesConsumer { private Map modules; private DebugMeta debugMeta; - SentryEvent(SentryId eventId, Date timestamp) { + SentryEvent(SentryId eventId, final Date timestamp) { this.eventId = eventId; this.timestamp = timestamp; } @@ -51,6 +51,11 @@ public SentryEvent() { this(new SentryId(), DateUtils.getCurrentDateTime()); } + @TestOnly + public SentryEvent(final Date timestamp) { + this(new SentryId(), timestamp); + } + public SentryId getEventId() { return eventId; } @@ -135,11 +140,6 @@ public void setEventId(SentryId eventId) { this.eventId = eventId; } - @TestOnly - public void setTimestamp(Date timestamp) { - this.timestamp = timestamp; - } - public void setThrowable(Throwable throwable) { this.throwable = throwable; } diff --git a/sentry-core/src/main/java/io/sentry/core/protocol/App.java b/sentry-core/src/main/java/io/sentry/core/protocol/App.java index a42edbfb0..7420965e1 100644 --- a/sentry-core/src/main/java/io/sentry/core/protocol/App.java +++ b/sentry-core/src/main/java/io/sentry/core/protocol/App.java @@ -28,7 +28,8 @@ public void setAppIdentifier(String appIdentifier) { } public Date getAppStartTime() { - return appStartTime != null ? (Date) appStartTime.clone() : null; + final Date appStartTimeRef = appStartTime; + return appStartTimeRef != null ? (Date) appStartTimeRef.clone() : null; } public void setAppStartTime(Date appStartTime) { diff --git a/sentry-core/src/main/java/io/sentry/core/protocol/Device.java b/sentry-core/src/main/java/io/sentry/core/protocol/Device.java index d1a16c30a..5be57221a 100644 --- a/sentry-core/src/main/java/io/sentry/core/protocol/Device.java +++ b/sentry-core/src/main/java/io/sentry/core/protocol/Device.java @@ -254,7 +254,8 @@ public void setScreenDpi(Integer screenDpi) { } public Date getBootTime() { - return bootTime != null ? (Date) bootTime.clone() : null; + final Date bootTimeRef = bootTime; + return bootTimeRef != null ? (Date) bootTimeRef.clone() : null; } public void setBootTime(Date bootTime) { diff --git a/sentry-core/src/main/java/io/sentry/core/protocol/User.java b/sentry-core/src/main/java/io/sentry/core/protocol/User.java index d9beec43b..49b6f41ed 100644 --- a/sentry-core/src/main/java/io/sentry/core/protocol/User.java +++ b/sentry-core/src/main/java/io/sentry/core/protocol/User.java @@ -119,12 +119,13 @@ Map getUnknown() { @Override public User clone() throws CloneNotSupportedException { - User clone = (User) super.clone(); + final User clone = (User) super.clone(); - if (other != null) { - Map otherClone = new ConcurrentHashMap<>(); + final Map otherRef = other; + if (otherRef != null) { + final Map otherClone = new ConcurrentHashMap<>(); - for (Map.Entry item : other.entrySet()) { + for (Map.Entry item : otherRef.entrySet()) { if (item != null) { otherClone.put(item.getKey(), item.getValue()); } @@ -135,10 +136,11 @@ public User clone() throws CloneNotSupportedException { clone.other = null; } - if (unknown != null) { - Map unknownClone = new HashMap<>(); + final Map unknownRef = unknown; + if (unknownRef != null) { + final Map unknownClone = new HashMap<>(); - for (Map.Entry item : unknown.entrySet()) { + for (Map.Entry item : unknownRef.entrySet()) { if (item != null) { unknownClone.put(item.getKey(), item.getValue()); } diff --git a/sentry-core/src/test/java/io/sentry/core/BreadcrumbTest.kt b/sentry-core/src/test/java/io/sentry/core/BreadcrumbTest.kt index 6ce902925..ed2cce7fb 100644 --- a/sentry-core/src/test/java/io/sentry/core/BreadcrumbTest.kt +++ b/sentry-core/src/test/java/io/sentry/core/BreadcrumbTest.kt @@ -1,6 +1,5 @@ package io.sentry.core -import java.util.Date import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertNotNull @@ -11,12 +10,10 @@ class BreadcrumbTest { fun `cloning breadcrumb wont have the same references`() { val breadcrumb = Breadcrumb() breadcrumb.message = "message" - val data = mutableMapOf(Pair("data", "data")) - breadcrumb.data = data + breadcrumb.setData("data", "data") val unknown = mapOf(Pair("unknown", "unknown")) breadcrumb.acceptUnknownProperties(unknown) - val date = Date() - breadcrumb.timestamp = date + breadcrumb.type = "type" val level = SentryLevel.DEBUG breadcrumb.level = level @@ -26,7 +23,6 @@ class BreadcrumbTest { assertNotNull(clone) assertNotSame(breadcrumb, clone) - assertNotSame(breadcrumb.timestamp, clone.timestamp) assertNotSame(breadcrumb.data, clone.data) @@ -37,13 +33,10 @@ class BreadcrumbTest { fun `cloning breadcrumb will have the same values`() { val breadcrumb = Breadcrumb() breadcrumb.message = "message" - val data = mutableMapOf(Pair("data", "data")) - breadcrumb.data = data + breadcrumb.setData("data", "data") val unknown = mapOf(Pair("unknown", "unknown")) breadcrumb.acceptUnknownProperties(unknown) - val date = Date() - val dateIso = DateUtils.getTimestamp(date) - breadcrumb.timestamp = date + breadcrumb.type = "type" val level = SentryLevel.DEBUG breadcrumb.level = level @@ -57,20 +50,16 @@ class BreadcrumbTest { assertEquals("type", clone.type) assertEquals(SentryLevel.DEBUG, clone.level) assertEquals("category", clone.category) - assertEquals(dateIso, DateUtils.getTimestamp(clone.timestamp)) } @Test fun `cloning breadcrumb and changing the original values wont change the clone values`() { val breadcrumb = Breadcrumb() breadcrumb.message = "message" - val data = mutableMapOf(Pair("data", "data")) - breadcrumb.data = data + breadcrumb.setData("data", "data") val unknown = mapOf(Pair("unknown", "unknown")) breadcrumb.acceptUnknownProperties(unknown) - val date = Date() - val dateIso = DateUtils.getTimestamp(date) - breadcrumb.timestamp = date + breadcrumb.type = "type" val level = SentryLevel.DEBUG breadcrumb.level = level @@ -83,7 +72,7 @@ class BreadcrumbTest { breadcrumb.data["otherData"] = "otherData" val newUnknown = mapOf(Pair("unknown", "newUnknown"), Pair("otherUnknown", "otherUnknown")) breadcrumb.acceptUnknownProperties(newUnknown) - breadcrumb.timestamp = Date() + breadcrumb.type = "newType" breadcrumb.level = SentryLevel.FATAL breadcrumb.category = "newCategory" @@ -96,7 +85,6 @@ class BreadcrumbTest { assertEquals("type", clone.type) assertEquals(SentryLevel.DEBUG, clone.level) assertEquals("category", clone.category) - assertEquals(dateIso, DateUtils.getTimestamp(clone.timestamp)) } @Test diff --git a/sentry-core/src/test/java/io/sentry/core/ScopeTest.kt b/sentry-core/src/test/java/io/sentry/core/ScopeTest.kt index ecb2450ef..8058e19c9 100644 --- a/sentry-core/src/test/java/io/sentry/core/ScopeTest.kt +++ b/sentry-core/src/test/java/io/sentry/core/ScopeTest.kt @@ -1,7 +1,6 @@ package io.sentry.core import io.sentry.core.protocol.User -import java.util.Date import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertNotNull @@ -30,11 +29,8 @@ class ScopeTest { val breadcrumb = Breadcrumb() breadcrumb.message = "message" - val data = mutableMapOf(Pair("data", "data")) - breadcrumb.data = data + breadcrumb.setData("data", "data") - val date = Date() - breadcrumb.timestamp = date breadcrumb.type = "type" breadcrumb.level = SentryLevel.DEBUG breadcrumb.category = "category"