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

NullPointerException when commit property value is null #596

Closed
Ramblurr opened this Issue Nov 8, 2017 · 7 comments

Comments

Projects
None yet
2 participants
@Ramblurr

Ramblurr commented Nov 8, 2017

It is possible to write a commit property whose value is null (in Java).

But when Javers later tries to read/parse this (by calling new JsonPrimitive() on this line), an NPE is thrown because JsonPrimitive doesn't handle null.

Caused by: java.lang.NullPointerException
	at com.google.gson.JsonPrimitive.isPrimitiveOrString(JsonPrimitive.java:278)
	at com.google.gson.JsonPrimitive.setValue(JsonPrimitive.java:101)
	at com.google.gson.JsonPrimitive.<init>(JsonPrimitive.java:65)
	at org.javers.core.json.typeadapter.commit.CommitPropertiesConverter.toJson(CommitPropertiesConverter.java:34)
	at org.javers.core.json.typeadapter.commit.CdoSnapshotAssembler.assembleCommitMetadata(CdoSnapshotAssembler.java:72)
	at org.javers.core.json.typeadapter.commit.CdoSnapshotAssembler.assemble(CdoSnapshotAssembler.java:22)
	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:1380)
	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:51)
	at org.javers.repository.api.JaversExtendedRepository.getLatest(JaversExtendedRepository.java:92)
	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:71)
	at org.javers.core.JaversCore.commit(JaversCore.java:82)
	at org.javers.spring.jpa.JaversTransactionalDecorator.commit(JaversTransactionalDecorator.java:65)
@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Nov 8, 2017

Thanks for reporting, would you like to contribute a PR with a fix? Looks easy to fix

@Ramblurr

This comment has been minimized.

Ramblurr commented Nov 9, 2017

What is the right behavior? Allow null writes (and subsequent reads) or silently discard them?

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Nov 9, 2017

I suggest to silently remove all nulled properties before write to JaversRepository. Properties are stored in a map, nulled key is equiv to no key.

@Ramblurr

This comment has been minimized.

Ramblurr commented Nov 10, 2017

A null key, yes I agree. But what about a null value?

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Nov 10, 2017

I mean, a key with null value is equiv to no key at all

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Nov 10, 2017

so we need to filter out all keys with null values

bartoszwalacik added a commit that referenced this issue Dec 20, 2017

Merge pull request #620 from tau3/bugfix/596/null-property-value
#596 fixed null values problem in CommitPropertiesConverter
@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Dec 20, 2017

fix released in 3.7.7

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