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.

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

[Rust] Ignore additional properties #671

Merged
merged 2 commits into from
Jul 30, 2018
Merged

[Rust] Ignore additional properties #671

merged 2 commits into from
Jul 30, 2018

Conversation

bjgill
Copy link
Contributor

@bjgill bjgill commented Jul 27, 2018

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: master, 4.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language. @frol @farcaller

Description of the PR

This is a workaround for #318. In swagger-codegen, we did not support additionalProperties and so ignored them.

In openapi-generator, we now get back a type (map), where we got back a null previously. postProcessModels doesn't have enough context to work out what sort of map is required (we don't know the types of the additionalProperties).

Thus, this workaround detects this specific failure case and fiddles with the dataType to get additionalProperties ignored again. This will allow us to generate from specs containing additionalProperties and have the resultant crates compile.

I've added in a warning to users to let them know what's happening.

I'm building on top of #658 to allow us to add a test to ensure that we don't regress this in future.

// them recognised correctly.
cm.isAlias = false;
cm.dataType = typeMapping.get(cm.dataType);
if (cm.dataType == "map") {
Copy link
Member

Choose a reason for hiding this comment

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

@bjgill should this be "map".equals(cm.dataType) instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point - I'll be consistent with the equivalent 5 lines previously, though - cm.dataType.equals("map")

Benjamin Gill added 2 commits July 30, 2018 10:18
rust-server doen't yet support them, and they cause quite a bit of havoc at the moment (ending up as the `HashMap` type).
@bjgill
Copy link
Contributor Author

bjgill commented Jul 30, 2018

Not sure why travis failed. Doesn't seem to be anything Rust-related, though. Do you have the ability to retry the job?

...
npm WARN deprecated [email protected]: Jade has been renamed to pug, please install the latest version of pug instead of jade
npm WARN deprecated [email protected]: This package is unmaintained. Use @sinonjs/formatio instead
npm WARN deprecated [email protected]: Please update to minimatch 3.0.2 or higher to avoid a RegExp DoS issue
npm WARN deprecated [email protected]: please upgrade to graceful-fs 4 for compatibility with current and future versions of Node.js
npm WARN deprecated [email protected]: In 6.x, the babel package has been deprecated in favor of babel-cli. Check https://opencollective.com/babel to support the Babel maintainers
No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself.
Check the details on how to adjust your build configuration on: https://docs.travis-ci.com/user/common-build-problems/#Build-times-out-because-no-output-was-received
The build has been terminated

@wing328
Copy link
Member

wing328 commented Jul 30, 2018

@bjgill Restarted the job. Let's see how it goes

@bjgill
Copy link
Contributor Author

bjgill commented Jul 30, 2018

That passed, thanks.

@wing328 wing328 merged commit a3e5185 into OpenAPITools:master Jul 30, 2018
@bjgill bjgill deleted the ignore-additional-properties branch July 30, 2018 15:11
@wing328 wing328 changed the title Ignore additional properties [Rust] Ignore additional properties Jul 31, 2018
A-Joshi pushed a commit to ihsmarkitoss/openapi-generator that referenced this pull request Feb 27, 2019
* Ignore additionalProperties

rust-server doen't yet support them, and they cause quite a bit of havoc at the moment (ending up as the `HashMap` type).

* Use .equals() rather than `==`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants