From 2cc2d080691e1b794bbfa4ce4b49e072c4bae2dd Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Thu, 18 Feb 2021 15:07:08 +0100 Subject: [PATCH 1/3] Add Request to the Scope. --- sentry/src/main/java/io/sentry/Scope.java | 22 +++++++++++++++++++ .../src/main/java/io/sentry/SentryClient.java | 3 +++ .../test/java/io/sentry/SentryClientTest.kt | 6 +++++ 3 files changed, 31 insertions(+) diff --git a/sentry/src/main/java/io/sentry/Scope.java b/sentry/src/main/java/io/sentry/Scope.java index cf7205882a..d88f15a9b3 100644 --- a/sentry/src/main/java/io/sentry/Scope.java +++ b/sentry/src/main/java/io/sentry/Scope.java @@ -1,6 +1,7 @@ package io.sentry; import io.sentry.protocol.Contexts; +import io.sentry.protocol.Request; import io.sentry.protocol.User; import io.sentry.util.Objects; import java.util.ArrayList; @@ -29,6 +30,9 @@ public final class Scope implements Cloneable { /** Scope's user */ private @Nullable User user; + /** Scope's request */ + private @Nullable Request request; + /** Scope's fingerprint */ private @NotNull List fingerprint = new ArrayList<>(); @@ -163,6 +167,24 @@ public void setUser(final @Nullable User user) { } } + /** + * Returns the Scope's request + * + * @return the request + */ + public @Nullable Request getRequest() { + return request; + } + + /** + * Sets the Scope's request + * + * @param request the request + */ + public void setRequest(final @Nullable Request request) { + this.request = request; + } + /** * Returns the Scope's fingerprint list * diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index 46b071a45b..9979eae28f 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -421,6 +421,9 @@ public void captureSession(final @NotNull Session session, final @Nullable Objec if (event.getUser() == null) { event.setUser(scope.getUser()); } + if (event.getRequest() == null) { + event.setRequest(scope.getRequest()); + } if (event.getFingerprints() == null) { event.setFingerprints(scope.getFingerprint()); } diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt index 8a3f97a837..71b78c4b70 100644 --- a/sentry/src/test/java/io/sentry/SentryClientTest.kt +++ b/sentry/src/test/java/io/sentry/SentryClientTest.kt @@ -262,6 +262,9 @@ class SentryClientTest { assertEquals("fp", event.fingerprints[0]) assertEquals("id", event.user.id) assertEquals(SentryLevel.FATAL, event.level) + assertNotNull(event.request) { + assertEquals("post", it.method) + } } @Test @@ -855,6 +858,9 @@ class SentryClientTest { user = User().apply { id = "id" } + request = Request().apply { + method = "post" + } } } From ac0da4f19378cc125e65ae705cba6043f25c433a Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Thu, 18 Feb 2021 15:10:46 +0100 Subject: [PATCH 2/3] Changelog. --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 35c959ac08..6690ea58f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ * Fix: Make the ANR Atomic flags immutable * Enchancement: Integration interface better compatibility with Kotlin null-safety * Enchancement: Simplify Sentry configuration in Spring integration (#1259) +* Enchancement: Add Request to the Scope. #1270 Breaking Changes: * Enchancement: SentryExceptionResolver should not send handled errors by default (#1248). From 029fab18c7d4a2158f8e642f9807b2344fe840f2 Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Thu, 18 Feb 2021 17:34:37 +0100 Subject: [PATCH 3/3] Include Request in Scope clone and clear. --- sentry/api/sentry.api | 6 +- sentry/src/main/java/io/sentry/Scope.java | 4 + .../main/java/io/sentry/protocol/Request.java | 35 +++++++- sentry/src/test/java/io/sentry/ScopeTest.kt | 32 +++++++- .../java/io/sentry/protocol/RequestTest.kt | 81 +++++++++++++++++++ 5 files changed, 154 insertions(+), 4 deletions(-) create mode 100644 sentry/src/test/java/io/sentry/protocol/RequestTest.kt diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 0fdce7aacd..9d5109282e 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -436,6 +436,7 @@ public final class io/sentry/Scope : java/lang/Cloneable { public synthetic fun clone ()Ljava/lang/Object; public fun getContexts ()Lio/sentry/protocol/Contexts; public fun getLevel ()Lio/sentry/SentryLevel; + public fun getRequest ()Lio/sentry/protocol/Request; public fun getSpan ()Lio/sentry/ISpan; public fun getTransaction ()Lio/sentry/ITransaction; public fun getTransactionName ()Ljava/lang/String; @@ -450,6 +451,7 @@ public final class io/sentry/Scope : java/lang/Cloneable { public fun setExtra (Ljava/lang/String;Ljava/lang/String;)V public fun setFingerprint (Ljava/util/List;)V public fun setLevel (Lio/sentry/SentryLevel;)V + public fun setRequest (Lio/sentry/protocol/Request;)V public fun setTag (Ljava/lang/String;Ljava/lang/String;)V public fun setTransaction (Lio/sentry/ITransaction;)V public fun setTransaction (Ljava/lang/String;)V @@ -1463,9 +1465,11 @@ public final class io/sentry/protocol/OperatingSystem : io/sentry/IUnknownProper public fun setVersion (Ljava/lang/String;)V } -public final class io/sentry/protocol/Request : io/sentry/IUnknownPropertiesConsumer { +public final class io/sentry/protocol/Request : io/sentry/IUnknownPropertiesConsumer, java/lang/Cloneable { public fun ()V public fun acceptUnknownProperties (Ljava/util/Map;)V + public fun clone ()Lio/sentry/protocol/Request; + public synthetic fun clone ()Ljava/lang/Object; public fun getCookies ()Ljava/lang/String; public fun getData ()Ljava/lang/Object; public fun getEnvs ()Ljava/util/Map; diff --git a/sentry/src/main/java/io/sentry/Scope.java b/sentry/src/main/java/io/sentry/Scope.java index d88f15a9b3..d5f56bbb19 100644 --- a/sentry/src/main/java/io/sentry/Scope.java +++ b/sentry/src/main/java/io/sentry/Scope.java @@ -309,6 +309,7 @@ public void clear() { level = null; transaction = null; user = null; + request = null; fingerprint.clear(); breadcrumbs.clear(); tags.clear(); @@ -511,6 +512,9 @@ public void addAttachment(final @NotNull Attachment attachment) { final User userRef = user; clone.user = userRef != null ? userRef.clone() : null; + final Request requestRef = request; + clone.request = requestRef != null ? requestRef.clone() : null; + clone.fingerprint = new ArrayList<>(fingerprint); clone.eventProcessors = new CopyOnWriteArrayList<>(eventProcessors); diff --git a/sentry/src/main/java/io/sentry/protocol/Request.java b/sentry/src/main/java/io/sentry/protocol/Request.java index 6a26ea7e14..42b6aadb26 100644 --- a/sentry/src/main/java/io/sentry/protocol/Request.java +++ b/sentry/src/main/java/io/sentry/protocol/Request.java @@ -1,8 +1,12 @@ package io.sentry.protocol; import io.sentry.IUnknownPropertiesConsumer; +import io.sentry.util.CollectionUtils; import java.util.Map; import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.TestOnly; /** * Http request information. @@ -40,7 +44,7 @@ * csrftoken=u32t4o3tb3gg43; _gat=1;", "headers": { "content-type": "text/html" }, "env": { * "REMOTE_ADDR": "192.168.0.1" } } } ``` */ -public final class Request implements IUnknownPropertiesConsumer { +public final class Request implements Cloneable, IUnknownPropertiesConsumer { /** * The URL of the request if available. * @@ -159,9 +163,38 @@ public void setOthers(Map other) { this.other = other; } + /** + * the Request's unknown fields + * + * @return the unknown map + */ + @TestOnly + @Nullable + Map getUnknown() { + return unknown; + } + @ApiStatus.Internal @Override public void acceptUnknownProperties(Map unknown) { this.unknown = unknown; } + + /** + * Clones an User aka deep copy + * + * @return the cloned User + * @throws CloneNotSupportedException if the User is not cloneable + */ + @Override + public @NotNull Request clone() throws CloneNotSupportedException { + final Request clone = (Request) super.clone(); + + clone.headers = CollectionUtils.shallowCopy(headers); + clone.env = CollectionUtils.shallowCopy(env); + clone.other = CollectionUtils.shallowCopy(other); + clone.unknown = CollectionUtils.shallowCopy(unknown); + + return clone; + } } diff --git a/sentry/src/test/java/io/sentry/ScopeTest.kt b/sentry/src/test/java/io/sentry/ScopeTest.kt index c4ac15130f..1ddc465f6f 100644 --- a/sentry/src/test/java/io/sentry/ScopeTest.kt +++ b/sentry/src/test/java/io/sentry/ScopeTest.kt @@ -5,6 +5,7 @@ import com.nhaarman.mockitokotlin2.eq import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.never import com.nhaarman.mockitokotlin2.verify +import io.sentry.protocol.Request import io.sentry.protocol.User import io.sentry.test.callMethod import java.util.concurrent.CopyOnWriteArrayList @@ -33,6 +34,18 @@ class ScopeTest { scope.user = user + val request = Request() + request.method = "post" + request.cookies = "cookies" + request.data = "cookies" + request.envs = mapOf("env" to "value") + request.headers = mapOf("header" to "value") + request.others = mapOf("other" to "value") + request.queryString = "?foo=bar" + request.url = "http://localhost:8080/url" + + scope.request = request + val fingerprints = mutableListOf("abc", "def") scope.fingerprint = fingerprints @@ -56,6 +69,7 @@ class ScopeTest { assertNotNull(clone) assertNotSame(scope, clone) assertNotSame(scope.user, clone.user) + assertNotSame(scope.request, clone.request) assertNotSame(scope.contexts, clone.contexts) assertNotSame(scope.fingerprint, clone.fingerprint) assertNotSame(scope.breadcrumbs, clone.breadcrumbs) @@ -73,9 +87,12 @@ class ScopeTest { val user = User() user.id = "123" - scope.user = user + val request = Request() + request.method = "get" + scope.request = request + val fingerprints = mutableListOf("abc") scope.fingerprint = fingerprints @@ -98,6 +115,8 @@ class ScopeTest { assertEquals("123", clone.user?.id) + assertEquals("get", clone.request?.method) + assertEquals("abc", clone.fingerprint.first()) assertEquals("message", clone.breadcrumbs.first().message) @@ -123,9 +142,12 @@ class ScopeTest { val user = User() user.id = "123" - scope.user = user + val request = Request() + request.method = "get" + scope.request = request + val fingerprints = mutableListOf("abc") scope.fingerprint = fingerprints @@ -146,6 +168,7 @@ class ScopeTest { scope.level = SentryLevel.FATAL user.id = "456" + request.method = "post" scope.setTransaction(SentryTransaction("newTransaction", "op")) @@ -166,6 +189,8 @@ class ScopeTest { assertEquals("123", clone.user?.id) + assertEquals("get", clone.request?.method) + assertEquals("abc", clone.fingerprint.first()) assertEquals(1, clone.fingerprint.size) @@ -191,6 +216,7 @@ class ScopeTest { scope.level = SentryLevel.WARNING scope.setTransaction(SentryTransaction("", "op")) scope.user = User() + scope.request = Request() scope.fingerprint = mutableListOf("finger") scope.addBreadcrumb(Breadcrumb()) scope.setTag("some", "tag") @@ -202,6 +228,8 @@ class ScopeTest { assertNull(scope.level) assertNull(scope.transaction) + assertNull(scope.user) + assertNull(scope.request) assertEquals(0, scope.fingerprint.size) assertEquals(0, scope.breadcrumbs.size) assertEquals(0, scope.tags.size) diff --git a/sentry/src/test/java/io/sentry/protocol/RequestTest.kt b/sentry/src/test/java/io/sentry/protocol/RequestTest.kt new file mode 100644 index 0000000000..2d550709a9 --- /dev/null +++ b/sentry/src/test/java/io/sentry/protocol/RequestTest.kt @@ -0,0 +1,81 @@ +package io.sentry.protocol + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNotSame +import kotlin.test.assertNull + +class RequestTest { + @Test + fun `cloning request wont have the same references`() { + val request = createRequest() + val clone = request.clone() + + assertNotNull(clone) + assertNotSame(request, clone) + + assertNotSame(request.others, clone.others) + + assertNotSame(request.unknown, clone.unknown) + } + + @Test + fun `cloning request will have the same values`() { + val request = createRequest() + val clone = request.clone() + + assertEquals("get", clone.method) + assertEquals("http://localhost:8080", clone.url) + assertEquals("?foo=bar", clone.queryString) + assertEquals("envs", clone.envs!!["envs"]) + assertEquals("others", clone.others!!["others"]) + assertEquals("unknown", clone.unknown!!["unknown"]) + } + + @Test + fun `cloning request and changing the original values wont change the clone values`() { + val request = createRequest() + val clone = request.clone() + + request.method = "post" + request.url = "http://another-host:8081/" + request.queryString = "?xxx=yyy" + request.envs!!["envs"] = "newEnvs" + request.others!!["others"] = "newOthers" + request.others!!["anotherOne"] = "anotherOne" + val newUnknown = mapOf(Pair("unknown", "newUnknown"), Pair("otherUnknown", "otherUnknown")) + request.acceptUnknownProperties(newUnknown) + + assertEquals("get", clone.method) + assertEquals("http://localhost:8080", clone.url) + assertEquals("?foo=bar", clone.queryString) + assertEquals("envs", clone.envs!!["envs"]) + assertEquals(1, clone.envs!!.size) + assertEquals("others", clone.others!!["others"]) + assertEquals(1, clone.others!!.size) + assertEquals("unknown", clone.unknown!!["unknown"]) + assertEquals(1, clone.unknown!!.size) + } + + @Test + fun `setting null others do not crash`() { + val request = createRequest() + request.others = null + + assertNull(request.others) + } + + private fun createRequest(): Request { + return Request().apply { + method = "get" + url = "http://localhost:8080" + queryString = "?foo=bar" + envs = mutableMapOf(Pair("envs", "envs")) + val others = mutableMapOf(Pair("others", "others")) + setOthers(others) + val unknown = mapOf(Pair("unknown", "unknown")) + acceptUnknownProperties(unknown) + } + } +}