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

Support openApiNullable in jaxrs-cxf-client generator #8863

Merged

Conversation

cowclaw
Copy link
Contributor

@cowclaw cowclaw commented Mar 1, 2021

Until now the jaxrs-cxf-client generator does not support the openApiNullable config option although https://openapi-generator.tech/docs/generators/jaxrs-cxf-client lists is explicitly. Support for openApiNullable using jackson-databind-nullable (https://github.com/OpenAPITools/jackson-databind-nullable) is added in this PR.

Hints for the reviewer:

  • The PR is best reviewed commit-by-commit. The commits are written to be most reviewer-friendly.
  • The first commit introduces a new petstore sample for jaxrs-cxf-client with additional property jackson set to true. It reuses modules/openapi-generator/src/test/resources/3_0/petstore-with-fake-endpoints-models-for-testing.yaml which already provides many nullable properties.
  • The remainder of the PR subsequently adjusts the code generation. Each commit updates the mustache file and the generated code samples (by running the generator after adjusting the template).

cc: @wing328 @bbdouglas @sreeshas @jfiala @lukoyanov @cbornet @jeff9finger @karismann @Zomzog @lwlee2608 @nmuesch

Thanks in advance!

PR checklist

  • Read the contribution guidelines.
  • Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • Run the following to build the project and update samples:
    ./mvnw clean package 
    ./bin/generate-samples.sh
    ./bin/utils/export_docs_generators.sh
    
    Commit all changed files.
    This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
    These must match the expectations made by your contribution.
    You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
    For Windows users, please run the script in Git BASH.
  • File the PR against the correct branch: master, 5.1.x, 6.0.x
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

@wing328
Copy link
Member

wing328 commented Mar 24, 2021

The output samples/openapi3/client/petstore/jaxrs-cxf-client-jackson-nullable/ doesn't compile:

[ERROR]   symbol:   variable JsonNullable
[ERROR]   location: class org.openapitools.model.EnumTest
[ERROR] /Users/williamcheng/Code/openapi-generator/samples/openapi3/client/petstore/jaxrs-cxf-client-jackson-nullable/src/gen/java/org/openapitools/model/EnumTest.java:[285,24] cannot find symbol
[ERROR]   symbol:   variable JsonNullable
[ERROR]   location: class org.openapitools.model.EnumTest
[ERROR] /Users/williamcheng/Code/openapi-generator/samples/openapi3/client/petstore/jaxrs-cxf-client-jackson-nullable/src/gen/java/org/openapitools/model/EnumTest.java:[294,22] cannot find symbol
[ERROR]   symbol:   variable JsonNullable
[ERROR]   location: class org.openapitools.model.EnumTest

Can you please take a look?

@cowclaw
Copy link
Contributor Author

cowclaw commented Mar 24, 2021

Hello @wing328,

thanks for the review!

I indeed forgot to include jackson-databind-nullable as a dependency for the new sample. This is fixed in 7946e6e364d1cf4d550e6063361178294a83cee4.

The sample compiles now. Also the mvn integration-test -f samples/openapi3/client/petstore/jaxrs-cxf-client-jackson-nullable/pom.xml runs successfully.

Thanks and best regards,
Florian

BTW: shall I update to current master?

@cowclaw
Copy link
Contributor Author

cowclaw commented Mar 24, 2021

Please note that circleci build is still failing because the fix for #8922 is not (yet) included in the MR. I guess it will pass again after update to current master.

@wing328
Copy link
Member

wing328 commented Mar 25, 2021

Please note that circleci build is still failing because the fix for #8922 is not (yet) included in the MR. I guess it will pass again after update to current master.

Can you please pull the latest master into your branch to fix it?

@cowclaw cowclaw force-pushed the jaxrs_cxf_client_support_openapi_nullable branch from 7946e6e to 2424567 Compare March 25, 2021 15:54
@cowclaw
Copy link
Contributor Author

cowclaw commented Mar 25, 2021

Please note that circleci build is still failing because the fix for #8922 is not (yet) included in the MR. I guess it will pass again after update to current master.

Can you please pull the latest master into your branch to fix it?

done, thanks!

@cowclaw
Copy link
Contributor Author

cowclaw commented Mar 25, 2021

Now circleci failed with a different error. But I don't have an idea why. To me it does not seem related to my changes.

Any help is greatly appreciated @wing328. Thanks in advance!

