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

clone method - race condition free #226

Merged
merged 9 commits into from Jan 19, 2020
Merged

clone method - race condition free #226

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e52f994
clone race condition free
marandaneto Jan 14, 2020
0b39ff6
mark as current thread if its the main thread
marandaneto Jan 15, 2020
5f1f907
Merge branch 'master' of github.com:getsentry/sentry-android
marandaneto Jan 15, 2020
275763b
fix merge conflict
marandaneto Jan 15, 2020
bb848ce
Merge branch 'master' into ref/clone
marandaneto Jan 15, 2020
0816669
refactoring
marandaneto Jan 18, 2020
9e66251
fix merge conflict
marandaneto Jan 18, 2020
f3697b4
code review
marandaneto Jan 19, 2020
82156bf
fix merge conflict
marandaneto Jan 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 9 additions & 22 deletions sentry-android-core/src/test/java/io/sentry/android/core/AndroidSerializerTest.kt
View file
Open in desktop
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
Open in desktop
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;
marandaneto marked this conversation as resolved.
Show resolved Hide resolved
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
Open in desktop
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
Open in desktop
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