Skip to content

Commit d167cb0

Browse files
committed
Apply review suggestions
1 parent 1e7f10e commit d167cb0

8 files changed

Lines changed: 32 additions & 26 deletions

File tree

dd-sdk-android-core/api/apiSurface

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ interface com.datadog.android.api.instrumentation.network.HttpResponseInfo
177177
val headers: Map<String, List<String>>
178178
val contentType: String?
179179
val contentLength: Long?
180-
val request: HttpRequestInfo
180+
val request: HttpRequestInfo?
181181
interface com.datadog.android.api.instrumentation.network.MutableHttpRequestInfo
182182
fun newBuilder(): HttpRequestInfoBuilder
183183
data class com.datadog.android.api.net.Request

dd-sdk-android-core/src/main/kotlin/com/datadog/android/api/instrumentation/network/HttpResponseInfo.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ interface HttpResponseInfo {
4040
val contentLength: Long?
4141

4242
/**
43-
* Represents the HTTP request associated with the response.
43+
* Represents the HTTP request associated with the response, or null when not available.
4444
*/
45-
val request: HttpRequestInfo
45+
val request: HttpRequestInfo?
4646
}

integrations/dd-sdk-android-cronet/src/main/kotlin/com/datadog/android/cronet/internal/CronetHttpResponseInfo.kt

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,13 @@
55
*/
66
package com.datadog.android.cronet.internal
77

8-
import com.datadog.android.api.instrumentation.network.HttpRequestInfo
98
import com.datadog.android.api.instrumentation.network.HttpResponseInfo
109
import com.datadog.android.internal.network.HttpSpec
1110
import org.chromium.net.UrlResponseInfo
1211

