unitsofmeasurement / uom-lib

Units of Measurement Libraries
Other
14 stars 13 forks source link

60: Jackson deserialization for Quantity #72

Closed bantu closed 3 years ago

bantu commented 3 years ago

Fix #60

As requested here: https://github.com/unitsofmeasurement/uom-lib/issues/60#issuecomment-938682634


This change is Reviewable

GregJohnStewart commented 3 years ago

@bantu Looks like I can't push to your branch, but this was my edit:

QuantityJsonDerserializer.java:92 (right before return statement:

if(value == null){
            throw new JsonParseException(jp, "Value not found for quantity type.");
        }
        if(unit == null){
            throw new JsonParseException(jp, "Unit not found for quantity type.");
        }
        if(scale == null){
            throw new JsonParseException(jp, "Scale not found for quantity type.");
        }
bantu commented 3 years ago

@bantu Looks like I can't push to your branch

You need to accept my repo invitation.

GregJohnStewart commented 3 years ago

That would help! added my changes, thanks

bantu commented 3 years ago

I just discovered that this implementation does not at all work for objects containing multiple Quantity objects.

bantu commented 3 years ago

I suppose that while ((currentToken = jp.nextValue()) != null) { eats all tokens. Note that I wrote this code quite some time ago.

bantu commented 3 years ago

Reimplemented and simplified. Commits should probably be squashed.

keilw commented 3 years ago

Unfortunately it breaks the build ;-/

keilw commented 3 years ago

@bantu The common library was missing in module-info.

lucasvc commented 3 years ago

Would not be good to add a serializer too? Just as matter of forcing same serialization field names - even Jackson by default use the class member name/method name.

GregJohnStewart commented 3 years ago

@lucasvc The Jackson already serializes values fine, just doesn't know off the back how to deserialize

@keilw what's the status on this? need another pull request out?

lucasvc commented 3 years ago

@GregJohnStewart I know, as I said

even Jackson by default use the class member name/method name

But then you leave to Jackson that task, which could change in the future.

bantu commented 2 years ago

Unfortunately it breaks the build ;-/

Hmm. Why only after merge?

bantu commented 2 years ago

@keilw It looks like this change has not shipped yet. Are there any plans on shipping it any time soon?

fref commented 1 year ago

+1 on having a release with this change

keilw commented 1 year ago

We plan an MR2 of JSR 385 in the course of this quater, so there will be new releases of uom-lib-common (before the RI) and the other libraries (after MR2 of API and RI) soon anyway. Check out the release plan for MR2 for the overall progress. There are still some critical parts, I assume we'll have them done by the end of Q1, if we get a hand with some tasks, it might be sooner ;-)

fref commented 1 year ago

How could we be of some help?