From 90bfee5b7650b7ea6fb8bb41df85265a782ac940 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Fri, 13 May 2022 17:46:15 +0200 Subject: [PATCH] Add attachment methods to Hints directly --- sentry/api/sentry.api | 15 ++--- .../src/main/java/io/sentry/SentryClient.java | 4 +- .../java/io/sentry/hints/Attachments.java | 39 ------------ .../src/main/java/io/sentry/hints/Hints.java | 59 +++++++++++++++---- .../test/java/io/sentry/SentryClientTest.kt | 20 +++---- .../java/io/sentry/hints/AttachmentsTest.kt | 34 +++++------ .../test/java/io/sentry/hints/HintsTest.kt | 4 +- 7 files changed, 85 insertions(+), 90 deletions(-) delete mode 100644 sentry/src/main/java/io/sentry/hints/Attachments.java diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 106d720b6b2..41cb04950ce 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -1836,15 +1836,6 @@ public final class io/sentry/exception/SentryEnvelopeException : java/lang/Excep public abstract interface class io/sentry/hints/ApplyScopeData { } -public final class io/sentry/hints/Attachments { - public fun ()V - public fun add (Lio/sentry/Attachment;)V - public fun addAll (Ljava/util/List;)V - public fun clear ()V - public fun getAll ()Ljava/util/List; - public fun replaceAll (Ljava/util/List;)V -} - public abstract interface class io/sentry/hints/Cached { } @@ -1858,10 +1849,14 @@ public abstract interface class io/sentry/hints/Flushable { public final class io/sentry/hints/Hints { public fun ()V + public fun addAttachment (Lio/sentry/Attachment;)V + public fun addAttachments (Ljava/util/List;)V + public fun clearAttachments ()V public fun get (Ljava/lang/String;)Ljava/lang/Object; public fun getAs (Ljava/lang/String;Ljava/lang/Class;)Ljava/lang/Object; - public fun getAttachments ()Lio/sentry/hints/Attachments; + public fun getAttachments ()Ljava/util/List; public fun remove (Ljava/lang/String;)V + public fun replaceAttachments (Ljava/util/List;)V public fun set (Ljava/lang/String;Ljava/lang/Object;)V public static fun withAttachment (Lio/sentry/Attachment;)Lio/sentry/hints/Hints; public static fun withAttachments (Ljava/util/List;)Lio/sentry/hints/Hints; diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index f28e2d2b7ef..87331482cf1 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -196,7 +196,7 @@ private boolean shouldApplyScopeData( private void addScopeAttachmentsToHints(@Nullable Scope scope, @NotNull Hints hints) { if (scope != null) { - hints.getAttachments().addAll(scope.getAttachments()); + hints.addAttachments(scope.getAttachments()); } } @@ -227,7 +227,7 @@ private boolean shouldSendSessionUpdateForDroppedEvent( } private @Nullable List getAttachments(final @NotNull Hints hints) { - @NotNull final List attachments = hints.getAttachments().getAll(); + @NotNull final List attachments = hints.getAttachments(); @Nullable final Attachment screenshot = hints.getAs(SENTRY_SCREENSHOT, Attachment.class); if (screenshot != null) { diff --git a/sentry/src/main/java/io/sentry/hints/Attachments.java b/sentry/src/main/java/io/sentry/hints/Attachments.java deleted file mode 100644 index 4ff6b37755d..00000000000 --- a/sentry/src/main/java/io/sentry/hints/Attachments.java +++ /dev/null @@ -1,39 +0,0 @@ -package io.sentry.hints; - -import io.sentry.Attachment; -import java.util.List; -import java.util.concurrent.CopyOnWriteArrayList; -import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; - -public final class Attachments { - - private final @NotNull List internalStorage = new CopyOnWriteArrayList<>(); - - public void add(@Nullable Attachment attachment) { - if (attachment != null) { - internalStorage.add(attachment); - } - } - - @SuppressWarnings("unchecked") - public void addAll(@Nullable List attachments) { - if (attachments != null) { - internalStorage.addAll(attachments); - } - } - - @SuppressWarnings("unchecked") - public @NotNull List getAll() { - return new CopyOnWriteArrayList<>(internalStorage); - } - - public void replaceAll(@Nullable List attachments) { - clear(); - addAll(attachments); - } - - public void clear() { - internalStorage.clear(); - } -} diff --git a/sentry/src/main/java/io/sentry/hints/Hints.java b/sentry/src/main/java/io/sentry/hints/Hints.java index fce6fd20f96..5954f709d2e 100644 --- a/sentry/src/main/java/io/sentry/hints/Hints.java +++ b/sentry/src/main/java/io/sentry/hints/Hints.java @@ -6,6 +6,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.CopyOnWriteArrayList; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -15,13 +16,13 @@ public final class Hints { public static @NotNull Hints withAttachment(@Nullable Attachment attachment) { @NotNull final Hints hints = new Hints(); - hints.getAttachments().add(attachment); + hints.addAttachment(attachment); return hints; } public static @NotNull Hints withAttachments(@Nullable List attachments) { @NotNull final Hints hints = new Hints(); - hints.getAttachments().addAll(attachments); + hints.addAttachments(attachments); return hints; } @@ -48,16 +49,54 @@ public void remove(@NotNull String hintName) { internalStorage.remove(hintName); } - public @NotNull Attachments getAttachments() { - if (internalStorage.containsKey(SENTRY_ATTACHMENTS)) { - Attachments container = getAs(SENTRY_ATTACHMENTS, Attachments.class); - if (container != null) { - return container; - } + @SuppressWarnings("unchecked") + public @NotNull List getAttachments() { + List attachments = getAs(SENTRY_ATTACHMENTS, List.class); + if (attachments != null) { + return new CopyOnWriteArrayList<>(attachments); + } + + return new CopyOnWriteArrayList<>(); + } + + public void replaceAttachments(@Nullable List attachments) { + clearAttachments(); + addAttachments(attachments); + } + + public void clearAttachments() { + internalStorage.put(SENTRY_ATTACHMENTS, new CopyOnWriteArrayList()); + } + + @SuppressWarnings("unchecked") + public void addAttachment(@Nullable Attachment attachment) { + if (attachment == null) { + return; } - Attachments attachments = new Attachments(); + List existingAttachments = getAs(SENTRY_ATTACHMENTS, List.class); + if (existingAttachments != null) { + existingAttachments.add(attachment); + return; + } + + List attachments = new CopyOnWriteArrayList<>(); + attachments.add(attachment); internalStorage.put(SENTRY_ATTACHMENTS, attachments); - return attachments; + } + + @SuppressWarnings("unchecked") + public void addAttachments(@Nullable List attachments) { + if (attachments == null) { + return; + } + + List existingAttachments = getAs(SENTRY_ATTACHMENTS, List.class); + if (existingAttachments != null) { + existingAttachments.addAll(attachments); + return; + } + + internalStorage.put(SENTRY_ATTACHMENTS, new CopyOnWriteArrayList<>(attachments)); } } diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt index bd58f9e752c..c5662306bef 100644 --- a/sentry/src/test/java/io/sentry/SentryClientTest.kt +++ b/sentry/src/test/java/io/sentry/SentryClientTest.kt @@ -1618,8 +1618,8 @@ class SentryClientTest { fun `can add to attachments in beforeSend`() { val sut = fixture.getSut { options -> options.setBeforeSend { event, hints -> - assertEquals(listOf(fixture.attachment, fixture.attachment2), hints.attachments.all) - hints.attachments.add(fixture.attachment3) + assertEquals(listOf(fixture.attachment, fixture.attachment2), hints.attachments) + hints.addAttachment(fixture.attachment3) event } } @@ -1635,7 +1635,7 @@ class SentryClientTest { fun `can replace attachments in beforeSend`() { val sut = fixture.getSut { options -> options.setBeforeSend { event, hints -> - hints.attachments.replaceAll(listOf(fixture.attachment)) + hints.replaceAttachments(listOf(fixture.attachment)) event } } @@ -1652,8 +1652,8 @@ class SentryClientTest { val sut = fixture.getSut { options -> options.addEventProcessor(object : EventProcessor { override fun process(event: SentryEvent, hints: Hints): SentryEvent? { - assertEquals(listOf(fixture.attachment, fixture.attachment2), hints.attachments.all) - hints.attachments.add(fixture.attachment3) + assertEquals(listOf(fixture.attachment, fixture.attachment2), hints.attachments) + hints.addAttachment(fixture.attachment3) return event } @@ -1678,7 +1678,7 @@ class SentryClientTest { val sut = fixture.getSut { options -> options.addEventProcessor(object : EventProcessor { override fun process(event: SentryEvent, hints: Hints): SentryEvent? { - hints.attachments.replaceAll(listOf(fixture.attachment)) + hints.replaceAttachments(listOf(fixture.attachment)) return event } @@ -1740,8 +1740,8 @@ class SentryClientTest { transaction: SentryTransaction, hints: Hints ): SentryTransaction? { - assertEquals(listOf(fixture.attachment, fixture.attachment2), hints.attachments.all) - hints.attachments.add(fixture.attachment3) + assertEquals(listOf(fixture.attachment, fixture.attachment2), hints.attachments) + hints.addAttachment(fixture.attachment3) return transaction } }) @@ -1771,7 +1771,7 @@ class SentryClientTest { transaction: SentryTransaction, hints: Hints ): SentryTransaction? { - hints.attachments.replaceAll(listOf(fixture.attachment)) + hints.replaceAttachments(listOf(fixture.attachment)) return transaction } }) @@ -1809,7 +1809,7 @@ class SentryClientTest { fun `adding attachments in beforeBreadcrumb ignores them`() { val sut = fixture.getSut { options -> options.setBeforeBreadcrumb { breadcrumb, hints -> - hints.attachments.add(fixture.attachment) + hints.addAttachment(fixture.attachment) breadcrumb } } diff --git a/sentry/src/test/java/io/sentry/hints/AttachmentsTest.kt b/sentry/src/test/java/io/sentry/hints/AttachmentsTest.kt index 02cf8307601..54cd453f40a 100644 --- a/sentry/src/test/java/io/sentry/hints/AttachmentsTest.kt +++ b/sentry/src/test/java/io/sentry/hints/AttachmentsTest.kt @@ -8,50 +8,50 @@ class AttachmentsTest { @Test fun `can add an attachment`() { - val container = Attachments() + val hints = Hints() val attachment = newAttachment("test1") - container.add(attachment) + hints.addAttachment(attachment) - assertEquals(listOf(attachment), container.all) + assertEquals(listOf(attachment), hints.attachments) } @Test fun `can add multiple attachments`() { - val container = Attachments() + val attachments = Hints() val attachment1 = newAttachment("test1") val attachment2 = newAttachment("test2") - container.add(attachment1) - container.add(attachment2) + attachments.addAttachment(attachment1) + attachments.addAttachment(attachment2) - assertEquals(listOf(attachment1, attachment2), container.all) + assertEquals(listOf(attachment1, attachment2), attachments.attachments) } @Test fun `after reset list is empty`() { - val container = Attachments() + val hints = Hints() val attachment1 = newAttachment("test1") val attachment2 = newAttachment("test2") - container.add(attachment1) - container.add(attachment2) + hints.addAttachment(attachment1) + hints.addAttachment(attachment2) - container.clear() + hints.clearAttachments() - assertEquals(emptyList(), container.all) + assertEquals(emptyList(), hints.attachments) } @Test fun `after replace list contains only new item`() { - val container = Attachments() + val container = Hints() val attachment1 = newAttachment("test1") val attachment2 = newAttachment("test2") val attachment3 = newAttachment("test2") val attachment4 = newAttachment("test2") - container.add(attachment1) - container.add(attachment2) + container.addAttachment(attachment1) + container.addAttachment(attachment2) - container.replaceAll(listOf(attachment3, attachment4)) + container.replaceAttachments(listOf(attachment3, attachment4)) - assertEquals(listOf(attachment3, attachment4), container.all) + assertEquals(listOf(attachment3, attachment4), container.attachments) } companion object { diff --git a/sentry/src/test/java/io/sentry/hints/HintsTest.kt b/sentry/src/test/java/io/sentry/hints/HintsTest.kt index 5d332d089fa..313b94ab9d7 100644 --- a/sentry/src/test/java/io/sentry/hints/HintsTest.kt +++ b/sentry/src/test/java/io/sentry/hints/HintsTest.kt @@ -88,7 +88,7 @@ class HintsTest { fun `can create hints with attachment`() { val attachment = newAttachment("test1") val hints = Hints.withAttachment(attachment) - assertEquals(listOf(attachment), hints.attachments.all) + assertEquals(listOf(attachment), hints.attachments) } @Test @@ -96,6 +96,6 @@ class HintsTest { val attachment1 = newAttachment("test1") val attachment2 = newAttachment("test1") val hints = Hints.withAttachments(listOf(attachment1, attachment2)) - assertEquals(listOf(attachment1, attachment2), hints.attachments.all) + assertEquals(listOf(attachment1, attachment2), hints.attachments) } }