Skip to content

Commit c29549a

Browse files
author
Ronald Holshausen
committedFeb 9, 2020
fix: message pacts with JSON contents were being formatted as strings #1011
·
4_7_0-beta_04_0_6
1 parent fc6fe46 commit c29549a

File tree

11 files changed

+133
-40
lines changed

11 files changed

+133
-40
lines changed
 

‎consumer/pact-jvm-consumer-groovy/src/main/groovy/au/com/dius/pact/consumer/groovy/messaging/PactMessageBuilder.groovy

+8-2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import au.com.dius.pact.consumer.groovy.BaseBuilder
55
import au.com.dius.pact.consumer.groovy.Matcher
66
import au.com.dius.pact.consumer.groovy.PactBodyBuilder
77
import au.com.dius.pact.core.model.Consumer
8+
import au.com.dius.pact.core.model.ContentType
89
import au.com.dius.pact.core.model.InvalidPactException
910
import au.com.dius.pact.core.model.OptionalBody
1011
import au.com.dius.pact.core.model.PactSpecVersion
@@ -101,14 +102,19 @@ class PactMessageBuilder extends BaseBuilder {
101102
if (messages.empty) {
102103
throw new InvalidPactException('expectsToReceive is required before withContent')
103104
}
105+
106+
def contentType = ContentType.JSON.contentType
104107
if (options.contentType) {
108+
contentType = options.contentType
105109
messages.last().metaData.contentType = options.contentType
110+
} else if (messages.last().metaData.contentType) {
111+
contentType = messages.last().metaData.contentType
106112
}
107113

108-
def body = new PactBodyBuilder(mimetype: options.contentType, prettyPrintBody: options.prettyPrint)
114+
def body = new PactBodyBuilder(mimetype: contentType, prettyPrintBody: options.prettyPrint)
109115
closure.delegate = body
110116
closure.call()
111-
messages.last().contents = OptionalBody.body(body.body.bytes)
117+
messages.last().contents = OptionalBody.body(body.body.bytes, new ContentType(contentType))
112118
messages.last().matchingRules.addCategory(body.matchers)
113119

114120
this

‎consumer/pact-jvm-consumer-groovy/src/test/groovy/au/com/dius/pact/consumer/groovy/messaging/PactMessageBuilderSpec.groovy

+34-2
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import au.com.dius.pact.core.model.matchingrules.MatchingRuleGroup
44
import au.com.dius.pact.core.model.matchingrules.RegexMatcher
55
import au.com.dius.pact.core.model.messaging.Message
66
import groovy.json.JsonSlurper
7+
import spock.lang.Issue
78
import spock.lang.Specification
89

910
class PactMessageBuilderSpec extends Specification {
@@ -49,8 +50,7 @@ class PactMessageBuilderSpec extends Specification {
4950
builder {
5051
given 'the provider has data for a message'
5152
expectsToReceive 'a confirmation message for a group order'
52-
withMetaData(contentType: 'application/json')
53-
withContent {
53+
withContent('application/json') {
5454
name 'Bob'
5555
date = '2000-01-01'
5656
status 'bad'
@@ -117,4 +117,36 @@ class PactMessageBuilderSpec extends Specification {
117117
true
118118
}
119119

120+
@Issue('#1011')
121+
def 'invalid body format test'() {
122+
given:
123+
def pactMessageBuilder = new PactMessageBuilder().with {
124+
serviceConsumer 'consumer'
125+
hasPactWith 'provider'
126+
expectsToReceive 'feed entry'
127+
withMetaData(contentType: 'application/json')
128+
withContent {
129+
type 'foo'
130+
data {
131+
reference {
132+
id string('abc')
133+
}
134+
}
135+
}
136+
}
137+
138+
expect:
139+
pactMessageBuilder.run { Message message ->
140+
def feedEntry = message.contentsAsString()
141+
assert feedEntry == '''{
142+
| "type": "foo",
143+
| "data": {
144+
| "reference": {
145+
| "id": "abc"
146+
| }
147+
| }
148+
|}'''.stripMargin()
149+
assert message.jsonContents
150+
}
151+
}
120152
}

‎consumer/pact-jvm-consumer/src/main/kotlin/au/com/dius/pact/consumer/MessagePactBuilder.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ class MessagePactBuilder(
8888
} else {
8989
value
9090
}
91-
}
91+
}.toMutableMap()
9292
return this
9393
}
9494

‎core/model/src/main/kotlin/au/com/dius/pact/core/model/RequestResponseInteraction.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ open class RequestResponseInteraction @JvmOverloads constructor(
119119

120120
private fun parseBody(httpPart: HttpPart): Any? {
121121
return if (httpPart.jsonBody() && httpPart.body.isPresent()) {
122-
val body = Json.fromJson(JsonParser().parse(httpPart.body.valueAsString()))
122+
val body = Json.fromJson(JsonParser.parseString(httpPart.body.valueAsString()))
123123
if (body is String) {
124124
httpPart.body.valueAsString()
125125
} else {

‎core/model/src/main/kotlin/au/com/dius/pact/core/model/messaging/Message.kt

+34-19
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import au.com.dius.pact.core.model.generators.Generators
88
import au.com.dius.pact.core.model.matchingrules.MatchingRules
99
import au.com.dius.pact.core.model.matchingrules.MatchingRulesImpl
1010
import au.com.dius.pact.core.support.Json
11+
import au.com.dius.pact.core.support.isNotEmpty
1112
import com.github.salomonbrys.kotson.array
1213
import com.github.salomonbrys.kotson.obj
1314
import com.google.gson.JsonObject
@@ -26,18 +27,22 @@ class Message @JvmOverloads constructor(
2627
var contents: OptionalBody = OptionalBody.missing(),
2728
var matchingRules: MatchingRules = MatchingRulesImpl(),
2829
var generators: Generators = Generators(),
29-
var metaData: Map<String, Any?> = mapOf(),
30+
var metaData: MutableMap<String, Any?> = mutableMapOf(),
3031
override val interactionId: String? = null
3132
) : Interaction {
3233

3334
fun contentsAsBytes() = contents.orEmpty()
3435

3536
fun contentsAsString() = contents.valueAsString()
3637

37-
fun getContentType() = Companion.getContentType(metaData)
38+
fun getContentType() = if (contents.isPresent() && contents.contentType.contentType.isNotEmpty()) {
39+
contents.contentType.contentType
40+
} else {
41+
getContentType(metaData)
42+
}
3843

3944
@Deprecated("Use the content type associated with the message body")
40-
fun getParsedContentType() = parseContentType(this.getContentType())
45+
fun getParsedContentType() = parseContentType(this.getContentType() ?: "")
4146

4247
override fun toMap(pactSpecVersion: PactSpecVersion): Map<String, Any?> {
4348
val map: MutableMap<String, Any?> = mutableMapOf(
@@ -47,7 +52,7 @@ class Message @JvmOverloads constructor(
4752
if (!contents.isMissing()) {
4853
map["contents"] = when {
4954
isJsonContents() -> {
50-
val json = JsonParser().parse(contents.valueAsString())
55+
val json = JsonParser.parseString(contents.valueAsString())
5156
if (json.isJsonPrimitive && json.asJsonPrimitive.isString) {
5257
contents.valueAsString()
5358
} else {
@@ -71,19 +76,23 @@ class Message @JvmOverloads constructor(
7176

7277
private fun isJsonContents(): Boolean {
7378
return if (contents.isPresent()) {
74-
val mimeType = contents.contentType.asMimeType()
75-
isJson(mimeType)
79+
val contentType = getContentType(metaData)
80+
if (contentType.isNotEmpty()) {
81+
isJson(contentType)
82+
} else {
83+
isJson(contents.contentType.asMimeType())
84+
}
7685
} else {
7786
false
7887
}
7988
}
8089

8190
fun formatContents(): String {
8291
return if (contents.isPresent()) {
83-
val mimeType = contents.contentType.asMimeType()
92+
val contentType = getContentType(metaData) ?: contents.contentType.asMimeType()
8493
when {
85-
isJson(mimeType) -> Json.gsonPretty.toJson(JsonParser().parse(contents.valueAsString()))
86-
isOctetStream(mimeType) -> Base64.encodeBase64String(contentsAsBytes())
94+
isJson(contentType) -> Json.gsonPretty.toJson(JsonParser.parseString(contents.valueAsString()))
95+
isOctetStream(contentType) -> Base64.encodeBase64String(contentsAsBytes())
8796
else -> contents.valueAsString()
8897
}
8998
} else {
@@ -128,12 +137,13 @@ class Message @JvmOverloads constructor(
128137
}
129138

130139
fun withMetaData(metadata: Map<String, Any>): Message {
131-
this.metaData = metadata
140+
this.metaData = metadata.toMutableMap()
132141
return this
133142
}
134143

135144
companion object : KLogging() {
136145
const val JSON = "application/json"
146+
const val TEXT = "text/plain"
137147

138148
/**
139149
* Builds a message from a Map
@@ -171,23 +181,28 @@ class Message @JvmOverloads constructor(
171181
else Generators()
172182

173183
return Message(Json.toString(json["description"]), providerStates,
174-
contents, matchingRules, generators, metaData, Json.toString(json["_id"]))
184+
contents, matchingRules, generators, metaData.toMutableMap(), Json.toString(json["_id"]))
175185
}
176186

177187
@Deprecated("Use the content type associated with the message body")
178-
private fun parseContentType(contentType: String): ContentType? {
179-
return try {
180-
ContentType.parse(contentType)
181-
} catch (e: RuntimeException) {
182-
logger.debug(e) { "Failed to parse content type '$contentType'" }
188+
private fun parseContentType(contentType: String?): ContentType? {
189+
return if (contentType.isNotEmpty()) {
190+
try {
191+
ContentType.parse(contentType)
192+
} catch (e: RuntimeException) {
193+
logger.debug(e) { "Failed to parse content type '$contentType'" }
194+
null
195+
}
196+
} else {
183197
null
184198
}
185199
}
186200

187-
fun getContentType(metaData: Map<String, Any?>): String {
188-
return metaData.entries.find {
201+
@Deprecated("Use the content type associated with the message body")
202+
fun getContentType(metaData: Map<String, Any?>): String? {
203+
return parseContentType(metaData.entries.find {
189204
it.key.toLowerCase() == "contenttype" || it.key.toLowerCase() == "content-type"
190-
}?.value?.toString() ?: JSON
205+
}?.value?.toString())?.mimeType
191206
}
192207

193208
private fun isJson(contentType: String?) =

‎core/model/src/test/groovy/au/com/dius/pact/core/model/messaging/MessageSpec.groovy

+35-1
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,40 @@ class MessageSpec extends Specification {
126126
new MatchingRulesImpl(), new Generators(), [contentType: contentType])
127127
}
128128

129+
@Unroll
130+
def 'message to map handles message content correctly - with only metadata'() {
131+
expect:
132+
message.toMap(PactSpecVersion.V3).contents == contents
133+
134+
where:
135+
136+
body | contentType | contents
137+
'{"A": "Value A", "B": "Value B"}' | 'application/json' | [A: 'Value A', B: 'Value B']
138+
'{"A": "Value A", "B": "Value B"}' | '' | '{"A": "Value A", "B": "Value B"}'
139+
'1 2 3 4' | 'text/plain' | '1 2 3 4'
140+
new String([1, 2, 3, 4] as byte[]) | 'application/octet-stream' | 'AQIDBA=='
141+
142+
message = new Message('test', [], OptionalBody.body(body.bytes),
143+
new MatchingRulesImpl(), new Generators(), [contentType: contentType])
144+
}
145+
146+
@Unroll
147+
def 'message to map handles message content correctly - with no metadata'() {
148+
expect:
149+
message.toMap(PactSpecVersion.V3).contents == contents
150+
151+
where:
152+
153+
body | contentType | contents
154+
'{"A": "Value A", "B": "Value B"}' | 'application/json' | [A: 'Value A', B: 'Value B']
155+
'{"A": "Value A", "B": "Value B"}' | '' | '{"A": "Value A", "B": "Value B"}'
156+
'1 2 3 4' | 'text/plain' | '1 2 3 4'
157+
new String([1, 2, 3, 4] as byte[]) | 'application/octet-stream' | 'AQIDBA=='
158+
159+
message = new Message('test', [], OptionalBody.body(body.bytes, new ContentType(contentType)),
160+
new MatchingRulesImpl(), new Generators(), [:])
161+
}
162+
129163
@Unroll
130164
def 'get content type test'() {
131165
expect:
@@ -137,7 +171,7 @@ class MessageSpec extends Specification {
137171
'contentType' | 'application/json' | 'application/json'
138172
'Content-Type' | 'text/plain' | 'text/plain'
139173
'contenttype' | 'application/octet-stream' | 'application/octet-stream'
140-
'none' | 'none' | 'application/json'
174+
'none' | 'none' | null
141175

142176
message = new Message('Test').withMetaData([(key): contentType])
143177
}

‎provider/pact-jvm-provider-gradle/src/test/groovy/au/com/dius/pact/provider/gradle/ResponseComparisonSpec.groovy

+1-1
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ class ResponseComparisonSpec extends Specification {
9999
'application/octet-stream;charset=UTF-8' | '{"a": 100.0, "b": "test"}' | '{"a": 100.0, "b": "test"}'
100100
'application/octet-stream' | '{"a": 100.0, "b": "test"}' | '{"a": 100.0, "b": "test"}'
101101
'' | '{"a": 100.0, "b": "test"}' | '{"a": 100.0, "b": "test"}'
102-
null | '{"a": 100.0, "b": "test"}' | '{"a":100.0,"b":"test"}'
102+
null | '{"a": 100.0, "b": "test"}' | '{"a": 100.0, "b": "test"}'
103103

104104
}
105105

‎provider/pact-jvm-provider-junit/src/test/resources/amqp_pacts/message_test_consumer-test_provider.json

+3
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@
88
"messages": [
99
{
1010
"description": "a test message",
11+
"metaData": {
12+
"contentType": "application/json"
13+
},
1114
"contents": {
1215
"testParam1": "value1",
1316
"testParam2": "value2"

‎provider/pact-jvm-provider-junit5/src/test/resources/amqp_pacts/message_test_consumer-test_provider.json

+3
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@
88
"messages": [
99
{
1010
"description": "a test message",
11+
"metaData": {
12+
"contentType": "application/json"
13+
},
1114
"contents": {
1215
"testParam1": "value1",
1316
"testParam2": "value2"

‎provider/pact-jvm-provider/src/main/kotlin/au/com/dius/pact/provider/ResponseComparison.kt

+10-9
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import au.com.dius.pact.core.matchers.Mismatch
1313
import au.com.dius.pact.core.matchers.ResponseMatching
1414
import au.com.dius.pact.core.matchers.StatusMismatch
1515
import au.com.dius.pact.core.matchers.generateDiff
16+
import au.com.dius.pact.core.model.ContentType
1617
import au.com.dius.pact.core.model.OptionalBody
1718
import au.com.dius.pact.core.model.Response
1819
import au.com.dius.pact.core.model.isNullOrEmpty
@@ -21,7 +22,6 @@ import au.com.dius.pact.core.support.Json
2122
import com.github.salomonbrys.kotson.jsonObject
2223
import com.google.gson.JsonParser
2324
import mu.KLogging
24-
import org.apache.http.entity.ContentType
2525
import java.nio.charset.Charset
2626

2727
data class BodyComparisonResult(
@@ -48,7 +48,7 @@ class ResponseComparison(
4848
val expectedHeaders: Map<String, List<String>>,
4949
val expectedBody: OptionalBody,
5050
val isJsonBody: Boolean,
51-
val actualResponseContentType: ContentType,
51+
val actualResponseContentType: org.apache.http.entity.ContentType,
5252
val actualBody: String?
5353
) {
5454

@@ -114,7 +114,7 @@ class ResponseComparison(
114114
actualHeaders: Map<String, List<String>>,
115115
actualBody: String?
116116
): ComparisonResult {
117-
val actualResponseContentType = actualResponse["contentType"] as ContentType
117+
val actualResponseContentType = actualResponse["contentType"] as org.apache.http.entity.ContentType
118118
val comparison = ResponseComparison(response.headers, response.body, response.jsonBody(),
119119
actualResponseContentType, actualBody)
120120
val mismatches = ResponseMatching.responseMismatches(response, Response(actualStatus,
@@ -134,18 +134,19 @@ class ResponseComparison(
134134
else -> Matching.compareMessageMetadata(message.metaData, metadata, message.matchingRules)
135135
}
136136

137-
val contentType = message.getParsedContentType()!!
137+
val messageContentType = message.getContentType()
138+
val contentType = if (messageContentType.isNullOrEmpty()) Message.TEXT else messageContentType
138139
val responseComparison = ResponseComparison(
139-
mapOf("Content-Type" to listOf(message.getContentType())), message.contents,
140-
contentType.mimeType == ContentType.APPLICATION_JSON.mimeType,
141-
contentType, actual.valueAsString())
140+
mapOf("Content-Type" to listOf(contentType)), message.contents,
141+
contentType == ContentType.JSON.contentType,
142+
org.apache.http.entity.ContentType.parse(contentType), actual.valueAsString())
142143
return ComparisonResult(bodyMismatches = responseComparison.bodyResult(bodyMismatches),
143144
metadataMismatches = metadataMismatches.groupBy { it.key })
144145
}
145146

146147
@JvmStatic
147148
private fun compareMessageBody(message: Message, actual: OptionalBody): MutableList<BodyMismatch> {
148-
val result = MatchingConfig.lookupBodyMatcher(message.getParsedContentType()?.mimeType.orEmpty())
149+
val result = MatchingConfig.lookupBodyMatcher(message.getContentType().orEmpty())
149150
var bodyMismatches = mutableListOf<BodyMismatch>()
150151
if (result != null) {
151152
bodyMismatches = result.matchBody(message.contents, actual, true, message.matchingRules)
@@ -154,7 +155,7 @@ class ResponseComparison(
154155
val expectedBody = message.contents.valueAsString()
155156
if (expectedBody.isNotEmpty() && actual.isNullOrEmpty()) {
156157
bodyMismatches.add(BodyMismatch(expectedBody, null, "Expected body '$expectedBody' but was missing"))
157-
} else if (actual.valueAsString() != expectedBody) {
158+
} else if (expectedBody.isNotEmpty() && actual.valueAsString() != expectedBody) {
158159
bodyMismatches.add(BodyMismatch(expectedBody, actual.valueAsString(),
159160
"Actual body '${actual.valueAsString()}' is not equal to the expected body '$expectedBody'"))
160161
}

‎provider/pact-jvm-provider/src/test/groovy/au/com/dius/pact/provider/MessageComparisonSpec.groovy

+3-4
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import spock.lang.Specification
88

99
class MessageComparisonSpec extends Specification {
1010

11-
def 'compares the message contents as JSON by default'() {
11+
def 'compares the message contents as TEXT by default'() {
1212
given:
1313
def message = new Message('test', [], OptionalBody.body('{"a":1,"b":"2"}'.bytes))
1414
def actual = OptionalBody.body('{"a":1,"b":"3"}'.bytes)
@@ -19,7 +19,7 @@ class MessageComparisonSpec extends Specification {
1919
then:
2020
result.isRight()
2121
result.b.mismatches.collectEntries { [ it.key, it.value*.description() ] } == [
22-
'$.b': ['Expected "2" but received "3"']
22+
'/': ['Actual body \'{"a":1,"b":"3"}\' is not equal to the expected body \'{"a":1,"b":"2"}\'']
2323
]
2424
}
2525

@@ -35,8 +35,7 @@ class MessageComparisonSpec extends Specification {
3535
then:
3636
result.isRight()
3737
result.b.mismatches.collectEntries { [ it.key, it.value*.description() ] } == [
38-
'/': ["Expected body '{\"a\":1,\"b\":\"2\"}' to match '{\"a\":1,\"b\":\"3\"}' using equality but did " +
39-
'not match']
38+
'/': ['Expected body \'{"a":1,"b":"2"}\' to match \'{"a":1,"b":"3"}\' using equality but did not match']
4039
]
4140
}
4241

0 commit comments

Comments
 (0)
Please sign in to comment.