Skip to content

Conversation

@jimschubert
Copy link
Member

@jimschubert jimschubert commented Mar 26, 2019

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, ./bin/openapi3/{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.

Description of the PR

This is a rebase of #2436. Refer to that PR for discussion. This is work toward cleanup defined in #2285.

/cc @A-Joshi for visibility

private void setCliOption(CliOption cliOption) throws IllegalArgumentException {
if (additionalProperties.containsKey(cliOption.getOpt())) {
cliOption.setOptValue(additionalProperties.get(cliOption.getOpt()).toString());
if (classModifier.getOptValue() == null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy paste error - line should have cliOption.getOptValue not classModifier.getOptvalue

@wing328 wing328 added this to the 4.0.0 milestone Mar 27, 2019
@jimschubert jimschubert self-assigned this Mar 30, 2019
@A-Joshi
Copy link
Contributor

A-Joshi commented Mar 31, 2019 via email

* master: (48 commits)
  [Typescript AngularJS] fix Extra package prefix in api parameters operations (OpenAPITools#2522)
  OpenAPITools#1023 - [Scala] Use status family during response processing (OpenAPITools#1024)
  Generate setters for readonly properties in server code (OpenAPITools#1582)
  [JS] fix NPE for null string and improve Travis config file (OpenAPITools#2553)
  [elm] Update ISO 8601 library (fixes missing time zone designator) (OpenAPITools#2545)
  [csharp] update sample after OpenAPITools#2528 (OpenAPITools#2550)
  [JavaScript] fix index.js, ApiClient.js and test files generated to incorrect location (OpenAPITools#2511)
  Aspnetcore nullable support (OpenAPITools#2529)
  Csharp nullable support (OpenAPITools#2528)
  [C++] [Qt5] Add enum support for client and server (OpenAPITools#2339)
  Fixed typo in migration-from-swagger-codegen.md (OpenAPITools#2548)
  [TypeScript Client] fix install Aurelia + fix use deprecated function (OpenAPITools#2514)
  [KOTLIN] fix var name not correctly sanitized (OpenAPITools#2537)
  Update swagger-parser to '2.0.11-OpenAPITools.org-1' (OpenAPITools#2262)
  Add @karismann to Java and Kotlin technical committee (OpenAPITools#2542)
  Add GoDaddy to the list of companies using OpenAPI Generator (OpenAPITools#2541)
  [Kotlin SpringBoot Server] alternative: fix optional parameter not correctly declared in service (OpenAPITools#2539)
  improve indentation, update dependencies (OpenAPITools#2521)
  update kotlin spring samples
  [JAVA] Use specified data type in enum's fromValue instead of string (OpenAPITools#2347)
  ...
@jimschubert
Copy link
Member Author

Merging master into this branch locally, and regenerating the sample produces invalid annotations. Example:

         /// <summary>
         /// User Status
         /// </summary>
         /// <value>User Status</value>
-        [DataMember(Name="userStatus")]
+        [DataMember(Name="userStatus", EmitDefaultValue=)]
         public int? UserStatus { get; set; }

@wing328 do you know of a recent change which would have caused this?

I'm also seeing a warning and error dump when running the sample script. Possibly related?

$ ./bin/aspnetcore-petstore-server.sh
# START SCRIPT: ./bin/aspnetcore-petstore-server.sh
OpenJDK 64-Bit Server VM warning: ignoring option MaxPermSize=256M; support was removed in 8.0
[main] WARN  i.s.parser.util.DeserializationUtils - Error snake-parsing yaml content
io.swagger.parser.util.DeserializationUtils$SnakeException: Exception safe-checking yaml content  (maxDepth 2000)
	at io.swagger.parser.util.DeserializationUtils$CustomSnakeYamlConstructor.getSingleData(DeserializationUtils.java:300)
	at org.yaml.snakeyaml.Yaml.loadFromReader(Yaml.java:450)
	at org.yaml.snakeyaml.Yaml.load(Yaml.java:369)
	at io.swagger.parser.util.DeserializationUtils.readYamlTree(DeserializationUtils.java:137)
	at io.swagger.parser.Swagger20Parser.deserializeYaml(Swagger20Parser.java:83)
	at io.swagger.parser.Swagger20Parser.readWithInfo(Swagger20Parser.java:64)
	at io.swagger.parser.SwaggerParser.readWithInfo(SwaggerParser.java:32)
	at io.swagger.v3.parser.converter.SwaggerConverter.readLocation(SwaggerConverter.java:92)
	at io.swagger.parser.OpenAPIParser.readLocation(OpenAPIParser.java:19)
	at org.openapitools.codegen.config.CodegenConfigurator.toClientOptInput(CodegenConfigurator.java:606)
	at org.openapitools.codegen.cmd.Generate.run(Generate.java:367)
	at org.openapitools.codegen.OpenAPIGenerator.main(OpenAPIGenerator.java:60)
Caused by: java.lang.IllegalAccessError: tried to access field org.yaml.snakeyaml.constructor.BaseConstructor.composer from class io.swagger.parser.util.DeserializationUtils$CustomSnakeYamlConstructor
	at io.swagger.parser.util.DeserializationUtils$CustomSnakeYamlConstructor.getSingleData(DeserializationUtils.java:279)
	... 11 common frames omitted
[main] WARN  o.o.codegen.utils.ModelUtils - Multiple schemas found in content, returning only the first one
[main] WARN  o.o.codegen.utils.ModelUtils - Multiple schemas found in content, returning only the first one
[main] WARN  o.o.codegen.utils.ModelUtils - Multiple schemas found in content, returning only the first one

@wing328
Copy link
Member

wing328 commented Apr 1, 2019

[main] WARN i.s.parser.util.DeserializationUtils - Error snake-parsing yaml content

I noticed that too after the parser upgrade.

@wing328
Copy link
Member

wing328 commented Apr 1, 2019

@wing328 do you know of a recent change which would have caused this?

I also notice that in the latest master and I think I know why. Let me try to fix it later today.

@wing328
Copy link
Member

wing328 commented Apr 1, 2019

@wing328 do you know of a recent change which would have caused this?

The issue has been fixed via #2558

I'll file another PR to decommission the emitDefaultValue option as it should be controlled by the nullable attribute instead.

@wing328
Copy link
Member

wing328 commented Apr 1, 2019

FYI. Filed #2559 to remove the emitDefaultValue option.

@A-Joshi
Copy link
Contributor

A-Joshi commented Apr 1, 2019 via email

@jimschubert jimschubert merged commit 128da8e into OpenAPITools:master Apr 2, 2019
@jimschubert jimschubert deleted the ihsmarkitoss-feature/cleanup branch April 2, 2019 01:07
@jimschubert
Copy link
Member Author

@A-Joshi merged. This should unblock you for any additional work you'd had planned.

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.

3 participants