Update the openapi.generator plugin from version 4.0.0-beta to 4.0.2 #6
Conversation
…fix issues around paths containing . characters. REFERENCE: OpenAPITools/openapi-generator#1895 As part of this up rev, certain unit tests began failing related to String and URI conflicts. After some testing, found that removing the "format: uri" tag from the yaml file resolved the issue. I did this in an attempt to keep the method signature String given the implied unit test functionality. I did not do a full audit and it may be recommended that we confirm this before merging by someone more familiar with the code generation.
| @@ -230,7 +230,6 @@ components: | |||
| required: true | |||
| schema: | |||
| type: string | |||
| format: uri | |||
There was a problem hiding this comment.
❗️ Although this API is not really out yet, this change is removing a restriction on the attribute that was there in the previous version. We shouldn't be changing the spec to fix test cases. If the new generator is generating the code as a URI now as opposed to a string as before then this is what it should be.
There was a problem hiding this comment.
I'm not certain that I follow the logic.
If the API originally was generating "String someMethod()" and is now generating "URI someMethod()"; wouldn't it be more consistent in the API to require String to keep backwards compatibility. Or, are you saying that you prefer the harder restriction of URI and because this API is not released; the signature change won't matter (thereby change the unit test)?
There was a problem hiding this comment.
Currently what matters is the open api spec and not how the code was generated. The fact that the generator didn't put the right restrictions based on how it was specified is not an indication of what the specification of that version was. I think we need to stick with URI and be as specific as we can be. Code generation is something that has to be fixed separately. In other words, one couldn't say that the spec was accepting any strings since the spec was saying it should a URI. The code might have been wrong bu that is not the spec.
There was a problem hiding this comment.
Ok, I followed that. I'll make the necessary changes and get back to you once the unit tests are passing. Thanks for the clarification.
| @@ -307,7 +306,6 @@ components: | |||
| example: 4.6.9 | |||
| url: | |||
| type: string | |||
| format: uri | |||
Update the openapi.generator plugin from version 4.0.0-beta to 4.0.2 fix issues around paths containing . characters.
REFERENCE: OpenAPITools/openapi-generator#1895
As part of this up rev, certain unit tests began failing related to String and URI conflicts. After some testing, found that removing the "format: uri" tag from the yaml file resolved the issue. I did this in an attempt to keep the method signature String given the implied unit test functionality.
I did not do a full audit and it may be recommended that we confirm this before merging by someone more familiar with the code generation.
Who is reviewing it (please choose AT LEAST two reviewers that need to approve the PR before it can get merged)?
@paouelle
How should this be tested? (List steps with links to updated documentation)
Run CI and review the Interfaces related to generated code
Checklist: