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

Handling schema change (property type change) in javers #511

Closed
anshuljaindc opened this Issue Mar 6, 2017 · 10 comments

Comments

Projects
None yet
4 participants
@anshuljaindc

anshuljaindc commented Mar 6, 2017

I have my java class which consists of Map<String, String>. I am using javers sql repository for storing my auditing data. I recently changed schema of my property to Map<String, List>. Now I am getting following exception on commits of the domain. I think Javers should internally handle any schema changes.

Current javers version - 2.8.2
Stackoverflow link for question - http://stackoverflow.com/questions/42610035/handling-schema-change-in-javers-auditing
com.google.gson.JsonSyntaxException: java.lang.IllegalStateException: Expected BEGIN_ARRAY but was STRING
at com.google.gson.Gson.fromJson(Gson.java:826) ~[gson-2.4.jar:?]
at com.google.gson.Gson.fromJson(Gson.java:879) ~[gson-2.4.jar:?]
at com.google.gson.Gson$1.deserialize(Gson.java:129) ~[gson-2.4.jar:?]
at org.javers.core.json.typeadapter.commit.CdoSnapshotStateDeserializer.decodePropertyValue(CdoSnapshotStateDeserializer.java:48) ~[javers-core-2.8.2.jar:?]
at ...

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Mar 7, 2017

Agreed that tracking property type changes would be useful feature in JaVers. Let me know if you are eager to contribute

@vguna

This comment has been minimized.

vguna commented Jan 4, 2018

Just stumbled across this and sounds like a deal breaker for choosing JaVers :(. Changes to entities are done often (e.g. renaming, adding or removing attributes, changing type). These are currently handled nicely with Liquibase in my service. Would that mean, I have to write two updates now? One using Liquibase and a "custom" one for updating JaVers repository and somehow "patching" the JSON in there?
Is this only a problem with changing types or also on renamed, addded, deleted attributes?

What JaVers operations are affected by such attribute changes? Only constructing a "real" Entity out of the snapshots/diffs or also just evaluating what has changed in the past (diffs)? Looking at the stacktrace above it seems that it also "simply" breaks suddenly on a simple save on the repository then, right?

So what are the recommendations of the JaVers team to handle such scenarios? I can't think of a generic mechanism that is able to perform e.g. all possible attribute type changes automagically :).

E.g. for my scenario it would be sufficient, that I can simply query for the diffs of an object over time. If a type doesn't match anymore (previous and actual), it would be fine for me to just let JaVers mark the attribute as "changed" and simply store the new type/value from now on. Without actually dying on a simple save operation. I something like this with custom hooks/listeners possible currently?

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Jan 4, 2018

Hi @vguna, lot of questions ...

class name and property name changes are already covered by @typename and @PropertyName annotations. You don't need to update existing Snapshots. Snapshots are immutable.

What is not covered is changing property type, for example from string to int, and I think this issue is about it.

@bartoszwalacik bartoszwalacik changed the title from Handling schema change in javers? to Handling schema change (property type changes) in javers Jan 4, 2018

@bartoszwalacik bartoszwalacik changed the title from Handling schema change (property type changes) in javers to Handling schema change (property type change) in javers Jan 4, 2018

@vguna

This comment has been minimized.

vguna commented Jan 4, 2018

Yeah, sorry about that, but some questions are still unanswered for me ;). If these are already covered by the documentation, please give me a hint where to look.

So I understand that changes of names can be handled by the given annotations.

What happens if attributes are removed or added? Are these handled transparently for me?

Where are the places where my (JaVers related) code could break due to the type problem and what is your recommendation to handle such scenarios?

Are there already hooks/listeners that I could use to handle such situations?

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Jan 4, 2018

@vguna, Issues is the place for bug reports and for feature requests. Questions can be asked on Stack.

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Jan 4, 2018

(for now we know only one scenario when javers breaks after domain class change: when you change a property type)

@vguna

This comment has been minimized.

vguna commented Jan 5, 2018

Ok, fair enough, thanks!

@f466162

This comment has been minimized.

f466162 commented Jul 13, 2018

Unfortunately I experience a similar issue with JaVers 3.10.2 after a schema change (was: reference to another Entity, now: String). Renaming attributes in Entities is not a solution; yet I got no clue what to do as migrating data would mean to lose the audit trail...

2018-07-12 23:40:45.834 ERROR 4914 --- [pool-2-thread-1] o.s.s.s.TaskUtils$LoggingErrorHandler    : Unexpected error occurred in scheduled task. [userId=ka0064, X-Tracking-Id=49c606a5-e60d-4f81-b088-1c9fec880c10, trackingId=49c606a5-e60d-4f81-b088-1c9fec880c10]

com.google.gson.JsonSyntaxException: java.lang.IllegalStateException: Expected STRING but was BEGIN_OBJECT at path $
	at com.google.gson.Gson.fromJson(Gson.java:939)
	at com.google.gson.Gson.fromJson(Gson.java:994)
	at com.google.gson.internal.bind.TreeTypeAdapter$GsonContextImpl.deserialize(TreeTypeAdapter.java:162)
	at org.javers.core.json.typeadapter.commit.CdoSnapshotStateDeserializer.decodePropertyValue(CdoSnapshotStateDeserializer.java:48)
	at org.javers.core.json.typeadapter.commit.CdoSnapshotStateDeserializer.deserialize(CdoSnapshotStateDeserializer.java:39)
	at org.javers.core.json.typeadapter.commit.CdoSnapshotTypeAdapter.deserializeSnapshotState(CdoSnapshotTypeAdapter.java:71)
	at org.javers.core.json.typeadapter.commit.CdoSnapshotTypeAdapter.fromJson(CdoSnapshotTypeAdapter.java:57)
	at org.javers.core.json.typeadapter.commit.CdoSnapshotTypeAdapter.fromJson(CdoSnapshotTypeAdapter.java:21)
	at org.javers.core.json.JsonConverterBuilder.lambda$registerJsonTypeAdapterForType$4(JsonConverterBuilder.java:166)
	at com.google.gson.internal.bind.TreeTypeAdapter.read(TreeTypeAdapter.java:69)
	at com.google.gson.Gson.fromJson(Gson.java:927)
	at com.google.gson.Gson.fromJson(Gson.java:994)
	at com.google.gson.Gson.fromJson(Gson.java:967)
	at org.javers.core.json.JsonConverter.fromJson(JsonConverter.java:74)
	at org.javers.core.json.JsonConverter.fromSerializedSnapshot(JsonConverter.java:82)
	at org.javers.repository.sql.finders.CdoSnapshotFinder.lambda$fetchCdoSnapshots$3(CdoSnapshotFinder.java:94)
	at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193)
	at java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1382)
	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481)
	at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471)
	at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:708)
	at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:499)
	at org.javers.common.collections.Lists.transform(Lists.java:68)
	at org.javers.repository.sql.finders.CdoSnapshotFinder.fetchCdoSnapshots(CdoSnapshotFinder.java:93)
	at org.javers.repository.sql.finders.CdoSnapshotFinder.lambda$getLatest$0(CdoSnapshotFinder.java:54)
	at java.util.Optional.map(Optional.java:215)
	at org.javers.repository.sql.finders.CdoSnapshotFinder.getLatest(CdoSnapshotFinder.java:52)
	at org.javers.repository.sql.JaversSqlRepository.getLatest(JaversSqlRepository.java:52)
	at org.javers.repository.api.JaversExtendedRepository.getLatest(JaversExtendedRepository.java:91)
	at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193)
	at java.util.HashMap$KeySpliterator.forEachRemaining(HashMap.java:1553)
	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481)
	at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471)
	at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:708)
	at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:499)
	at org.javers.core.snapshot.SnapshotGraphFactory.createLatest(SnapshotGraphFactory.java:29)
	at org.javers.core.commit.CommitFactory.create(CommitFactory.java:72)
	at org.javers.core.JaversCore.commit(JaversCore.java:81)
	at org.javers.spring.jpa.JaversTransactionalDecorator.commit(JaversTransactionalDecorator.java:67)
	at org.javers.spring.jpa.JaversTransactionalDecorator$$FastClassBySpringCGLIB$$acb40bd0.invoke(<generated>)
	at org.springframework.cglib.proxy.MethodProxy.invoke(MethodProxy.java:204)
	at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.invokeJoinpoint(CglibAopProxy.java:746)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:163)
	at org.springframework.transaction.interceptor.TransactionAspectSupport.invokeWithinTransaction(TransactionAspectSupport.java:294)
	at org.springframework.transaction.interceptor.TransactionInterceptor.invoke(TransactionInterceptor.java:98)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:185)
	at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:688)
	at org.javers.spring.jpa.JaversTransactionalDecorator$$EnhancerBySpringCGLIB$$6938c640.commit(<generated>)
	at org.javers.spring.auditable.aspect.springdata.OnSaveAuditChangeHandler.handle(OnSaveAuditChangeHandler.java:18)
	at org.javers.spring.auditable.aspect.springdata.AbstractSpringAuditableRepositoryAspect.applyVersionChanges(AbstractSpringAuditableRepositoryAspect.java:57)
	at org.javers.spring.auditable.aspect.springdata.AbstractSpringAuditableRepositoryAspect.lambda$onVersionEvent$0(AbstractSpringAuditableRepositoryAspect.java:38)
	at java.util.Optional.ifPresent(Optional.java:159)
	at org.javers.spring.auditable.aspect.springdata.AbstractSpringAuditableRepositoryAspect.onVersionEvent(AbstractSpringAuditableRepositoryAspect.java:35)
	at org.javers.spring.auditable.aspect.springdata.AbstractSpringAuditableRepositoryAspect.onSave(AbstractSpringAuditableRepositoryAspect.java:25)
	at org.javers.spring.auditable.aspect.springdatajpa.JaversSpringDataJpaAuditableRepositoryAspect.onSaveExecuted(JaversSpringDataJpaAuditableRepositoryAspect.java:34)
	at sun.reflect.GeneratedMethodAccessor106.invoke(Unknown Source)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.springframework.aop.aspectj.AbstractAspectJAdvice.invokeAdviceMethodWithGivenArgs(AbstractAspectJAdvice.java:644)
	at org.springframework.aop.aspectj.AbstractAspectJAdvice.invokeAdviceMethod(AbstractAspectJAdvice.java:626)
	at org.springframework.aop.aspectj.AspectJAfterReturningAdvice.afterReturning(AspectJAfterReturningAdvice.java:66)
	at org.springframework.aop.framework.adapter.AfterReturningAdviceInterceptor.invoke(AfterReturningAdviceInterceptor.java:53)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:174)
	at org.springframework.aop.interceptor.ExposeInvocationInterceptor.invoke(ExposeInvocationInterceptor.java:92)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:185)
	at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:212)
	at com.sun.proxy.$Proxy161.save(Unknown Source)
@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Jul 14, 2018

guys, this is open source, you are encouraged to contributing, you can start from a failing test case

bartoszwalacik added a commit that referenced this issue Jul 15, 2018

bartoszwalacik added a commit that referenced this issue Jul 20, 2018

#511
initial work

bartoszwalacik added a commit that referenced this issue Jul 22, 2018

bartoszwalacik added a commit that referenced this issue Jul 30, 2018

bartoszwalacik added a commit that referenced this issue Aug 1, 2018

bartoszwalacik added a commit that referenced this issue Aug 4, 2018

@bartoszwalacik bartoszwalacik added the fixed label Aug 4, 2018

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Aug 4, 2018

released in 3.11.0

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