openapi-generator icon indicating copy to clipboard operation
openapi-generator copied to clipboard

Remove @InputFile, use a simple @Input #13726

Open hdani9307 opened this issue 2 years ago • 3 comments

PR checklist

  • [x] Read the contribution guidelines.
  • [x] 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.
  • [x] 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.
  • [x] File the PR against the correct branch: master (6.1.0) (minor release - breaking changes with fallbacks), 7.0.x (breaking changes without fallbacks)
  • [ ] If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

hdani9307 avatar Oct 18 '22 10:10 hdani9307

👋 @hdani9307 could you briefly explain why you would like to make this change? What issue are you experiencing that you are trying to address with these changes?

erichaagdev avatar Oct 18 '22 13:10 erichaagdev

Hi @erichaagdev

I wanted to generate the API from an URL like https://raw.githubusercontent.com/OpenAPITools/openapi-generator/master/modules/openapi-generator-gradle-plugin/src/test/resources/specs/petstore-v3.0.yaml in my project, but because of the @InputFile annotation in the GenerateTask, it is only accepting file urls, however the http url inputspec is supported by the generator.

hdani9307 avatar Oct 18 '22 13:10 hdani9307

@hdani9307 thanks for the explanation. Annotating file inputs with @InputFile is required to properly support incremental build functionality (e.g. UP-TO-DATE). You can read the Incremental Build section of the Gradle documentation to learn more.

Introducing these changes as is would cause a regression for functionality that was previously added by #4492, #6472, and #6716.

With that being said, you may still be able to implement the functionality you want without causing a regression or impacting backwards compatibility. What I recommend is to leave inputSpec as is and instead add a new property (e.g. remoteInputSpec) which can be annotated with @Input and used only if inputSpec is not defined.

Since Gradle has no way to know if the contents of the remote spec have changed, you would need to conditionally disable caching if the new property is in-use. Proper incremental building would not be supported, but to generate a new set of classes caused by changes to the remote specification you can always run a clean.

Another option is to conditionally register an additional non-cacheable/non-incremental task which would first download the remote specification as a pre-requisite step to the generate task. This would allow the generate task to still fully support build caching and incremental building.

erichaagdev avatar Oct 18 '22 16:10 erichaagdev

@erichaagdev If I create a new remoteInputSpec property, I will have to make the inputSpec parameter optional. Currently it is a required property. Would it cause any regression issue?

hdani9307 avatar Oct 20 '22 13:10 hdani9307

@hdani9307 I don't believe marking a previously required property with @Optional would break any backwards compatibility. Removing the @Optional annotation from a property and making it required would, but that's not what you would be doing.

I'd also like to propose an alternative solution - one which only requires you to modify your own build scripts. You could leverage the gradle-download-task plugin and add a task downloading the remote OpenAPI specification. You would then use the location of the downloaded spec file as the value for inputSpec.

Here is a sample showing how that could be done: https://github.com/erichaagdev/gradle-samples/blob/main/remote-openapi-spec/build.gradle.kts

erichaagdev avatar Oct 20 '22 17:10 erichaagdev

@erichaagdev I have added the new remoteInputSpec property.

Thank you for your alternative suggestion, we are already using the same plugin as a workaround. It works well, I just wanted to fix this bug in the lib.

hdani9307 avatar Oct 27 '22 09:10 hdani9307

Could you also update the configuration section of the README to mention the new property?

erichaagdev avatar Oct 29 '22 17:10 erichaagdev

@hdani9307 thanks for the PR.

@erichaagdev thanks for reviewing the change.

wing328 avatar Nov 07 '22 15:11 wing328