From 8975aee9a8e53defe57b9ec6d898858986a57cb5 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Mon, 28 Aug 2017 17:30:47 +0200 Subject: [PATCH] Using testMatchers in DSL other than byRegex adds unsolicited isEmpty check for an array The converted body looks different when done via the String notation than it does when done via a map notation. When working with String body and when matchers are provided, even when all entries of a map / list got removed, the map / list itself remains. That leads to unnecessary creation of checks for empty collection. With this method we're checking if the JSON path matcher is related to array checking and we're trying to remove that trailing collection. All in all it's better to use the Groovy based notation for defining body... fixes #391 --- .../util/JsonToJsonPathsConverter.groovy | 32 +++++++ ...vcMethodBodyBuilderWithMatchersSpec.groovy | 90 +++++++++++++++++++ 2 files changed, 122 insertions(+) diff --git a/spring-cloud-contract-verifier/src/main/groovy/org/springframework/cloud/contract/verifier/util/JsonToJsonPathsConverter.groovy b/spring-cloud-contract-verifier/src/main/groovy/org/springframework/cloud/contract/verifier/util/JsonToJsonPathsConverter.groovy index 1d1c4c49ad..1935c7835b 100644 --- a/spring-cloud-contract-verifier/src/main/groovy/org/springframework/cloud/contract/verifier/util/JsonToJsonPathsConverter.groovy +++ b/spring-cloud-contract-verifier/src/main/groovy/org/springframework/cloud/contract/verifier/util/JsonToJsonPathsConverter.groovy @@ -50,6 +50,7 @@ class JsonToJsonPathsConverter { private static final Boolean SERVER_SIDE = false private static final Boolean CLIENT_SIDE = true + private static final String ANY_ARRAY_NOTATION_IN_JSONPATH = "[*]" private final ContractVerifierConfigProperties configProperties @@ -80,6 +81,7 @@ class JsonToJsonPathsConverter { bodyMatchers.jsonPathMatchers().each { BodyMatcher matcher -> try { context.delete(matcher.path()) + removeTrailingContainers(matcher, context) } catch (RuntimeException e) { if (log.isDebugEnabled()) { log.debug("Exception occurred while trying to delete path [${matcher.path()}]", e) @@ -90,6 +92,36 @@ class JsonToJsonPathsConverter { return jsonCopy } + /** + * Related to #391. The converted body looks different when done via the String notation than + * it does when done via a map notation. When working with String body and when matchers + * are provided, even when all entries of a map / list got removed, the map / list itself + * remains. That leads to unnecessary creation of checks for empty collection. With this method + * we're checking if the JSON path matcher is related to array checking and we're trying to + * remove that trailing collection. All in all it's better to use the Groovy based notation for + * defining body... + */ + private static void removeTrailingContainers(BodyMatcher matcher, DocumentContext context) { + if (matcher.path().contains(ANY_ARRAY_NOTATION_IN_JSONPATH)) { + String pathWithoutAnyArray = matcher.path().substring(0, matcher.path().lastIndexOf(ANY_ARRAY_NOTATION_IN_JSONPATH)) + def object = context.read(pathWithoutAnyArray) + if (object instanceof Iterable && containsOnlyEmptyElements(object)) { + context.delete(pathWithoutAnyArray) + } + } + } + + private static boolean containsOnlyEmptyElements(Object object) { + return object.every { + if (it instanceof Map) { + return it.isEmpty() + } else if (it instanceof List) { + return it.isEmpty() + } + return false + } + } + // Doing a clone doesn't work for nested lists... private static Object cloneBody(Object object) { if (object instanceof List) { diff --git a/spring-cloud-contract-verifier/src/test/groovy/org/springframework/cloud/contract/verifier/builder/MockMvcMethodBodyBuilderWithMatchersSpec.groovy b/spring-cloud-contract-verifier/src/test/groovy/org/springframework/cloud/contract/verifier/builder/MockMvcMethodBodyBuilderWithMatchersSpec.groovy index 96da05eaa6..61a256a99f 100644 --- a/spring-cloud-contract-verifier/src/test/groovy/org/springframework/cloud/contract/verifier/builder/MockMvcMethodBodyBuilderWithMatchersSpec.groovy +++ b/spring-cloud-contract-verifier/src/test/groovy/org/springframework/cloud/contract/verifier/builder/MockMvcMethodBodyBuilderWithMatchersSpec.groovy @@ -423,4 +423,94 @@ class MockMvcMethodBodyBuilderWithMatchersSpec extends Specification implements "JaxRsClientJUnitMethodBodyBuilder" | { Contract dsl -> new JaxRsClientJUnitMethodBodyBuilder(dsl, properties) } | '$' } + @Issue("#391") + def "should work for matchers and body with multiline string for [#methodBuilderName]"() { + given: + Contract contractDsl = Contract.make { + request { + name "ISSUE 391" + method 'GET' + urlPath '/item/factsheet?size=2&page=1' + headers { header "accept", "application/...json" } + } + response { + status 200 + body(""" + { + "items": [ + { + "id": "35309", + "title": "lorem ipsum" + } + ] + } + """) + testMatchers { + jsonPath('$.items[*].id', byRegex(nonBlank())) + jsonPath('$.items[*].title', byRegex(nonBlank())) + jsonPath('$.items[*]', byType { minOccurrence(2); maxOccurrence(2) }) + } + headers {header "content-type", "application/...json;charset=UTF-8"} + } + } + MethodBodyBuilder builder = methodBuilder(contractDsl) + BlockBuilder blockBuilder = new BlockBuilder(" ") + and: + builder.appendTo(blockBuilder) + String test = blockBuilder.toString() + when: + SyntaxChecker.tryToCompile(methodBuilderName, test) + then: + !test.contains('''assertThatJson(parsedJson).array("['items']").isEmpty()''') + where: + methodBuilderName | methodBuilder + "MockMvcSpockMethodBuilder" | { Contract dsl -> new MockMvcSpockMethodRequestProcessingBodyBuilder(dsl, properties) } + "MockMvcJUnitMethodBuilder" | { Contract dsl -> new MockMvcJUnitMethodBodyBuilder(dsl, properties) } + "JaxRsClientSpockMethodRequestProcessingBodyBuilder" | { Contract dsl -> new JaxRsClientSpockMethodRequestProcessingBodyBuilder(dsl, properties) } + "JaxRsClientJUnitMethodBodyBuilder" | { Contract dsl -> new JaxRsClientJUnitMethodBodyBuilder(dsl, properties) } + } + + @Issue("#391") + def "should work for matchers and body with multiline string with map body for [#methodBuilderName]"() { + given: + Contract contractDsl = Contract.make { + request { + name "ISSUE 391" + method 'GET' + urlPath '/item/factsheet?size=2&page=1' + headers { header "accept", "application/...json" } + } + response { + status 200 + body([ + "items": [ + "id" : "35309", + "title": "lorem ipsum" + ] + ]) + testMatchers { + jsonPath('$.items[*].id', byRegex(nonBlank())) + jsonPath('$.items[*].title', byRegex(nonBlank())) + jsonPath('$.items[*]', byType { minOccurrence(2); maxOccurrence(2) }) + } + headers {header "content-type", "application/...json;charset=UTF-8"} + } + } + MethodBodyBuilder builder = methodBuilder(contractDsl) + BlockBuilder blockBuilder = new BlockBuilder(" ") + and: + builder.appendTo(blockBuilder) + String test = blockBuilder.toString() + when: + SyntaxChecker.tryToCompile(methodBuilderName, test) + then: + !test.contains('''assertThatJson(parsedJson).array("['items']").isEmpty()''') + where: + methodBuilderName | methodBuilder + "MockMvcSpockMethodBuilder" | { Contract dsl -> new MockMvcSpockMethodRequestProcessingBodyBuilder(dsl, properties) } + "MockMvcJUnitMethodBuilder" | { Contract dsl -> new MockMvcJUnitMethodBodyBuilder(dsl, properties) } + "JaxRsClientSpockMethodRequestProcessingBodyBuilder" | { Contract dsl -> new JaxRsClientSpockMethodRequestProcessingBodyBuilder(dsl, properties) } + "JaxRsClientJUnitMethodBodyBuilder" | { Contract dsl -> new JaxRsClientJUnitMethodBodyBuilder(dsl, properties) } + } + }