Skip to content
This repository was archived by the owner on Jan 8, 2024. It is now read-only.

Commit 9050fb9

Browse files
benj-fletchbenj-fletch
authored
DW-3258 - Bug fixes (#5)
* DW-3258 - Bug fixes * DW-3258 - Further quote fixes * DW-3258 - Resolving PR comments
1 parent c3a0fbe commit 9050fb9

File tree

7 files changed

+44
-27
lines changed

7 files changed

+44
-27
lines changed

‎build.gradle.kts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,15 @@ tasks.withType<KotlinCompile> {
4747
tasks.withType<Test> {
4848
useJUnitPlatform()
4949
}
50+
51+
tasks {
52+
val sourcesJar by creating(Jar::class) {
53+
archiveClassifier.set("sources")
54+
from(sourceSets.main.get().allSource)
55+
}
56+
57+
artifacts {
58+
archives(sourcesJar)
59+
archives(jar)
60+
}
61+
}

‎src/main/kotlin/uk/gov/dwp/dataworks/logging/LogFields.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ object LogFields {
1414
var asJson = asJson()
1515

1616
private fun asJson(): String {
17-
return logFields.map { """"${it.key}":"${it.value}"""" }.joinToString(separator = ", ")
17+
return logFields.map { "\"${it.key}\":\"${it.value}\"" }.joinToString(separator = ", ")
1818
}
1919

2020
fun get(logField: String): String {

‎src/main/kotlin/uk/gov/dwp/dataworks/logging/LoggerLayoutAppender.kt

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import ch.qos.logback.core.LayoutBase
2525
* ```
2626
* Note that they may not be in the same order.
2727
*/
28-
object LoggerLayoutAppender : LayoutBase<ILoggingEvent>() {
28+
class LoggerLayoutAppender : LayoutBase<ILoggingEvent>() {
2929

3030
private var start_time_milliseconds = System.currentTimeMillis()
3131

@@ -38,6 +38,10 @@ object LoggerLayoutAppender : LayoutBase<ILoggingEvent>() {
3838
if (event == null) {
3939
return ""
4040
}
41-
return """{"timestamp":"${epochToUTCString(event.timeStamp)}", "log_level":"${event.level}", "message":${flattenString(event.formattedMessage)}, ${throwableProxyEventToJsonKeyPair(event)}"thread":"${event.threadName}", "logger":"${event.loggerName}", "duration_in_milliseconds":"${getDurationInMilliseconds(event.timeStamp)}", ${LogFields.asJson}}"""
41+
val formattedTimestamp = epochToUTCString(event.timeStamp)
42+
val formattedMessage = flattenString(event.formattedMessage).trim('"')
43+
val formattedException = throwableProxyEventToJsonKeyPair(event)
44+
val formattedDuration = getDurationInMilliseconds(event.timeStamp)
45+
return """{"timestamp":"$formattedTimestamp", "log_level":"${event.level}", "message":"$formattedMessage", $formattedException"thread":"${event.threadName}", "logger":"${event.loggerName}", "duration_in_milliseconds":"$formattedDuration", ${LogFields.asJson}}"""
4246
}
4347
}

‎src/main/kotlin/uk/gov/dwp/dataworks/logging/LoggingUtils.kt

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,28 +31,29 @@ fun throwableProxyEventToJsonKeyPair(event: ILoggingEvent): String {
3131
val throwableProxy = event.throwableProxy ?: return ""
3232

3333
val stackTrace = ThrowableProxyUtil.asString(throwableProxy)
34-
return """"exception":"${StringEscapeUtils.escapeJson(flattenString(stackTrace))}", """
34+
return "\"exception\":\"${StringEscapeUtils.escapeJson(flattenString(stackTrace))}\", "
3535
}
3636

3737
/**
3838
* Converts a message and a set of Tuples to an _almost_ formatted Json string. Tuples are first parsed as follows:
3939
* ```
40-
* "TupleKey1":"TupleValue1", "TupleKey2":"TupleValue2"
40+
* TupleKey1":"TupleValue1", "TupleKey2":"TupleValue2"
4141
* ```
4242
* Then the input message is escaped as per [StringEscapeUtils.escapeJson] and prepended with quotes. The resulting
4343
* output will look like the following:
4444
* ```
45-
* "input message contents", "TupleKey1":"TupleValue1", "TupleKey2":"TupleValue2"
45+
* input message contents", "TupleKey1":"TupleValue1", "TupleKey2":"TupleValue2"
4646
* ```
47+
* For call which do not pass contents to [tuples], [message] contents will be escaped and returned.
4748
*/
4849
fun semiFormattedTuples(message: String, vararg tuples: Pair<String, String>): String {
4950
if (tuples.isEmpty()) {
50-
return "\"$message\""
51+
return StringEscapeUtils.escapeJson(message)
5152
}
5253
val formattedTuples = tuples.joinToString(
53-
separator = "\", \"",
54-
transform = { """${it.first}":"${StringEscapeUtils.escapeJson(it.second)}""" })
55-
return """"${StringEscapeUtils.escapeJson(message)}", "$formattedTuples""""
54+
separator = ", ",
55+
transform = { "\"${it.first}\":\"${StringEscapeUtils.escapeJson(it.second)}\"" })
56+
return "${StringEscapeUtils.escapeJson(message)}\", $formattedTuples"
5657
}
5758

5859
private val dtf = DateTimeFormatter.ofPattern("YYYY-MM-dd'T'HH:mm:ss.SSS")

‎src/test/kotlin/uk/gov/dwp/dataworks/logging/DataworksLoggerTest.kt

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ class DataworksLoggerTest {
1212
DataworksLogger(mockLogger).debug("main-message", "key1" to "value1", "key2" to "value2")
1313

1414
verify(mockLogger, times(1)).isDebugEnabled
15-
verify(mockLogger, times(1)).debug("\"main-message\", \"key1\":\"value1\", \"key2\":\"value2\"")
15+
verify(mockLogger, times(1)).debug("main-message\", \"key1\":\"value1\", \"key2\":\"value2\"")
1616
verifyNoMoreInteractions(mockLogger)
1717
}
1818

@@ -24,7 +24,7 @@ class DataworksLoggerTest {
2424
DataworksLogger(mockLogger).info("main-message", "key1" to "value1", "key2" to "value2")
2525

2626
verify(mockLogger, times(1)).isInfoEnabled
27-
verify(mockLogger, times(1)).info("\"main-message\", \"key1\":\"value1\", \"key2\":\"value2\"")
27+
verify(mockLogger, times(1)).info("main-message\", \"key1\":\"value1\", \"key2\":\"value2\"")
2828
verifyNoMoreInteractions(mockLogger)
2929
}
3030

@@ -36,7 +36,7 @@ class DataworksLoggerTest {
3636
DataworksLogger(mockLogger).error("main-message", "key1" to "value1", "key2" to "value2")
3737

3838
verify(mockLogger, times(1)).isErrorEnabled
39-
verify(mockLogger, times(1)).error("\"main-message\", \"key1\":\"value1\", \"key2\":\"value2\"")
39+
verify(mockLogger, times(1)).error("main-message\", \"key1\":\"value1\", \"key2\":\"value2\"")
4040
verifyNoMoreInteractions(mockLogger)
4141
}
4242

@@ -49,7 +49,7 @@ class DataworksLoggerTest {
4949
DataworksLogger(mockLogger).error("main-message", exception, "key1" to "value1", "key2" to "value2")
5050

5151
verify(mockLogger, times(1)).isErrorEnabled
52-
verify(mockLogger, times(1)).error(eq("\"main-message\", \"key1\":\"value1\", \"key2\":\"value2\""), same(exception))
52+
verify(mockLogger, times(1)).error(eq("main-message\", \"key1\":\"value1\", \"key2\":\"value2\""), same(exception))
5353
verifyNoMoreInteractions(mockLogger)
5454
}
5555
}

‎src/test/kotlin/uk/gov/dwp/dataworks/logging/LogLayoutAppenderTest.kt

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,13 @@ import org.junit.jupiter.api.Test
1414
class LogLayoutAppenderTest {
1515
@Test
1616
fun `doLayout returns empty when called on null`() {
17-
val result = LoggerLayoutAppender.doLayout(null)
17+
val result = LoggerLayoutAppender().doLayout(null)
1818
assertThat(result).isEqualTo("")
1919
}
2020

2121
@Test
2222
fun `doLayout returns skinny JSON on empty event`() {
23-
val result = LoggerLayoutAppender.doLayout(mock())
23+
val result = LoggerLayoutAppender().doLayout(mock())
2424
assertJsonContainsCommonFields(result)
2525
assertJsonContainsField(result, "timestamp", "1970-01-01T00:00:00.000")
2626
assertJsonContainsField(result, "log_level", "null")
@@ -36,9 +36,9 @@ class LogLayoutAppenderTest {
3636
whenever(mockEvent.level).thenReturn(Level.WARN)
3737
whenever(mockEvent.threadName).thenReturn("my.thread.is.betty")
3838
whenever(mockEvent.loggerName).thenReturn("logger.name.is.mavis")
39-
whenever(mockEvent.formattedMessage).thenReturn("\"some message about stuff\"")
39+
whenever(mockEvent.formattedMessage).thenReturn("some message about stuff")
4040
whenever(mockEvent.hasCallerData()).thenReturn(false)
41-
val result = LoggerLayoutAppender.doLayout(mockEvent)
41+
val result = LoggerLayoutAppender().doLayout(mockEvent)
4242
println(result)
4343

4444
assertJsonContainsCommonFields(result)
@@ -52,10 +52,10 @@ class LogLayoutAppenderTest {
5252
@Test
5353
fun `doLayout will flatten multiline messages`() {
5454
val mockEvent = mock<ILoggingEvent>()
55-
whenever(mockEvent.formattedMessage).thenReturn("\"some\nmessage\nabout\nstuff with\ttabs\"")
55+
whenever(mockEvent.formattedMessage).thenReturn("some\nmessage\nabout\nstuff with\ttabs")
5656
whenever(mockEvent.hasCallerData()).thenReturn(false)
5757

58-
val result = LoggerLayoutAppender.doLayout(mockEvent)
58+
val result = LoggerLayoutAppender().doLayout(mockEvent)
5959
println(result)
6060

6161
assertJsonContainsCommonFields(result)
@@ -69,7 +69,7 @@ class LogLayoutAppenderTest {
6969
whenever(mockEvent.formattedMessage).thenReturn(embeddedTokens)
7070
whenever(mockEvent.hasCallerData()).thenReturn(false)
7171

72-
val result = LoggerLayoutAppender.doLayout(mockEvent)
72+
val result = LoggerLayoutAppender().doLayout(mockEvent)
7373
println(result)
7474
assertJsonContainsCommonFields(result)
7575
assertJsonContainsField(result, "key1", "value1")
@@ -80,10 +80,10 @@ class LogLayoutAppenderTest {
8080
@Test
8181
fun `doLayout should not escape json as that would mess with our custom static log methods which do`() {
8282
val mockEvent = mock<ILoggingEvent>()
83-
whenever(mockEvent.formattedMessage).thenReturn("\"message-/:'!@\"")
83+
whenever(mockEvent.formattedMessage).thenReturn("message-/:'!@")
8484
whenever(mockEvent.hasCallerData()).thenReturn(false)
8585

86-
val result = LoggerLayoutAppender.doLayout(mockEvent)
86+
val result = LoggerLayoutAppender().doLayout(mockEvent)
8787
assertJsonContainsField(result, "message", "message-/:'!@")
8888
}
8989

@@ -100,7 +100,7 @@ class LogLayoutAppenderTest {
100100
ThrowableProxyUtil.build(stubThrowable, catchMe2(), ThrowableProxy(catchMe3()))
101101
whenever(mockEvent.throwableProxy).thenReturn(stubThrowable as IThrowableProxy)
102102

103-
val result = LoggerLayoutAppender.doLayout(mockEvent)
103+
val result = LoggerLayoutAppender().doLayout(mockEvent)
104104
assertThat(result).containsPattern("\"exception\":\".*\"")
105105
assertThat(result).contains("i am an exception")
106106
}

‎src/test/kotlin/uk/gov/dwp/dataworks/logging/LoggingUtilsTest.kt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,18 +14,18 @@ import org.junit.jupiter.api.Test
1414
class LoggingUtilsTest {
1515
@Test
1616
fun `semiFormattedTuples will format as partial JSON without matching key-value pairs`() {
17-
assertThat("\"my-message\"").isEqualTo(semiFormattedTuples("my-message"))
17+
assertThat("my-message").isEqualTo(semiFormattedTuples("my-message"))
1818
}
1919

2020
@Test
2121
fun `semiFormattedTuples will format as partial JSON with matching key-value pairs`() {
22-
assertThat("\"my-message\", \"key1\":\"value1\", \"key2\":\"value2\"")
22+
assertThat("my-message\", \"key1\":\"value1\", \"key2\":\"value2\"")
2323
.isEqualTo(semiFormattedTuples("my-message", "key1" to "value1", "key2" to "value2"))
2424
}
2525

2626
@Test
2727
fun `semiFormattedTuples will escape JSON in message and Tuple values`() {
28-
assertThat("\"message-\\/:'!@\\u00A3\$%^&*()\\n\\t\\r\", \"key-unchanged\":\"value-\\/:!@\\u00A3\$%^&*()\\n\\t\\r\"")
28+
assertThat("message-\\/:'!@\\u00A3\$%^&*()\\n\\t\\r\", \"key-unchanged\":\"value-\\/:!@\\u00A3\$%^&*()\\n\\t\\r\"")
2929
.isEqualTo(semiFormattedTuples("message-/:'!@£\$%^&*()\n\t\r", "key-unchanged" to "value-/:!@£\$%^&*()\n\t\r"))
3030
}
3131

0 commit comments

Comments
 (0)