unitsofmeasurement / unit-api

Units of Measurement API
http://unitsofmeasurement.github.io/unit-api/
Other
180 stars 42 forks source link

improve interoperability of Quantity with Hibernate JPA #238

Closed ChMThiel closed 2 years ago

ChMThiel commented 2 years ago

In order to store Quantities in a JPA-Entity, one has to implement a javax.persistence.AttributeConverter, that maps between a Quantity-Object (stored in a @Column-field of the entity) and the value stored in the corresponding DB-Table-Column.

When using Hibernate as JPA-Provider this works fine, but following warning is logged: WARN [org.hib.typ.des.jav.JavaTypeDescriptorRegistry] (main) HHH000481: Encountered Java type [interface javax.measure.Quantity] for which we could not locate a JavaTypeDescriptor and which does not appear to implement equals and/or hashCode. This can lead to significant performance problems when performing equality/dirty checking involving this Java type. Consider registering a custom JavaTypeDescriptor or at least implementing equals/hashCode. As you can see, two possible solutions are given:

Since java 8 it is possible to provide default-implementations of methods in interfaces. IMHO it would be nice, to move/copy the hashCode/equals implementation in like in AbstractQuantity to the interface javax.measure.Quantity like this:

default boolean equals(Object obj) {
    if (this == obj) {
       return true;
     }
    if (obj instanceof Quantity<?>) {
         Quantity<?> that = (Quantity<?>) obj;
         return Objects.equals(getUnit(), that.getUnit()) && 
                 Objects.equals(getScale(), that.getScale()) && 
                 Objects.equals(getValue(), that.getValue());
     }
     return false;
 }

 default int hashCode() {
     return Objects.hash(getUnit(), getScale(), getValue());
 }
keilw commented 2 years ago

That would require a new version of the API, but although we strictly speaking added new methods these are methods already present and expected by the TCK, so adding one or the other to the interface seems unproblematic. Will have a more detailed look, but it should be doable in a 2.1.4 release instead of a full Maintenance Release 2 of the API.

ChMThiel commented 2 years ago

When will the 2.1.4 be released?

keilw commented 2 years ago

We'll have to see, but I don't spot any significant must-haves in https://github.com/unitsofmeasurement/unit-api/issues (especially Vector API is far away and may not come before at least another MR) so I would say relatively soon, probably after summer. It should be the basis for another Indriya release with the same number, where there are 3 issues to be worked on: https://github.com/unitsofmeasurement/indriya/milestone/10

keilw commented 2 years ago

@ChMThiel Unfortunately the above code does not work, it's illegal to override any methods of java.lang.Object with a default interface method. We declare toString() as an abstract signature method, that's possible, but with default it isn't.

So it looks like only option 1:

ChMThiel commented 2 years ago

it's illegal to override any methods of java.lang.Object with a default interface method

arrrggg... i didn't consider that :(

About the hibernate JavaTypeDescriptor: I'm no expert either, but i will give it a try in my project.

desruisseaux commented 2 years ago

Anyway I was about to reply that while I had no objection, the implications are a little bit larger than just adding 2 default methods. The following invariants must be true in all cases:

Because the default methods would have used a if (obj instanceof Quantity<?>) check (as opposed to checking for a specific implementation), ensuring that the above invariants are always true would have required that all implementations does something equivalent to what the default methods do. It means that what the default methods do would need to become part of Quantity.hashCode() and Quantity.equals(Object) contract. This is similar to List.equals(Object) and List.hashCode(), which define precisely how those methods shall compute their values.

keilw commented 2 years ago

But List.equals(Object) and List.hashCode() are just abstract stubs, not default methods which they also cannot be, unless a future Java version was to change that. We could add a // Comparison and hashing block to Quantity if it helps, @ChMThiel wdyt?

For anything Hibernate specific, that cannot be done elsewhere e.g. in Jadira (which had no release after 2018, so not sure, if it even fully supports JSR 385 aside from 363) we would have uom-lib.