fix: Datastore lazy reference save/update behavior #1205
Conversation
…). added tests and other minor changes.
There was a problem hiding this comment.
I always worry about making lazy loading more complicated -- it could make saving much slower because now you have to go and read each entity before saving.
I have two general questions related to this:
- In the original reproduction scenario, I get that saving the child individually fails. But what happens if you invoke save on the parent entity instead? Does the child gets updated properly?
- If the child entity did not get updated, will this logic still trigger? I'd like to avoid triggering each child retrieval unless it's absolutely necessary.
| @@ -523,7 +527,7 @@ private static StructuredQuery.OrderBy createOrderBy( | |||
|
|
|||
| private List<Entity> convertToEntityForSave( | |||
| Object entity, Set<Key> persistedEntities, Key... ancestors) { | |||
| if (ancestors != null) { | |||
| if (ancestors.length != 0) { | |||
There was a problem hiding this comment.
Can it still be null? Maybe ancestors != null && ancestors.length != 0?
There was a problem hiding this comment.
Initially I thought that since varargs are treated as arrays, checking length should be enough. In our case, when there is no ancestors, no argument of type Key is passed down, and ancestors would be an empty array.
But since you point this out, I found if only a null is passed in, then ancestors == null. So I'll add both checks just to be cautious.
| // public void setId(String id) { | ||
| // this.id = id; | ||
| // } | ||
|
|
There was a problem hiding this comment.
Oh, missed this one.
...store/src/test/java/com/google/cloud/spring/data/datastore/it/DatastoreIntegrationTests.java
Show resolved
Hide resolved
...d-gcp-data-datastore/src/main/java/com/google/cloud/spring/data/datastore/core/LazyUtil.java
Show resolved
Hide resolved
|
SonarCloud Quality Gate failed. |
The issue:
Long, it would not fail, but rather saved to Datastore as a new entity with null values.Trying to fix by loading the real entity when attempting to save. When the entity is not loaded/evaluated, it will still fall to original logic without loading it.
fixes #1203
SonarCloud failed on test coverage, I'll look into adding tests.