johanhaleby / occurrent

Unintrusive Event Sourcing Library for the JVM
https://occurrent.org
120 stars 16 forks source link

SpringMongoEventStore: Avoid converting BigDecimal to Double in cloudEvent #120

Closed AShynkevich closed 1 year ago

AShynkevich commented 2 years ago

Occurent version: 0.13.1 & 0.14.6

There is an issue where the entity contains any BigDecimal type with a big number.

For instance, we've got an event with BigDecimal value

data class EventEntity(val id: String, val value: BigDecimal...)  

before saving the event the library attempts to convert cloudEvent to Document: image

after this the converted document has ugly Double value with lost precision and this data is written to the Event storage.

E.g. I've sent the BigDecimal number 123456789012345.62 converting I've got 1.2345678901234562E14. Format with lost precisions demands additional efforts to identify and restore the number.

The cause is the class org.occurrent.eventstore.mongodb.cloudevent.DocumentCloudEventWriter with static method:

public static Document toDocument(CloudEvent event) {
        DocumentCloudEventWriter writer = new DocumentCloudEventWriter(new Document());
        return CloudEventUtils.toReader(event).read(writer);
}

where creates cloud event writer with default Document object with default parsing rules.

Is it possible to provide the ability to admit Bigdecimal during converting or make converting cloud events more flexible?

johanhaleby commented 2 years ago

Gah, great find, thanks for reporting. It would really help if you could provide a short test case (or code snippet) that replicates the error. Hopefully I'll find some time to think about, and maybe fix, the issue on friday :)

AShynkevich commented 2 years ago

Sure you can find the case here: https://github.com/AShynkevich/occurrent-test-case

AShynkevich commented 1 year ago

Hello, do we have any updates?

johanhaleby commented 1 year ago

Hi @AShynkevich! Unfortunately I haven't found time to look at it yet, but rest assured I haven't forgotten about this. My suspicion is that this is a Jackson configuration issue. I.e. that you need to annotate the BigDecimal property somehow. But I'm not sure so I will have to look into it more. Thanks for your patience.

johanhaleby commented 1 year ago

@AShynkevich Sorry for letting you wait for this long, and thanks a lot for the awesome project you provided (with README and everything!!) for me to replicate your issue.

However, I don't think this is a problem with Occurrent per se. Users are responsible for how serialization/deserialization works. In this case, Jackson, by default, serializes BigDecimal as a float/double value. If you add this annotation, Jackson will instead treat it as a string (and then I think it should work):

@JsonFormat(shape = JsonFormat.Shape.STRING)
 private BigDecimal salary;

Now, one of the selling points with Occurrent is that you don't (necessarily) need to clutter your domain code and events with third-party annotations. This is of course always a trade-off that has to be made, but you can also configure the ObjectMapper instance to serialize all instances of BigDecimal as String, and thus you don't need annotations. I don't know how to do this off the top of my head, but I'm sure you can find it in the Jackson docs somewhere.

AShynkevich commented 1 year ago

Hi @johanhaleby, It sounds like a solution. Thank you for your effort and good explanation. Hopefully, we'll contact again! :-)