Skip to content

Commit

Permalink
Fix some tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@Stexxe
Stexxe committed Dec 19, 2024
1 parent 7fcd1f2 commit 3f1751a
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 45 deletions.
13 changes: 7 additions & 6 deletions ...r-client-plugins/ktor-client-logging/common/src/io/ktor/client/plugins/logging/Logging.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,9 @@ public val Logging: ClientPlugin<LoggingConfig> = createClientPlugin("Logging",

if (level.headers && contentLength != null) {
logger.log("--> ${request.method.value} $uri")
} else if (headers.contains(HttpHeaders.ContentEncoding)) {
} 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)")
Expand Down Expand Up @@ -198,7 +200,6 @@ public val Logging: ClientPlugin<LoggingConfig> = createClientPlugin("Logging",
}
}


suspend fun logResponseStdFormat(response: HttpResponse): HttpResponse {
if (level == LogLevel.NONE) return response

Expand All @@ -207,13 +208,13 @@ public val Logging: ClientPlugin<LoggingConfig> = createClientPlugin("Logging",
val duration = response.responseTime.timestamp - response.requestTime.timestamp

if ((level == LogLevel.HEADERS || level == LogLevel.BODY) && contentLength != null) {
logger.log("<-- ${response.status} ${request.url.pathQuery()} ${response.version} (${duration}ms)")
logger.log("<-- ${response.status} ${request.url.pathQuery()} (${duration}ms)")
} else if (response.headers[HttpHeaders.ContentEncoding] == "gzip") {
logger.log("<-- ${response.status} ${request.url.pathQuery()} ${response.version} (${duration}ms)")
logger.log("<-- ${response.status} ${request.url.pathQuery()} (${duration}ms)")
} else if (level.info && contentLength != null) {
logger.log("<-- ${response.status} ${request.url.pathQuery()} ${response.version} (${duration}ms, $contentLength-byte body)")
logger.log("<-- ${response.status} ${request.url.pathQuery()} (${duration}ms, $contentLength-byte body)")
} else {
logger.log("<-- ${response.status} ${request.url.pathQuery()} ${response.version} (${duration}ms, unknown-byte body)")
logger.log("<-- ${response.status} ${request.url.pathQuery()} (${duration}ms, unknown-byte body)")
}

if (level.headers || level == LogLevel.BODY) {
Expand Down
120 changes: 81 additions & 39 deletions ...ient-plugins/ktor-client-logging/jvm/test/io/ktor/client/plugins/logging/NewFormatTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,18 @@ class NewFormatTest {
log = LogRecorder()
}

@Test
fun noLoggingWhenLevelNone() = testWithLevel(LogLevel.NONE, handle = { respondWithLength() }) { client ->
client.get("/")
log.assertNoMoreLogs()
}

@Test
fun basicGet() = testWithLevel(LogLevel.INFO, handle = { respondWithLength() }) { client ->
client.get("/")

log.assertLogEqual("--> GET /")
.assertLogMatch(Regex("""<-- 200 OK / HTTP/1.1 \(\d+ms, 0-byte body\)"""))
.assertLogMatch(Regex("""<-- 200 OK / \(\d+ms, 0-byte body\)"""))
.assertNoMoreLogs()
}

Expand All @@ -83,17 +89,16 @@ class NewFormatTest {
}

log.assertLogEqual("--> POST / (5-byte body)")
.assertLogMatch(Regex("""<-- 200 OK / HTTP/1.1 \(\d+ms, 0-byte body\)"""))
.assertLogMatch(Regex("""<-- 200 OK / \(\d+ms, 0-byte body\)"""))
.assertNoMoreLogs()

}

@Test
fun basicGet404() = testWithLevel(LogLevel.INFO, handle = { respondWithLength("", HttpStatusCode.NotFound) }) { client ->
client.get("/")

log.assertLogEqual("--> GET /")
.assertLogMatch(Regex("""<-- 404 Not Found / HTTP/1.1 \(\d+ms, 0-byte body\)"""))
.assertLogMatch(Regex("""<-- 404 Not Found / \(\d+ms, 0-byte body\)"""))
.assertNoMoreLogs()
}

Expand All @@ -102,7 +107,7 @@ class NewFormatTest {
client.get("/some/resource")

log.assertLogEqual("--> GET /some/resource")
.assertLogMatch(Regex("""<-- 200 OK /some/resource HTTP/1.1 \(\d+ms, 0-byte body\)"""))
.assertLogMatch(Regex("""<-- 200 OK /some/resource \(\d+ms, 0-byte body\)"""))
.assertNoMoreLogs()
}

Expand All @@ -111,20 +116,16 @@ class NewFormatTest {
client.get("/?a=1&b=2&c=3")

log.assertLogEqual("--> GET /?a=1&b=2&c=3")
.assertLogMatch(Regex("""<-- 200 OK /\?a=1&b=2&c=3 HTTP/1.1 \(\d+ms, 0-byte body\)"""))
.assertLogMatch(Regex("""<-- 200 OK /\?a=1&b=2&c=3 \(\d+ms, 0-byte body\)"""))
.assertNoMoreLogs()
}

@Test
fun basicGetNonEmptyBody() = testWithLevel(LogLevel.INFO, handle = {
respond("hello", headers = Headers.build {
append(HttpHeaders.ContentLength, "5")
})
}) { client ->
fun basicGetNonEmptyBody() = testWithLevel(LogLevel.INFO, handle = { respondWithLength("hello") }) { client ->
client.get("/")

log.assertLogEqual("--> GET /")
.assertLogMatch(Regex("""<-- 200 OK / HTTP/1.1 \(\d+ms, 5-byte body\)"""))
.assertLogMatch(Regex("""<-- 200 OK / \(\d+ms, 5-byte body\)"""))
.assertNoMoreLogs()
}

Expand All @@ -133,9 +134,8 @@ class NewFormatTest {
client.post("/")

log.assertLogEqual("--> POST / (0-byte body)")
.assertLogMatch(Regex("""<-- 200 OK / HTTP/1.1 \(\d+ms, 0-byte body\)"""))
.assertLogMatch(Regex("""<-- 200 OK / \(\d+ms, 0-byte body\)"""))
.assertNoMoreLogs()

}

@Test
Expand All @@ -155,7 +155,7 @@ class NewFormatTest {
}

log.assertLogEqual("--> POST / (0-byte body)")
.assertLogMatch(Regex("""<-- 200 OK / HTTP/1.1 \(\d+ms, 0-byte body\)"""))
.assertLogMatch(Regex("""<-- 200 OK / \(\d+ms, 0-byte body\)"""))
.assertNoMoreLogs()

}
Expand All @@ -169,7 +169,7 @@ class NewFormatTest {
}

log.assertLogEqual("--> POST / (unknown-byte body)")
.assertLogMatch(Regex("""<-- 200 OK / HTTP/1.1 \(\d+ms, 0-byte body\)"""))
.assertLogMatch(Regex("""<-- 200 OK / \(\d+ms, 0-byte body\)"""))
.assertNoMoreLogs()
}

Expand All @@ -184,7 +184,7 @@ class NewFormatTest {
}

log.assertLogEqual("--> POST / (11-byte body)")
.assertLogMatch(Regex("""<-- 200 OK / HTTP/1.1 \(\d+ms, 0-byte body\)"""))
.assertLogMatch(Regex("""<-- 200 OK / \(\d+ms, 0-byte body\)"""))
.assertNoMoreLogs()
}

