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

PR for openapi-generator-maven-plugin inputSpec -- Allow jar: URLs #18576

Merged
merged 10 commits into from
May 21, 2024

Conversation

parenko
Copy link
Contributor

@parenko parenko commented May 6, 2024

This is a PR in response to #10016. It changes <inputSpec/> processing to:

  1. Search a classpath of Maven compilation dependencies for resources.
  2. Allow URLs of the form jar:jar-file-specific-url!/spec.yaml

In the case that a compilation classpath resource is specified, the resource URL is passed to the swagger-parser OpenAPIV3Parser instead of the <inputSpec/> string since the OpenAPIV3Parser does not have access to the Maven compilation classpath. This requires swagger-api/swagger-parser#1592 and swagger-api/swagger-parser#1593.

modules/openapi-generator-maven-plugin/src/test/java/org/openapitools/codegen/plugin/CodeGenMojoTest.java has been expanded to include tests for resource and URL input specs.

(This is a copy of PR #10037. I've rebased original branch on master, fixed merge conflicts and removed swagger-parser 2.0.28-SNAPSHOT since 2.1.19 is used now)

CC @wing328, @jimschubert

@parenko parenko requested a review from jimschubert as a code owner May 6, 2024 05:06
@parenko parenko force-pushed the fix-10016 branch 4 times, most recently from bf30b60 to 0d43b21 Compare May 6, 2024 21:34
@wing328
Copy link
Member

wing328 commented May 8, 2024

@parenko
Copy link
Contributor Author

parenko commented May 8, 2024

can you please also add a test or 2 in https://github.com/OpenAPITools/openapi-generator/blob/master/.github/workflows/maven-plugin-tests.yaml#L44 ?

Will do so. I have to fix other tests, also. I have so issues to run the build locally.

@parenko parenko marked this pull request as draft May 12, 2024 09:43
@parenko parenko force-pushed the fix-10016 branch 3 times, most recently from 81144af to 3ee767d Compare May 18, 2024 22:13
allen-ball and others added 6 commits May 19, 2024 23:36
E.g., jar:jar-specific-uri!/spec.yml.
specifications:

* URLs of the form jar:jar-specific-uri!/spec.yaml

* Resources on the compilation classpath

in addition to the existing FILE test case.
else it is a remote URL && url is not empty
@parenko parenko force-pushed the fix-10016 branch 2 times, most recently from 69c1d77 to 904c663 Compare May 19, 2024 21:49
@parenko parenko marked this pull request as ready for review May 19, 2024 22:07
@parenko parenko marked this pull request as draft May 19, 2024 22:08
@parenko parenko marked this pull request as ready for review May 19, 2024 22:45
@parenko
Copy link
Contributor Author

parenko commented May 19, 2024

@wing328 instead of adding another pom to https://github.com/OpenAPITools/openapi-generator/blob/master/.github/workflows/maven-plugin-tests.yaml#L44 , I've extended https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator-maven-plugin/examples/java-client.xml with another execution, pointing to a jar as inputSpec.

There are also tests in CodeGenMojoTest.java. For inputSpec as classpath, url or file there are other examples and tests, thus I did not additional examples.

<dependency>
<groupId>org.apache.maven</groupId>
<artifactId>maven-resolver-provider</artifactId>
<version>3.9.4</version>
Copy link
Member

Choose a reason for hiding this comment

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

can w use 3.9.6 instead which is the latest stable 3.x version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've aligned it with other maven dependencies. Should I update other references to 3.9.6, also? Or just use a property to reference same maven version and update in another PR?

https://github.com/apache/maven/blob/master/pom.xml#L308

Copy link
Contributor Author

@parenko parenko May 20, 2024

Choose a reason for hiding this comment

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

Updated maven dependencies to 3.9.6 via same property. Now it is clearer that they belong together updated.

@wing328 wing328 merged commit 9a35914 into OpenAPITools:master May 21, 2024
16 checks passed
@wing328
Copy link
Member

wing328 commented May 21, 2024

@parenko thanks for the PR, which has been merged into the master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants