Skip to content
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

Jump to bottom

[csharp][generichost] Modernizes handling of composed schemas #15865

Merged
merged 2 commits into from
Jun 17, 2023
Merged

[csharp][generichost] Modernizes handling of composed schemas #15865

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
636060a
removed hotfixes, improved composed schema handling
devhl-labs Jun 17, 2023
a501719
fix copy paste bug
devhl-labs Jun 17, 2023
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
6 changes: 0 additions & 6 deletions modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,12 +270,6 @@ void configureGeneratorProperties() {
InlineModelResolver inlineModelResolver = new InlineModelResolver();
inlineModelResolver.setInlineSchemaNameMapping(config.inlineSchemaNameMapping());
inlineModelResolver.setInlineSchemaNameDefaults(config.inlineSchemaNameDefault());
if (inlineModelResolver.refactorAllOfInlineSchemas == null && // option not set
config instanceof CSharpClientCodegen) { // default to true for csharp-netcore generator
inlineModelResolver.refactorAllOfInlineSchemas = true;
LOGGER.info("inlineModelResolver.refactorAllOfInlineSchemas is default to true instead of false for `csharp-netcore` generator." +
"Add --inline-schema-name-defaults REFACTOR_ALLOF_INLINE_SCHEMAS=false in CLI for example to set it to false instead.");
}

inlineModelResolver.flatten(openAPI);
}
Expand Down
53 changes: 43 additions & 10 deletions ...api-generator/src/main/java/org/openapitools/codegen/languages/AbstractCSharpCodegen.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,32 @@ public Map<String, ModelsMap> postProcessAllModels(Map<String, ModelsMap> objs)
CodegenModel model = ModelUtils.getModelByName(entry.getKey(), objs);
removeCircularReferencesInComposedSchemas(model);

CodegenComposedSchemas composedSchemas = model.getComposedSchemas();
if (composedSchemas != null) {
List<CodegenProperty> allOf = composedSchemas.getAllOf();
if (allOf != null) {
for(CodegenProperty property : allOf) {
property.name = patchPropertyName(model, property.baseType);
}
}

List<CodegenProperty> anyOf = composedSchemas.getAnyOf();
if (anyOf != null) {
for(CodegenProperty property : anyOf) {
property.name = patchPropertyName(model, property.baseType);
property.isNullable = true;
}
}

List<CodegenProperty> oneOf = composedSchemas.getOneOf();
if (oneOf != null) {
for(CodegenProperty property : oneOf) {
property.name = patchPropertyName(model, property.baseType);
property.isNullable = true;
}
}
}

// https://github.com/OpenAPITools/openapi-generator/issues/12324
// TODO: why do these collections contain different instances?
// fixing allVars should suffice instead of patching every collection
Expand Down Expand Up @@ -526,7 +552,22 @@ public Map<String, ModelsMap> postProcessAllModels(Map<String, ModelsMap> objs)
return processed;
}

private void patchProperty(Map<String, CodegenModel> enumRefs, CodegenModel model, CodegenProperty property) {
private String patchPropertyName(CodegenModel model, String value) {
// the casing will be wrong if we just set the name to escapeReservedWord
// if we try to fix it with camelize, underscores get stripped out
// so test if the name was escaped and then replace var with Var
String tmpPropertyName = escapeReservedWord(model, value);
if (!value.equals(tmpPropertyName) || value.startsWith(this.invalidNamePrefix)) {
value = tmpPropertyName;
String firstCharacter = value.substring(0, 1);
value = value.substring(1);
value = firstCharacter.toUpperCase(Locale.ROOT) + value;
}

return value;
}

protected void patchProperty(Map<String, CodegenModel> enumRefs, CodegenModel model, CodegenProperty property) {
if (enumRefs.containsKey(property.dataType)) {
// Handle any enum properties referred to by $ref.
// This is different in C# than most other generators, because enums in C# are compiled to integral types,
Expand Down Expand Up @@ -554,15 +595,7 @@ private void patchProperty(Map<String, CodegenModel> enumRefs, CodegenModel mode
}

String tmpPropertyName = escapeReservedWord(model, property.name);
if (!property.name.equals(tmpPropertyName) || property.name.startsWith(this.invalidNamePrefix)) {
// the casing will be wrong if we just set the name to escapeReservedWord
// if we try to fix it with camelize, underscores get stripped out
// so test if the name was escaped and then replace var with Var
property.name = tmpPropertyName;
String firstCharacter = property.name.substring(0, 1);
property.name = property.name.substring(1);
property.name = firstCharacter.toUpperCase(Locale.ROOT) + property.name;
}
property.name = patchPropertyName(model, property.name);

// fix incorrect data types for maps of maps
if (property.datatypeWithEnum.endsWith(", List>") && property.items != null) {
Expand Down
107 changes: 14 additions & 93 deletions ...enapi-generator/src/main/java/org/openapitools/codegen/languages/CSharpClientCodegen.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import io.swagger.v3.oas.models.media.Schema;
import io.swagger.v3.oas.models.Operation;
import io.swagger.v3.oas.models.servers.Server;
import org.mozilla.javascript.optimizer.Codegen;
import org.openapitools.codegen.*;
import org.openapitools.codegen.meta.features.*;
import org.openapitools.codegen.model.ModelMap;
Expand Down Expand Up @@ -1520,6 +1521,19 @@ public String toInstantiationType(Schema schema) {
}
}

@Override
protected void patchProperty(Map<String, CodegenModel> enumRefs, CodegenModel model, CodegenProperty property) {
super.patchProperty(enumRefs, model, property);

if (!GENERICHOST.equals(getLibrary()) || model.parentModel == null) {
return;
}

if (model.parentModel.allVars.stream().anyMatch(v -> v.baseName.equals(property.baseName))){
property.isInherited = true;
}
}

@Override
public ModelsMap postProcessModels(ModelsMap objs) {
objs = super.postProcessModels(objs);
Expand Down Expand Up @@ -1564,75 +1578,6 @@ public ModelsMap postProcessModels(ModelsMap objs) {
return objs;
}

/**
* ISSUE: https://github.com/OpenAPITools/openapi-generator/issues/11846
* Ensures that a model has all inherited properties
* Check modules\openapi-generator\src\test\resources\3_0\java\petstore-with-fake-endpoints-models-for-testing-with-http-signature.yaml
* Without this method, property petType in GrandparentAnimal will not make it through ParentPet and into ChildCat
*/
private void ensureInheritedPropertiesArePresent(CodegenModel derivedModel) {
// every c# generator should definitely want this, or we should fix the issue
// still, lets avoid breaking changes :(
if (Boolean.FALSE.equals(GENERICHOST.equals(getLibrary()))) {
return;
}

if (derivedModel.parentModel == null) {
return;
}

for (CodegenProperty parentProperty : derivedModel.parentModel.allVars) {
if (Boolean.FALSE.equals(derivedModel.allVars.stream().anyMatch(v -> v.baseName.equals(parentProperty.baseName)))) {
CodegenProperty clone = parentProperty.clone();
clone.isInherited = true;
LOGGER.debug("Inherited property " + clone.name + " from model" + derivedModel.parentModel.classname + " was not found in " + derivedModel.classname + ". Adding a clone now.");
derivedModel.allVars.add(clone);
}
}

ensureInheritedPropertiesArePresent(derivedModel.parentModel);
}

/**
* Invoked by {@link DefaultGenerator} after all models have been post-processed, allowing for a last pass of codegen-specific model cleanup.
*
* @param objs Current state of codegen object model.
* @return An in-place modified state of the codegen object model.
*/
@Override
public Map<String, ModelsMap> postProcessAllModels(Map<String, ModelsMap> objs) {
objs = super.postProcessAllModels(objs);

// other libraries probably want these fixes, but lets avoid breaking changes for now
if (Boolean.FALSE.equals(GENERICHOST.equals(getLibrary()))) {
return objs;
}

ArrayList<CodegenModel> allModels = new ArrayList<>();
for (String key : objs.keySet()) {
CodegenModel model = ModelUtils.getModelByName(key, objs);
allModels.add(model);
}

for (CodegenModel cm : allModels) {
cm.anyOf.forEach(anyOf -> removePropertiesDeclaredInComposedClass(anyOf, allModels, cm));
cm.oneOf.forEach(oneOf -> removePropertiesDeclaredInComposedClass(oneOf, allModels, cm));
cm.allOf.forEach(allOf -> removePropertiesDeclaredInComposedClass(allOf, allModels, cm));

if (cm.getComposedSchemas() != null && cm.getComposedSchemas().getAllOf() != null && !cm.getComposedSchemas().getAllOf().isEmpty()) {
cm.getComposedSchemas().getAllOf().forEach(allOf -> {
if (allOf.dataType.equals(cm.parent)) {
allOf.isInherited = true;
}
});
}

ensureInheritedPropertiesArePresent(cm);
}

return objs;
}

/**
* Return true if the property being passed is a C# value type
*
Expand All @@ -1647,30 +1592,6 @@ protected boolean isValueType(CodegenProperty var) {
: this.getValueTypes().contains(var.dataType) || var.isEnum;
}

/**
* Removes properties from a model which are also defined in a composed class.
*
* @param className The name which may be a composed model
* @param allModels A collection of all CodegenModel
* @param cm The CodegenModel to correct
*/
private void removePropertiesDeclaredInComposedClass(String className, List<CodegenModel> allModels, CodegenModel cm) {
CodegenModel otherModel = allModels.stream().filter(m -> m.classname.equals(className)).findFirst().orElse(null);
if (otherModel == null) {
return;
}

otherModel.readWriteVars.stream().filter(v -> cm.readWriteVars.stream().anyMatch(cmV -> cmV.baseName.equals(v.baseName))).collect(Collectors.toList())
.forEach(v -> {
cm.readWriteVars.removeIf(item -> item.baseName.equals(v.baseName));
cm.vars.removeIf(item -> item.baseName.equals(v.baseName));
cm.readOnlyVars.removeIf(item -> item.baseName.equals(v.baseName));
cm.requiredVars.removeIf(item -> item.baseName.equals(v.baseName));
cm.allVars.removeIf(item -> item.baseName.equals(v.baseName));
cm.nonNullableVars.removeIf(item -> item.baseName.equals(v.baseName));
});
}

@Override
public void postProcess() {
System.out.println("################################################################################");
Expand Down
Loading