Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multi cast upstream response for Chucker consumption. #267

Merged
merged 19 commits into from
Mar 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ interceptor.redactHeader("Auth-Token", "User-Session");

### Skip-Inspection οΈπŸ•΅οΈ

If you need to selectively skip Chucker inspection on some endpoints or on particular requests you can add a special header - `Skip-ChuckerInterceptor: true`. This will inform Chucker to not process this request. Chucker will also strip this header from any request before sending it to a server.
If you need to selectively skip Chucker inspection on some endpoints or on particular requests you can add a special header - `Skip-Chucker-Interceptor: true`. This will inform Chucker to not process this request. Chucker will also strip this header from any request before sending it to a server.

If you use `OkHttp` directly, create requests like below.

Expand Down
137 changes: 103 additions & 34 deletions library/src/main/java/com/chuckerteam/chucker/api/ChuckerInterceptor.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,14 @@ package com.chuckerteam.chucker.api

import android.content.Context
import com.chuckerteam.chucker.internal.data.entity.HttpTransaction
import com.chuckerteam.chucker.internal.support.AndroidCacheFileFactory
import com.chuckerteam.chucker.internal.support.FileFactory
import com.chuckerteam.chucker.internal.support.IOUtils
import com.chuckerteam.chucker.internal.support.TeeSource
import com.chuckerteam.chucker.internal.support.contentLength
import com.chuckerteam.chucker.internal.support.contentType
import com.chuckerteam.chucker.internal.support.isGzipped
import java.io.File
import java.io.IOException
import java.nio.charset.Charset
import okhttp3.Headers
Expand All @@ -15,8 +19,7 @@ import okhttp3.Response
import okhttp3.ResponseBody
import okio.Buffer
import okio.GzipSource

private const val MAX_BLOB_SIZE = 1000_000L
import okio.Okio

/**
* An OkHttp Interceptor which persists and displays HTTP activity
Expand All @@ -27,16 +30,39 @@ private const val MAX_BLOB_SIZE = 1000_000L
* @param maxContentLength The maximum length for request and response content
* before their truncation. Warning: setting this value too high may cause unexpected
* results.
* @param fileFactory Provider for [File]s where Chucker will save temporary responses before
* processing them.
* @param headersToRedact a [Set] of headers you want to redact. They will be replaced
* with a `**` in the Chucker UI.
*/
class ChuckerInterceptor @JvmOverloads constructor(
class ChuckerInterceptor internal constructor(
private val context: Context,
private val collector: ChuckerCollector = ChuckerCollector(context),
private val maxContentLength: Long = 250000L,
private val fileFactory: FileFactory,
headersToRedact: Set<String> = emptySet()
) : Interceptor {

/**
* An OkHttp Interceptor which persists and displays HTTP activity
* in your application for later inspection.
*
* @param context An Android [Context]
* @param collector A [ChuckerCollector] to customize data retention
* @param maxContentLength The maximum length for request and response content
* before their truncation. Warning: setting this value too high may cause unexpected
* results.
* @param headersToRedact a [Set] of headers you want to redact. They will be replaced
* with a `**` in the Chucker UI.
*/
@JvmOverloads
constructor(
context: Context,
collector: ChuckerCollector = ChuckerCollector(context),
maxContentLength: Long = 250000L,
headersToRedact: Set<String> = emptySet()
) : this(context, collector, maxContentLength, AndroidCacheFileFactory(context), headersToRedact)

private val io: IOUtils = IOUtils(context)
private val headersToRedact: MutableSet<String> = headersToRedact.toMutableSet()

Expand Down Expand Up @@ -69,10 +95,8 @@ class ChuckerInterceptor @JvmOverloads constructor(
throw e
}

val processedResponse = processResponse(response, transaction)
collector.onResponseReceived(transaction)

return processedResponse
processResponseMetadata(response, transaction)
return multiCastResponseBody(response, transaction)
}

/**
Expand Down Expand Up @@ -113,9 +137,12 @@ class ChuckerInterceptor @JvmOverloads constructor(
}

/**
* Processes a [Response] and populates corresponding fields of a [HttpTransaction].
* Processes [Response] metadata and populates corresponding fields of a [HttpTransaction].
*/
private fun processResponse(response: Response, transaction: HttpTransaction): Response {
private fun processResponseMetadata(
response: Response,
transaction: HttpTransaction
) {
val responseEncodingIsSupported = io.bodyHasSupportedEncoding(response.headers().get(CONTENT_ENCODING))

transaction.apply {
Expand All @@ -140,50 +167,63 @@ class ChuckerInterceptor @JvmOverloads constructor(

tookMs = (response.receivedResponseAtMillis() - response.sentRequestAtMillis())
}

return if (responseEncodingIsSupported) {
processResponseBody(response, transaction)
} else {
response
}
}

/**
* Processes a [ResponseBody] and populates corresponding fields of a [HttpTransaction].
* Multi casts a [Response] body if it is available and downstreams it to a file which will
* be available for Chucker to consume and save in the [transaction] at some point in the future
* when the end user reads bytes form the [response].
*/
private fun processResponseBody(response: Response, transaction: HttpTransaction): Response {
val responseBody = response.body() ?: return response
private fun multiCastResponseBody(
response: Response,
transaction: HttpTransaction
): Response {
val responseBody = response.body()
if (responseBody == null) {
collector.onResponseReceived(transaction)
return response
}

val contentType = responseBody.contentType()
val charset = contentType?.charset(UTF8) ?: UTF8
val contentLength = responseBody.contentLength()

val responseSource = if (response.isGzipped) {
GzipSource(responseBody.source())
} else {
responseBody.source()
}
val buffer = Buffer().apply { responseSource.use { writeAll(it) } }
val teeSource = TeeSource(
responseBody.source(),
fileFactory.create(),
ChuckerTransactionTeeCallback(response, transaction),
maxContentLength
)

return response.newBuilder()
.body(ResponseBody.create(contentType, contentLength, Okio.buffer(teeSource)))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I insist on returning the original Response object further in chain without any builders, cloning, etc.
We had enough side effects and issues with providing processed or somehow changed objects.
And while I see that here we should get the same object I still don't like that it is recreated.
In some issues we already mentioned that Chucker should be as transparent as possible and this is possible only if we do copy only for Chucker processing without anything like that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see how it would be possible to stream original bytes to two sinks at the same time without replacing the response. This is a mechanism that is used under the hood by OkHttp for caching (just a simpler version). It is also recommended way by Square (square/okio#186). Do you have any other approach in mind that would solve the problems addressed by this?

.build()
}

private fun processResponseBody(
response: Response,
responseBodyBuffer: Buffer,
transaction: HttpTransaction
) {
val responseBody = response.body() ?: return

if (io.isPlaintext(buffer)) {
val contentType = responseBody.contentType()
val charset = contentType?.charset(UTF8) ?: UTF8

if (io.isPlaintext(responseBodyBuffer)) {
transaction.isResponseBodyPlainText = true
if (contentLength != 0L) {
transaction.responseBody = buffer.clone().readString(charset)
if (responseBodyBuffer.size() != 0L) {
transaction.responseBody = responseBodyBuffer.readString(charset)
}
} else {
transaction.isResponseBodyPlainText = false

val isImageContentType =
(contentType?.toString()?.contains(CONTENT_TYPE_IMAGE, ignoreCase = true) == true)

if (isImageContentType && buffer.size() < MAX_BLOB_SIZE) {
transaction.responseImageData = buffer.clone().readByteArray()
if (isImageContentType && (responseBodyBuffer.size() < MAX_BLOB_SIZE)) {
transaction.responseImageData = responseBodyBuffer.readByteArray()
}
}

return response.newBuilder()
.body(ResponseBody.create(contentType, contentLength, buffer))
.build()
}

/** Overrides all headers from [headersToRedact] with `**` */
Expand All @@ -197,6 +237,35 @@ class ChuckerInterceptor @JvmOverloads constructor(
return builder.build()
}

private inner class ChuckerTransactionTeeCallback(
private val response: Response,
private val transaction: HttpTransaction
) : TeeSource.Callback {
override fun onSuccess(file: File) {
val buffer = readResponseBuffer(file, response.isGzipped)
file.delete()
processResponseBody(response, buffer, transaction)
collector.onResponseReceived(transaction)
}

override fun onFailure(exception: IOException, file: File) {
file.delete()
collector.onResponseReceived(transaction)
}

private fun readResponseBuffer(responseBody: File, isGzipped: Boolean): Buffer {
val bufferedSource = Okio.buffer(Okio.source(responseBody))
val source = if (isGzipped) {
GzipSource(bufferedSource)
} else {
bufferedSource
}
return Buffer().apply {
writeAll(source)
}
}
}

companion object {
private val UTF8 = Charset.forName("UTF-8")

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package com.chuckerteam.chucker.internal.support

import android.content.Context
import java.io.File
import java.util.concurrent.atomic.AtomicLong

internal class AndroidCacheFileFactory(
context: Context
) : FileFactory {
private val fileDir = context.cacheDir
private val uniqueIdGenerator = AtomicLong()

override fun create(): File {
return File(fileDir, "chucker-${uniqueIdGenerator.getAndIncrement()}")
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package com.chuckerteam.chucker.internal.support

import java.io.File

internal interface FileFactory {
fun create(): File
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package com.chuckerteam.chucker.internal.support

import java.io.File
import java.io.IOException
import okio.Buffer
import okio.Okio
import okio.Source
import okio.Timeout

/**
* A source that acts as a tee operator - https://en.wikipedia.org/wiki/Tee_(command).
*
* It takes the input [upstream] and reads from it serving the bytes to the end consumer
* like a regular [Source]. While bytes are read from the [upstream] the are also copied
* to a [sideChannel] file. After the [upstream] is depleted or when a failure occurs
* an appropriate [callback] method is called.
*
* Failure is considered any [IOException] during reading the bytes or exceeding [readBytesLimit] length.
*/
internal class TeeSource(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please leave some comments around as otherwise this code will harder to maintain.
For example explaining that you're writing a special prefix on every file, etc...

private val upstream: Source,
private val sideChannel: File,
private val callback: Callback,
private val readBytesLimit: Long = Long.MAX_VALUE
) : Source {
private val sideStream = Okio.buffer(Okio.sink(sideChannel))
private var totalBytesRead = 0L
private var reachedLimit = false
private var upstreamFailed = false

override fun read(sink: Buffer, byteCount: Long): Long {
val bytesRead = try {
upstream.read(sink, byteCount)
} catch (e: IOException) {
callSideChannelFailure(e)
throw e
}

if (bytesRead == -1L) {
sideStream.close()
return -1L
}

totalBytesRead += bytesRead
if (!reachedLimit && (totalBytesRead <= readBytesLimit)) {
val offset = sink.size() - bytesRead
sink.copyTo(sideStream.buffer(), offset, bytesRead)
sideStream.emitCompleteSegments()
return bytesRead
}
if (!reachedLimit) {
reachedLimit = true
sideStream.close()
callSideChannelFailure(IOException("Capacity of $readBytesLimit bytes exceeded"))
}

return bytesRead
}

override fun close() {
sideStream.close()
upstream.close()
if (!upstreamFailed) {
callback.onSuccess(sideChannel)
}
}

override fun timeout(): Timeout = upstream.timeout()

private fun callSideChannelFailure(exception: IOException) {
if (!upstreamFailed) {
upstreamFailed = true
callback.onFailure(exception, sideChannel)
}
}

interface Callback {
/**
* Called when the upstream was successfully copied to the [file].
*/
fun onSuccess(file: File)

/**
* Called when there was an issue while copying bytes to the [file].
*
* It might occur due to an exception thrown while reading bytes or due to exceeding capacity limit.
*/
fun onFailure(exception: IOException, file: File)
}
Comment on lines +77 to +89
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to go with a callback instead of magic headers. It gives type safety and is more extensible. In the future it is possible to add onContentLengthAvailable(length: Long) method in order to resolve #209.

}
Loading