Skip to content
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

com.sap.cloud.sdk.datamodel.odata omit empty fields on serialisation #742

Open
p3rh2n opened this issue Mar 28, 2022 · 6 comments
Open

com.sap.cloud.sdk.datamodel.odata omit empty fields on serialisation #742

p3rh2n opened this issue Mar 28, 2022 · 6 comments
Labels
enhancement New feature or request feature request Java Related to documentation on the SAP Cloud SDK for Java waiting for release

Comments

@p3rh2n
Copy link

p3rh2n commented Mar 28, 2022

Hi all!

In the context of an OData update request we run into an error response from the SAP service called. We are using com.sap.cloud.sdk.datamodel:odata-core in version 3.65.0. The Method in Question is com.sap.cloud.sdk.datamodel.odata.helper.FluentHelperUpdate.executeRequest().

When serializing in REST calls with data from com.sap.cloud.sdk.datamodel.odata, empty fields are also rendered into the output:

{
    "insuranceId" : "ABC123",
    "terminationDate" : null,
    "terminationReason" : "UNKNOWN",
    "terminationWish" : null,
    "terminationReceived" : "/Date(1648027328522)/",
    "force" : false
}

This fails on the SAP side with HttpResult 400 - bad request, because these fields are either not allowed to be empty or not allowed to be included at all.
We are now looking for a way / a setting to prevent this, so that something like

{
    "insuranceId" : "ABC123",
    "terminationReason" : "UNKNOWN",
    "terminationReceived" : "/Date(1648027328522)/",
    "force" : false
}

is rendered out.

As a workaround, we are using com.sap.cloud.sdk.datamodel.odata.helper.FluentHelperUpdate.excludingFields() right now as follows.

final var emptyEntityFields = new ArrayList<EntityField>();
            if(cancellation.getTerminationDate() == null) emptyEntityFields.add(new EntityField("terminationDate"));
            if(cancellation.getTerminationWish() == null) emptyEntityFields.add(new EntityField("terminationWish"));
            this.dmeEpaService.updateTerminationRequest(terminationRequest)
                    .replacingEntity()
                    .excludingFields(emptyEntityFields.toArray(EntityField[]::new))
                    .executeRequest(this.httpDestination);

This workaround is cumbersome and needed on every request affected. As an example, in the jackson environment there is the setting @JsonInclude(JsonInclude.Include.NON_NULL) to accomplish this for every entity class annotated that way. See com.fasterxml.jackson.annotation
Enum JsonInclude.Include.NON_NULL
for details an an eample.

Thanks in advance and best regards

Peter Hahn

@p3rh2n p3rh2n added the Java Related to documentation on the SAP Cloud SDK for Java label Mar 28, 2022
@Johannes-Schneider
Copy link
Contributor

Hi @p3rh2n,

thanks for reaching out to us!

In your code, I can see the following line:

.replacingEntity()

As per our JavaDoc, this method does the following:

Allows to control that the request to update the entity is sent with the HTTP method PUT and its payload contains all fields of the entity, regardless which of them have been changed.

In other words, you are not actually updating an existing entity (where only the actually changed fields are of interest) but rather replacing it.
As the documentation says, such a request is expected to contain all fields of an entity, regardless of whether they have been changed or whether they are empty.

Can you please try simply removing the .replacingEntity() call in your request builder?
This should result in a HTTP PATCH request (instead of using HTTP PUT) where the payload should consist of only those fields that have been changed.


Nevertheless, it is still unexpected to see a server error when sending such a request.
May I ask how you retrieved the entity in the first place? Are you simply running a HTTP GET request against your service, modify the retrieved entity and then trying to send the update?

Best regards,
Johannes

@p3rh2n
Copy link
Author

p3rh2n commented Mar 28, 2022

Hi @Johannes-Schneider ,

thanks for the quick answer!

We have to call POST here, since PUT is not specified in the interface.

However, I think that regardless of whether I want to create (POST) or refresh (PUT) an object, it should be possible to not output unset fields (@nullable == null) at all. This also complies with the REST best pracices.

For this API, I feel a method like excludeNull() in this given fluent API would be more appropriate than the Jackson-kind @annotations I originally proposed.

Maybe an additional optional argument makes sense to control behavior like isNull(), isEmpty(), isBlank() and the like.

Best regards
Peter

@Johannes-Schneider
Copy link
Contributor

Hi @p3rh2n,

thanks for the clarification.

As you have noticed, our (typed) OData clients currently lack such a functionality.
However, we can definitely see the benefit of offering some additional API to support use cases like yours, so we are considering this issue as a new feature request.


In addition, we would like to further understand the details as of why you are seeing 400 HTTP responses from the service with the payload you outlined above.
It looks like such a payload should be perfectly fine (even though a bit too verbose), so we wouldn't expect the service to complain.

Would you be able to point us to the service you are consuming? (i.e. is this a S/4 Cloud/OnPremise service?)

Best regards,
Johannes

@p3rh2n
Copy link
Author

p3rh2n commented Mar 30, 2022

Hi @Johannes-Schneider ,

unfortunately I'm not really savvy on the SAP side. What I understood is that the interface can be configured on the SAP side to ignore the empty fields. However, we did not want to change this on the SAP side, as this was triggered by a code change on the side of our microservice. Often, however, such a change is not easily possible, since the interfaces are specified by separate organizational units.

We felt it made sense from a RESTful architecture style perspective to not transmit empty fields (and mutely ignore unknown ones), too.

For deeper details on the SAP side, I could reach out to colleagues there.

Best regards
Peter

@artemkovalyov
Copy link
Member

artemkovalyov commented Apr 4, 2022

Hey @p3rh2n,

We've added this feature request to the backlog. Do you have any deadlines to have this or workaround is OK for now?
Is this the service you're using dmeEpaService?

We'll keep the issue open to inform you when the feature is released or ask for additional clarifications while implementing it.

On a side note, we'd be interested to hear your opinion about the SDK and projects you're using it for.
Would you be open to a feedback call? If yes, let me know can I approach you to set it up?

@DirtyAssign
Copy link

Hi @artemkovalyov,

I am a colleague of @p3rh2n.
Our workaround works, which has @p3rh2n implemented.

We are using your cloud SDK a lot. A good reason was to prevent CVE's >= 7 . The SDK is up-to-date and we don't need to program OData Request by hand.

Do you have a timeline for us?

Best Regards from cologne!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feature request Java Related to documentation on the SAP Cloud SDK for Java waiting for release
Projects
None yet
Development

No branches or pull requests

4 participants