Skip to content

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.

Sign up for GitHub

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

Jump to bottom
Open
rpanackal wants to merge 26 commits into
base: main
Choose a base branch
from
Open

feat: [OpenAPI] Make Spring Optional with Apache HttpClient #1046

rpanackal wants to merge 26 commits into from

Conversation

@rpanackal
Copy link
Member

@rpanackal rpanackal commented Dec 29, 2025 •
edited
Loading

Context

SAP/cloud-sdk-java-backlog#464.

There are two objectives to this PR

  1. Make Spring dependencies optional by introducing Apache HttpClient reliance.
  2. Serve as base/reference branch to compare all upcoming changes/PRs. This PR contains, all required files and api to support apache-httpclient template 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:

  • New package com.sap.cloud.sdk.services.openapi.apache with ApiClient, BaseApi and other supporting classes added
  • New openapi-api-apache-sample module as e2e test with sample code generation with apache/ApiClient

Edit: Merged features

Definition of Done

  • Functionality scope stated & covered
  • Tests cover the scope above
  • Error handling created / updated & covered by the tests above
  • Documentation updated
  • Release notes updated

@rpanackal rpanackal force-pushed the feat/openapi/optional-spring-base branch from f47b1db to cde369f Compare December 30, 2025 09:26
newtork
newtork reviewed Dec 30, 2025
View reviewed changes
...he-sample/src/main/java/com/sap/cloud/sdk/datamodel/openapi/apache/sample/api/OrdersApi.java Outdated
* @throws ApiException
* if fails to make API call
*/
public Order ordersPost( @javax.annotation.Nonnull Order order )
Copy link
Contributor

@newtork newtork Dec 30, 2025

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.

Copy link
Member Author

@rpanackal rpanackal Jan 12, 2026

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

newtork
newtork reviewed Dec 30, 2025
View reviewed changes
...che-sample/src/main/java/com/sap/cloud/sdk/datamodel/openapi/apache/sample/api/SodasApi.java Outdated
* @throws ApiException
* if fails to make API call
*/
public File sodasDownloadIdGet( @javax.annotation.Nonnull Long id )
Copy link
Contributor

@newtork newtork Dec 30, 2025

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?

Copy link
Contributor

@newtork newtork Dec 30, 2025

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 (?)

Copy link
Member Author

@rpanackal rpanackal Jan 12, 2026 •
edited
Loading

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.

Copy link
Member Author

@rpanackal rpanackal Jan 12, 2026 •
edited
Loading

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 byte[]

@newtork newtork added the do not merge Pull request must not be merged label Dec 30, 2025
@newtork newtork mentioned this pull request Dec 30, 2025
Merged
@sap-email-compliance
Copy link

sap-email-compliance bot commented Jan 8, 2026

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 ospo@sap.com.

@rpanackal rpanackal self-assigned this Jan 8, 2026
@rpanackal rpanackal removed the do not merge Pull request must not be merged label Jan 12, 2026
@rpanackal rpanackal added the please review Request to review a pull request label Jan 12, 2026
@rpanackal rpanackal removed the please review Request to review a pull request label Jan 12, 2026
@rpanackal rpanackal added the please review Request to review a pull request label Jan 12, 2026
CharlesDuboisSAP
CharlesDuboisSAP reviewed Jan 13, 2026
View reviewed changes
Copy link
Contributor

@CharlesDuboisSAP CharlesDuboisSAP left a comment

Choose a reason for hiding this comment

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

Could we see it in AI SDK?

...che-sample/src/main/java/com/sap/cloud/sdk/datamodel/openapi/apache/sample/api/SodasApi.java
return apiClient
.invokeAPI(
localVarPath,
"GET",
Copy link
Contributor

@CharlesDuboisSAP CharlesDuboisSAP Jan 13, 2026

Choose a reason for hiding this comment

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

Why not use Apache's Method enum?

Copy link
Member Author

@rpanackal rpanackal Jan 13, 2026

Choose a reason for hiding this comment

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

I attempted this just now. This adds the following dependency for our users

<dependency>
      <groupId>org.apache.httpcomponents.core5</groupId>
      <artifactId>httpcore5</artifactId>
  </dependency>

Do you think its still worth it? The httpMethod string is set by the openapi generator and I think should be safe to accept a string here.

Copy link
Member Author

@rpanackal rpanackal Jan 13, 2026

Choose a reason for hiding this comment

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

I made changes to make use of Apache's Method enum within internal code. But not in public api signature to avoid dependency declaration for the users.

.../openapi/openapi-core/src/main/java/com/sap/cloud/sdk/services/openapi/apache/ApiClient.java
* @return A new ApiClient instance
*/
@Nonnull
public static ApiClient fromHttpClient( @Nonnull final CloseableHttpClient httpClient )
Copy link
Contributor

@CharlesDuboisSAP CharlesDuboisSAP Jan 13, 2026

Choose a reason for hiding this comment

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

Why public?

Copy link
Member Author

@rpanackal rpanackal Jan 13, 2026

Choose a reason for hiding this comment

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

In Spring version, users could create an apiclient/ApiClient instance with RestTemplate object. This was used to define workarounds on the client.

The fromHttpClient serves similar purpose for Apache client.

...model/openapi/openapi-core/src/main/java/com/sap/cloud/sdk/services/openapi/apache/Pair.java
* Represents a name-value pair used for query parameters and form data.
*/
@Value
public class Pair
Copy link
Contributor

@CharlesDuboisSAP CharlesDuboisSAP Jan 13, 2026

Choose a reason for hiding this comment

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

Did you think about using apache Pair?

Copy link
Member Author

@rpanackal rpanackal Jan 13, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, I simply preferred to not add another Apache class to our public API. (No hard opinion)

Our custom Pair class is quite simple and probably doesnt need maintenance. Also, we would require users to declare yet another dependency. Let me know if you still prefer to use the apache class.

@rpanackal
Copy link
Member Author

rpanackal commented Jan 13, 2026 •
edited
Loading

Could we see it in AI SDK?

@CharlesDuboisSAP You can checkout branch

rpanackal and others added 5 commits January 13, 2026 17:01
@rpanackal @CharlesDuboisSAP
Update datamodel/openapi/openapi-generator/src/main/resources/openapi…
4835a34 
…-generator/mustache-templates/libraries/apache-httpclient/api.mustache

Co-authored-by: Charles Dubois <103174266+CharlesDuboisSAP@users.noreply.github.com>
@rpanackal
Review: Apache Method enum and ctor formatting
de76c72
@rpanackal
Merge remote-tracking branch 'origin/feat/openapi/optional-spring-bas…
067ddc8 
…e' into feat/openapi/optional-spring-base
@rpanackal @CharlesDuboisSAP
Update datamodel/openapi/openapi-core/src/main/java/com/sap/cloud/sdk…
de01665 
…/services/openapi/apache/ApiClient.java

Co-authored-by: Charles Dubois <103174266+CharlesDuboisSAP@users.noreply.github.com>
@rpanackal
Fix compliance issue and formatting
b7d623e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@newtork newtork newtork left review comments

@CharlesDuboisSAP CharlesDuboisSAP CharlesDuboisSAP left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@rpanackal rpanackal

Labels

please review Request to review a pull request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@rpanackal @newtork @CharlesDuboisSAP