Skip to content
This repository has been archived by the owner on Aug 30, 2023. It is now read-only.

Commit

Permalink
clone method - race condition free (#226)
Browse files Browse the repository at this point in the history
  • Loading branch information
@marandaneto
marandaneto authored Jan 19, 2020
1 parent f1a6cf1 commit d2655be
Show file tree
Hide file tree
Showing 10 changed files with 100 additions and 128 deletions.
31 changes: 9 additions & 22 deletions sentry-android-core/src/test/java/io/sentry/android/core/AndroidSerializerTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)

Expand All @@ -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\"}"

Expand All @@ -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\"}"

Expand All @@ -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}"

Expand All @@ -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<String, Any>()
objects["int"] = 1
Expand All @@ -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<String, Any>()
objects["int"] = 1
Expand All @@ -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()
Expand All @@ -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\"}}}"

Expand All @@ -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()
Expand All @@ -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\"}}}"

Expand All @@ -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\"}"
Expand All @@ -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\"}"

Expand All @@ -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
}
}
Expand Down
43 changes: 23 additions & 20 deletions sentry-core/src/main/java/io/sentry/core/Breadcrumb.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, String> data;
private Map<String, String> data = new ConcurrentHashMap<>();
private String category;
private SentryLevel level;
private Map<String, Object> unknown;

Breadcrumb(Date timestamp) {
Breadcrumb(final Date timestamp) {
this.timestamp = timestamp;
}

Expand All @@ -35,10 +36,6 @@ public Date getTimestamp() {
return (Date) timestamp.clone();
}

void setTimestamp(Date timestamp) {
this.timestamp = timestamp;
}

public String getMessage() {
return message;
}
Expand All @@ -55,12 +52,16 @@ public void setType(String type) {
this.type = type;
}

public Map<String, String> getData() {
Map<String, String> getData() {
return data;
}

public void setData(Map<String, String> 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() {
Expand Down Expand Up @@ -92,14 +93,13 @@ Map<String, Object> 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<String, String> dataClone = new ConcurrentHashMap<>();
final Map<String, String> dataRef = data;
if (dataRef != null) {
final Map<String, String> dataClone = new ConcurrentHashMap<>();

for (Map.Entry<String, String> item : data.entrySet()) {
for (Map.Entry<String, String> item : dataRef.entrySet()) {
if (item != null) {
dataClone.put(item.getKey(), item.getValue());
}
Expand All @@ -110,10 +110,11 @@ public Breadcrumb clone() throws CloneNotSupportedException {
clone.data = null;
}

if (unknown != null) {
Map<String, Object> unknownClone = new HashMap<>();
final Map<String, Object> unknownRef = unknown;
if (unknownRef != null) {
final Map<String, Object> unknownClone = new HashMap<>();

for (Map.Entry<String, Object> item : unknown.entrySet()) {
for (Map.Entry<String, Object> item : unknownRef.entrySet()) {
if (item != null) {
unknownClone.put(item.getKey(), item.getValue()); // shallow copy
}
Expand All @@ -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;
}
Expand Down
82 changes: 39 additions & 43 deletions sentry-core/src/main/java/io/sentry/core/Scope.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -19,7 +20,7 @@ public final class Scope implements Cloneable {
private @NotNull Queue<Breadcrumb> breadcrumbs;
private @NotNull Map<String, String> tags = new ConcurrentHashMap<>();
private @NotNull Map<String, Object> extra = new ConcurrentHashMap<>();
private @NotNull List<EventProcessor> eventProcessors = new ArrayList<>();
private @NotNull List<EventProcessor> eventProcessors = new CopyOnWriteArrayList<>();
private final @NotNull SentryOptions options;

public Scope(final @NotNull SentryOptions options) {
Expand Down Expand Up @@ -79,12 +80,7 @@ Queue<Breadcrumb> getBreadcrumbs() {
"The BeforeBreadcrumbCallback callback threw an exception. It will be added as breadcrumb and continue.",
e);

Map<String, String> 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;
}
Expand Down Expand Up @@ -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<Breadcrumb> 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<String, String> tagsClone = new ConcurrentHashMap<>();
final SentryLevel levelRef = level;
clone.level =
levelRef != null ? SentryLevel.valueOf(levelRef.name().toUpperCase(Locale.ROOT)) : null;

for (Map.Entry<String, String> 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<Breadcrumb> breadcrumbsRef = breadcrumbs;

Queue<Breadcrumb> breadcrumbsClone = createBreadcrumbsList(options.getMaxBreadcrumbs());

for (Breadcrumb item : breadcrumbsRef) {
final Breadcrumb breadcrumbClone = item.clone();
breadcrumbsClone.add(breadcrumbClone);
}
clone.breadcrumbs = breadcrumbsClone;

if (extra != null) {
Map<String, Object> extraClone = new HashMap<>();
final Map<String, String> tagsRef = tags;

for (Map.Entry<String, Object> item : extra.entrySet()) {
if (item != null) {
extraClone.put(item.getKey(), item.getValue());
}
final Map<String, String> tagsClone = new ConcurrentHashMap<>();

for (Map.Entry<String, String> item : tagsRef.entrySet()) {
if (item != null) {
tagsClone.put(item.getKey(), item.getValue());
}
}

clone.extra = extraClone;
} else {
clone.extra = null;
clone.tags = tagsClone;

final Map<String, Object> extraRef = extra;

Map<String, Object> extraClone = new HashMap<>();

for (Map.Entry<String, Object> item : extraRef.entrySet()) {
if (item != null) {
extraClone.put(item.getKey(), item.getValue());
}
}

clone.extra = extraClone;

return clone;
}

Expand Down
4 changes: 1 addition & 3 deletions sentry-core/src/main/java/io/sentry/core/SentryClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, String> 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);
}
}
Expand Down
Loading

0 comments on commit d2655be

Please sign in to comment.