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

ClassCastException with joda-time objects #224

Closed
asaf-romano opened this Issue Oct 11, 2015 · 13 comments

Comments

Projects
None yet
3 participants
@asaf-romano

asaf-romano commented Oct 11, 2015

Hey there,

Trying to use the library on an object containing various jodatime's DateTime fields, I get the following exception:
java.util.concurrent.CompletionException: java.lang.ClassCastException: org.joda.time.chrono.ISOChronology cannot be cast to org.javers.core.metamodel.object.GlobalId at java.util.concurrent.CompletableFuture.encodeThrowable(CompletableFuture.java:273) ~[na:1.8.0_60] at java.util.concurrent.CompletableFuture.completeThrowable(CompletableFuture.java:280) ~[na:1.8.0_60] at java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1592) [na:1.8.0_60] at java.util.concurrent.CompletableFuture$AsyncSupply.exec(CompletableFuture.java:1582) [na:1.8.0_60] at java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:289) ~[na:1.8.0_60] at java.util.concurrent.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1056) ~[na:1.8.0_60] at java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1692) ~[na:1.8.0_60] at java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:157) ~[na:1.8.0_60] Caused by: java.lang.ClassCastException: org.joda.time.chrono.ISOChronology cannot be cast to org.javers.core.metamodel.object.GlobalId at org.javers.core.graph.ObjectNode.getReference(ObjectNode.java:75) ~[javers-core-1.3.13.jar:na] at org.javers.core.diff.RealNodePair.getRightGlobalId(RealNodePair.java:61) ~[javers-core-1.3.13.jar:na] at org.javers.core.diff.appenders.ReferenceChangeAppender.calculateChanges(ReferenceChangeAppender.java:25) ~[javers-core-1.3.13.jar:na] at org.javers.core.diff.appenders.ReferenceChangeAppender.calculateChanges(ReferenceChangeAppender.java:15) ~[javers-core-1.3.13.jar:na] at org.javers.core.diff.DiffFactory.appendChanges(DiffFactory.java:142) ~[javers-core-1.3.13.jar:na] at org.javers.core.diff.DiffFactory.appendPropertyChanges(DiffFactory.java:132) ~[javers-core-1.3.13.jar:na] at org.javers.core.diff.DiffFactory.createAndAppendChanges(DiffFactory.java:114) ~[javers-core-1.3.13.jar:na] at org.javers.core.diff.DiffFactory.create(DiffFactory.java:65) ~[javers-core-1.3.13.jar:na] at org.javers.core.diff.DiffFactory.compare(DiffFactory.java:58) ~[javers-core-1.3.13.jar:na] at org.javers.core.JaversCore.compare(JaversCore.java:94) ~[javers-core-1.3.13.jar:na]

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Oct 11, 2015

looks similar to #218

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Oct 11, 2015

we are working on fixing this

@asaf-romano

This comment has been minimized.

asaf-romano commented Oct 11, 2015

FWIW, unlike the case described there, this one isn't random at all. One DateTime object breaks compare permanently (btw, this DateTime object is also wrapped in Optional).

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Oct 11, 2015

If you submit a failing test which isoletes the problem, it would help us to find the bug

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Oct 13, 2015

fix for #218 is released in fresh version 1.3.14, available at https://oss.sonatype.org/content/repositories/releases/ and in few hours in Maven Central, Could you check if it solves your issue (I think it will)

@szmeti

This comment has been minimized.

szmeti commented Oct 13, 2015

We have run into the same issue just now. We are still getting the above exception with 1.3.14

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Oct 13, 2015

ok, so could you please submit a failing test which isolets this issue? you can push it to your forked Javers repository

jboroczki added a commit to corballis/javers that referenced this issue Oct 14, 2015

@szmeti

This comment has been minimized.

szmeti commented Oct 14, 2015

Failing test is available in 3a03e1d.

It seems that if two DateTimes have different chronologies, it cannot handle it. If both DateTime instances use the same chronology, then it works fine.

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Oct 14, 2015

chronology ... so when two dates are using diffrent callendars?

In fact, there is no out-of-the-box support for org.joda.time.DateTime in JaVers
(only 2 Joda types have built in cmparators: LocalDateTime and LocalDate)

Try to write and register CustomPropertyComparator (see http://javers.org/documentation/diff-configuration/#custom-comparators) for org.joda.time.DateTime, and then we can push it to JaVers core.

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Oct 16, 2015

@szmeti your failing test is fine, I have ClassCastException reproduced on my maschine ...

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Oct 16, 2015

I mean that lack of support for joda DateTime as ValueType should not result in ClassCastException

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Oct 16, 2015

found a bug in our Optional.equals() method

bartoszwalacik added a commit that referenced this issue Oct 16, 2015

#224
fixed bug in Optional.equals()
@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Oct 17, 2015

bug is fixed in v 1.3.17
but to have better diff experience, remember to register DateTime as ValueType and consider implementing JSON TypeAdapter for it, see http://javers.org/documentation/repository-examples/#json-type-adapter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment