From b82f7a4943462df0cb96f04f6ebe43793eb6e3fc Mon Sep 17 00:00:00 2001 From: "Aleksei.Tirman" Date: Thu, 19 Dec 2024 18:15:44 +0200 Subject: [PATCH] Refactoring --- .../io/ktor/client/plugins/logging/Logging.kt | 187 +++++++++--------- .../client/plugins/logging/NewFormatTest.kt | 55 +++++- 2 files changed, 146 insertions(+), 96 deletions(-) diff --git a/ktor-client/ktor-client-plugins/ktor-client-logging/common/src/io/ktor/client/plugins/logging/Logging.kt b/ktor-client/ktor-client-plugins/ktor-client-logging/common/src/io/ktor/client/plugins/logging/Logging.kt index b9c7964eff..f5bcadda70 100644 --- a/ktor-client/ktor-client-plugins/ktor-client-logging/common/src/io/ktor/client/plugins/logging/Logging.kt +++ b/ktor-client/ktor-client-plugins/ktor-client-logging/common/src/io/ktor/client/plugins/logging/Logging.kt @@ -129,75 +129,71 @@ public val Logging: ClientPlugin = createClientPlugin("Logging", if (level == LogLevel.NONE) return val uri = URLBuilder().takeFrom(request.url).build().pathQuery() + val body = request.body + val headers = HeadersBuilder().apply { + if (body is OutgoingContent && request.method != HttpMethod.Get && body !is EmptyContent) { + body.contentType?.let { + appendIfNameAbsent(HttpHeaders.ContentType, it.toString()) + } + body.contentLength?.let { + appendIfNameAbsent(HttpHeaders.ContentLength, it.toString()) + } + } + appendAll(request.headers) + }.build() - if (request.method == HttpMethod.Get) { - logger.log("--> ${request.method.value} $uri") - } else { - val headers = HeadersBuilder().apply { appendAll(request.headers) }.build() + val contentLength = headers[HttpHeaders.ContentLength]?.toLongOrNull() + val startLine = when { + (request.method == HttpMethod.Get) + || ((level.headers || level.body) && contentLength != null) + || (level.headers && contentLength == null) + || headers.contains(HttpHeaders.ContentEncoding) -> "--> ${request.method.value} $uri" - val body = request.body - var contentLength = headers[HttpHeaders.ContentLength]?.toLongOrNull() - if (contentLength == null && request.method != HttpMethod.Get && body is OutgoingContent && body.contentLength != null) { - contentLength = body.contentLength - } + level.info && contentLength != null -> "--> ${request.method.value} $uri ($contentLength-byte body)" + + body is OutgoingContent.WriteChannelContent + || body is OutgoingContent.ReadChannelContent -> "--> ${request.method.value} $uri (unknown-byte body)" - if (level.headers && contentLength != null) { - logger.log("--> ${request.method.value} $uri") - } else if (level.headers && contentLength == null) { - logger.log("--> ${request.method.value} $uri") - }else if (headers.contains(HttpHeaders.ContentEncoding)) { - logger.log("--> ${request.method.value} $uri") - } else if (level.info && contentLength != null) { - logger.log("--> ${request.method.value} $uri ($contentLength-byte body)") - } else if (request.body is OutgoingContent.WriteChannelContent || request.body is OutgoingContent.ReadChannelContent) { - logger.log("--> ${request.method.value} $uri (unknown-byte body)") - } else { + else -> { val size = calcRequestBodySize(request.body, headers) - logger.log("--> ${request.method.value} $uri ($size-byte body)") + "--> ${request.method.value} $uri ($size-byte body)" } } - if (level.headers || level == LogLevel.BODY) { - val headers = HeadersBuilder().apply { - val body = request.body - if (body is OutgoingContent && request.method != HttpMethod.Get && body !is EmptyContent) { - body.contentType?.let { - appendIfNameAbsent(HttpHeaders.ContentType, it.toString()) - } - body.contentLength?.let { - appendIfNameAbsent(HttpHeaders.ContentLength, it.toString()) - } - } - appendAll(request.headers) - }.build() + logger.log(startLine) - for ((name, values) in headers.entries()) { - logger.log("$name: ${values.joinToString(separator = ", ")}") - } + if (!level.headers && level != LogLevel.BODY) { + return + } + + for ((name, values) in headers.entries()) { + logger.log("$name: ${values.joinToString(separator = ", ")}") + } + if (level != LogLevel.BODY || request.method == HttpMethod.Get) { logger.log("--> END ${request.method.value}") return } - if (level.body) { - logger.log("") - val body = request.body + logger.log("") - if (body is OutgoingContent) { - if (request.headers[HttpHeaders.ContentEncoding] == "gzip") { - val (newBody, size) = logOutgoingContent(body) { channel -> - GZipEncoder.decode(channel) - } - - logger.log("--> END ${request.method.value} ($size-byte, gzipped)") - } else { - val (newBody, size) = logOutgoingContent(body) - logger.log("--> END ${request.method.value} ($size-byte)") - } + if (body !is OutgoingContent) { + logger.log("--> END ${request.method.value}") + return + } - logger.log("--> END ${request.method.value}") + val endLine = if (request.headers[HttpHeaders.ContentEncoding] == "gzip") { + val (newBody, size) = logOutgoingContent(body) { channel -> + GZipEncoder.decode(channel) } + + "--> END ${request.method.value} ($size-byte, gzipped)" + } else { + val (newBody, size) = logOutgoingContent(body) + "--> END ${request.method.value} ($size-byte)" } + + logger.log(endLine) } suspend fun logResponseStdFormat(response: HttpResponse): HttpResponse { @@ -207,59 +203,60 @@ public val Logging: ClientPlugin = createClientPlugin("Logging", val request = response.request val duration = response.responseTime.timestamp - response.requestTime.timestamp - if ((level == LogLevel.HEADERS || level == LogLevel.BODY) && contentLength != null) { - logger.log("<-- ${response.status} ${request.url.pathQuery()} (${duration}ms)") - } else if (response.headers[HttpHeaders.ContentEncoding] == "gzip") { - logger.log("<-- ${response.status} ${request.url.pathQuery()} (${duration}ms)") - } else if (level.info && contentLength != null) { - logger.log("<-- ${response.status} ${request.url.pathQuery()} (${duration}ms, $contentLength-byte body)") - } else { - logger.log("<-- ${response.status} ${request.url.pathQuery()} (${duration}ms, unknown-byte body)") + val startLine = when { + ((level == LogLevel.HEADERS || level == LogLevel.BODY) && contentLength != null) + || (response.headers[HttpHeaders.ContentEncoding] == "gzip") -> "<-- ${response.status} ${request.url.pathQuery()} (${duration}ms)" + + level.info && contentLength != null -> "<-- ${response.status} ${request.url.pathQuery()} (${duration}ms, $contentLength-byte body)" + + else -> "<-- ${response.status} ${request.url.pathQuery()} (${duration}ms, unknown-byte body)" } - if (level.headers || level == LogLevel.BODY) { - for ((name, values) in response.headers.entries()) { - logger.log("$name: ${values.joinToString(separator = ", ")}") - } + logger.log(startLine) + + if (!level.headers && level != LogLevel.BODY) { + return response } - if (level.body) { - if (contentLength != null && contentLength == 0L) { - logger.log("<-- END HTTP") - return response - } + for ((name, values) in response.headers.entries()) { + logger.log("$name: ${values.joinToString(separator = ", ")}") + } - val (origChannel, newChannel) = response.rawContent.split(response) - logger.log("") - logger.log(newChannel.readRemaining().readText()) + if (!level.body) { logger.log("<-- END HTTP") + return response + } - return object : HttpResponse() { - override val call: HttpClientCall - get() = response.call - override val status: HttpStatusCode - get() = response.status - override val version: HttpProtocolVersion - get() = response.version - override val requestTime: GMTDate - get() = response.requestTime - override val responseTime: GMTDate - get() = response.responseTime - - @InternalAPI - override val rawContent: ByteReadChannel - get() = origChannel - override val headers: Headers - get() = response.headers - override val coroutineContext: CoroutineContext - get() = response.coroutineContext - } + if (contentLength != null && contentLength == 0L) { + logger.log("<-- END HTTP (${duration}ms, $contentLength-byte body)") + return response } - if (level.headers) { - logger.log("<-- END HTTP") + val (origChannel, newChannel) = response.rawContent.split(response) + logger.log("") + logger.log(newChannel.readRemaining().readText()) + logger.log("<-- END HTTP") + + return object : HttpResponse() { + override val call: HttpClientCall + get() = response.call + override val status: HttpStatusCode + get() = response.status + override val version: HttpProtocolVersion + get() = response.version + override val requestTime: GMTDate + get() = response.requestTime + override val responseTime: GMTDate + get() = response.responseTime + + @InternalAPI + override val rawContent: ByteReadChannel + get() = origChannel + override val headers: Headers + get() = response.headers + override val coroutineContext: CoroutineContext + get() = response.coroutineContext } - return response } @OptIn(DelicateCoroutinesApi::class) diff --git a/ktor-client/ktor-client-plugins/ktor-client-logging/jvm/test/io/ktor/client/plugins/logging/NewFormatTest.kt b/ktor-client/ktor-client-plugins/ktor-client-logging/jvm/test/io/ktor/client/plugins/logging/NewFormatTest.kt index 59f6d41ef1..794c969f2a 100644 --- a/ktor-client/ktor-client-plugins/ktor-client-logging/jvm/test/io/ktor/client/plugins/logging/NewFormatTest.kt +++ b/ktor-client/ktor-client-plugins/ktor-client-logging/jvm/test/io/ktor/client/plugins/logging/NewFormatTest.kt @@ -469,10 +469,63 @@ class NewFormatTest { .assertLogEqual("--> END GET") .assertLogMatch(Regex("""<-- 200 OK / \(\d+ms\)""")) .assertLogEqual("Content-Length: 0") - .assertLogEqual("<-- END HTTP") + .assertLogMatch(Regex("""<-- END HTTP \(\d+ms, 0-byte body\)""")) .assertNoMoreLogs() } + @Test + fun bodyGet204() = testWithLevel(LogLevel.BODY, handle = { + respond("", status = HttpStatusCode.NoContent, headers = Headers.build { + append(HttpHeaders.ContentLength, "0") + }) + }) { client -> + client.get("/") + log.assertLogEqual("--> GET /") + .assertLogEqual("Accept-Charset: UTF-8") + .assertLogEqual("Accept: */*") + .assertLogEqual("--> END GET") + .assertLogMatch(Regex("""<-- 204 No Content / \(\d+ms\)""")) + .assertLogEqual("Content-Length: 0") + .assertLogMatch(Regex("""<-- END HTTP \(\d+ms, 0-byte body\)""")) + .assertNoMoreLogs() + } + + @Test + fun bodyGet205() = testWithLevel(LogLevel.BODY, handle = { + respond("", status = HttpStatusCode.ResetContent, headers = Headers.build { + append(HttpHeaders.ContentLength, "0") + }) + }) { client -> + client.get("/") + log.assertLogEqual("--> GET /") + .assertLogEqual("Accept-Charset: UTF-8") + .assertLogEqual("Accept: */*") + .assertLogEqual("--> END GET") + .assertLogMatch(Regex("""<-- 205 Reset Content / \(\d+ms\)""")) + .assertLogEqual("Content-Length: 0") + .assertLogMatch(Regex("""<-- END HTTP \(\d+ms, 0-byte body\)""")) + .assertNoMoreLogs() + } + +// @Test +// fun bodyPost() = testWithLevel(LogLevel.BODY, handle = { respondWithLength() }) { client -> +// client.post("/") { +// setBody("test") +// } +// log.assertLogEqual("--> POST /") +// .assertLogEqual("Content-Type: text/plain; charset=UTF-8") +// .assertLogEqual("Content-Length: 4") +// .assertLogEqual("Accept-Charset: UTF-8") +// .assertLogEqual("Accept: */*") +// .assertLogEqual("") +// .assertLogEqual("test") +// .assertLogEqual("--> END POST (4-byte body)") +// .assertLogMatch(Regex("""<-- 200 OK / \(\d+ms\)""")) +// .assertLogEqual("Content-Length: 0") +// .assertLogMatch(Regex("""<-- END HTTP \(\d+ms, 0-byte body\)""")) +// .assertNoMoreLogs() +// } + @Test fun bodyGetWithResponseBody() = testWithLevel(LogLevel.BODY, handle = { respondWithLength("hello!") }) { client -> client.get("/")