Skip to content

Commit 8319043

Browse files
jpfinnejpfinne
committed
Improve normalization
1 parent bcf2047 commit 8319043

8 files changed

Lines changed: 371 additions & 65 deletions

File tree

‎modules/openapi-generator/src/main/java/org/openapitools/codegen/OpenAPINormalizer.java‎

Lines changed: 139 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -646,7 +646,7 @@ protected void normalizeComponentsSchemas() {
646646
schemas.put(schemaName, normalizeSchema(schema, new HashSet<>()));
647647

648648
if (getRule(REPLACE_ONE_OF_BY_DISCRIMINATOR_MAPPING)) {
649-
ensureInheritance(schema, schemaName);
649+
ensureInheritanceForDiscriminatorMappings(schema, schemaName);
650650
}
651651
}
652652
}
@@ -1585,85 +1585,201 @@ protected Schema processSimplifyOneOf(Schema schema) {
15851585
* Ensure inheritance is correctly defined for OneOf and Discriminators.
15861586
*
15871587
* For schemas containing oneOf and discriminator.propertyName:
1588-
* Create the mappings as $refs
1589-
* Remove OneOf
1590-
*
1591-
* For referenced schemas, ensure that there is an allOf with this schema.
1588+
* <ul>
1589+
* <li>Create the mappings as $refs</li>
1590+
* <li>Remove OneOf</li>
1591+
* </ul>
15921592
*/
15931593
protected Schema processReplaceOneOfByMapping(Schema schema) {
15941594
if (!getRule(REPLACE_ONE_OF_BY_DISCRIMINATOR_MAPPING)) {
15951595
return schema;
15961596
}
15971597
Discriminator discriminator = schema.getDiscriminator();
15981598
if (discriminator != null) {
1599-
if (discriminator.getMapping() == null) {
1599+
boolean inlineSchema = isInlineSchema(schema);
1600+
if (inlineSchema) {
1601+
// the For referenced schemas, ensure that there is an allOf with this schema.
1602+
LOGGER.warn("Inline oneOf schema not supported by REPLACE_ONE_OF_BY_DISCRIMINATOR_MAPPING normalization");
1603+
return schema;
1604+
}
1605+
if (discriminator.getMapping() == null && discriminator.getPropertyName() != null) {
16001606
Map<String, String> mappings = new TreeMap<>();
16011607
discriminator.setMapping(mappings);
1602-
for (Object oneOfObject : schema.getOneOf()) {
1603-
Schema oneOf = (Schema) oneOfObject;
1608+
List<Schema> oneOfs = schema.getOneOf();
1609+
for (Schema oneOf: oneOfs) {
16041610
String refSchema = oneOf.get$ref();
16051611
if (refSchema != null) {
1606-
String name = getDiscriminatorValue(refSchema);
1612+
boolean hasProperty = findProperty(schema, discriminator.getPropertyName(), false, new HashSet<>()) != null;
1613+
String name = getDiscriminatorValue(refSchema, discriminator.getPropertyName(), hasProperty);
16071614
mappings.put(name, refSchema);
16081615
}
16091616
}
16101617
}
1611-
1612-
1618+
// remove oneOf and only keep the discriminator mapping
16131619
schema.setOneOf(null);
16141620
}
1621+
16151622
return schema;
16161623
}
16171624

1618-
protected String getDiscriminatorValue(String refSchema) {
1619-
String schemaName = refSchema.contains("/") ? refSchema.substring(refSchema.lastIndexOf('/') + 1) : refSchema;;
1625+
private boolean isInlineSchema(Schema schema) {
1626+
if (openAPI.getComponents()!=null && openAPI.getComponents().getSchemas()!=null) {
1627+
int identity = System.identityHashCode(schema);
1628+
for (Schema componentSchema: openAPI.getComponents().getSchemas().values()) {
1629+
if (System.identityHashCode(componentSchema) == identity) {
1630+
return false;
1631+
}
1632+
}
1633+
}
1634+
return true;
1635+
}
1636+
1637+
/**
1638+
* Best effort to retrieve a good discriminator value.
1639+
* By order of precedence:
1640+
* <ul>
1641+
* <li>x-discriminator-value</li>
1642+
* <li>single enum value for attribute used by the discriminator.propertyName</li>
1643+
* <li>hame of the schema</li>
1644+
* </ul>
1645+
*
1646+
* @param refSchema $ref value like #/components/schemas/Dog
1647+
* @param discriminatorPropertyName name of the property used in the discriminator mapping
1648+
* @param propertyAlreadyPresent if true, delete the property in the referenced schemas to avoid duplicates
1649+
*
1650+
* @return the name
1651+
*/
1652+
protected String getDiscriminatorValue(String refSchema, String discriminatorPropertyName, boolean propertyAlreadyPresent) {
1653+
String schemaName = ModelUtils.getSimpleRef(refSchema);
16201654
Schema schema = ModelUtils.getSchema(openAPI, schemaName);
1655+
Schema property = findProperty(schema, discriminatorPropertyName, propertyAlreadyPresent, new HashSet<>());
16211656
if (schema != null && schema.getExtensions() != null) {
16221657
Object discriminatorValue = schema.getExtensions().get("x-discriminator-value");
16231658
if (discriminatorValue != null) {
16241659
return discriminatorValue.toString();
16251660
}
16261661
}
1662+
1663+
// find the discriminator value as a unique enum value
1664+
if (property != null) {
1665+
List enums = property.getEnum();
1666+
if (enums != null && enums.size() == 1) {
1667+
return enums.get(0).toString();
1668+
}
1669+
}
1670+
16271671
return schemaName;
16281672
}
16291673

1674+
/**
1675+
* find a property under the schema.
1676+
*
1677+
* @param schema
1678+
* @param propertyName property to find
1679+
* @param toDelete if true delete the found property
1680+
* @param visitedSchemas avoid infinite recursion
1681+
* @return found property or null if not found.
1682+
*/
1683+
private Schema findProperty(Schema schema, String propertyName, boolean toDelete, HashSet<Object> visitedSchemas) {
1684+
if (propertyName == null || schema == null || visitedSchemas.contains(schema)) {
1685+
return null;
1686+
}
1687+
visitedSchemas.add(schema);
1688+
Map<String, Schema> properties = schema.getProperties();
1689+
if (properties != null) {
1690+
Schema property = properties.get(propertyName);
1691+
if (property != null) {
1692+
if (toDelete) {
1693+
if (schema.getProperties().remove(propertyName) != null) {
1694+
schema.setProperties(null);
1695+
}
1696+
}
1697+
return property;
1698+
}
1699+
}
1700+
List<Schema> allOfs = schema.getAllOf();
1701+
if (allOfs != null) {
1702+
for (Schema child : allOfs) {
1703+
Schema found = findProperty(child, propertyName, toDelete, visitedSchemas);
1704+
if (found != null) {
1705+
return found;
1706+
}
1707+
}
1708+
}
1709+
1710+
return null;
1711+
}
1712+
16301713

1631-
protected void ensureInheritance(Schema parent, String parentName) {
1714+
/**
1715+
* ensure that all schemas referenced in the discriminator mapping has an allOf to the parent schema.
1716+
*
1717+
* This allows DefaultCodeGen to detect inheritance.
1718+
*
1719+
* @param parent parent schma
1720+
* @param parentName name of the parent schema
1721+
*/
1722+
protected void ensureInheritanceForDiscriminatorMappings(Schema parent, String parentName) {
16321723
Discriminator discriminator = parent.getDiscriminator();
16331724
if (discriminator != null && discriminator.getMapping() != null) {
1634-
16351725
for (String mapping : discriminator.getMapping().values()) {
1636-
String refSchemaName = getDiscriminatorValue(mapping);
1726+
String refSchemaName = ModelUtils.getSimpleRef(mapping);
16371727
Schema child = ModelUtils.getSchema(openAPI, refSchemaName);
16381728
if (child != null) {
1639-
ensureInheritance(parent, child, parentName);
1729+
if (parentName != null) {
1730+
ensureInheritanceForDiscriminatorMappings(parent, child, parentName, new HashSet<>());
1731+
}
16401732
}
16411733
}
16421734
}
16431735
}
16441736

1645-
protected void ensureInheritance(Schema parent, Schema child, String parentName) {
1737+
/**
1738+
* If not already present, add in the child an allOf referencing the parent.
1739+
*/
1740+
protected void ensureInheritanceForDiscriminatorMappings(Schema parent, Schema child, String parentName, Set<Schema> visitedSchemas) {
16461741
String reference = "#/components/schemas/" + parentName;
16471742
List<Schema> allOf = child.getAllOf();
16481743
if (allOf != null) {
1649-
if (hasParent(child, parent, reference)) {
1744+
if (hasParent(parent, child, reference, visitedSchemas)) {
1745+
// already done, so no need to add
16501746
return;
16511747
}
1748+
Schema refToParent = new Schema<>().$ref(reference);
1749+
allOf.add(refToParent);
16521750
} else {
16531751
allOf = new ArrayList<>();
16541752
child.setAllOf(allOf);
1753+
Schema refToParent = new Schema<>().$ref(reference);
1754+
allOf.add(refToParent);
1755+
if (child.getProperties() != null) {
1756+
// move the properties inside the new allOf.
1757+
Schema childProperties = new Schema<>().properties(child.getProperties());
1758+
allOf.add(childProperties);
1759+
child.setProperties(null);
1760+
child.setType(null);
1761+
}
16551762
}
1656-
Schema refToParent = new Schema<>().$ref(reference);
1657-
allOf.add(refToParent);
16581763
}
16591764

1660-
private boolean hasParent(Schema child, Schema parent, String reference) {
1765+
/**
1766+
* return true if the child as an allOf referencing the parent scham.
1767+
*/
1768+
private boolean hasParent(Schema parent, Schema child, String reference, Set<Schema> visitedSchemas) {
16611769
if (child.get$ref() != null && child.get$ref().equals(reference)) {
16621770
return true;
16631771
}
16641772
List<Schema> allOf = child.getAllOf();
16651773
if (allOf != null) {
1666-
return allOf.stream().anyMatch(s -> hasParent(s, parent, reference));
1774+
for (Schema schema : allOf) {
1775+
if (visitedSchemas.contains(schema)) {
1776+
return false;
1777+
}
1778+
visitedSchemas.add(schema);
1779+
if (hasParent(schema, parent, reference, visitedSchemas)) {
1780+
return true;
1781+
}
1782+
}
16671783
}
16681784
return false;
16691785
}

‎modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,10 @@
1717

1818
package org.openapitools.codegen.utils;
1919

20+
import com.fasterxml.jackson.core.JsonProcessingException;
2021
import com.fasterxml.jackson.databind.JsonNode;
2122
import com.fasterxml.jackson.databind.ObjectMapper;
23+
import io.swagger.util.Yaml;
2224
import io.swagger.v3.core.util.AnnotationsUtils;
2325
import io.swagger.v3.oas.models.OpenAPI;
2426
import io.swagger.v3.oas.models.Operation;
@@ -2645,4 +2647,20 @@ public LinkedHashSet<String> build() {
26452647
}
26462648
}
26472649
}
2650+
2651+
2652+
/*
2653+
* Simplest dump of an openApi contract on the console.
2654+
*
2655+
* Only use for debugging.
2656+
*/
2657+
public static void dumpAsYaml(OpenAPI openAPI) {
2658+
ObjectMapper mapper = Yaml.mapper();
2659+
try {
2660+
String yaml = mapper.writeValueAsString(openAPI);
2661+
System.out.println(yaml);
2662+
} catch (JsonProcessingException e) {
2663+
throw new RuntimeException(e);
2664+
}
2665+
}
26482666
}

‎modules/openapi-generator/src/test/java/org/openapitools/codegen/OpenAPINormalizerTest.java‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1507,7 +1507,6 @@ public Schema normalizeSchema(Schema schema, Set<Schema> visitedSchemas) {
15071507

15081508
@Test
15091509
public void testREPLACE_ONE_OF_BY_DISCRIMINATOR_MAPPING() {
1510-
// to test array schema processing in 3.1 spec
15111510
OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/oneOf_issue_23527.yaml");
15121511

15131512
Map<String, String> inputRules = Map.of("REPLACE_ONE_OF_BY_DISCRIMINATOR_MAPPING", "true");
@@ -1519,5 +1518,30 @@ public void testREPLACE_ONE_OF_BY_DISCRIMINATOR_MAPPING() {
15191518
assertEquals(mapping, Map.of("MultiPolygon", "#/components/schemas/Multi-Polygon", "Polygon", "#/components/schemas/Polygon" ));
15201519
}
15211520

1521+
@Test
1522+
public void issue_14769() {
1523+
OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/oneOf_issue_14769.yaml");
1524+
Map<String, String> inputRules = Map.of("REPLACE_ONE_OF_BY_DISCRIMINATOR_MAPPING", "true");
1525+
OpenAPINormalizer openAPINormalizer = new OpenAPINormalizer(openAPI, inputRules);
1526+
openAPINormalizer.normalize();
1527+
// ModelUtils.dumpAsYaml(openAPI);
1528+
Schema vehicle = openAPI.getComponents().getSchemas().get("Vehicle");
1529+
Map<String, String> mapping = vehicle.getDiscriminator().getMapping();
1530+
assertEquals(mapping, Map.of("car", "#/components/schemas/Car", "plane", "#/components/schemas/Plane" ));
1531+
Schema car = openAPI.getComponents().getSchemas().get("Car");
1532+
assertFalse(car.getProperties().containsKey("type"));
1533+
}
1534+
1535+
@Test
1536+
public void oneOf_issue_23276() {
1537+
OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/oneOf_issue_23276.yaml");
1538+
Map<String, String> inputRules = Map.of("REPLACE_ONE_OF_BY_DISCRIMINATOR_MAPPING", "true");
1539+
OpenAPINormalizer openAPINormalizer = new OpenAPINormalizer(openAPI, inputRules);
1540+
openAPINormalizer.normalize();
1541+
// ModelUtils.dumpAsYaml(openAPI);
1542+
Schema payload = (Schema)openAPI.getComponents().getSchemas().get("DeviceLifecycleEvent").getProperties().get("payload");
1543+
// inline oneOf are not converted
1544+
assertNotNull(payload.getOneOf());
1545+
}
15221546

15231547
}

‎modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java‎

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,6 @@
7272
import static org.openapitools.codegen.CodegenConstants.*;
7373
import static org.openapitools.codegen.TestUtils.*;
7474
import static org.openapitools.codegen.languages.JavaClientCodegen.*;
75-
import static org.openapitools.codegen.languages.SpringCodegen.SPRING_BOOT;
7675
import static org.testng.Assert.*;
7776

7877
public class JavaClientCodegenTest {
@@ -4371,26 +4370,6 @@ void issue_912() {
43714370
.containsWithName("JsonSubTypes")
43724371
.recursivelyContainsWithNameAndAttributes("JsonSubTypes.Type", Map.of("value", "Folder.class", "name", "\"folder\""))
43734372
.recursivelyContainsWithNameAndAttributes("JsonSubTypes.Type", Map.of("value", "Source.class", "name", "\"source\""));
4374-
4375-
}
4376-
4377-
@Test
4378-
void issue_19261() {
4379-
Map<String, File> files = generateFromContract("src/test/resources/3_0/oneOf_issue_19261.yaml", APACHE,
4380-
Map.of(),
4381-
codegen -> codegen.addOpenapiNormalizer("REPLACE_ONE_OF_BY_DISCRIMINATOR_MAPPING", "true")
4382-
.addInlineSchemaOption("REFACTOR_ALLOF_INLINE_SCHEMAS", "true"));
4383-
JavaFileAssert.assertThat(files.get("Product.java"))
4384-
.isNormalClass()
4385-
.doesNotExtendsClasses()
4386-
.fileContains("AboType type")
4387-
.assertTypeAnnotations()
4388-
.containsWithNameAndAttributes("JsonTypeInfo", Map.of("include", "JsonTypeInfo.As.PROPERTY", "property", "\"type\""))
4389-
.containsWithName("JsonSubTypes")
4390-
.recursivelyContainsWithNameAndAttributes("JsonSubTypes.Type", Map.of("value", "HomeProduct.class", "name", "\"home\""))
4391-
.recursivelyContainsWithNameAndAttributes("JsonSubTypes.Type", Map.of("value", "InternetProduct.class", "name", "\"internet\""));
4392-
JavaFileAssert.assertThat(files.get("InternetProduct.java"))
4393-
.extendsClass("Product");
43944373
}
43954374

43964375
}

‎modules/openapi-generator/src/test/java/org/openapitools/codegen/java/spring/SpringCodegenTest.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6762,4 +6762,12 @@ void oneof_polymorphism_and_inheritance() throws IOException {
67626762
.isNormalClass()
67636763
.extendsClass("FruitDto");
67646764
}
6765+
6766+
@Test
6767+
public void issue_14769() throws IOException {
6768+
Map<String, File> files = generateFromContract("src/test/resources/3_0/oneOf_issue_14769.yaml", SPRING_BOOT,
6769+
Map.of(MODEL_NAME_SUFFIX, "Dto",
6770+
GENERATE_MODEL_DOCS, false, GENERATE_APIS, false, INTERFACE_ONLY, true),
6771+
codegen -> codegen.addOpenapiNormalizer("REPLACE_ONE_OF_BY_DISCRIMINATOR_MAPPING", "true"));
6772+
}
67656773
}

‎modules/openapi-generator/src/test/resources/3_0/oneOf_issue_14769.yaml‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
openapi: 3.0.1
2+
info:
3+
title: discriminator_enums
4+
version: '1.0.0'
5+
components:
6+
schemas:
7+
Vehicle:
8+
type: object
9+
required:
10+
- id
11+
- type
12+
properties:
13+
id:
14+
type: integer
15+
type:
16+
type: string
17+
model:
18+
type: string
19+
name:
20+
type: string
21+
oneOf:
22+
- $ref: '#/components/schemas/Car'
23+
- $ref: '#/components/schemas/Plane'
24+
discriminator:
25+
propertyName: type
26+
27+
Car:
28+
type: object
29+
properties:
30+
type:
31+
enum:
32+
- car
33+
has_4_wheel_drive:
34+
type: boolean
35+
36+
Plane:
37+
type: object
38+
properties:
39+
type:
40+
enum:
41+
- plane
42+
has_reactor:
43+
type: boolean
44+
nb_passengers:
45+
type: integer

0 commit comments

Comments
 (0)