Skip to content

Commit

Permalink
fix(rust): oneOf generation for client (#17915)
Browse files Browse the repository at this point in the history
* fix(rust): discriminator mapping to serde rename

Discriminator mapping has been ignored in some cases.
Even existing samples had wrong definition in some cases

This PR addresses this

* fix(rust): `oneOf` generation for client

Solves #17869 and #17896 and also includes unmerged $17898

Unfortunately it affects quite a lot of code, but we can see that only client-side models were affected by re-generation.
I tried to split this PR to several, but they're really coupled and hard to create a chain of PRs.

* fix: indentation in `impl Default`

* missing fixes

* fix: correct typeDeclaration with unaliased schema

* style: improve indentation for models

* fix: user toModelName for aliases of oneOf

* refactor: unify `getTypeDeclaration` for rust

* cover the case when `mapping` has the same `ref` for different mapping names

* test: add test for previous change

* style: remove extra qualified path to models

* add some comments

* fix(build): use method of `List` instead of specific for `LinkedList`
  • Loading branch information
DDtKey committed Feb 24, 2024
1 parent 15af1ce commit 518b29d
Show file tree
Hide file tree
Showing 335 changed files with 3,663 additions and 1,099 deletions.
8 changes: 8 additions & 0 deletions bin/configs/rust-hyper-oneOf-array-map-import.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
generatorName: rust
outputDir: samples/client/others/rust/hyper/oneOf-array-map
library: hyper
inputSpec: modules/openapi-generator/src/test/resources/3_0/oneOfArrayMapImport.yaml
templateDir: modules/openapi-generator/src/main/resources/rust
additionalProperties:
supportAsync: false
packageName: oneof-array-map-hyper
8 changes: 8 additions & 0 deletions bin/configs/rust-hyper-oneOf-reuseRef.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
generatorName: rust
outputDir: samples/client/others/rust/hyper/oneOf-reuseRef
library: hyper
inputSpec: modules/openapi-generator/src/test/resources/3_0/oneOf_reuseRef.yaml
templateDir: modules/openapi-generator/src/main/resources/rust
additionalProperties:
supportAsync: false
packageName: oneof-reuse-ref-hyper
8 changes: 8 additions & 0 deletions bin/configs/rust-reqwest-oneOf-array-map-import.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
generatorName: rust
outputDir: samples/client/others/rust/reqwest/oneOf-array-map
library: reqwest
inputSpec: modules/openapi-generator/src/test/resources/3_0/oneOfArrayMapImport.yaml
templateDir: modules/openapi-generator/src/main/resources/rust
additionalProperties:
supportAsync: false
packageName: oneof-array-map-reqwest
8 changes: 8 additions & 0 deletions bin/configs/rust-reqwest-oneOf-reuseRef.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
generatorName: rust
outputDir: samples/client/others/rust/reqwest/oneOf-reuseRef
library: reqwest
inputSpec: modules/openapi-generator/src/test/resources/3_0/oneOf_reuseRef.yaml
templateDir: modules/openapi-generator/src/main/resources/rust
additionalProperties:
supportAsync: false
packageName: oneof-reuse-ref-reqwest
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Strings;
import org.openapitools.codegen.CodegenConfig;
import org.openapitools.codegen.CodegenProperty;
import org.openapitools.codegen.DefaultCodegen;
import org.openapitools.codegen.GeneratorLanguage;
import io.swagger.v3.oas.models.media.ArraySchema;
import io.swagger.v3.oas.models.media.FileSchema;
import io.swagger.v3.oas.models.media.Schema;
import org.openapitools.codegen.*;
import org.openapitools.codegen.utils.ModelUtils;
import org.openapitools.codegen.utils.StringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -232,6 +233,83 @@ public String sanitizeIdentifier(String name, CasingType casingType, String esca
return name;
}

@Override
public String getTypeDeclaration(Schema p) {
if (ModelUtils.isArraySchema(p)) {
ArraySchema ap = (ArraySchema) p;
Schema inner = ap.getItems();
String innerType = getTypeDeclaration(inner);
return typeMapping.get("array") + "<" + innerType + ">";
} else if (ModelUtils.isMapSchema(p)) {
Schema inner = ModelUtils.getAdditionalProperties(p);
String innerType = getTypeDeclaration(inner);
StringBuilder typeDeclaration = new StringBuilder(typeMapping.get("map")).append("<").append(typeMapping.get("string")).append(", ");
typeDeclaration.append(innerType).append(">");
return typeDeclaration.toString();
} else if (!org.apache.commons.lang3.StringUtils.isEmpty(p.get$ref())) {
String datatype;
try {
datatype = toModelName(ModelUtils.getSimpleRef(p.get$ref()));
datatype = "models::" + toModelName(datatype);
} catch (Exception e) {
LOGGER.warn("Error obtaining the datatype from schema (model):{}. Datatype default to Object", p);
datatype = "Object";
LOGGER.error(e.getMessage(), e);
}
return datatype;
} else if (p instanceof FileSchema) {
return typeMapping.get("file");
}

String oasType = getSchemaType(p);
if (typeMapping.containsKey(oasType)) {
return typeMapping.get(oasType);
}

if (typeMapping.containsValue(oasType)) {
return oasType;
}

if (languageSpecificPrimitives.contains(oasType)) {
return oasType;
}

return "models::" + toModelName(oasType);
}

@Override
public CodegenModel fromModel(String name, Schema model) {
LOGGER.trace("Creating model from schema: {}", model);

Map<String, Schema> allDefinitions = ModelUtils.getSchemas(this.openAPI);
CodegenModel mdl = super.fromModel(name, model);

mdl.vendorExtensions.put("x-upper-case-name", name.toUpperCase(Locale.ROOT));
if (!org.apache.commons.lang3.StringUtils.isEmpty(model.get$ref())) {
Schema schema = allDefinitions.get(ModelUtils.getSimpleRef(model.get$ref()));
mdl.dataType = typeMapping.get(schema.getType());
}
if (ModelUtils.isArraySchema(model)) {
if (typeMapping.containsKey(mdl.arrayModelType)) {
mdl.arrayModelType = typeMapping.get(mdl.arrayModelType);
} else {
mdl.arrayModelType = toModelName(mdl.arrayModelType);
}
} else if ((!mdl.anyOf.isEmpty()) || (!mdl.oneOf.isEmpty())) {
mdl.dataType = getSchemaType(model);
}

Schema additionalProperties = ModelUtils.getAdditionalProperties(model);

if (additionalProperties != null) {
mdl.additionalPropertiesType = getTypeDeclaration(additionalProperties);
}

LOGGER.trace("Created model: {}", mdl);

return mdl;
}

@Override
public String toVarName(String name) {
// obtain the name from nameMapping directly if provided
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import io.swagger.v3.oas.models.Operation;
import io.swagger.v3.oas.models.info.Info;
import io.swagger.v3.oas.models.media.ArraySchema;
import io.swagger.v3.oas.models.media.FileSchema;
import io.swagger.v3.oas.models.media.Schema;
import io.swagger.v3.oas.models.parameters.Parameter;
import io.swagger.v3.oas.models.parameters.RequestBody;
Expand Down Expand Up @@ -703,41 +702,6 @@ public CodegenParameter fromRequestBody(RequestBody body, Set<String> imports, S
return codegenParameter;
}

@Override
public String getTypeDeclaration(Schema p) {
if (ModelUtils.isArraySchema(p)) {
ArraySchema ap = (ArraySchema) p;
Schema inner = ap.getItems();
String innerType = getTypeDeclaration(inner);
return typeMapping.get("array") + "<" + innerType + ">";
} else if (ModelUtils.isMapSchema(p)) {
Schema inner = ModelUtils.getAdditionalProperties(p);
String innerType = getTypeDeclaration(inner);
StringBuilder typeDeclaration = new StringBuilder(typeMapping.get("map")).append("<").append(typeMapping.get("string")).append(", ");
typeDeclaration.append(innerType).append(">");
return typeDeclaration.toString();
} else if (!StringUtils.isEmpty(p.get$ref())) {
String datatype;
try {
datatype = p.get$ref();

if (datatype.indexOf("#/components/schemas/") == 0) {
datatype = toModelName(datatype.substring("#/components/schemas/".length()));
datatype = "models::" + datatype;
}
} catch (Exception e) {
LOGGER.warn("Error obtaining the datatype from schema (model):{}. Datatype default to Object", p);
datatype = "Object";
LOGGER.error(e.getMessage(), e);
}
return datatype;
} else if (p instanceof FileSchema) {
return typeMapping.get("File");
}

return super.getTypeDeclaration(p);
}

@Override
public String toInstantiationType(Schema p) {
if (ModelUtils.isArraySchema(p)) {
Expand All @@ -752,39 +716,6 @@ public String toInstantiationType(Schema p) {
}
}

@Override
public CodegenModel fromModel(String name, Schema model) {
LOGGER.trace("Creating model from schema: {}", model);

Map<String, Schema> allDefinitions = ModelUtils.getSchemas(this.openAPI);
CodegenModel mdl = super.fromModel(name, model);

mdl.vendorExtensions.put("x-upper-case-name", name.toUpperCase(Locale.ROOT));
if (!StringUtils.isEmpty(model.get$ref())) {
Schema schema = allDefinitions.get(ModelUtils.getSimpleRef(model.get$ref()));
mdl.dataType = typeMapping.get(schema.getType());
}
if (ModelUtils.isArraySchema(model)) {
if (typeMapping.containsKey(mdl.arrayModelType)) {
mdl.arrayModelType = typeMapping.get(mdl.arrayModelType);
} else {
mdl.arrayModelType = toModelName(mdl.arrayModelType);
}
} else if ((!mdl.anyOf.isEmpty()) || (!mdl.oneOf.isEmpty())) {
mdl.dataType = getSchemaType(model);
}

Schema additionalProperties = ModelUtils.getAdditionalProperties(model);

if (additionalProperties != null) {
mdl.additionalPropertiesType = getTypeDeclaration(additionalProperties);
}

LOGGER.trace("Created model: {}", mdl);

return mdl;
}

@Override
public Map<String, Object> postProcessSupportingFileData(Map<String, Object> bundle) {
generateYAMLSpecFile(bundle);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@

import com.samskivert.mustache.Mustache;
import com.samskivert.mustache.Template;
import io.swagger.v3.oas.models.media.ArraySchema;
import io.swagger.v3.oas.models.media.Schema;
import io.swagger.v3.oas.models.media.StringSchema;
import io.swagger.v3.oas.models.media.*;
import io.swagger.v3.parser.util.SchemaTypeUtil;
import joptsimple.internal.Strings;
import org.openapitools.codegen.*;
Expand All @@ -41,8 +39,7 @@
import java.math.BigDecimal;
import java.math.BigInteger;
import java.util.*;

import static org.openapitools.codegen.utils.StringUtils.camelize;
import java.util.stream.Collectors;

public class RustClientCodegen extends AbstractRustCodegen implements CodegenConfig {
private final Logger LOGGER = LoggerFactory.getLogger(RustClientCodegen.class);
Expand Down Expand Up @@ -149,11 +146,13 @@ public RustClientCodegen() {
typeMapping.clear();
typeMapping.put("integer", "i32");
typeMapping.put("long", "i64");
typeMapping.put("number", "f32");
typeMapping.put("number", "f64");
typeMapping.put("float", "f32");
typeMapping.put("double", "f64");
typeMapping.put("boolean", "bool");
typeMapping.put("string", "String");
typeMapping.put("array", "Vec");
typeMapping.put("map", "std::collections::HashMap");
typeMapping.put("UUID", "uuid::Uuid");
typeMapping.put("URI", "String");
typeMapping.put("date", "string");
Expand Down Expand Up @@ -205,11 +204,72 @@ public RustClientCodegen() {
setLibrary(REQWEST_LIBRARY);
}

@Override
public CodegenModel fromModel(String name, Schema model) {
CodegenModel mdl = super.fromModel(name, model);

// set correct names and baseNames to oneOf in composed-schema to use as enum variant names & mapping
if (mdl.getComposedSchemas() != null && mdl.getComposedSchemas().getOneOf() != null
&& !mdl.getComposedSchemas().getOneOf().isEmpty()) {

List<CodegenProperty> newOneOfs = mdl.getComposedSchemas().getOneOf().stream()
.map(CodegenProperty::clone)
.collect(Collectors.toList());
List<Schema> schemas = ModelUtils.getInterfaces(model);
if (newOneOfs.size() != schemas.size()) {
// For safety reasons, this should never happen unless there is an error in the code
throw new RuntimeException("oneOf size does not match the model");
}

Map<String, String> refsMapping = Optional.ofNullable(model.getDiscriminator())
.map(Discriminator::getMapping).orElse(Collections.emptyMap());

// Reverse mapped references to use as baseName for oneOF, but different keys may point to the same $ref.
// Thus, we group them by the value
Map<String, List<String>> mappedNamesByRef = refsMapping.entrySet().stream()
.collect(Collectors.groupingBy(Map.Entry::getValue,
Collectors.mapping(Map.Entry::getKey, Collectors.toList())
));

for (int i = 0; i < newOneOfs.size(); i++) {
CodegenProperty oneOf = newOneOfs.get(i);
Schema schema = schemas.get(i);

if (mappedNamesByRef.containsKey(schema.get$ref())) {
// prefer mapped names if present
// remove mapping not in order not to reuse for the next occurrence of the ref
List<String> names = mappedNamesByRef.get(schema.get$ref());
String mappedName = names.remove(0);
oneOf.setBaseName(mappedName);
oneOf.setName(toModelName(mappedName));
} else if (!org.apache.commons.lang3.StringUtils.isEmpty(schema.get$ref())) {
// use $ref if it's reference
String refName = ModelUtils.getSimpleRef(schema.get$ref());
if (refName != null) {
String modelName = toModelName(refName);
oneOf.setName(modelName);
oneOf.setBaseName(refName);
}
} else {
// In-placed type (primitive), because there is no mapping or ref for it.
// use camelized `title` if present, otherwise use `type`
String oneOfName = Optional.ofNullable(schema.getTitle()).orElseGet(schema::getType);
oneOf.setName(toModelName(oneOfName));
}
}

mdl.getComposedSchemas().setOneOf(newOneOfs);
}

return mdl;
}

@Override
public ModelsMap postProcessModels(ModelsMap objs) {
// Remove the discriminator field from the model, serde will take care of this
for (ModelMap model : objs.getModels()) {
CodegenModel cm = model.getModel();

if (cm.discriminator != null) {
String reserved_var_name = cm.discriminator.getPropertyBaseName();

Expand Down Expand Up @@ -399,43 +459,9 @@ public String modelDocFileFolder() {

@Override
public String getTypeDeclaration(Schema p) {
// use unaliased schema for client-side
Schema unaliasSchema = unaliasSchema(p);
if (ModelUtils.isArraySchema(unaliasSchema)) {
ArraySchema ap = (ArraySchema) unaliasSchema;
Schema inner = ap.getItems();
if (inner == null) {
LOGGER.warn("{}(array property) does not have a proper inner type defined.Default to string",
ap.getName());
inner = new StringSchema().description("TODO default missing array inner type to string");
}
return "Vec<" + getTypeDeclaration(inner) + ">";
} else if (ModelUtils.isMapSchema(unaliasSchema)) {
Schema inner = ModelUtils.getAdditionalProperties(unaliasSchema);
if (inner == null) {
LOGGER.warn("{}(map property) does not have a proper inner type defined. Default to string", unaliasSchema.getName());
inner = new StringSchema().description("TODO default missing map inner type to string");
}
return "::std::collections::HashMap<String, " + getTypeDeclaration(inner) + ">";
}

// Not using the supertype invocation, because we want to UpperCamelize
// the type.
String schemaType = getSchemaType(unaliasSchema);
if (typeMapping.containsKey(schemaType)) {
return typeMapping.get(schemaType);
}

if (typeMapping.containsValue(schemaType)) {
return schemaType;
}

if (languageSpecificPrimitives.contains(schemaType)) {
return schemaType;
}

// return fully-qualified model name
// crate::models::{{classnameFile}}::{{classname}}
return "crate::models::" + toModelName(schemaType);
return super.getTypeDeclaration(unaliasSchema);
}

@Override
Expand Down
Loading

0 comments on commit 518b29d

Please sign in to comment.