Skip to content

[dart-dio] fix issue with multi layered discriminators #16136

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.

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
wants to merge 10 commits into
base: master
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,15 @@ public boolean equals(Object o) {
public int hashCode() {
return Objects.hash(mappingName, modelName);
}

@Override
public String toString() {
final StringBuffer sb = new StringBuffer("MappedModel{");
sb.append("mappingName='").append(mappingName).append('\'');
sb.append(", modelName='").append(modelName).append('\'');
sb.append('}');
return sb.toString();
}
Comment on lines +190 to +198
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wing328 is this change appropriate for the core generator ?

}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@
import com.google.common.collect.Sets;
import com.samskivert.mustache.Mustache;
import com.samskivert.mustache.Template;

import io.swagger.v3.oas.models.OpenAPI;
import io.swagger.v3.oas.models.media.Discriminator;
import io.swagger.v3.oas.models.media.Schema;

import org.apache.commons.lang3.ObjectUtils;
import org.apache.commons.lang3.StringUtils;
import org.openapitools.codegen.*;
Expand Down Expand Up @@ -84,12 +86,12 @@ public DartDioClientCodegen() {
ClientModificationFeature.Authorizations,
ClientModificationFeature.UserAgent
).includeSchemaSupportFeatures(
SchemaSupportFeature.Polymorphism,
SchemaSupportFeature.Union,
SchemaSupportFeature.Composite,
SchemaSupportFeature.allOf,
SchemaSupportFeature.oneOf,
SchemaSupportFeature.anyOf
SchemaSupportFeature.Polymorphism,
SchemaSupportFeature.Union,
SchemaSupportFeature.Composite,
SchemaSupportFeature.allOf,
SchemaSupportFeature.oneOf,
SchemaSupportFeature.anyOf
)
);
generatorMetadata = GeneratorMetadata.newBuilder(generatorMetadata)
Expand Down Expand Up @@ -175,8 +177,7 @@ public void processOpts() {
if (!additionalProperties.containsKey(FINAL_PROPERTIES)) {
additionalProperties.put(FINAL_PROPERTIES, Boolean.parseBoolean(FINAL_PROPERTIES_DEFAULT_VALUE));
LOGGER.debug("finalProperties not set, using default {}", FINAL_PROPERTIES_DEFAULT_VALUE);
}
else {
} else {
additionalProperties.put(FINAL_PROPERTIES, Boolean.parseBoolean(additionalProperties.get(FINAL_PROPERTIES).toString()));
}

Expand Down Expand Up @@ -368,6 +369,7 @@ private void syncRootTypesWithInnerVars(Map<String, ModelsMap> objs) {
syncRootTypesWithInnerVars(allModels, model);
}
}

private void syncRootTypesWithInnerVars(Map<String, CodegenModel> objs, CodegenModel model) {
List<CodegenProperty> allVars = new ArrayList<>();
allVars.addAll(((Collection<CodegenProperty>) model.vendorExtensions.get(kSelfAndAncestorOnlyProps)));
Expand All @@ -387,6 +389,7 @@ private void syncRootTypesWithInnerVars(Map<String, CodegenModel> objs, CodegenM
}
}
}

