Skip to content

[Rust Server] Improve XML support #2504

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

Merged
merged 1 commit into from
Apr 22, 2019
Merged

[Rust Server] Improve XML support #2504

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
12 changes: 9 additions & 3 deletions bin/rust-server-petstore.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,16 @@ then
mvn -B clean package
fi

for spec_path in modules/openapi-generator/src/test/resources/2_0/rust-server/* ; do
for spec_path in modules/openapi-generator/src/test/resources/*/rust-server/* ; do
export JAVA_OPTS="${JAVA_OPTS} -XX:MaxPermSize=256M -Xmx1024M -DloggerPath=conf/log4j.properties"
spec=$(basename "$spec_path" | sed 's/.yaml//')
ags="generate -t modules/openapi-generator/src/main/resources/rust-server -i $spec_path -g rust-server -o samples/server/petstore/rust-server/output/$spec -DpackageName=$spec --additional-properties hideGenerationTimestamp=true $@"
args="generate --template-dir modules/openapi-generator/src/main/resources/rust-server
--input-spec $spec_path
--generator-name rust-server
--output samples/server/petstore/rust-server/output/$spec
-DpackageName=$spec
--additional-properties hideGenerationTimestamp=true
$@"

java $JAVA_OPTS -jar $executable $ags
java $JAVA_OPTS -jar $executable $args
done
114 changes: 62 additions & 52 deletions ...openapi-generator/src/main/java/org/openapitools/codegen/languages/RustServerCodegen.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,17 @@

package org.openapitools.codegen.languages;

import io.swagger.v3.core.util.Json;
import io.swagger.v3.oas.models.OpenAPI;
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.media.StringSchema;
import io.swagger.v3.oas.models.media.XML;
import io.swagger.v3.oas.models.parameters.Parameter;
import io.swagger.v3.oas.models.parameters.RequestBody;
import io.swagger.v3.oas.models.servers.Server;
import org.apache.commons.lang3.StringUtils;
import org.openapitools.codegen.*;
Expand Down Expand Up @@ -64,6 +67,9 @@ public class RustServerCodegen extends DefaultCodegen implements CodegenConfig {
public RustServerCodegen() {
super();

// Force generating models for wrapping types
ModelUtils.setGenerateAliasAsModel(true);
Copy link
Member

Choose a reason for hiding this comment

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

@richardwhiuk thanks for the PR. Do you mind explaining why we must enforce this option to true?

Some users simply want to reuse the same array definition through out the spec with aliases.

Copy link
Contributor Author

@richardwhiuk richardwhiuk Apr 21, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry, this should only apply to the Rust Generator - I need to figure out where to move it to so it's only invoked when the Rust generator is in use.

Currently, the generator assumes that the model will exist when generating the API bindings.

This means that it currently generates compile errors like:

error[E0412]: cannot find type `AnotherXmlArray` in module `models`
  --> output/openapi-v3/src/lib.rs:94:52
   |
94 |     fn xml_other_put(&self, string: Option<models::AnotherXmlArray>, context: &C) -> Box<Future<Item=XmlOtherPutResponse, Error=ApiError>>;
   |                                                    ^^^^^^^^^^^^^^^ did you mean `AnotherXmlInner`?

(for non aliasing, that should be Option<Vec<....>>.)

Worse, this is a backwards compatibility break - previously the generator did generate aliasing - #1729 changes this so the default is not to generate aliasing, meaning that the 4.x branch generates Rust code that doesn't compile.

Longer term we can look at enhancing the Rust Server code generator to support generating code which compiles when aliasing is disabled (though unit structs in the manner that the Rust generator generates are fairly idiomatic, and allow for additional methods to be declared on them).

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, this should only apply to the Rust Generator - I need to figure out where to move it to so it's only invoked when the Rust generator is in use.

Currently, it's already the case, which means only the Rust server generator will have this option enabled.

Thanks for the detailed explanation. We can certainly support generating array/map as model later on.

Copy link
Member

Choose a reason for hiding this comment

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

Currently, it's already the case, which means only the Rust server generator will have this option enabled.

I know what you mean now. I'll need to uncomment this line and show a warning message for the time being.

Copy link
Member

Choose a reason for hiding this comment

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

Filed #2714

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think the constructor must get run at startup, which means the other generators pick this up.

I'm slightly surprised we've defaulted generateAliasAsModel to false - this seems like the reverse of the default in 3.x?

Copy link
Member

Choose a reason for hiding this comment

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

4.x is a branch with breaking changes (with or without fallback) and the main reason is that most generators do not support alias (array/map) as model so that's we set generateAliasAsModel to false as the default.


// Show the generation timestamp by default
hideGenerationTimestamp = Boolean.FALSE;

Expand Down Expand Up @@ -742,6 +748,39 @@ public boolean isDataTypeFile(final String dataType) {
return dataType != null && dataType.equals(typeMapping.get("File").toString());
}

// This is a really terrible hack. We're working around the fact that the
// base version of `fromRequestBody` checks to see whether the body is a
// ref. If so, it unwraps the reference and replaces it with its inner
// type. This causes problems in rust-server, as it means that we use inner
// types in the API, rather than the correct outer type.
//
// Thus, we grab the inner schema beforehand, and then tinker afterwards to
// restore things to sensible values.
@Override
public CodegenParameter fromRequestBody(RequestBody body, Set<String> imports, String bodyParameterName) {
Schema original_schema = ModelUtils.getSchemaFromRequestBody(body);
CodegenParameter codegenParameter = super.fromRequestBody(body, imports, bodyParameterName);

if (StringUtils.isNotBlank(original_schema.get$ref())) {
// Undo the mess `super.fromRequestBody` made - re-wrap the inner
// type.
codegenParameter.dataType = getTypeDeclaration(original_schema);
codegenParameter.isPrimitiveType = false;
codegenParameter.isListContainer = false;
codegenParameter.isString = false;

// This is a model, so should only have an example if explicitly
// defined.
if (codegenParameter.vendorExtensions != null && codegenParameter.vendorExtensions.containsKey("x-example")) {
codegenParameter.example = Json.pretty(codegenParameter.vendorExtensions.get("x-example"));
} else {
codegenParameter.example = null;
}
}

return codegenParameter;
}

@Override
public String getTypeDeclaration(Schema p) {
if (ModelUtils.isArraySchema(p)) {
Expand Down Expand Up @@ -779,35 +818,6 @@ public String getTypeDeclaration(Schema p) {
return super.getTypeDeclaration(p);
}

private boolean isNonPrimitive(CodegenParameter parameter) {
return !parameter.isString && !parameter.isNumeric && !parameter.isByteArray &&
!parameter.isBinary && !parameter.isFile && !parameter.isBoolean &&
!parameter.isDate && !parameter.isDateTime && !parameter.isUuid &&
!parameter.isListContainer && !parameter.isMapContainer &&
!languageSpecificPrimitives.contains(parameter.dataType);
}

@Override
public CodegenParameter fromParameter(Parameter param, Set<String> imports) {
CodegenParameter parameter = super.fromParameter(param, imports);
if (isNonPrimitive(parameter)) {
String name = "models::" + getTypeDeclaration(parameter.dataType);
parameter.dataType = name;
}

return parameter;
}

@Override
public void postProcessParameter(CodegenParameter parameter) {
// If this parameter is not a primitive type, prefix it with "models::"
// to ensure it's namespaced correctly in the Rust code.
if (isNonPrimitive(parameter)) {
String name = "models::" + getTypeDeclaration(parameter.dataType);
parameter.dataType = name;
}
}

@Override
public String toInstantiationType(Schema p) {
if (ModelUtils.isArraySchema(p)) {
Expand All @@ -833,17 +843,34 @@ public CodegenModel fromModel(String name, Schema model) {
}
if (ModelUtils.isArraySchema(model)) {
ArraySchema am = (ArraySchema) model;
String xmlName = null;

// Detect XML list where the inner item is defined directly.
if ((am.getItems() != null) &&
(am.getItems().getXml() != null)) {
xmlName = am.getItems().getXml().getName();
}

// If this model's items require wrapping in xml, squirrel
// away the xml name so we can insert it into the relevant model fields.
String xmlName = am.getItems().getXml().getName();
if (xmlName != null) {
mdl.vendorExtensions.put("itemXmlName", xmlName);
modelXmlNames.put("models::" + mdl.classname, xmlName);
// Detect XML list where the inner item is a reference.
if (am.getXml() != null && am.getXml().getWrapped() &&
am.getItems() != null &&
!StringUtils.isEmpty(am.getItems().get$ref())) {
Schema inner_schema = allDefinitions.get(
ModelUtils.getSimpleRef(am.getItems().get$ref()));

if (inner_schema.getXml() != null &&
inner_schema.getXml().getName() != null) {
xmlName = inner_schema.getXml().getName();
}
}

// If this model's items require wrapping in xml, squirrel away the
// xml name so we can insert it into the relevant model fields.
if (xmlName != null) {
mdl.vendorExtensions.put("itemXmlName", xmlName);
modelXmlNames.put("models::" + mdl.classname, xmlName);
}

mdl.arrayModelType = toModelName(mdl.arrayModelType);
}

Expand Down Expand Up @@ -1076,23 +1103,6 @@ public Map<String, Object> postProcessModels(Map<String, Object> objs) {
return super.postProcessModelsEnum(objs);
}

private boolean paramHasXmlNamespace(CodegenParameter param, Map<String, Schema> definitions) {
Object refName = param.vendorExtensions.get("refName");

if ((refName != null) && (refName instanceof String)) {
String name = (String) refName;
Schema model = definitions.get(ModelUtils.getSimpleRef(name));

if (model != null) {
XML xml = model.getXml();
if ((xml != null) && (xml.getNamespace() != null)) {
return true;
}
}
}
return false;
}

private void processParam(CodegenParameter param, CodegenOperation op) {
String example = null;

Expand Down
42 changes: 25 additions & 17 deletions modules/openapi-generator/src/main/resources/rust-server/client-mod.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,23 +280,31 @@ impl<F, C> Api<C> for Client<F> where
request.headers_mut().set(ContentType(mimetypes::requests::{{#vendorExtensions}}{{{uppercase_operation_id}}}{{/vendorExtensions}}.clone()));
request.set_body(body.into_bytes());{{/-last}}{{/formParams}}{{/vendorExtensions}}{{#bodyParam}}{{#-first}}
// Body parameter
{{/-first}}{{#vendorExtensions}}{{#required}}{{#consumesPlainText}} let body = param_{{{paramName}}};{{/consumesPlainText}}{{#consumesXml}}
{{^has_namespace}} let body = serde_xml_rs::to_string(&param_{{{paramName}}}).expect("impossible to fail to serialize");{{/has_namespace}}{{#has_namespace}}
let mut namespaces = BTreeMap::new();
// An empty string is used to indicate a global namespace in xmltree.
namespaces.insert("".to_string(), models::namespaces::{{{uppercase_data_type}}}.clone());
let body = serde_xml_rs::to_string_with_namespaces(&param_{{{paramName}}}, namespaces).expect("impossible to fail to serialize");{{/has_namespace}}{{/consumesXml}}{{#consumesJson}}
let body = serde_json::to_string(&param_{{{paramName}}}).expect("impossible to fail to serialize");{{/consumesJson}}
{{/required}}{{^required}}{{#consumesPlainText}} let body = param_{{{paramName}}};
{{/consumesPlainText}}{{^consumesPlainText}} let body = param_{{{paramName}}}.map(|ref body| {
{{#consumesXml}}
{{^has_namespace}} serde_xml_rs::to_string(body).expect("impossible to fail to serialize"){{/has_namespace}}{{#has_namespace}}
let mut namespaces = BTreeMap::new();
// An empty string is used to indicate a global namespace in xmltree.
namespaces.insert("".to_string(), models::namespaces::{{{uppercase_data_type}}}.clone());
serde_xml_rs::to_string_with_namespaces(body, namespaces).expect("impossible to fail to serialize"){{/has_namespace}}{{/consumesXml}}{{#consumesJson}}
serde_json::to_string(body).expect("impossible to fail to serialize"){{/consumesJson}}
});{{/consumesPlainText}}{{/required}}{{/vendorExtensions}}{{/bodyParam}}
{{/-first}}
{{#vendorExtensions}}
{{#consumesPlainText}}
let body = param_{{{paramName}}};
{{/consumesPlainText}}
{{#required}}
{{#consumesXml}}
let body = param_{{{paramName}}}.to_xml();
{{/consumesXml}}
{{#consumesJson}}
let body = serde_json::to_string(&param_{{{paramName}}}).expect("impossible to fail to serialize");
{{/consumesJson}}
{{/required}}
{{^required}}
let body = param_{{{paramName}}}.map(|ref body| {
{{#consumesXml}}
body.to_xml()
{{/consumesXml}}
{{#consumesJson}}
serde_json::to_string(body).expect("impossible to fail to serialize")
{{/consumesJson}}
});
{{/required}}
{{/vendorExtensions}}
{{/bodyParam}}

{{#bodyParam}}{{^required}}if let Some(body) = body {
{{/required}} request.set_body(body.into_bytes());
Expand Down
2 changes: 2 additions & 0 deletions modules/openapi-generator/src/main/resources/rust-server/example-server_lib.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ use std::marker::PhantomData;
use hyper;
use {{{externCrateName}}};
use swagger::{Has, XSpanIdString};
{{#hasAuthMethods}}
use swagger::auth::Authorization;
{{/hasAuthMethods}}

pub struct NewService<C>{
marker: PhantomData<C>
Expand Down
51 changes: 42 additions & 9 deletions modules/openapi-generator/src/main/resources/rust-server/models.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,17 @@
extern crate chrono;
extern crate uuid;

{{#usesXml}}use serde_xml_rs;{{/usesXml}}
{{#usesXml}}
use serde_xml_rs;
{{/usesXml}}
use serde::ser::Serializer;

{{#usesXml}}
use std::collections::{HashMap, BTreeMap};
{{/usesXml}}
{{^usesXml}}
use std::collections::HashMap;
{{/usesXml}}
use models;
use swagger;

Expand Down Expand Up @@ -70,6 +77,7 @@ impl ::std::ops::DerefMut for {{{classname}}} {
}

{{/dataType}}{{^dataType}}{{#arrayModelType}}{{#vendorExtensions}}{{#itemXmlName}}// Utility function for wrapping list elements when serializing xml
#[allow(non_snake_case)]
fn wrap_in_{{{itemXmlName}}}<S>(item: &Vec<{{{arrayModelType}}}>, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
Expand All @@ -78,7 +86,7 @@ where
}

{{/itemXmlName}}{{/vendorExtensions}}{{! vec}}#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct {{{classname}}}(Vec<{{{arrayModelType}}}>);
pub struct {{{classname}}}({{#vendorExtensions}}{{#itemXmlName}}#[serde(serialize_with = "wrap_in_{{{itemXmlName}}}")]{{/itemXmlName}}{{/vendorExtensions}}Vec<{{{arrayModelType}}}>);

impl ::std::convert::From<Vec<{{{arrayModelType}}}>> for {{{classname}}} {
fn from(x: Vec<{{{arrayModelType}}}>) -> Self {
Expand Down Expand Up @@ -164,12 +172,37 @@ impl {{{classname}}} {
}
}
}
{{/arrayModelType}}{{/dataType}}{{/isEnum}}{{/model}}{{/models}}{{#usesXmlNamespaces}}
//XML namespaces
pub mod namespaces {
lazy_static!{
{{#models}}{{#model}}{{#xmlNamespace}}pub static ref {{#vendorExtensions}}{{{upperCaseName}}}{{/vendorExtensions}}: String = "{{{xmlNamespace}}}".to_string();
{{/xmlNamespace}}{{/model}}{{/models}}
}
{{/arrayModelType}}
{{/dataType}}
{{/isEnum}}

{{#usesXml}}
{{#usesXmlNamespaces}}
{{#xmlNamespace}}
impl {{{classname}}} {
/// Associated constant for this model's XML namespace.
#[allow(dead_code)]
pub const NAMESPACE: &'static str = "{{{xmlNamespace}}}";
}

{{/xmlNamespace}}
{{/usesXmlNamespaces}}
impl {{{classname}}} {
/// Helper function to allow us to convert this model to an XML string.
/// Will panic if serialisation fails.
#[allow(dead_code)]
pub(crate) fn to_xml(&self) -> String {
{{#xmlNamespace}}
let mut namespaces = BTreeMap::new();
// An empty string is used to indicate a global namespace in xmltree.
namespaces.insert("".to_string(), Self::NAMESPACE.to_string());
serde_xml_rs::to_string_with_namespaces(&self, namespaces).expect("impossible to fail to serialize")
{{/xmlNamespace}}
{{^xmlNamespace}}
serde_xml_rs::to_string(&self).expect("impossible to fail to serialize")
{{/xmlNamespace}}
}
}
{{/usesXml}}
{{/model}}
{{/models}}
2 changes: 1 addition & 1 deletion modules/openapi-generator/src/main/resources/rust-server/server-mod.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ where
{{/has_namespace}}{{#has_namespace}}
let mut namespaces = BTreeMap::new();
// An empty string is used to indicate a global namespace in xmltree.
namespaces.insert("".to_string(), models::namespaces::{{{uppercase_data_type}}}.clone());
namespaces.insert("".to_string(), {{{dataType}}}::NAMESPACE.to_string());
let body = serde_xml_rs::to_string_with_namespaces(&body, namespaces).expect("impossible to fail to serialize");
{{/has_namespace}}{{/producesXml}}{{#producesJson}}
let body = serde_json::to_string(&body).expect("impossible to fail to serialize");
Expand Down
1 change: 1 addition & 0 deletions modules/openapi-generator/src/test/resources/2_0/rust-server/rust-server-test.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ paths:
description: Success
schema:
type: object

# Requests with arbitrary JSON currently fail.
# post:
# summary: Send an arbitrary JSON blob
Expand Down
Loading