-
Notifications
You must be signed in to change notification settings - Fork 23
feat: [OpenAPI] Make Spring Optional with Apache HttpClient #1046
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
base: main
Are you sure you want to change the base?
Conversation
f47b1db to
cde369f
Compare
...he-sample/src/main/java/com/sap/cloud/sdk/datamodel/openapi/apache/sample/api/OrdersApi.java
Show resolved
Hide resolved
| * @throws ApiException | ||
| * if fails to make API call | ||
| */ | ||
| public Order ordersPost( @javax.annotation.Nonnull Order order ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Question/Optional)
It may be worth our time to check for alternatives to javax.annotation.Nonnull null-indicator annotations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Surely Jspecify is an option. But I guess that would merit a larger refactoring throughout the sdk
...he-sample/src/main/java/com/sap/cloud/sdk/datamodel/openapi/apache/sample/api/OrdersApi.java
Outdated
Show resolved
Hide resolved
...che-sample/src/main/java/com/sap/cloud/sdk/datamodel/openapi/apache/sample/api/SodasApi.java
Outdated
Show resolved
Hide resolved
| * @throws ApiException | ||
| * if fails to make API call | ||
| */ | ||
| public File sodasDownloadIdGet( @javax.annotation.Nonnull Long id ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Question/Minor)
For better comparison, would you consider offering your byte[] fix here too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Additionally)
The method is not returning @Nonnull annotation on result (?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Question/Minor)
For better comparison, would you consider offering your byte[] fix here too?
I have not included it yet. The reason being the fix for Spring RestTemplate doesnt extend well for Apache HttpClient. Spring relies on <useAbstractForFiles>true</useAbstractForFiles>, spring only feature. We would have to create a feature toggle and invest some more effort here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevermind. I found out in apache templates, setting
<typeMappings>
<typeMapping>File=byte[]</typeMapping>
</typeMappings>
already works, affecting both method parameter type and return type. This is unlike Spring resttemplate, where only the return type was influenced. We had to accept org.springframework.core.io.Resource on method parameter eg:PromptTemplatesApi.importPromptTemplate, by enabling useAbstractForFiles.
To summarize, we already support bye[]
...che-sample/src/main/java/com/sap/cloud/sdk/datamodel/openapi/apache/sample/api/SodasApi.java
Show resolved
Hide resolved
...ache-sample/src/main/java/com/sap/cloud/sdk/datamodel/openapi/apache/sample/model/Fanta.java
Show resolved
Hide resolved
...el/openapi/openapi-core/src/main/java/com/sap/cloud/sdk/services/openapi/apache/BaseApi.java
Outdated
Show resolved
Hide resolved
|
SAP employees are expected to use their SAP-email address for commits related to their work. Our compliance check has detected usage of an email other than a SAP one by a SAP employee. Please update your pull request accordingly. If you think this is wrong or need any assistance, please contact [email protected]. |
Context
SAP/cloud-sdk-java-backlog#464.
There are two objectives to this PR
apache-httpclienttemplate based code generation. We will (soon) remove a large chunk of introduced files, reduce public API and even refactor code.To maintain backward compatibility Spring framework dependencies are marked optional but template files, or any public API are still maintained without any changes.
Feature scope:
com.sap.cloud.sdk.services.openapi.apachewithApiClient,BaseApiand other supporting classes addedopenapi-api-apache-samplemodule as e2e test with sample code generation with apache/ApiClientEdit: Merged features
Definition of Done
Error handling created / updated & covered by the tests aboveDocumentation updatedRelease notes updated