diff --git a/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/processors/ExternalRefProcessor.java b/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/processors/ExternalRefProcessor.java index 682e91e201..96958c0374 100644 --- a/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/processors/ExternalRefProcessor.java +++ b/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/processors/ExternalRefProcessor.java @@ -230,10 +230,12 @@ private void processSchema(Schema property, String file) { } if (property instanceof ComposedSchema) { ComposedSchema composed = (ComposedSchema) property; - final Map refMap = Optional.ofNullable(composed.getDiscriminator()) + // discriminator values grouped by the $ref they map to: several values may map to the same schema + final Map> discriminatorValuesByRef = Optional.ofNullable(composed.getDiscriminator()) .map(Discriminator::getMapping).orElse(Collections.emptyMap()).entrySet() - .stream().collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); - Map refCache = (!refMap.isEmpty() && + .stream().collect(Collectors.groupingBy(Map.Entry::getValue, + Collectors.mapping(Map.Entry::getKey, Collectors.toList()))); + Map refCache = (!discriminatorValuesByRef.isEmpty() && (composed.getAnyOf() != null || composed.getOneOf() != null)) ? Stream.of( composed.getAnyOf(), composed.getOneOf() ) @@ -247,12 +249,13 @@ private void processSchema(Schema property, String file) { processProperties(composed.getAnyOf(), file); processProperties(composed.getOneOf(), file); - if (!refMap.isEmpty() && !refCache.isEmpty()) { + if (!discriminatorValuesByRef.isEmpty() && !refCache.isEmpty()) { refCache.entrySet() .stream().filter(e -> !e.getKey().equals(e.getValue().get$ref())) .forEach(entry -> { String newRef = entry.getValue().get$ref(); - property.getDiscriminator().getMapping().put(refMap.get(entry.getKey()), newRef); + discriminatorValuesByRef.getOrDefault(entry.getKey(), Collections.emptyList()) + .forEach(value -> property.getDiscriminator().getMapping().put(value, newRef)); }); } } diff --git a/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/processors/SchemaProcessor.java b/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/processors/SchemaProcessor.java index 9e6e7fafb2..9183cba215 100644 --- a/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/processors/SchemaProcessor.java +++ b/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/processors/SchemaProcessor.java @@ -183,20 +183,18 @@ private void changeDiscriminatorMapping(ComposedSchema composedSchema, String ol String oldName = RefUtils.computeDefinitionName(oldRef); String newName = RefUtils.computeDefinitionName(newRef); - String mappingName = null; + // several discriminator values may map to the same schema: update all of them + boolean mapped = false; if (discriminator.getMapping() != null) { - for (String name : discriminator.getMapping().keySet()) { - if (oldRef.equals(discriminator.getMapping().get(name))) { - mappingName = name; - break; + for (Map.Entry entry : discriminator.getMapping().entrySet()) { + if (oldRef.equals(entry.getValue())) { + entry.setValue(newRef); + mapped = true; } } - if (mappingName != null) { - discriminator.getMapping().put(mappingName, newRef); - } } - if (mappingName == null && !oldName.equals(newName)) { + if (!mapped && !oldName.equals(newName)) { if (discriminator.getMapping() == null) { discriminator.setMapping(new HashMap()); } diff --git a/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/Issue2406Test.java b/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/Issue2406Test.java new file mode 100644 index 0000000000..c23ae748ed --- /dev/null +++ b/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/Issue2406Test.java @@ -0,0 +1,41 @@ +package io.swagger.v3.parser.test; + +import io.swagger.v3.oas.models.media.Discriminator; +import io.swagger.v3.oas.models.media.Schema; +import io.swagger.v3.parser.OpenAPIV3Parser; +import io.swagger.v3.parser.core.models.ParseOptions; +import io.swagger.v3.parser.core.models.SwaggerParseResult; +import org.junit.Test; + +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; + +public class Issue2406Test { + + @Test + public void shouldUpdateDiscriminatorMappingOfOneOfNestedInArrayItemsWithExternalRefs() { + ParseOptions options = new ParseOptions(); + options.setResolve(true); + SwaggerParseResult result = new OpenAPIV3Parser().readLocation("issue-2406/openapi.yaml", null, options); + + assertEquals(result.getMessages().toString(), 0, result.getMessages().size()); + + Schema searchRequest = result.getOpenAPI().getComponents().getSchemas().get("SearchRequest"); + Schema items = ((Schema) searchRequest.getProperties().get("query")).getItems(); + List oneOfRefs = items.getOneOf().stream().map(Schema::get$ref).collect(Collectors.toList()); + assertEquals(Arrays.asList("#/components/schemas/TypeA", "#/components/schemas/TypeB"), oneOfRefs); + + Discriminator discriminator = items.getDiscriminator(); + assertEquals("kind", discriminator.getPropertyName()); + // no entry with a null key must be added, and the mapping must follow the rewritten oneOf refs + assertFalse(discriminator.getMapping().containsKey(null)); + assertEquals(3, discriminator.getMapping().size()); + assertEquals("#/components/schemas/TypeA", discriminator.getMapping().get("a")); + assertEquals("#/components/schemas/TypeB", discriminator.getMapping().get("b")); + assertEquals("#/components/schemas/TypeB", discriminator.getMapping().get("bLegacy")); + } +} diff --git a/modules/swagger-parser-v3/src/test/resources/issue-2406/openapi.yaml b/modules/swagger-parser-v3/src/test/resources/issue-2406/openapi.yaml new file mode 100644 index 0000000000..55494cfd17 --- /dev/null +++ b/modules/swagger-parser-v3/src/test/resources/issue-2406/openapi.yaml @@ -0,0 +1,17 @@ +openapi: 3.0.3 +info: + title: Discriminator mapping of a oneOf nested in array items with external refs + version: 1.0.0 +paths: + /search: + post: + operationId: search + requestBody: + required: true + content: + application/json: + schema: + $ref: './schemas/SearchRequest.yaml' + responses: + '204': + description: OK diff --git a/modules/swagger-parser-v3/src/test/resources/issue-2406/schemas/SearchRequest.yaml b/modules/swagger-parser-v3/src/test/resources/issue-2406/schemas/SearchRequest.yaml new file mode 100644 index 0000000000..8486070418 --- /dev/null +++ b/modules/swagger-parser-v3/src/test/resources/issue-2406/schemas/SearchRequest.yaml @@ -0,0 +1,15 @@ +type: object +properties: + query: + type: array + items: + oneOf: + - $ref: './TypeA.yaml' + - $ref: './TypeB.yaml' + discriminator: + propertyName: kind + mapping: + a: './TypeA.yaml' + b: './TypeB.yaml' + # several discriminator values may map to the same schema + bLegacy: './TypeB.yaml' diff --git a/modules/swagger-parser-v3/src/test/resources/issue-2406/schemas/TypeA.yaml b/modules/swagger-parser-v3/src/test/resources/issue-2406/schemas/TypeA.yaml new file mode 100644 index 0000000000..fec0216653 --- /dev/null +++ b/modules/swagger-parser-v3/src/test/resources/issue-2406/schemas/TypeA.yaml @@ -0,0 +1,7 @@ +type: object +required: [kind] +properties: + kind: + type: string + valueA: + type: string diff --git a/modules/swagger-parser-v3/src/test/resources/issue-2406/schemas/TypeB.yaml b/modules/swagger-parser-v3/src/test/resources/issue-2406/schemas/TypeB.yaml new file mode 100644 index 0000000000..9e61287f30 --- /dev/null +++ b/modules/swagger-parser-v3/src/test/resources/issue-2406/schemas/TypeB.yaml @@ -0,0 +1,7 @@ +type: object +required: [kind] +properties: + kind: + type: string + valueB: + type: integer