private final String kIsChild = "x-is-child";
private final String kIsParent = "x-is-parent";
private final String kIsPure = "x-is-pure";
Expand All @@ -397,6 +400,7 @@ private void syncRootTypesWithInnerVars(Map<String, CodegenModel> objs, CodegenM
private final String kSelfAndAncestorOnlyProps = "x-self-and-ancestor-only-props";
private final String kHasSelfAndAncestorOnlyProps = "x-has-self-and-ancestor-only-props";
private final String kParentDiscriminator = "x-parent-discriminator";
private final String kDiscriminatorSelfMappedModel = "x-discriminator-self-mapped-model";

// adapts codegen models and property to dart rules of inheritance
private void adaptToDartInheritance(Map<String, ModelsMap> objs) {
Expand Down Expand Up @@ -426,7 +430,6 @@ private void adaptToDartInheritance(Map<String, ModelsMap> objs) {
}



Set<String> allPureClasses = new HashSet<>();
// set isChild,isParent,isPure
for (java.util.Map.Entry<String, CodegenModel> cmEntry : allModels.entrySet()) {
Expand All @@ -448,9 +451,9 @@ private void adaptToDartInheritance(Map<String, ModelsMap> objs) {
cm.vendorExtensions.put(kIsPure, isPure);
if (!isParent && (cm.oneOf == null || cm.oneOf.isEmpty())) {
//discriminator has no meaning here
if (cm.discriminator!=null) {
if (cm.discriminator != null) {
cm.vendorExtensions.put(kParentDiscriminator, cm.discriminator);
cm.discriminator=null;
cm.discriminator = null;
}

}
Expand Down Expand Up @@ -553,8 +556,8 @@ private void adaptToDartInheritance(Map<String, ModelsMap> objs) {
protected CodegenDiscriminator createDiscriminator(String schemaName, Schema schema) {
CodegenDiscriminator sub = super.createDiscriminator(schemaName, schema);
Discriminator originalDiscriminator = schema.getDiscriminator();
if (originalDiscriminator!=null) {
Map<String,String> originalMapping = originalDiscriminator.getMapping();
if (originalDiscriminator != null) {
Map<String, String> originalMapping = originalDiscriminator.getMapping();
if (originalMapping != null && !originalMapping.isEmpty()) {
//we already have a discriminator mapping, remove everything else
for (MappedModel currentMappings : new HashSet<>(sub.getMappedModels())) {
Expand All @@ -570,6 +573,120 @@ protected CodegenDiscriminator createDiscriminator(String schemaName, Schema sch
return sub;
}

private boolean isParentRecursive(CodegenModel codegenModel, CodegenModel parentModel, boolean allowParentIsModel) {
final String classname = parentModel.getClassname();
final String childClassName = codegenModel.getClassname();

if ((allowParentIsModel && classname == childClassName)
|| (codegenModel.parent == classname)
|| (codegenModel.getAllParents() != null && codegenModel.getAllParents().contains(classname))
|| (codegenModel.interfaces != null && codegenModel.interfaces.contains(classname))
|| parentModel.anyOf.contains(childClassName)
|| parentModel.oneOf.contains(childClassName)
|| (codegenModel.parentModel != null && isParentRecursive(codegenModel.parentModel, parentModel, true))) {
return true;
}

return false;
}

/// sort the models by first putting the childModels and then the parent models
private Set<MappedModel> sortFilteredModels(Set<MappedModel> unSortedModels) {
final Set<MappedModel> sortedModels = new HashSet<MappedModel>();
final Set<MappedModel> notSortedModels = new HashSet<MappedModel>();
for (MappedModel mappedModel : unSortedModels) {
final CodegenModel mappedCodegenModel = mappedModel.getModel();
if (unSortedModels.stream().anyMatch(m -> m != mappedModel && isParentRecursive(mappedCodegenModel, m.getModel(), true))) {
notSortedModels.add(mappedModel);
} else {
sortedModels.add(mappedModel);
}
}

final Set<MappedModel> finishedSortedModels = new LinkedHashSet<MappedModel>();
if (!notSortedModels.isEmpty()) {
final Set<MappedModel> sortedUnsortedModels = sortFilteredModels(notSortedModels);
finishedSortedModels.addAll(sortedUnsortedModels);
}
finishedSortedModels.addAll(sortedModels);
return finishedSortedModels;
}

// filter out all the models, which are not submodels of this model
private void filterMappedModels(CodegenModel cm) {

if (cm.discriminator != null && cm.discriminator.getMappedModels() != null) {
final CodegenDiscriminator discriminator = cm.getDiscriminator();
final String classname = cm.getClassname();
final Set<MappedModel> filteredModels = new HashSet<MappedModel>();
final Set<MappedModel> removedModels = new HashSet<MappedModel>();
for (MappedModel mappedModel : discriminator.getMappedModels()) {
final CodegenModel mappedCodegenModel = mappedModel.getModel();
// filter out all the models, which are not submodels of this model
if (isParentRecursive(mappedCodegenModel, cm, false)) {
filteredModels.add(mappedModel);
} else {
// set the default for the model, which is the same as the model itself
final String mappedCodegenModelClassname = mappedCodegenModel.getClassname();
if(classname == mappedCodegenModelClassname) {
LOGGER.info("Removing model {} from discriminator of model {} because it is the model itself", mappedCodegenModel.getClassname(), classname);
discriminator.getVendorExtensions().put(kDiscriminatorSelfMappedModel, mappedModel);
} else {
LOGGER.info("Removing model {} from discriminator of model {} because it is not a submodel", mappedCodegenModel.getClassname(), classname);
}
removedModels.add(mappedModel);
}
}

// sort the models by first putting the childModels and then the parent models
final Set<MappedModel> sortedModels = sortFilteredModels(filteredModels);

cm.discriminator.setMappedModels(sortedModels);

Set<String> filteredImports = new HashSet<>();
for (String mImport : cm.getImports()) {
boolean found = false;
if (cm.parent != null) {
String parentString = rewriteImport(cm.getParent(), true);
found = parentString.equals(mImport);
}
if (!found) {
for (MappedModel mappedModel : filteredModels) {
String mappedModelImport = rewriteImport(mappedModel.getModelName(), true);
if (mappedModelImport.equals(mImport)) {
found = true;
break;
}
}
}
if (!found) {
for (CodegenProperty codegenProperty : cm.getAllVars()) {
String codeGenImport = rewriteImport(codegenProperty.getDataType(), true);
if (codeGenImport.equals(mImport)) {
found = true;
break;
}
}
}
if (!found) {
boolean removeIt = false;
for (MappedModel removedModel : removedModels) {
String removedImport = rewriteImport(removedModel.getModelName(), true);
if (removedImport.equals(mImport)) {
removeIt = true;
break;
}
}
found = !removeIt;
}
if (found) {
filteredImports.add(mImport);
}
}
cm.setImports(filteredImports);
}
}

@Override
public Map<String, ModelsMap> postProcessAllModels(Map<String, ModelsMap> objs) {
objs = super.postProcessAllModels(objs);
Expand All @@ -584,6 +701,9 @@ public Map<String, ModelsMap> postProcessAllModels(Map<String, ModelsMap> objs)
CodegenModel cm = mo.getModel();
cm.imports = rewriteImports(cm.imports, true);
cm.vendorExtensions.put("x-has-vars", !cm.vars.isEmpty());

// filter out all the models, which are not submodels of this model
filterMappedModels(cm);
}
}

Expand Down Expand Up @@ -650,7 +770,7 @@ public OperationsMap postProcessOperationsWithModels(OperationsMap objs, List<Mo
// longer in use.
if (op.allParams.stream().noneMatch(param -> param.dataType.equals("Uint8List"))
&& op.responses.stream().filter(response -> response.dataType != null)
.noneMatch(response -> response.dataType.equals("Uint8List"))) {
.noneMatch(response -> response.dataType.equals("Uint8List"))) {
// Remove unused imports after processing
op.imports.remove("Uint8List");
}
Expand Down Expand Up @@ -704,6 +824,7 @@ private void addBuiltValueSerializerImport(String type) {

/**
* Adds the serializer to the global list of custom built_value serializers.
*
* @param serializer
*/
private void addBuiltValueSerializer(BuiltValueSerializer serializer) {
Expand All @@ -717,32 +838,39 @@ private void addBuiltValueSerializer(BuiltValueSerializer serializer) {
private Set<String> rewriteImports(Set<String> originalImports, boolean isModel) {
Set<String> resultImports = Sets.newHashSet();
for (String modelImport : originalImports) {
if (modelImport.startsWith("BuiltList", 0)) {
modelImport = "BuiltList";
} else if (modelImport.startsWith("BuiltSet", 0)) {
modelImport = "BuiltSet";
} else if (modelImport.startsWith("BuiltMap", 0)) {
modelImport = "BuiltMap";
final String rewriteString = rewriteImport(modelImport, isModel);
if (rewriteString != null) {
resultImports.add(rewriteString);
}
}
return resultImports;
}

if (imports.containsKey(modelImport)) {
String i = imports.get(modelImport);
if (Objects.equals(i, DIO_IMPORT) && !isModel) {
// Don't add imports to operations that are already imported
continue;
}
resultImports.add(i);
} else if (importMapping().containsKey(modelImport)) {
resultImports.add(importMapping().get(modelImport));
} else if (modelImport.startsWith("dart:")) { // import dart:* directly
resultImports.add(modelImport);
} else if (modelImport.startsWith("package:")) { // e.g. package:openapi/src/model/child.dart
resultImports.add(modelImport);
} else {
resultImports.add("package:" + pubName + "/" + sourceFolder + "/" + modelPackage() + "/" + underscore(modelImport) + ".dart");
private String rewriteImport(String modelImport, boolean isModel) {
if (modelImport.startsWith("BuiltList", 0)) {
modelImport = "BuiltList";
} else if (modelImport.startsWith("BuiltSet", 0)) {
modelImport = "BuiltSet";
} else if (modelImport.startsWith("BuiltMap", 0)) {
modelImport = "BuiltMap";
}

if (imports.containsKey(modelImport)) {
String i = imports.get(modelImport);
if (Objects.equals(i, DIO_IMPORT) && !isModel) {
// Don't add imports to operations that are already imported
return null;
}
return i;
} else if (importMapping().containsKey(modelImport)) {
return importMapping().get(modelImport);
} else if (modelImport.startsWith("dart:")) { // import dart:* directly
return modelImport;
} else if (modelImport.startsWith("package:")) { // e.g. package:openapi/src/model/child.dart
return modelImport;
} else {
return "package:" + pubName + "/" + sourceFolder + "/" + modelPackage() + "/" + underscore(modelImport) + ".dart";
}
return resultImports;
}

static class BuiltValueSerializer {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ extension {{classname}}DiscriminatorExt on {{classname}} {
return r'{{mappingName}}';
}
{{/mappedModels}}
return null;
return {{#discriminator.vendorExtensions.x-discriminator-self-mapped-model}}r'{{mappingName}}'{{/discriminator.vendorExtensions.x-discriminator-self-mapped-model}}{{^discriminator.vendorExtensions.x-discriminator-self-mapped-model}}null{{/discriminator.vendorExtensions.x-discriminator-self-mapped-model}};
}
}
extension {{classname}}BuilderDiscriminatorExt on {{classname}}Builder {
Expand All @@ -15,6 +15,6 @@ extension {{classname}}BuilderDiscriminatorExt on {{classname}}Builder {
return r'{{mappingName}}';
}
{{/mappedModels}}
return null;
return {{#discriminator.vendorExtensions.x-discriminator-self-mapped-model}}r'{{mappingName}}'{{/discriminator.vendorExtensions.x-discriminator-self-mapped-model}}{{^discriminator.vendorExtensions.x-discriminator-self-mapped-model}}null{{/discriminator.vendorExtensions.x-discriminator-self-mapped-model}};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@
{{#mappedModels}}
r'{{mappingName}}': {{modelName}},
{{/mappedModels}}
};{{/hasDiscriminatorWithNonEmptyMapping}}
{{#vendorExtensions.x-discriminator-self-mapped-model}} r'{{mappingName}}': {{modelName}},
{{/vendorExtensions.x-discriminator-self-mapped-model}} };{{/hasDiscriminatorWithNonEmptyMapping}}

{{/discriminator}}{{^vendorExtensions.x-is-parent}} {{classname}}._();

Expand Down
Loading