Skip to content

feat: Add ODataClient for Apache HttpClient5#1169

Open
Jonas-Isr wants to merge 26 commits into
mainfrom
odata-client-hc5
Open

feat: Add ODataClient for Apache HttpClient5#1169
Jonas-Isr wants to merge 26 commits into
mainfrom
odata-client-hc5

Conversation

@Jonas-Isr
Copy link
Copy Markdown
Member

@Jonas-Isr Jonas-Isr commented May 8, 2026

Context

SAP/cloud-sdk-java-backlog#504

This PR adds a new module to the Cloud SDK: datamodel/odata-client-apache-httpclient5. This aims to enable users to use Apaches HttpClient v5 in conjunction with our OData functionalities. I tried to stick as closely to the v4 implementation as possible while respecting the design choices of the new HttpClient v5.

This PR does not consider the high-level OData v2 and v4 modules. This will be handled in the future.

Comments regarding review

The first commit of this branch only features the copying of the v4 version of the module. If you want to see the changes that are made compared to our v4 version, look at the changes between the current state of the branch compared to the state after the first commit.

Currently, nothing is marked beta. It might be reasonable to mark (parts of) the new code as such. I am happy to discuss this in the review.

I am also happy to do pair reviews.

Feature scope:

  • Add new module for HttpClient v5 (by copying over the old v4 module and adapting it to v5)
  • Tests

Design decisions

CSRF Token Handling

HC4: ODataRequestGeneric.tryExecuteWithCsrfToken() issued a HEAD request to the service path before every mutating request, using DefaultCsrfTokenRetriever.

HC5: Moved to CsrfTokenInterceptor, an HttpRequestInterceptor registered in DefaultApacheHttpClient5Factory. It intercepts mutating requests at the HTTP client level before they are sent. Thus, all calls with ODataRequestGeneric.tryExecute() use it. tryExecuteWithCsrfToken() therefor does not exist. (This is a breaking change for users that update to HC5. There is of course also the option to keep the method and simply forward this to tryExecute but I felt this is even more confusing since then it looks like the two methods should do different things. Happy to discuss this.)

Why: HC5's HttpClient interface no longer exposes an easy way to issue a pre-request side-effect at the OData layer. The interceptor mechanism is the idiomatic HC5 way to inject cross-cutting behavior. It also means CSRF handling applies to all HTTP clients built by the factory, not just OData requests. If this is not wanted, we can add a flag to the builder of the DefaultApacheHttpClient5Factory to make this opt-in/opt-out.

Notes:

  • The interceptor sends HEAD to the service root (derived by stripping the last path segment) rather than the full resource URI, matching HC4's behaviour of scoping the token to the service path.
  • Compared to the HC4 version, if no CSRF token header is returned, there is no CsrfTokenRetrievalException thrown. Instead, the problem is only logged (at WARN level). This is a necessary evil since we use HC5's interceptor pattern for CSRF tokens and throwing the exception there is not possible.

HTTP Execution Method

HC4: In httpClient.execute(HttpUriRequest) the client owns the response lifecycle.

HC5: In httpClient.executeOpen(null, httpRequest, null) the caller owns the response lifecycle and is responsible for closing it.

Why: The HC5 execute(request, responseHandler) pattern closes the response after the handler returns. Since the OData layer needs to keep the response open for streaming/deserialization, executeOpen is the only way to do this that I saw.

Connection Timeout Exception

HC4 uses ConnectionPoolTimeoutException from org.apache.http.conn.

HC5: uses ConnectionRequestTimeoutException from org.apache.hc.core5.http.

Why: HC5 renamed and restructured exception types. The exception is caught in ODataHttpRequest to produce a descriptive ODataConnectionException about connection leaks.

Content-Type on JSON Entities

HC4: ContentType.APPLICATION_JSON had no charset and thus getContentType() returned "application/json".

HC5: ContentType.APPLICATION_JSON includes charset=UTF-8 and this getContentType() would return "application/json; charset=UTF-8".

Fix in ODataRequestUpdate: ComparableHttpEntity now uses ContentType.create("application/json") (no charset). ODataRequestCreate sets "application/json" as a literal string. Both avoid the unwanted charset suffix that would appear in batch request bodies.

UriRequestMerger interface

HC4: removeDuplicateQueryParameters was an instance method in ODataRequestResultGeneric that cast httpClient to UriQueryMerger. UriQueryMerger was implemented by HttpClientWrapper and exposed the destination-contributed query parameters (base URL params + URL.queries... properties) via mergeRequestUri(URI.create("")).

HC5: i tried to do it in the same way. I added UriQueryMerger as a public interface in the connectivity-apache-httpclient5 module, implemented by ApacheHttpClient5Wrapper. ODataRequestResultGeneric casts httpClient to UriQueryMerger identically to HC4, strips destination-contributed params that appear verbatim in the next link, and cleans up the resulting query string.

Definition of Done

  • Functionality scope stated & covered
  • Tests cover the scope above
  • Error handling created / updated & covered by the tests above
  • Documentation updated
    • I will consider documentation after the initial code reviews
  • Release notes updated

@Jonas-Isr Jonas-Isr self-assigned this May 8, 2026
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.

2 participants