1312
internal data class CronetHttpResponseInfo(
1413
private val response: UrlResponseInfo,
15-
override val request: HttpRequestInfo
14+
override val request: CronetHttpRequestInfo?
1615
) : HttpResponseInfo {
1716

1817
override val url: String get() = response.url

integrations/dd-sdk-android-cronet/src/main/kotlin/com/datadog/android/cronet/internal/CronetRequestCallback.kt

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -102,25 +102,25 @@ internal class CronetRequestCallback(
102102
) = delegate.onReadCompleted(request, info, byteBuffer)
103103

104104
private fun finishSuccessRequestTracing(response: UrlResponseInfo?) {
105-
val requestInfo = apmTracingStateHolder.get()?.createRequestInfo()
106-
?: distributedTracingStateHolder.get()?.createRequestInfo()
107-
val responseInfo = if (response != null && requestInfo != null) {
108-
CronetHttpResponseInfo(response, requestInfo)
109-
} else {
110-
null
111-
}
112-
113105
apmTracingStateHolder.getAndSet(null)?.let { apmTracingState ->
114-
if (responseInfo != null) {
115-
apmNetworkInstrumentation?.onResponseSucceeded(apmTracingState, responseInfo)
106+
if (response != null) {
107+
val requestInfo = apmTracingState.createRequestInfo() as? CronetHttpRequestInfo
108+
apmNetworkInstrumentation?.onResponseSucceeded(
109+
apmTracingState,
110+
CronetHttpResponseInfo(response, requestInfo)
111+
)
116112
}
117113
} ?: apmNetworkInstrumentation?.reportInstrumentationError {
118114
"Request tracing state not found for ${response?.url}. Instrumentation may be broken."
119115
}
120116

121117
distributedTracingStateHolder.getAndSet(null)?.let { distributedTracingState ->
122-
if (responseInfo != null) {
123-
distributedTracingInstrumentation?.onResponseSucceeded(distributedTracingState, responseInfo)
118+
if (response != null) {
119+
val requestInfo = distributedTracingState.createRequestInfo() as? CronetHttpRequestInfo
120+
distributedTracingInstrumentation?.onResponseSucceeded(
121+
distributedTracingState,
122+
CronetHttpResponseInfo(response, requestInfo)
123+
)
124124
}
125125
}
126126
}

integrations/dd-sdk-android-cronet/src/main/kotlin/com/datadog/android/cronet/internal/CronetRequestFinishedInfoListener.kt

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
package com.datadog.android.cronet.internal
77

88
import androidx.annotation.WorkerThread
9-
import com.datadog.android.api.instrumentation.network.HttpRequestInfo
109
import com.datadog.android.rum.RumAttributes
1110
import com.datadog.android.rum.internal.domain.event.ResourceTiming
1211
import com.datadog.android.rum.internal.net.RumNetworkInstrumentation
@@ -25,7 +24,7 @@ internal class CronetRequestFinishedInfoListener(
2524

2625
@WorkerThread
2726
override fun onRequestFinished(finishedInfo: RequestFinishedInfo) {
28-
val requestInfo = finishedInfo.annotations?.filterIsInstance<HttpRequestInfo>()?.firstOrNull()
27+
val requestInfo = finishedInfo.annotations?.filterIsInstance<CronetHttpRequestInfo>()?.firstOrNull()
2928
val distributingTracingState = finishedInfo.annotations?.filterIsInstance<RequestTracingState>()?.firstOrNull()
3029
if (requestInfo == null) {
3130
rumNetworkInstrumentation.reportInstrumentationError {

integrations/dd-sdk-android-cronet/src/test/kotlin/com/datadog/android/cronet/internal/CronetRequestCallbackTest.kt

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,6 @@ internal class CronetRequestCallbackTest {
388388
// Given
389389
val mockDistributedTracingState: RequestTracingState = mock()
390390
whenever(mockDistributedTracingInstrumentation.onRequest(fakeRequestInfo)) doReturn mockDistributedTracingState
391-
whenever(mockDistributedTracingState.createRequestInfo()) doReturn mockMutableRequestInfo
392391
testedCallback.onRequestStarted(fakeRequestInfo)
393392

394393
// When
@@ -477,7 +476,6 @@ internal class CronetRequestCallbackTest {
477476
// Given
478477
val mockDistributedTracingState: RequestTracingState = mock()
479478
whenever(mockDistributedTracingInstrumentation.onRequest(fakeRequestInfo)) doReturn mockDistributedTracingState
480-
whenever(mockDistributedTracingState.createRequestInfo()) doReturn mockMutableRequestInfo
481479
testedCallback.onRequestStarted(fakeRequestInfo)
482480

483481
// When

integrations/dd-sdk-android-cronet/src/test/kotlin/com/datadog/android/cronet/internal/CronetRequestFinishedInfoListenerTest.kt

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
*/
66
package com.datadog.android.cronet.internal
77

8-
import com.datadog.android.api.instrumentation.network.HttpRequestInfo
98
import com.datadog.android.cronet.internal.CronetRequestFinishedInfoListener.Companion.minus
109
import com.datadog.android.rum.internal.domain.event.ResourceTiming
1110
import com.datadog.android.rum.internal.net.RumNetworkInstrumentation
@@ -54,8 +53,8 @@ internal class CronetRequestFinishedInfoListenerTest {
5453
@Mock
5554
private lateinit var mockResponseInfo: UrlResponseInfo
5655

57-
@Forgery
58-
private lateinit var fakeRequestInfo: HttpRequestInfo
56+
@Mock
57+
private lateinit var fakeRequestInfo: CronetHttpRequestInfo
5958

6059
private lateinit var testedListener: CronetRequestFinishedInfoListener
6160

integrations/dd-sdk-android-cronet/src/test/kotlin/com/datadog/android/cronet/internal/CronetResponseInfoTest.kt

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66

77
package com.datadog.android.cronet.internal
88

9-
import com.datadog.android.api.instrumentation.network.HttpRequestInfo
109
import com.datadog.android.internal.network.HttpSpec
1110
import com.datadog.android.utils.forge.Configurator
1211
import com.datadog.tools.unit.forge.exhaustiveAttributes
@@ -40,7 +39,7 @@ internal class CronetResponseInfoTest {
4039
lateinit var mockUrlResponseInfo: UrlResponseInfo
4140

4241
@Mock
43-
lateinit var mockRequestInfo: HttpRequestInfo
42+
lateinit var mockRequestInfo: CronetHttpRequestInfo
4443

4544
lateinit var fakeHeaders: Map<String, List<String>>
4645

@@ -134,6 +133,18 @@ internal class CronetResponseInfoTest {
134133
assertThat(result).isSameAs(mockRequestInfo)
135134
}
136135

136+
@Test
137+
fun `M return null W request property { null request }`() {
138+
// Given
139+
val responseInfo = CronetHttpResponseInfo(mockUrlResponseInfo, null)
140+
141+
// When
142+
val result = responseInfo.request
143+
144+
// Then
145+
assertThat(result).isNull()
146+
}
147+
137148
@Test
138149
fun `M return content length from header W computeContentLength() { header present }`(
139150
@LongForgery(min = 1) fakeContentLength: Long

0 commit comments

Comments
 (0)