Expand All @@ -199,7 +199,7 @@ class NewFormatTest {
assertEquals("hello", response.bodyAsText())

log.assertLogEqual("--> POST / (unknown-byte body)")
.assertLogMatch(Regex("""<-- 200 OK / HTTP/1.1 \(\d+ms, 5-byte body\)"""))
.assertLogMatch(Regex("""<-- 200 OK / \(\d+ms, 5-byte body\)"""))
.assertNoMoreLogs()
}

Expand All @@ -214,7 +214,7 @@ class NewFormatTest {
}

log.assertLogEqual("--> POST / (unknown-byte body)")
.assertLogMatch(Regex("""<-- 200 OK / HTTP/1.1 \(\d+ms, 0-byte body\)"""))
.assertLogMatch(Regex("""<-- 200 OK / \(\d+ms, 0-byte body\)"""))
.assertNoMoreLogs()
}

Expand All @@ -231,7 +231,7 @@ class NewFormatTest {
}

log.assertLogEqual("--> POST / (11-byte body)")
.assertLogMatch(Regex("""<-- 200 OK / HTTP/1.1 \(\d+ms, 0-byte body\)"""))
.assertLogMatch(Regex("""<-- 200 OK / \(\d+ms, 0-byte body\)"""))
.assertNoMoreLogs()
}

