Skip to content

refactor: centralize common schema checks in ModelUtils #23608

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Mattias-Sehlstedt wants to merge 1 commit into
base: master
Choose a base branch
from
Open

refactor: centralize common schema checks in ModelUtils #23608

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2515,7 +2515,7 @@ private String getPrimitiveType(Schema schema) {
// Note: the value of a free-form object cannot be an arbitrary type. Per OAS specification,
// it must be a map of string to values.
return "object";
} else if (schema.getProperties() != null && !schema.getProperties().isEmpty()) { // having property implies it's a model
} else if (ModelUtils.hasProperties(schema)) { // having property implies it's a model
return "object";
} else if (ModelUtils.isAnyType(schema)) {
return "AnyType";
Expand Down Expand Up @@ -2707,8 +2707,8 @@ protected void updateModelForComposedSchema(CodegenModel m, Schema schema, Map<S
List<String> allRequired = new ArrayList<>();

// if schema has properties outside of allOf/oneOf/anyOf also add them to m
if (composed.getProperties() != null && !composed.getProperties().isEmpty()) {
if (composed.getOneOf() != null && !composed.getOneOf().isEmpty()) {
if (ModelUtils.hasProperties(composed)) {
if (ModelUtils.hasOneOf(composed)) {
LOGGER.warn("'oneOf' is intended to include only the additional optional OAS extension discriminator object. " +
"For more details, see https://json-schema.org/draft/2019-09/json-schema-core.html#rfc.section.9.2.1.3 and the OAS section on 'Composition and Inheritance'.");
}
Expand Down Expand Up @@ -3362,7 +3362,7 @@ private CodegenProperty discriminatorFound(String composedSchemaName, Schema sc,
}
}
}
if (composedSchema.getOneOf() != null && composedSchema.getOneOf().size() != 0) {
if (ModelUtils.hasOneOf(composedSchema)) {
// All oneOf definitions must contain the discriminator
CodegenProperty cp = new CodegenProperty();
for (Object oneOf : composedSchema.getOneOf()) {
Expand All @@ -3388,7 +3388,7 @@ private CodegenProperty discriminatorFound(String composedSchemaName, Schema sc,
}
return cp;
}
if (composedSchema.getAnyOf() != null && composedSchema.getAnyOf().size() != 0) {
if (ModelUtils.hasAnyOf(composedSchema)) {
// All anyOf definitions must contain the discriminator because a min of one must be selected
CodegenProperty cp = new CodegenProperty();
for (Object anyOf : composedSchema.getAnyOf()) {
Expand Down Expand Up @@ -3457,7 +3457,7 @@ private Discriminator recursiveGetDiscriminator(Schema sc, ArrayList<Schema> vis
}
}
}
if (composedSchema.getOneOf() != null && composedSchema.getOneOf().size() != 0) {
if (ModelUtils.hasOneOf(composedSchema)) {
// All oneOf definitions must contain the discriminator
Integer hasDiscriminatorCnt = 0;
Integer hasNullTypeCnt = 0;
Expand Down Expand Up @@ -3792,7 +3792,7 @@ protected void addProperties(Map<String, Schema> properties, List<String> requir
}
if (ModelUtils.isComposedSchema(schema)) {
// fix issue #16797 and #15796, constructor fail by missing parent required params
if (schema.getProperties() != null && !schema.getProperties().isEmpty()) {
if (ModelUtils.hasProperties(schema)) {
properties.putAll(schema.getProperties());
}

Expand Down Expand Up @@ -8599,7 +8599,7 @@ public void setRemoveEnumValuePrefix(final boolean removeEnumValuePrefix) {
* @param name name of the parent oneOf schema
*/
public void addOneOfNameExtension(Schema schema, String name) {
if (schema.getOneOf() != null && !schema.getOneOf().isEmpty()) {
if (ModelUtils.hasOneOf(schema)) {
schema.addExtension(X_ONE_OF_NAME, name);
}
}
Expand Down
27 changes: 12 additions & 15 deletions modules/openapi-generator/src/main/java/org/openapitools/codegen/InlineModelResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import io.swagger.v3.oas.models.parameters.RequestBody;
import io.swagger.v3.oas.models.responses.ApiResponse;
import io.swagger.v3.oas.models.responses.ApiResponses;
import java.util.stream.Collectors;
import org.apache.commons.lang3.StringUtils;
import org.openapitools.codegen.utils.ModelUtils;
import org.slf4j.Logger;
Expand Down Expand Up @@ -236,9 +235,9 @@ private boolean isModelNeeded(Schema schema, Set<Schema> visitedSchemas) {
if (resolveInlineEnums && schema.getEnum() != null && schema.getEnum().size() > 0) {
return true;
}
if (schema.getType() == null || "object".equals(schema.getType())) {
if (schema.getType() == null || ModelUtils.isObjectTypeOAS30(schema)) {
// object or undeclared type with properties
if (schema.getProperties() != null && schema.getProperties().size() > 0) {
if (ModelUtils.hasProperties(schema)) {
return true;
}
}
Expand All @@ -264,7 +263,7 @@ private boolean isModelNeeded(Schema schema, Set<Schema> visitedSchemas) {
return isModelNeeded((Schema) schema.getAllOf().get(0), visitedSchemas);
}

if (schema.getAllOf() != null && !schema.getAllOf().isEmpty()) {
if (ModelUtils.hasAllOf(schema)) {
// check to ensure at least one of the allOf item is model
for (Object inner : schema.getAllOf()) {
if (isModelNeeded(ModelUtils.getReferencedSchema(openAPI, (Schema) inner), visitedSchemas)) {
Expand All @@ -275,10 +274,10 @@ private boolean isModelNeeded(Schema schema, Set<Schema> visitedSchemas) {
return false;
}

if (schema.getAnyOf() != null && !schema.getAnyOf().isEmpty()) {
if (ModelUtils.hasAnyOf(schema)) {
return true;
}
if (schema.getOneOf() != null && !schema.getOneOf().isEmpty()) {
if (ModelUtils.hasOneOf(schema)) {
return true;
}
}
Expand All @@ -297,9 +296,9 @@ private void gatherInlineModels(Schema schema, String modelPrefix) {
if (schema.get$ref() != null) {
// if ref already, no inline schemas should be present but check for
// any to catch OpenAPI violations
if (isModelNeeded(schema) || "object".equals(schema.getType()) ||
if (isModelNeeded(schema) || ModelUtils.isObjectTypeOAS30(schema) ||
schema.getProperties() != null || schema.getAdditionalProperties() != null ||
ModelUtils.isComposedSchema(schema)) {
ModelUtils.isComposedSchema(schema)) {
LOGGER.error("Illegal schema found with $ref combined with other properties," +
" no properties should be defined alongside a $ref:\n " + schema.toString());
}
Expand All @@ -308,7 +307,7 @@ private void gatherInlineModels(Schema schema, String modelPrefix) {
// Check object models / any type models / composed models for properties,
// if the schema has a type defined that is not "object" it should not define
// any properties
if (schema.getType() == null || "object".equals(schema.getType())) {
if (schema.getType() == null || ModelUtils.isObjectTypeOAS30(schema)) {
// Check properties and recurse, each property could be its own inline model
Map<String, Schema> props = schema.getProperties();
if (props != null) {
Expand Down Expand Up @@ -640,10 +639,8 @@ private void flattenComposedChildren(String key, List<Schema> children, boolean
ListIterator<Schema> listIterator = children.listIterator();
while (listIterator.hasNext()) {
Schema component = listIterator.next();
if ((component != null) &&
(component.get$ref() == null) &&
((component.getProperties() != null && !component.getProperties().isEmpty()) ||
(component.getEnum() != null && !component.getEnum().isEmpty()))) {
boolean componentDoesNotHaveRef = component != null && component.get$ref() == null;
if (componentDoesNotHaveRef && (ModelUtils.hasProperties(component) || ModelUtils.hasEnum(component))) {
// If a `title` attribute is defined in the inline schema, codegen uses it to name the
// inline schema. Otherwise, we'll use the default naming such as InlineObject1, etc.
// We know that this is not the best way to name the model.
Expand Down Expand Up @@ -839,7 +836,7 @@ private void flattenProperties(OpenAPI openAPI, Map<String, Schema> properties,
Schema inner = ModelUtils.getSchemaItems(property);
if (ModelUtils.isObjectSchema(inner)) {
Schema op = inner;
if (op.getProperties() != null && op.getProperties().size() > 0) {
if (ModelUtils.hasProperties(op)) {
flattenProperties(openAPI, op.getProperties(), path);
String modelName = resolveModelName(op.getTitle(), path + "_" + key);
Schema innerModel = modelFromProperty(openAPI, op, modelName);
Expand Down Expand Up @@ -869,7 +866,7 @@ private void flattenProperties(OpenAPI openAPI, Map<String, Schema> properties,
Schema inner = ModelUtils.getAdditionalProperties(property);
if (ModelUtils.isObjectSchema(inner)) {
Schema op = inner;
if (op.getProperties() != null && op.getProperties().size() > 0) {
if (ModelUtils.hasProperties(op)) {
flattenProperties(openAPI, op.getProperties(), path);
String modelName = resolveModelName(op.getTitle(), path + "_" + key);
Schema innerModel = modelFromProperty(openAPI, op, modelName);
Expand Down
29 changes: 12 additions & 17 deletions modules/openapi-generator/src/main/java/org/openapitools/codegen/OpenAPINormalizer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -760,19 +760,19 @@ public Schema normalizeSchema(Schema schema, Set<Schema> visitedSchemas) {
schema = normalizeComplexComposedSchema(schema, visitedSchemas);
}

if (schema.getAllOf() != null && !schema.getAllOf().isEmpty()) {
if (ModelUtils.hasAllOf(schema)) {
return normalizeAllOf(schema, visitedSchemas);
}

if (schema.getOneOf() != null && !schema.getOneOf().isEmpty()) {
if (ModelUtils.hasOneOf(schema)) {
return normalizeOneOf(schema, visitedSchemas);
}

if (schema.getAnyOf() != null && !schema.getAnyOf().isEmpty()) {
if (ModelUtils.hasAnyOf(schema)) {
return normalizeAnyOf(schema, visitedSchemas);
}

if (schema.getProperties() != null && !schema.getProperties().isEmpty()) {
if (ModelUtils.hasProperties(schema)) {
normalizeProperties(schema, visitedSchemas);
}

Expand All @@ -781,7 +781,7 @@ public Schema normalizeSchema(Schema schema, Set<Schema> visitedSchemas) {
}

return schema;
} else if (schema.getProperties() != null && !schema.getProperties().isEmpty()) {
} else if (ModelUtils.hasProperties(schema)) {
normalizeProperties(schema, visitedSchemas);
} else if (schema.getAdditionalProperties() instanceof Schema) { // map
normalizeMapSchema(schema);
Expand Down Expand Up @@ -1109,7 +1109,7 @@ protected Schema normalizeAnyOf(Schema schema, Set<Schema> visitedSchemas) {

protected Schema normalizeComplexComposedSchema(Schema schema, Set<Schema> visitedSchemas) {
// loop through properties, if any
if (schema.getProperties() != null && !schema.getProperties().isEmpty()) {
if (ModelUtils.hasProperties(schema)) {
normalizeProperties(schema, visitedSchemas);
}

Expand Down Expand Up @@ -1294,10 +1294,9 @@ protected void processRemoveAnyOfOneOfAndKeepPropertiesOnly(Schema schema) {
return;
}

if (((schema.getOneOf() != null && !schema.getOneOf().isEmpty())
|| (schema.getAnyOf() != null && !schema.getAnyOf().isEmpty())) // has anyOf or oneOf
&& (schema.getProperties() != null && !schema.getProperties().isEmpty()) // has properties
&& schema.getAllOf() == null) { // not allOf
boolean hasAnyOfOrOneOf = ModelUtils.hasOneOf(schema) || ModelUtils.hasAnyOf(schema);
boolean notAllOf = schema.getAllOf() == null;
if (hasAnyOfOrOneOf && ModelUtils.hasProperties(schema) && notAllOf) {
// clear oneOf, anyOf
schema.setOneOf(null);
schema.setAnyOf(null);
Expand Down Expand Up @@ -1374,9 +1373,7 @@ protected Schema processSimplifyAnyOfEnum(Schema schema) {
if (schema.getAnyOf() == null || schema.getAnyOf().isEmpty()) {
return schema;
}
if(schema.getOneOf() != null && !schema.getOneOf().isEmpty() ||
schema.getAllOf() != null && !schema.getAllOf().isEmpty() ||
schema.getNot() != null) {
if(ModelUtils.hasOneOf(schema) || ModelUtils.hasAllOf(schema) || schema.getNot() != null) {
//only convert to enum if anyOf is the only composition
return schema;
}
Expand All @@ -1399,9 +1396,7 @@ protected Schema processSimplifyOneOfEnum(Schema schema) {
if (schema.getOneOf() == null || schema.getOneOf().isEmpty()) {
return schema;
}
if(schema.getAnyOf() != null && !schema.getAnyOf().isEmpty() ||
schema.getAllOf() != null && !schema.getAllOf().isEmpty() ||
schema.getNot() != null) {
if(ModelUtils.hasAnyOf(schema) || ModelUtils.hasAllOf(schema) || schema.getNot() != null) {
//only convert to enum if oneOf is the only composition
return schema;
}
Expand Down Expand Up @@ -2109,7 +2104,7 @@ protected void processNormalizeOtherThanObjectWithProperties(Schema schema) {
// Check object models / any type models / composed models for properties,
// if the schema has a type defined that is not "object" it should not define
// any properties
if (schema.getType() != null && !"object".equals(schema.getType())) {
if (schema.getType() != null && !ModelUtils.isObjectTypeOAS30(schema)) {
schema.setProperties(null);
}
}
Expand Down
2 changes: 1 addition & 1 deletion ...enapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractJavaCodegen.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1383,7 +1383,7 @@ public String toDefaultValue(CodegenProperty cp, Schema schema) {
}
return toArrayDefaultValue(cp, schema);
} else if (ModelUtils.isMapSchema(schema) && !(ModelUtils.isComposedSchema(schema))) {
if (schema.getProperties() != null && schema.getProperties().size() > 0) {
if (ModelUtils.hasProperties(schema)) {
// object is complex object with free-form additional properties
if (schema.getDefault() != null) {
return super.toDefaultValue(schema);
Expand Down
2 changes: 1 addition & 1 deletion ...penapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractPhpCodegen.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ public String getSchemaType(Schema p) {
openAPIType = "UNKNOWN_OPENAPI_TYPE";
}

if ((p.getAnyOf() != null && !p.getAnyOf().isEmpty()) || (p.getOneOf() != null && !p.getOneOf().isEmpty())) {
if (ModelUtils.hasAnyOf(p) || ModelUtils.hasOneOf(p)) {
return openAPIType;
}

Expand Down
2 changes: 1 addition & 1 deletion ...api-generator/src/main/java/org/openapitools/codegen/languages/AbstractPythonCodegen.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,7 @@ private String toExampleValueRecursive(Schema schema, List<Schema> includedSchem

// if required and optionals
List<String> reqs = new ArrayList<>();
if (schema.getProperties() != null && !schema.getProperties().isEmpty()) {
if (ModelUtils.hasProperties(schema)) {
for (Object toAdd : schema.getProperties().keySet()) {
reqs.add((String) toAdd);
}
Expand Down
2 changes: 1 addition & 1 deletion ...tor/src/main/java/org/openapitools/codegen/languages/AbstractPythonPydanticV1Codegen.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,7 @@ private String toExampleValueRecursive(Schema schema, List<Schema> includedSchem

// if required and optionals
List<String> reqs = new ArrayList<>();
if (schema.getProperties() != null && !schema.getProperties().isEmpty()) {
if (ModelUtils.hasProperties(schema)) {
for (Object toAdd : schema.getProperties().keySet()) {
reqs.add((String) toAdd);
}
Expand Down
Loading
Loading