Skip to content

Conversation

@rpanackal
Copy link
Member

Context

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

Feature scope:

  • Introduce a ResponseMetadataListener that accepts OpenApiResponse object which includes the metadata
  • Make case insensitive key match for headers using TreeMap

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

- models only
- formatting
- add api classes
- no more shared client in *Api.java instances
- *Api.invoke method bugged and unused
- Remove debugging, connectionTimeout, defaultHeaderMap, defaultCookieMap
- add TODOs
- public setBasePath
- Remove addDefaultHeader, setUserAgent,
- Make as manny methods static
- Remove deprecated getStatusCode and getResponseHeaders
- public parameterToPairs
- Make as many methods static
- rename ApiClientResponseHandler to DefaultApiResponseHandler
- finish up ApiClient todos
rpanackal and others added 18 commits January 13, 2026 11:07
…-generator/mustache-templates/libraries/apache-httpclient/api.mustache

Co-authored-by: Charles Dubois <103174266+CharlesDuboisSAP@users.noreply.github.com>
…/services/openapi/apache/ApiClient.java

Co-authored-by: Charles Dubois <103174266+CharlesDuboisSAP@users.noreply.github.com>
…st-apache-client-templates

# Conflicts:
#	datamodel/openapi/openapi-generator/src/main/resources/openapi-generator/mustache-templates/libraries/apache-httpclient/api.mustache
…st-apache-client-templates

# Conflicts:
#	datamodel/openapi/openapi-generator/src/main/resources/openapi-generator/mustache-templates/libraries/apache-httpclient/api.mustache
…#1062)

Moving to single larger PR (sorry), because its getting difficult to sync changes across. Both PRs have been thoroughly review at this point.
…sponse-status-and-header

# Conflicts:
#	datamodel/openapi/openapi-generator/src/main/resources/openapi-generator/mustache-templates/libraries/apache-httpclient/operationBody.mustache
#	datamodel/openapi/openapi-generator/src/test/resources/DataModelGeneratorApacheIntegrationTest/api-class-for-ai-sdk/output/com/sap/cloud/sdk/services/builder/api/AwesomeSodaApi.java
#	datamodel/openapi/openapi-generator/src/test/resources/DataModelGeneratorApacheIntegrationTest/api-class-for-ai-sdk/output/com/sap/cloud/sdk/services/builder/api/AwesomeSodasApi.java
#	datamodel/openapi/openapi-generator/src/test/resources/DataModelGeneratorApacheIntegrationTest/api-class-for-ai-sdk/output/com/sap/cloud/sdk/services/builder/api/DefaultApi.java
#	datamodel/openapi/openapi-generator/src/test/resources/DataModelGeneratorApacheIntegrationTest/api-class-vendor-extension-json/output/com/sap/cloud/sdk/services/apiclassvendorextension/api/AwesomeSodaApi.java
#	datamodel/openapi/openapi-generator/src/test/resources/DataModelGeneratorApacheIntegrationTest/api-class-vendor-extension-json/output/com/sap/cloud/sdk/services/apiclassvendorextension/api/AwesomeSodasApi.java
#	datamodel/openapi/openapi-generator/src/test/resources/DataModelGeneratorApacheIntegrationTest/api-class-vendor-extension-json/output/com/sap/cloud/sdk/services/apiclassvendorextension/api/DefaultApi.java
#	datamodel/openapi/openapi-generator/src/test/resources/DataModelGeneratorApacheIntegrationTest/api-class-vendor-extension-yaml/output/com/sap/cloud/sdk/services/apiclassvendorextension/api/DefaultApi.java
#	datamodel/openapi/openapi-generator/src/test/resources/DataModelGeneratorApacheIntegrationTest/inlineobject-schemas-enabled/output/com/sap/cloud/sdk/services/inlineobject/api/DefaultApi.java
#	datamodel/openapi/openapi-generator/src/test/resources/DataModelGeneratorApacheIntegrationTest/partial-generation/output/com/sap/cloud/sdk/services/builder/api/DefaultApi.java
#	datamodel/openapi/openapi-generator/src/test/resources/DataModelGeneratorApacheIntegrationTest/remove-operation-id-prefix/output/com/sap/cloud/sdk/services/builder/api/OrdersApi.java
#	datamodel/openapi/openapi-generator/src/test/resources/DataModelGeneratorApacheIntegrationTest/remove-operation-id-prefix/output/com/sap/cloud/sdk/services/builder/api/SodasApi.java
# Conflicts:
#	datamodel/openapi/openapi-core/src/main/java/com/sap/cloud/sdk/services/openapi/apache/ApiClient.java
#	datamodel/openapi/openapi-core/src/main/java/com/sap/cloud/sdk/services/openapi/apache/DefaultApiResponseHandler.java
#	datamodel/openapi/openapi-core/src/main/java/com/sap/cloud/sdk/services/openapi/apache/OpenApiResponse.java
OpenApiResponse( final int statusCode, @Nonnull final Map<String, List<String>> headers )
{
this.statusCode = statusCode;
this.headers = Map.copyOf(headers);
Copy link
Member Author

Choose a reason for hiding this comment

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

Map.copy() doesn't return a TreeMap and therefore removes the nice case insensitive matching of headers. Instead I opted to make the treeMap unmodifiable at creation site with Collections.unmodifiableMap()

@FunctionalInterface
public interface ResponseMetadataListener
{
void onResponse( OpenApiResponse response );
Copy link
Member Author

@rpanackal rpanackal Jan 16, 2026

Choose a reason for hiding this comment

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

I could create a new class OpenApiResponseMetadata or similar instead of re-using or maybe even abusing OpenApiResponse class. What do you think?

@sap-email-compliance
Copy link

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.

* @since 5.25.0
*/
@FunctionalInterface
public interface ResponseMetadataListener
Copy link
Contributor

Choose a reason for hiding this comment

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

(Minor/Nitpick)

ResponseMetadataListener isn't very easy name.
What would you think of OpenApiResponseListener?

newtork
newtork previously approved these changes Jan 16, 2026
Copy link
Contributor

@newtork newtork left a comment

Choose a reason for hiding this comment

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

Apart from minor question regarding naming: looks good!


@With
private final ResponseMetadataListener responseMetadataListener;
private final OpenApiResponseListener responseMetadataListener;
Copy link
Member

Choose a reason for hiding this comment

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

(Very Minor/Question)

Would it make sense to also rename the variable to openApiResponseListener to make it more consistent? (Same in DefaultApiResponseHandler.)

Copy link
Member

@Jonas-Isr Jonas-Isr left a comment

Choose a reason for hiding this comment

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

LGTM


@With
private final ResponseMetadataListener responseMetadataListener;
private final OpenApiResponseListener openApiResponseListener;
Copy link
Contributor

@newtork newtork Jan 20, 2026

Choose a reason for hiding this comment

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

(Minor)

Could we add the @Beta annotation here?

While I'm okay with current state, I feel like having options in the future would be nice:

  • rename it, e.g. to withResponseListener
  • extend interface API, e.g. with HttpResponse reference, settings for error handling

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.

4 participants