[SEVERE] built_value_generator:built_value on lib/model/enum_arrays.dart:
An error `FormatterException` occurred while formatting the generated source for
  `package:openapi/model/enum_arrays.dart`
which was output to
  `lib/model/enum_arrays.built_value.g.part`.
This may indicate an issue in the generator, the input source code, or in the
source formatter.
Could not format because the source could not be parsed:

line 43, column 5: Expected an identifier.
   ╷
43 │ '\\$': 'dollar',
   │     ^
   ╵
line 39, column 15: Expected an identifier.
   ╷
39 │ 'dollar': '\\$',
   │               ^
   ╵
[INFO] 15.5s elapsed, 47/63 actions completed.
[INFO] 16.5s elapsed, 78/94 actions completed.
[INFO] 17.5s elapsed, 128/144 actions completed.
[INFO] Running build completed, took 18.0s

[INFO] Caching finalized dependency graph...
[INFO] Caching finalized dependency graph completed, took 113ms

[SEVERE] Failed after 18.2s
[ERROR] Failed to execute goal org.codehaus.mojo:exec-maven-plugin:1.2.1:exec (pub-run-build-runner) on project DartDioPetstoreClientLibFakeTests: Command execution failed. Process exited with an error: 1 (Exit value: 1) -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
[ERROR] 
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR]   mvn <goals> -rf :DartDioPetstoreClientLibFakeTests

@cowclaw cowclaw force-pushed the jaxrs_cxf_client_support_openapi_nullable branch from 2424567 to bd6f316 Compare March 26, 2021 21:15
@cowclaw
Copy link
Contributor Author

cowclaw commented Mar 26, 2021

Again updated to current master. 🤞

@cowclaw cowclaw force-pushed the jaxrs_cxf_client_support_openapi_nullable branch 2 times, most recently from 96ba1a4 to 8a78afd Compare March 27, 2021 21:00
@cowclaw
Copy link
Contributor Author

cowclaw commented Mar 27, 2021

@wing328

The errors that let circleci fail are changing. I still don't see a connection to my changes. But it's always the parallel build #2 that fails.

Resolving dependencies...
Git error. Command: `git rev-list --max-count=1 2d7bb10369be49d3c53a6567cc76e8580b5d133d`
stdout: 
stderr: fatal: bad object 2d7bb10369be49d3c53a6567cc76e8580b5d133d
exit code: 128
[ERROR] Failed to execute goal org.codehaus.mojo:exec-maven-plugin:1.2.1:exec (tests-pub-get) on project DartDioPetstoreClientLibFakeTests: Command execution failed. Process exited with an error: 69 (Exit value: 69) -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
[ERROR] 
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR]   mvn <goals> -rf :DartDioPetstoreClientLibFakeTests
2021-03-27 21:03:12.688:INFO::main: Logging initialized @106ms
2021-03-27 21:03:12.699:INFO:oejr.Runner:main: Runner
2021-03-27 21:03:12.827:INFO:oejs.Server:main: jetty-9.2.9.v20150224
Mar 27, 2021 9:03:16 PM com.sun.jersey.api.core.PackagesResourceConfig init
INFO: Scanning for root resource and provider classes in the packages:
  io.swagger.jaxrs.json
  io.swagger.sample.util
  io.swagger.sample.resource
  io.swagger.jaxrs.listing
Mar 27, 2021 9:03:16 PM com.sun.jersey.api.core.ScanningResourceConfig logClasses
INFO: Root resource classes found:
  class io.swagger.sample.resource.PetResource
  class io.swagger.jaxrs.listing.ApiListingResource
  class io.swagger.sample.resource.PetStoreResource
  class io.swagger.jaxrs.listing.AcceptHeaderApiListingResource
  class io.swagger.sample.resource.UserResource
Mar 27, 2021 9:03:16 PM com.sun.jersey.api.core.ScanningResourceConfig logClasses
INFO: Provider classes found:
  class io.swagger.sample.util.JacksonJsonProvider
  class io.swagger.sample.resource.SampleExceptionMapper
  class io.swagger.jaxrs.listing.SwaggerSerializers
