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

Non-changing update of an entity value of type BigDecimal leads to: itemToUpdate.getChangedFields().size() >0 #704

Open
itmstr01 opened this issue Jan 28, 2025 · 6 comments
Labels
bug Something isn't working

Comments

@itmstr01
Copy link

Describe the Bug

I read an Item-Object of a Bom. The value of componentQuantity is "1" as a BigDecimal.
I updated the value of componentQuantity with "1.0", wich should be mathematically the same!
But now the expression itemToUpdate.getChangedFields().size() >0 is true!
The reason is this behavior: new BigDecimal("1.0").equals(new BigDecimal("1")) == false
Sadly the VdmObject.getChangedFields() uses the equals-method for delta-detection.
With BigDecimals you have to use "compareTo() == 0" for equality-testing.
This behavior can lead to performance issues and huge network traffic.

Steps to Reproduce

See bug description

Expected Behavior

itemToUpdate.getChangedFields().size() == 0

Screenshots

No response

Used Versions

  • Java and Maven version via mvn --version:
Apache Maven 3.9.9 (8e8579a9e76f7d015ee5ec7bfcdc97d260186937)
Maven home: C:\Program Files\JetBrains\IntelliJ IDEA 2024.1.4\plugins\maven\lib\maven3
Java version: 17.0.13, vendor: Amazon.com Inc., runtime: C:\Java\CorrettoJdk17.0.13_11
Default locale: de_DE, platform encoding: UTF-8
OS name: "windows 10", version: "10.0", arch: "amd64", family: "windows"
  • SAP Cloud SDK version: 5.15.0

  • Spring Boot or CAP version: no

  • Dependency tree via `mvn dependency:tree`
    not needed because of obvious behavior.
    

Code Examples

not needed because of obvious behavior.

Stack Trace

No response

Log File

Log file ...

Affected Development Phase

Release

Impact

Impaired

Timeline

Needed end of Feb.

@itmstr01 itmstr01 added the bug Something isn't working label Jan 28, 2025
@Jonas-Isr
Copy link
Member

Hi @itmstr01,

what is your use case for this? Can't you do the check/conversion before the update?

Best,
Jonas

@itmstr01
Copy link
Author

itmstr01 commented Jan 29, 2025

First of all: Can you guarantee that the BigDecimal value within the entity always contains no decimal places if it contains an integer value? (and no extra zeros on fractional values) That would be the prerequisite for solving the problem on our side.

We get the data via XML. The XML is merged into the entity for updating. The merge process is configurable via a mapping definition. To solve the problem on our side, I have to change the merge process. This is not an easy thing to do in our case.

We cannot control the format inside the XML, so one can give 6E2 instead of 600. And that is not equal!

@Jonas-Isr
Copy link
Member

I am looking into this and will let you know as soon as I have more information.

@Jonas-Isr
Copy link
Member

Hi @itmstr01,

our SDK is aligned with the OData standard about EDM.Decimal. The OData Documentation states under 11.4.2.1:

Responses for the other primitive types follow the rules booleanValue, byteValue, dateValue, dateTimeOffsetValue, decimalValue, doubleValue, durationValue, enumValue, guidValue, int16Value, int32Value, int64Value, sbyteValue, singleValue, and timeOfDayValue in [OData-ABNF].

In the OData-ABNF it says under 7. Literal Data Values:
decimalValue = [ SIGN ] 1*DIGIT [ "." 1*DIGIT ] [ "e" [ SIGN ] 1*DIGIT ] / nanInfinity.

Thus, in order to stay aligned with the OData standard, we do not treat different decimal values (that adhere to the above ABNF) as if they were other values, even if those other values mathematically are equivalent. This means, unfortunately, that we cannot change the behavior of the SDK as you suggested.

Nevertheless, we don't change the decimal value internally in the SDK (in serialisation or else). So what the server sends out is propagated through the SDK without changing the format. This should make it possible for you to solve the issue on your side. Regarding implementation, one way might be to overwrite VdmObject.getChangedFields(), for example by using VdmObject.toMapOfFields() to get all fields of the object and compare these before and after the update.

I hope this helps you!

Best Regards,
Jonas

@itmstr01
Copy link
Author

How can I overwrite methods in VdmObject without changing the generated Entities by hand?
Is it possible to adapt the generator-template somehow?

@Jonas-Isr
Copy link
Member

I didn't mean that you can/should change the generated entities themselves. What i ment was creating a subclass that extends the generated class. In that subclass you then can for example overwrite getChangedFields().

Let's assume you have the generated class Product where the problem occurs. You could then create a class MyProduct as such:

static class MyProduct extends Product {    
  @Nonnull  
  @Override    
  public Map<String, Object> getChangedFields() {    
    var changedFields =  super.getChangedFields();
    // your logic here
    return actuallyChangedFields;
  }
}

To use this new class, you would, however, probably need to manually transfer the fields from each instance of Product to a new instance of MyProduct via getters and setter. Depending on how complex Product is and how many different generated classes making problems like Product you have, this might unfortunately be a bit of manual work. (I do not know, of course, whether you can leverage your merging step in some way to make this transfer easier.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants