From 0fa5b5eae37141a0124ff9f7d5bdfeb38928bb7a Mon Sep 17 00:00:00 2001 From: Ronald Holshausen Date: Wed, 15 Jun 2022 17:29:07 +1000 Subject: [PATCH] fix: make the use of content type overrides consistent #1569 --- .../dius/pact/core/matchers/MatchingConfig.kt | 9 ++++--- .../core/matchers/MatchingConfigSpec.groovy | 2 ++ .../com/dius/pact/core/model/ContentType.kt | 26 ++++++++++++++++--- .../pact/core/model/ContentTypeSpec.groovy | 8 ++++++ .../dius/pact/core/model/HttpPartSpec.groovy | 16 ++++++++++++ 5 files changed, 53 insertions(+), 8 deletions(-) diff --git a/core/matchers/src/main/kotlin/au/com/dius/pact/core/matchers/MatchingConfig.kt b/core/matchers/src/main/kotlin/au/com/dius/pact/core/matchers/MatchingConfig.kt index 2428d82dbd..101021ff9d 100644 --- a/core/matchers/src/main/kotlin/au/com/dius/pact/core/matchers/MatchingConfig.kt +++ b/core/matchers/src/main/kotlin/au/com/dius/pact/core/matchers/MatchingConfig.kt @@ -22,11 +22,11 @@ object MatchingConfig { @JvmStatic fun lookupContentMatcher(contentType: String?): ContentMatcher? { return if (contentType != null) { - val contentType1 = ContentType(contentType) - val contentMatcher = CatalogueManager.findContentMatcher(contentType1) + val ct = ContentType(contentType) + val contentMatcher = CatalogueManager.findContentMatcher(ct) if (contentMatcher != null) { if (!contentMatcher.isCore) { - PluginContentMatcher(contentMatcher, contentType1) + PluginContentMatcher(contentMatcher, ct) } else { coreContentMatcher(contentType) } @@ -44,7 +44,8 @@ object MatchingConfig { val clazz = Class.forName(matcher).kotlin (clazz.objectInstance ?: clazz.createInstance()) as ContentMatcher? } else { - when (System.getProperty("pact.content_type.override.$contentType")) { + val ct = ContentType(contentType) + when (ct.override()) { "json" -> JsonContentMatcher "text" -> PlainTextContentMatcher() else -> null diff --git a/core/matchers/src/test/groovy/au/com/dius/pact/core/matchers/MatchingConfigSpec.groovy b/core/matchers/src/test/groovy/au/com/dius/pact/core/matchers/MatchingConfigSpec.groovy index 182b235118..5c30c8bfcb 100644 --- a/core/matchers/src/test/groovy/au/com/dius/pact/core/matchers/MatchingConfigSpec.groovy +++ b/core/matchers/src/test/groovy/au/com/dius/pact/core/matchers/MatchingConfigSpec.groovy @@ -9,6 +9,7 @@ class MatchingConfigSpec extends Specification { def setupSpec() { System.setProperty('pact.content_type.override.application/x-thrift', 'json') + System.setProperty('pact.content_type.override.application.x-bob', 'json') System.setProperty('pact.content_type.override.application/x-other', 'text') } @@ -27,6 +28,7 @@ class MatchingConfigSpec extends Specification { 'application/json-rpc' | 'au.com.dius.pact.core.matchers.JsonContentMatcher' 'application/jsonrequest' | 'au.com.dius.pact.core.matchers.JsonContentMatcher' 'application/x-thrift' | 'au.com.dius.pact.core.matchers.JsonContentMatcher' + 'application/x-bob' | 'au.com.dius.pact.core.matchers.JsonBodyMatcher' 'application/x-other' | 'au.com.dius.pact.core.matchers.PlainTextContentMatcher' } } diff --git a/core/model/src/main/kotlin/au/com/dius/pact/core/model/ContentType.kt b/core/model/src/main/kotlin/au/com/dius/pact/core/model/ContentType.kt index fcaa59d4c5..16fd9004a2 100644 --- a/core/model/src/main/kotlin/au/com/dius/pact/core/model/ContentType.kt +++ b/core/model/src/main/kotlin/au/com/dius/pact/core/model/ContentType.kt @@ -16,7 +16,7 @@ class ContentType(val contentType: MediaType?) { fun isJson(): Boolean { return if (contentType != null) { - when (System.getProperty("pact.content_type.override.${contentType.baseType}")) { + when (overrideForContentType(contentType)) { "json" -> true else -> { if ("vnd.schemaregistry.v1+json" == contentType.subtype) @@ -33,7 +33,7 @@ class ContentType(val contentType: MediaType?) { } fun isXml(): Boolean = if (contentType != null) { - when (System.getProperty("pact.content_type.override.${contentType.baseType}")) { + when (overrideForContentType(contentType)) { "xml" -> true else -> xmlRegex.matches(contentType.subtype.toLowerCase()) } @@ -81,8 +81,7 @@ class ContentType(val contentType: MediaType?) { val superType = registry.getSupertype(contentType) ?: MediaType.OCTET_STREAM val type = contentType.type val baseType = superType.type - val override = System.getProperty("pact.content_type.override.$type.${contentType.subtype}") - ?: System.getProperty("pact.content_type.override.$type/${contentType.subtype}") + val override = overrideForContentType(contentType) when { override.isNotEmpty() -> override == "binary" type == "text" || baseType == "text" -> false @@ -133,6 +132,12 @@ class ContentType(val contentType: MediaType?) { } } + /** + * If there is an override defined for the content type. Overrides are defined with a JVM system property + * in the format pact.content_type.override..=text|json|binary|... + */ + fun override() = overrideForContentType(this.contentType) + companion object : KLogging() { @JvmStatic fun fromString(contentType: String?) = if (contentType.isNullOrEmpty()) { @@ -164,5 +169,18 @@ class ContentType(val contentType: MediaType?) { val XML = ContentType("application/xml") @JvmStatic val KAFKA_SCHEMA_REGISTRY_JSON = ContentType("application/vnd.schemaregistry.v1+json") + + /** + * If there is an override defined for the given content type. Overrides are defined with a JVM system property + * in the format pact.content_type.override..=text|json|binary|... + */ + fun overrideForContentType(contentType: MediaType?): String? { + return if (contentType != null) { + (System.getProperty("pact.content_type.override.${contentType.type}.${contentType.subtype}") + ?: System.getProperty("pact.content_type.override.${contentType.type}/${contentType.subtype}")) + } else { + null + } + } } } diff --git a/core/model/src/test/groovy/au/com/dius/pact/core/model/ContentTypeSpec.groovy b/core/model/src/test/groovy/au/com/dius/pact/core/model/ContentTypeSpec.groovy index 2fc87db341..ac82d41822 100644 --- a/core/model/src/test/groovy/au/com/dius/pact/core/model/ContentTypeSpec.groovy +++ b/core/model/src/test/groovy/au/com/dius/pact/core/model/ContentTypeSpec.groovy @@ -15,6 +15,10 @@ class ContentTypeSpec extends Specification { System.setProperty('pact.content_type.override.application/x-other', 'text') System.setProperty('pact.content_type.override.application/x-bin', 'binary') System.setProperty('pact.content_type.override.application/x-ml', 'xml') + System.setProperty('pact.content_type.override.application.other', 'json') + System.setProperty('pact.content_type.override.application.other-bin', 'binary') + System.setProperty('pact.content_type.override.application.other-text', 'text') + System.setProperty('pact.content_type.override.application.other-xml', 'xml') } @Unroll @@ -35,6 +39,7 @@ class ContentTypeSpec extends Specification { 'application/x-thrift' || true 'application/x-other' || false 'application/graphql' || true + 'application/other' || true contentType = new ContentType(value) } @@ -78,6 +83,7 @@ class ContentTypeSpec extends Specification { 'application/STUFF+XML' || true 'application/x-ml' || true 'application/x-thrift' || false + 'application/other-xml' || true contentType = new ContentType(value) } @@ -123,6 +129,8 @@ class ContentTypeSpec extends Specification { 'multipart/form-data' || true 'application/x-www-form-urlencoded' || false 'application/x-bin' || true + 'application/other-bin' || true + 'application/other' || false contentType = new ContentType(value) } diff --git a/core/model/src/test/groovy/au/com/dius/pact/core/model/HttpPartSpec.groovy b/core/model/src/test/groovy/au/com/dius/pact/core/model/HttpPartSpec.groovy index eeaa99e230..9fc82c8866 100644 --- a/core/model/src/test/groovy/au/com/dius/pact/core/model/HttpPartSpec.groovy +++ b/core/model/src/test/groovy/au/com/dius/pact/core/model/HttpPartSpec.groovy @@ -79,4 +79,20 @@ class HttpPartSpec extends Specification { 0 * decoder.decode(_) result.valueAsString() == '{}' } + + @Issue('#1569') + @RestoreSystemProperties + def 'takes into account content type overrides with dot format'() { + given: + def json = new JsonValue.Object([body: new JsonValue.StringValue('{}'.chars)]) + System.setProperty('pact.content_type.override.application.x-thrift', 'json') + def decoder = Mock(Base64.Decoder) + + when: + def result = HttpPart.extractBody(json, ContentType.fromString('application/x-thrift'), decoder) + + then: + 0 * decoder.decode(_) + result.valueAsString() == '{}' + } }