Mar 27, 2021 9:03:16 PM com.sun.jersey.server.impl.application.WebApplicationImpl _initiate
INFO: Initiating Jersey application, version 'Jersey: 1.13 06/29/2012 05:14 PM'
21:03:16,970 |-INFO in ch.qos.logback.classic.LoggerContext[default] - Could NOT find resource [logback.groovy]
21:03:16,970 |-INFO in ch.qos.logback.classic.LoggerContext[default] - Could NOT find resource [logback-test.xml]
21:03:16,970 |-INFO in ch.qos.logback.classic.LoggerContext[default] - Found resource [logback.xml] at [file:/petstore/webapp/WEB-INF/classes/logback.xml]
21:03:17,039 |-INFO in ch.qos.logback.classic.joran.action.ConfigurationAction - debug attribute not set
21:03:17,051 |-INFO in ch.qos.logback.core.joran.action.AppenderAction - About to instantiate appender of type [ch.qos.logback.core.ConsoleAppender]
21:03:17,054 |-INFO in ch.qos.logback.core.joran.action.AppenderAction - Naming appender as [STDOUT]
21:03:17,158 |-WARN in ch.qos.logback.core.ConsoleAppender[STDOUT] - This appender no longer admits a layout as a sub-component, set an encoder instead.
21:03:17,158 |-WARN in ch.qos.logback.core.ConsoleAppender[STDOUT] - To ensure compatibility, wrapping your layout in LayoutWrappingEncoder.
21:03:17,158 |-WARN in ch.qos.logback.core.ConsoleAppender[STDOUT] - See also http://logback.qos.ch/codes.html#layoutInsteadOfEncoder for details
21:03:17,158 |-INFO in ch.qos.logback.classic.joran.action.LoggerAction - Setting level of logger [io.swagger] to INFO
21:03:17,158 |-INFO in ch.qos.logback.classic.joran.action.RootLoggerAction - Setting level of ROOT logger to ERROR
21:03:17,159 |-INFO in ch.qos.logback.core.joran.action.AppenderRefAction - Attaching appender named [STDOUT] to Logger[ROOT]
21:03:17,159 |-INFO in ch.qos.logback.classic.joran.action.ConfigurationAction - End of configuration.
21:03:17,168 |-INFO in ch.qos.logback.classic.joran.JoranConfigurator@750e2b97 - Registering current configuration as safe fallback point

2021-03-27 21:03:17.852:INFO:oejsh.ContextHandler:main: Started o.e.j.w.WebAppContext@5680a178{/,file:/petstore/webapp/,AVAILABLE}{file:/petstore/webapp/}
2021-03-27 21:03:17.856:INFO:oejs.AbstractNCSARequestLog:main: Opened /var/log/2021_03_27-requests.log
2021-03-27 21:03:17.880:INFO:oejs.ServerConnector:main: Started ServerConnector@5cc3e14d{HTTP/1.1}{0.0.0.0:8080}
2021-03-27 21:03:17.880:INFO:oejs.Server:main: Started @5301ms

Exited with code exit status 1

Is there a way to try and reproduce it locally? I tried running ./CI/circle_parallel.sh locally on my machine but to no avail.

What I think is strange is that we had a green build initially.
image

Might it help to close this PR and create a new one? Did you ever experience such behaviour with circleci?

@cowclaw cowclaw force-pushed the jaxrs_cxf_client_support_openapi_nullable branch from 8a78afd to b5ab59c Compare April 20, 2021 08:20
@cowclaw
Copy link
Contributor Author

cowclaw commented Apr 20, 2021

I rebased to current master again. Now all checks were successul, I did not make any further changes. Can you please continue the review @wing328 ?

Big thanks in adavance!

@cowclaw
Copy link
Contributor Author

cowclaw commented May 7, 2021

I rebased to current master again. Now all checks were successul, I did not make any further changes. Can you please continue the review @wing328 ?

Big thanks in adavance!

Hello @wing328,

may I kindly ask for review again?

@wing328 wing328 modified the milestones: 5.1.1, 5.2.0 May 10, 2021
@wing328 wing328 modified the milestones: 5.2.0, 5.2.1 Jul 13, 2021
@cowclaw
Copy link
Contributor Author

cowclaw commented Jul 15, 2021

I rebased to current master again. Now all checks were successul, I did not make any further changes. Can you please continue the review @wing328 ?
Big thanks in adavance!

Hello @wing328,

may I kindly ask for review again?

Hello @wing328, is there anything left for me to do so you can restart the review? Our team would really love to see this integrated in the main branch. :)

@wing328
Copy link
Member

wing328 commented Jul 19, 2021

Tested again and the result is good. Sorry for the delay in reviewing as there are too many PRs for review.

@wing328 wing328 merged commit c42e03e into OpenAPITools:master Jul 19, 2021
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.

2 participants