Skip to content

Commit 7a846a1

Browse files
jimschubertjimschubert
authored
[bug][core] Copy all attributes (not properties) on composed schemas when flattening models (#7106)
1 parent 6a08ec5 commit 7a846a1

File tree

4 files changed

+344
-13
lines changed

4 files changed

+344
-13
lines changed

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

Lines changed: 62 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@
1717

1818
package org.openapitools.codegen;
1919

20+
import com.fasterxml.jackson.core.JsonProcessingException;
21+
import com.fasterxml.jackson.core.util.DefaultPrettyPrinter;
22+
import com.fasterxml.jackson.databind.MapperFeature;
23+
import com.fasterxml.jackson.databind.ObjectMapper;
2024
import io.swagger.v3.core.util.Json;
2125
import io.swagger.v3.oas.models.*;
2226
import io.swagger.v3.oas.models.callbacks.Callback;
@@ -25,6 +29,7 @@
2529
import io.swagger.v3.oas.models.parameters.RequestBody;
2630
import io.swagger.v3.oas.models.responses.ApiResponse;
2731
import io.swagger.v3.oas.models.responses.ApiResponses;
32+
import org.apache.commons.lang3.StringUtils;
2833
import org.openapitools.codegen.utils.ModelUtils;
2934
import org.slf4j.Logger;
3035
import org.slf4j.LoggerFactory;
@@ -36,6 +41,17 @@ public class InlineModelResolver {
3641
private OpenAPI openapi;
3742
private Map<String, Schema> addedModels = new HashMap<String, Schema>();
3843
private Map<String, String> generatedSignature = new HashMap<String, String>();
44+
45+
// structure mapper sorts properties alphabetically on write to ensure models are
46+
// serialized consistently for lookup of existing models
47+
private static ObjectMapper structureMapper;
48+
49+
static {
50+
structureMapper = Json.mapper().copy();
51+
structureMapper.configure(MapperFeature.SORT_PROPERTIES_ALPHABETICALLY, true);
52+
structureMapper.writer(new DefaultPrettyPrinter());
53+
}
54+
3955
static Logger LOGGER = LoggerFactory.getLogger(InlineModelResolver.class);
4056

4157
void flatten(OpenAPI openapi) {
@@ -488,15 +504,25 @@ private String resolveModelName(String title, String key) {
488504
}
489505

490506
private String matchGenerated(Schema model) {
491-
String json = Json.pretty(model);
492-
if (generatedSignature.containsKey(json)) {
493-
return generatedSignature.get(json);
507+
try {
508+
String json = structureMapper.writeValueAsString(model);
509+
if (generatedSignature.containsKey(json)) {
510+
return generatedSignature.get(json);
511+
}
512+
} catch (JsonProcessingException e) {
513+
e.printStackTrace();
494514
}
515+
495516
return null;
496517
}
497518

498519
private void addGenerated(String name, Schema model) {
499-
generatedSignature.put(Json.pretty(model), name);
520+
try {
521+
String json = structureMapper.writeValueAsString(model);
522+
generatedSignature.put(json, name);
523+
} catch (JsonProcessingException e) {
524+
e.printStackTrace();
525+
}
500526
}
501527

502528
/**
@@ -620,22 +646,45 @@ private Schema modelFromProperty(OpenAPI openAPI, Schema object, String path) {
620646
}
621647
XML xml = object.getXml();
622648
Map<String, Schema> properties = object.getProperties();
649+
650+
// NOTE:
651+
// No need to null check setters below. All defaults in the new'd Schema are null, so setting to null would just be a noop.
623652
Schema model = new Schema();
624-
if (object.getType() != null) {
625-
model.setType(object.getType());
626-
}
627-
if (object.getFormat() != null) {
628-
// Even though the `format` keyword typically applies to primitive types only,
629-
// the JSON schema specification states `format` can be used for any model type instance
630-
// including object types.
631-
model.setFormat(object.getFormat());
632-
}
653+
model.setType(object.getType());
654+
655+
// Even though the `format` keyword typically applies to primitive types only,
656+
// the JSON schema specification states `format` can be used for any model type instance
657+
// including object types.
658+
model.setFormat(object.getFormat());
659+
633660
model.setDescription(description);
634661
model.setExample(example);
635662
model.setName(object.getName());
636663
model.setXml(xml);
637664
model.setRequired(object.getRequired());
638665
model.setNullable(object.getNullable());
666+
model.setDiscriminator(object.getDiscriminator());
667+
model.setWriteOnly(object.getWriteOnly());
668+
model.setUniqueItems(object.getUniqueItems());
669+
model.setTitle(object.getTitle());
670+
model.setReadOnly(object.getReadOnly());
671+
model.setPattern(object.getPattern());
672+
model.setNot(object.getNot());
673+
model.setMinProperties(object.getMinProperties());
674+
model.setMinLength(object.getMinLength());
675+
model.setMinItems(object.getMinItems());
676+
model.setMinimum(object.getMinimum());
677+
model.setMaxProperties(object.getMaxProperties());
678+
model.setMaxLength(object.getMaxLength());
679+
model.setMaxItems(object.getMaxItems());
680+
model.setMaximum(object.getMaximum());
681+
model.setExternalDocs(object.getExternalDocs());
682+
model.setExtensions(object.getExtensions());
683+
model.setExclusiveMinimum(object.getExclusiveMinimum());
684+
model.setExclusiveMaximum(object.getExclusiveMaximum());
685+
model.setExample(object.getExample());
686+
model.setDeprecated(object.getDeprecated());
687+
639688
if (properties != null) {
640689
flattenProperties(openAPI, properties, path);
641690
model.setProperties(properties);

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

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -785,6 +785,109 @@ public void objectComposedWithInline() {
785785
checkComposedChildren(openAPI, schema.getOneOf(), "oneOf");
786786
}
787787

788+
789+
@Test
790+
public void inheritanceWithInlineDiscriminator() {
791+
OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/2_0/regression_6905.yaml");
792+
new InlineModelResolver().flatten(openAPI);
793+
794+
assertTrue(openAPI.getComponents().getSchemas().get("PartyType") instanceof StringSchema);
795+
assertTrue(openAPI.getComponents().getSchemas().get("CustomerType") instanceof StringSchema);
796+
assertTrue(openAPI.getComponents().getSchemas().get("Entity") instanceof ObjectSchema);
797+
798+
assertTrue(openAPI.getComponents().getSchemas().get("Party") instanceof ComposedSchema);
799+
assertTrue(openAPI.getComponents().getSchemas().get("Contact") instanceof ComposedSchema);
800+
assertTrue(openAPI.getComponents().getSchemas().get("Customer") instanceof ComposedSchema);
801+
assertTrue(openAPI.getComponents().getSchemas().get("Person") instanceof ComposedSchema);
802+
assertTrue(openAPI.getComponents().getSchemas().get("Organization") instanceof ComposedSchema);
803+
804+
assertTrue(openAPI.getComponents().getSchemas().get("ApiError") instanceof ObjectSchema);
805+
806+
assertFalse(openAPI.getComponents().getSchemas().get("Party_allOf") instanceof ComposedSchema);
807+
assertFalse(openAPI.getComponents().getSchemas().get("Contact_allOf") instanceof ComposedSchema);
808+
assertFalse(openAPI.getComponents().getSchemas().get("Customer_allOf") instanceof ComposedSchema);
809+
assertFalse(openAPI.getComponents().getSchemas().get("Person_allOf") instanceof ComposedSchema);
810+
assertFalse(openAPI.getComponents().getSchemas().get("Organization_allOf") instanceof ComposedSchema);
811+
812+
// Party
813+
ComposedSchema party = (ComposedSchema) openAPI.getComponents().getSchemas().get("Party");
814+
List<Schema> partySchemas = party.getAllOf();
815+
Schema entity = ModelUtils.getReferencedSchema(openAPI, partySchemas.get(0));
816+
Schema partyAllOf = ModelUtils.getReferencedSchema(openAPI, partySchemas.get(1));
817+
818+
assertEquals(partySchemas.get(0).get$ref(), "#/components/schemas/Entity");
819+
assertEquals(partySchemas.get(1).get$ref(), "#/components/schemas/Party_allOf");
820+
821+
assertNull(party.getDiscriminator());
822+
assertNull(entity.getDiscriminator());
823+
assertNotNull(partyAllOf.getDiscriminator());
824+
assertEquals(partyAllOf.getDiscriminator().getPropertyName(), "party_type");
825+
assertEquals(partyAllOf.getRequired().get(0), "party_type");
826+
827+
828+
// Contact
829+
ComposedSchema contact = (ComposedSchema) openAPI.getComponents().getSchemas().get("Contact");
830+
Schema contactAllOf = ModelUtils.getReferencedSchema(openAPI, contact.getAllOf().get(1));
831+
832+
assertEquals(contact.getExtensions().get("x-discriminator-value"), "contact");
833+
assertEquals(contact.getAllOf().get(0).get$ref(), "#/components/schemas/Party");
834+
assertEquals(contact.getAllOf().get(1).get$ref(), "#/components/schemas/Contact_allOf");
835+
assertNull(contactAllOf.getDiscriminator());
836+
837+
838+
// Customer
839+
ComposedSchema customer = (ComposedSchema) openAPI.getComponents().getSchemas().get("Customer");
840+
List<Schema> customerSchemas = customer.getAllOf();
841+
Schema customerAllOf = ModelUtils.getReferencedSchema(openAPI, customerSchemas.get(1));
842+
843+
assertEquals(customerSchemas.get(0).get$ref(), "#/components/schemas/Party");
844+
assertNull(customer.getDiscriminator());
845+
assertEquals(customer.getExtensions().get("x-discriminator-value"), "customer");
846+
847+
// Discriminators are not defined at this level in the schema doc
848+
assertNull(customerSchemas.get(0).getDiscriminator());
849+
assertEquals(customerSchemas.get(1).get$ref(), "#/components/schemas/Customer_allOf");
850+
assertNull(customerSchemas.get(1).getDiscriminator());
851+
852+
// Customer -> Party where Customer defines it's own discriminator
853+
assertNotNull(customerAllOf.getDiscriminator());
854+
assertEquals(customerAllOf.getDiscriminator().getPropertyName(), "customer_type");
855+
assertEquals(customerAllOf.getRequired().get(0), "customer_type");
856+
857+
858+
// Person
859+
ComposedSchema person = (ComposedSchema) openAPI.getComponents().getSchemas().get("Person");
860+
List<Schema> personSchemas = person.getAllOf();
861+
Schema personAllOf = ModelUtils.getReferencedSchema(openAPI, personSchemas.get(1));
862+
863+
// Discriminators are not defined at this level in the schema doc
864+
assertEquals(personSchemas.get(0).get$ref(), "#/components/schemas/Customer");
865+
assertNull(personSchemas.get(0).getDiscriminator());
866+
assertEquals(personSchemas.get(1).get$ref(), "#/components/schemas/Person_allOf");
867+
assertNull(personSchemas.get(1).getDiscriminator());
868+
869+
// Person -> Customer -> Party, so discriminator is not at this level
870+
assertNull(person.getDiscriminator());
871+
assertEquals(person.getExtensions().get("x-discriminator-value"), "person");
872+
assertNull(personAllOf.getDiscriminator());
873+
874+
875+
// Organization
876+
ComposedSchema organization = (ComposedSchema) openAPI.getComponents().getSchemas().get("Organization");
877+
List<Schema> organizationSchemas = organization.getAllOf();
878+
Schema organizationAllOf = ModelUtils.getReferencedSchema(openAPI, organizationSchemas.get(1));
879+
880+
// Discriminators are not defined at this level in the schema doc
881+
assertEquals(organizationSchemas.get(0).get$ref(), "#/components/schemas/Customer");
882+
assertNull(organizationSchemas.get(0).getDiscriminator());
883+
assertEquals(organizationSchemas.get(1).get$ref(), "#/components/schemas/Organization_allOf");
884+
assertNull(organizationSchemas.get(1).getDiscriminator());
885+
886+
// Organization -> Customer -> Party, so discriminator is not at this level
887+
assertNull(organizationAllOf.getDiscriminator());
888+
assertEquals(organization.getExtensions().get("x-discriminator-value"), "organization");
889+
}
890+
788891
@Test
789892
public void arbitraryObjectModelWithArrayInlineWithTitle() {
790893
OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/inline_model_resolver.yaml");
@@ -888,4 +991,9 @@ public void callbacks() {
888991
assertTrue(properties.get("action") instanceof StringSchema);
889992
assertTrue(properties.get("data") instanceof StringSchema);
890993
}
994+
995+
@Test
996+
public void regresssion_6905() {
997+
998+
}
891999
}

0 commit comments

Comments
 (0)