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-server] Restore newtype and XML support #1705

Closed
wants to merge 17 commits into from
Closed

[rust-server] Restore newtype and XML support #1705

wants to merge 17 commits into from

Conversation

bjgill
Copy link
Contributor

@bjgill bjgill commented Dec 18, 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, 3.4.x, 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

As part of the migration from swagger-codegen to openapi-generator, the rust-server generator lost support for newtypes (e.g. pub struct OuterString(String)). Partly as a consequence, XML support was also broken.

This PR undoes the change made by the default generator to unwrap references. This restores the old behaviour of rust-server.

I've added in a sample to verify the XML behaviour. We've also successfully used this branch to generate a Rust client for one of our services currently migrating from swagger-codegen to openapi-generator.

This will be my final PR to openapi-generator on behalf of @Metaswitch. I am leaving the company at the end of the year. @rjw2 will be taking over my role in handling upstream contributions

Fixes #1590

Benjamin Gill added 17 commits December 18, 2018 13:41
This breaks the examples rather spectacularly, though.
Rather than re-writing fromRequestBody wholesale, we instead just subclass it. We now run the superclass method. By detecting whether the superclass method has just unwrapped an inner type it should not have, we can then perform the appropriate fiddling to undo the unwrapping.
Meaning that the other can test our behaviour in the absence of XML.
@auto-labeler
Copy link

auto-labeler bot commented Dec 18, 2018

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

Please review the labels and make any necessary changes.

@bjgill
Copy link
Contributor Author

bjgill commented Dec 18, 2018

Hmm. It looks as if #1296 may have broken this PR (I was working off master from early November).

@bjgill
Copy link
Contributor Author

bjgill commented Dec 19, 2018

Indeed - this patch relies on having the original type wrappers for arrays available, so that we can hang XML-handling code off them. It looks as if https://github.com/OpenAPITools/openapi-generator/pull/1296/files#diff-bd1652e990ffe227072a5c8908fd3054R442 is stripping out those wrappers, and hence breaking this PR.

I'm not immediately sure how to handle. This stripping happens early enough in openapi-generator that I can't see an easy way of working around it.

@wing328
Copy link
Member

wing328 commented Feb 13, 2019

I'm not immediately sure how to handle. This stripping happens early enough in openapi-generator that I can't see an easy way of working around it.

@bjgill What about setting the generateAliasAsModel option to true?

@wing328 wing328 added this to the 4.0.0 milestone Feb 13, 2019
@bjgill
Copy link
Contributor Author

bjgill commented Feb 13, 2019

Interesting. I was unaware of that option. I don't know what it is, but it certainly sounds promising.

In any case, I'm probably not going to have time to look at this PR in the near future. I'm happy to leave for another week or two in case someone else wants to pick this work up. Otherwise, I'll close this PR.

@bjgill bjgill closed this Feb 23, 2019
@richardwhiuk richardwhiuk deleted the rust-xml-support branch June 2, 2024 11:12
@richardwhiuk richardwhiuk restored the rust-xml-support branch June 2, 2024 11:12
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