Expand All @@ -241,7 +241,7 @@ class NewFormatTest {
}) { client ->
client.prepareGet("/").execute { response ->
log.assertLogEqual("--> GET /")
.assertLogMatch(Regex("""<-- 200 OK / HTTP/1.1 \(\d+ms, 10-byte body\)"""))
.assertLogMatch(Regex("""<-- 200 OK / \(\d+ms, 10-byte body\)"""))
.assertNoMoreLogs()
}
}
Expand All @@ -268,7 +268,7 @@ class NewFormatTest {
client.post("/")

log.assertLogEqual("--> POST / (0-byte body)")
.assertLogMatch(Regex("""<-- 200 OK / HTTP/1.1 \(\d+ms, unknown-byte body\)"""))
.assertLogMatch(Regex("""<-- 200 OK / \(\d+ms, unknown-byte body\)"""))
.assertNoMoreLogs()
}
}
Expand All @@ -281,7 +281,7 @@ class NewFormatTest {
.assertLogEqual("Accept-Charset: UTF-8")
.assertLogEqual("Accept: */*")
.assertLogEqual("--> END GET")
.assertLogMatch(Regex("""<-- 200 OK / HTTP/1.1 \(\d+ms\)"""))
.assertLogMatch(Regex("""<-- 200 OK / \(\d+ms\)"""))
.assertLogEqual("Content-Length: 0")
.assertLogEqual("<-- END HTTP")
.assertNoMoreLogs()
Expand All @@ -299,7 +299,27 @@ class NewFormatTest {
.assertLogEqual("Accept-Charset: UTF-8")
.assertLogEqual("Accept: */*")
.assertLogEqual("--> END POST")
.assertLogMatch(Regex("""<-- 200 OK /post HTTP/1.1 \(\d+ms\)"""))
.assertLogMatch(Regex("""<-- 200 OK /post \(\d+ms\)"""))
.assertLogEqual("Content-Length: 0")
.assertLogEqual("<-- END HTTP")
.assertNoMoreLogs()
}

@Test
fun headersNoLength() = testWithLevel(LogLevel.HEADERS, handle = { respondWithLength() }) { client ->
client.post("/post") {
setBody(object : OutgoingContent.WriteChannelContent() {
override suspend fun writeTo(channel: ByteWriteChannel) {
channel.writeStringUtf8("test")
}
})
}

log.assertLogEqual("--> POST /post")
.assertLogEqual("Accept-Charset: UTF-8")
.assertLogEqual("Accept: */*")
.assertLogEqual("--> END POST")
.assertLogMatch(Regex("""<-- 200 OK /post \(\d+ms\)"""))
.assertLogEqual("Content-Length: 0")
.assertLogEqual("<-- END HTTP")
.assertNoMoreLogs()
Expand All @@ -321,9 +341,27 @@ class NewFormatTest {
.assertLogEqual("Accept-Charset: UTF-8")
.assertLogEqual("Accept: */*")
.assertLogEqual("--> END GET")
.assertLogMatch(Regex("""<-- 200 OK / HTTP/1.1 \(\d+ms\)"""))
.assertLogMatch(Regex("""<-- 200 OK / \(\d+ms\)"""))
.assertLogEqual("Custom-Response: value")
.assertLogEqual("Content-Length: 5")
.assertLogEqual("Content-Type: text/plain")
.assertLogEqual("<-- END HTTP")
.assertNoMoreLogs()
}

@Test
fun headersResponseBody() = testWithLevel(LogLevel.HEADERS, handle = {
respondWithLength("test", contentType = ContentType.Text.Html)
}) { client ->
client.get("/")

log.assertLogEqual("--> GET /")
.assertLogEqual("Accept-Charset: UTF-8")
.assertLogEqual("Accept: */*")
.assertLogEqual("--> END GET")
.assertLogMatch(Regex("""<-- 200 OK / \(\d+ms\)"""))
.assertLogEqual("Content-Length: 4")
.assertLogEqual("Content-Type: text/html")
.assertLogEqual("<-- END HTTP")
.assertNoMoreLogs()
}
Expand All @@ -342,8 +380,9 @@ class NewFormatTest {
.assertLogEqual("Accept-Charset: UTF-8")
.assertLogEqual("Accept: */*")
.assertLogEqual("--> END POST")
.assertLogMatch(Regex("""<-- 200 OK / HTTP/1.1 \(\d+ms\)"""))
.assertLogMatch(Regex("""<-- 200 OK / \(\d+ms\)"""))
.assertLogEqual("Content-Length: 3")
.assertLogEqual("Content-Type: text/plain")
.assertLogEqual("<-- END HTTP")
.assertNoMoreLogs()
}
Expand Down Expand Up @@ -372,7 +411,7 @@ class NewFormatTest {
.assertLogEqual("Accept-Charset: UTF-8")
.assertLogEqual("Accept: */*")
.assertLogEqual("--> END POST")
.assertLogMatch(Regex("""<-- 200 OK / HTTP/1.1 \(\d+ms, unknown-byte body\)"""))
.assertLogMatch(Regex("""<-- 200 OK / \(\d+ms, unknown-byte body\)"""))
.assertLogEqual("<-- END HTTP")
.assertNoMoreLogs()
}
Expand Down Expand Up @@ -411,19 +450,15 @@ class NewFormatTest {
// .assertLogEqual("")
// .assertLogEqual("request".repeat(1024))
// .assertLogEqual("--> END POST (7168-byte, gzipped)")
// .assertLogMatch(Regex("""<-- 200 OK / HTTP/1.1 \(\d+ms\)"""))
// .assertLogMatch(Regex("""<-- 200 OK / \(\d+ms\)"""))
// .assertLogEqual("")
// .assertLogEqual("response".repeat(1024))
// .assertLogMatch(Regex("""<-- END HTTP \(\d+ms, 111-byte body, gzipped\)"""))
// .assertNoMoreLogs()
// }
// }

@Test
fun noLoggingWhenLevelNone() = testWithLevel(LogLevel.NONE, handle = { respondWithLength() }) { client ->
client.get("/")
log.assertNoMoreLogs()
}


@Test
fun bodyGet() = testWithLevel(LogLevel.BODY, handle = { respondWithLength() }) { client ->
Expand All @@ -432,7 +467,7 @@ class NewFormatTest {
.assertLogEqual("Accept-Charset: UTF-8")
.assertLogEqual("Accept: */*")
.assertLogEqual("--> END GET")
.assertLogMatch(Regex("""<-- 200 OK / HTTP/1.1 \(\d+ms\)"""))
.assertLogMatch(Regex("""<-- 200 OK / \(\d+ms\)"""))
.assertLogEqual("Content-Length: 0")
.assertLogEqual("<-- END HTTP")
.assertNoMoreLogs()
Expand All @@ -445,19 +480,24 @@ class NewFormatTest {
.assertLogEqual("Accept-Charset: UTF-8")
.assertLogEqual("Accept: */*")
.assertLogEqual("--> END GET")
.assertLogMatch(Regex("""<-- 200 OK / HTTP/1.1 \(\d+ms\)"""))
.assertLogMatch(Regex("""<-- 200 OK / \(\d+ms\)"""))
.assertLogEqual("Content-Length: 6")
.assertLogEqual("Content-Type: text/plain")
.assertLogEqual("")
.assertLogEqual("hello!")
.assertLogEqual("<-- END HTTP")
.assertNoMoreLogs()
}

@Test
fun basicChunkedResponseBody() = testWithLevel(LogLevel.INFO, handle = { respond(ByteReadChannel("test")) }) { client ->
fun basicChunkedResponseBody() = testWithLevel(LogLevel.INFO, handle = {
respond(ByteReadChannel("test"), headers = Headers.build {
append(HttpHeaders.TransferEncoding, "chunked")
})
}) { client ->
client.get("/")
log.assertLogEqual("--> GET /")
.assertLogMatch(Regex("""<-- 200 OK / HTTP/1.1 \(\d+ms, unknown-byte body\)"""))
.assertLogMatch(Regex("""<-- 200 OK / \(\d+ms, unknown-byte body\)"""))
.assertNoMoreLogs()
}

Expand All @@ -467,17 +507,19 @@ class NewFormatTest {
})
}

private fun MockRequestHandleScope.respondWithLength(body: String, status: HttpStatusCode = HttpStatusCode.OK, headers: Headers = Headers.Empty): HttpResponseData {
private fun MockRequestHandleScope.respondWithLength(body: String, status: HttpStatusCode = HttpStatusCode.OK, contentType: ContentType = ContentType.Text.Plain, headers: Headers = Headers.Empty): HttpResponseData {
return respond(ByteReadChannel(body), headers = Headers.build {
appendAll(headers)
append("Content-Length", body.length.toString())
set("Content-Type", contentType.toString())
}, status = status)
}

private fun MockRequestHandleScope.respondWithLength(body: ByteArray, status: HttpStatusCode = HttpStatusCode.OK, headers: Headers = Headers.Empty): HttpResponseData {
private fun MockRequestHandleScope.respondWithLength(body: ByteArray, status: HttpStatusCode = HttpStatusCode.OK, contentType: ContentType = ContentType.Text.Plain, headers: Headers = Headers.Empty): HttpResponseData {
return respond(ByteReadChannel(body), headers = Headers.build {
appendAll(headers)
append("Content-Length", body.size.toString())
set("Content-Type", contentType.toString())
}, status = status)
}

Expand Down

0 comments on commit 3f1751a

Please sign in to comment.