From 4da85cf0637159b467315d6f034be6bf8e7665ef Mon Sep 17 00:00:00 2001 From: Min Zhu Date: Fri, 29 Jul 2022 17:42:45 +0000 Subject: [PATCH 1/4] load lazy referenced entities before attempt to save. --- .../datastore/core/DatastoreTemplate.java | 11 +++++++++- .../spring/data/datastore/core/LazyUtil.java | 20 +++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/spring-cloud-gcp-data-datastore/src/main/java/com/google/cloud/spring/data/datastore/core/DatastoreTemplate.java b/spring-cloud-gcp-data-datastore/src/main/java/com/google/cloud/spring/data/datastore/core/DatastoreTemplate.java index f5e4cf06de..9383727212 100644 --- a/spring-cloud-gcp-data-datastore/src/main/java/com/google/cloud/spring/data/datastore/core/DatastoreTemplate.java +++ b/spring-cloud-gcp-data-datastore/src/main/java/com/google/cloud/spring/data/datastore/core/DatastoreTemplate.java @@ -523,11 +523,16 @@ private static StructuredQuery.OrderBy createOrderBy( private List convertToEntityForSave( Object entity, Set persistedEntities, Key... ancestors) { - if (ancestors != null) { + if (ancestors.length != 0) { for (Key ancestor : ancestors) { validateKey(entity, keyToPathElement(ancestor)); } } + // If entity is lazy referenced, now load all properties before attempt to save + if (LazyUtil.isLazy(entity)) { + entity = LazyUtil.getLazyValue(entity); + } + Key key = getKey(entity, true, ancestors); Builder builder = Entity.newBuilder(key); List entitiesToSave = new ArrayList<>(); @@ -565,6 +570,10 @@ private List getReferenceEntitiesForSave( value = ListValue.of(keyValues); } else { + // If property is lazy referenced, now load it before attempt to save. + if (LazyUtil.isLazy(val)) { + val = LazyUtil.getLazyValue(val); + } entitiesToSave.addAll( getEntitiesForSave(Collections.singletonList(val), persistedEntities)); Key key = getKey(val, false); diff --git a/spring-cloud-gcp-data-datastore/src/main/java/com/google/cloud/spring/data/datastore/core/LazyUtil.java b/spring-cloud-gcp-data-datastore/src/main/java/com/google/cloud/spring/data/datastore/core/LazyUtil.java index c76a1551db..846b23609c 100644 --- a/spring-cloud-gcp-data-datastore/src/main/java/com/google/cloud/spring/data/datastore/core/LazyUtil.java +++ b/spring-cloud-gcp-data-datastore/src/main/java/com/google/cloud/spring/data/datastore/core/LazyUtil.java @@ -90,6 +90,22 @@ public static boolean isLazyAndNotLoaded(Object object) { return false; } + static boolean isLazy(Object object) { + SimpleLazyDynamicInvocationHandler handler = getProxy(object); + if (handler != null) { + return handler.getKeys() != null; + } + return false; + } + + static Object getLazyValue(Object object) { + // validate it's object is lazy. + Assert.isTrue(isLazy(object), "A lazy loaded object is required."); + + SimpleLazyDynamicInvocationHandler proxy = getProxy(object); + return proxy.getValue(); + } + /** * Extract keys from a proxy object. * @@ -145,6 +161,10 @@ public Value getKeys() { return this.keys; } + T getValue() { + return value; + } + @Override public Object invoke(Object proxy, Method method, Object[] args) throws Throwable { if (!this.isEvaluated) { From d28b8a5a1228b84b2285583a87409953693ee9a1 Mon Sep 17 00:00:00 2001 From: Min Zhu Date: Fri, 29 Jul 2022 21:29:19 +0000 Subject: [PATCH 2/4] moved loading lazy field earlier in the sequence to cover for getKey(). added tests and other minor changes. --- .../datastore/core/DatastoreTemplate.java | 9 ++- .../spring/data/datastore/core/LazyUtil.java | 15 ++++- .../spring/data/datastore/it/ChildEntity.java | 48 +++++++++++++++ .../it/DatastoreIntegrationTests.java | 60 +++++++++++++++++++ .../spring/data/datastore/it/LazyEntity.java | 4 ++ 5 files changed, 129 insertions(+), 7 deletions(-) create mode 100644 spring-cloud-gcp-data-datastore/src/test/java/com/google/cloud/spring/data/datastore/it/ChildEntity.java diff --git a/spring-cloud-gcp-data-datastore/src/main/java/com/google/cloud/spring/data/datastore/core/DatastoreTemplate.java b/spring-cloud-gcp-data-datastore/src/main/java/com/google/cloud/spring/data/datastore/core/DatastoreTemplate.java index 9383727212..dadece7efb 100644 --- a/spring-cloud-gcp-data-datastore/src/main/java/com/google/cloud/spring/data/datastore/core/DatastoreTemplate.java +++ b/spring-cloud-gcp-data-datastore/src/main/java/com/google/cloud/spring/data/datastore/core/DatastoreTemplate.java @@ -165,6 +165,10 @@ private List getEntitiesForSave( Iterable entities, Set persisted, Key... ancestors) { List entitiesForSave = new LinkedList<>(); for (T entity : entities) { + // If entity is lazy referenced, now load all properties before attempt to save + if (LazyUtil.isLazy(entity)) { + entity = (T) LazyUtil.getLazyValue(entity); + } Key key = getKey(entity, true, ancestors); if (!persisted.contains(key)) { persisted.add(key); @@ -528,11 +532,6 @@ private List convertToEntityForSave( validateKey(entity, keyToPathElement(ancestor)); } } - // If entity is lazy referenced, now load all properties before attempt to save - if (LazyUtil.isLazy(entity)) { - entity = LazyUtil.getLazyValue(entity); - } - Key key = getKey(entity, true, ancestors); Builder builder = Entity.newBuilder(key); List entitiesToSave = new ArrayList<>(); diff --git a/spring-cloud-gcp-data-datastore/src/main/java/com/google/cloud/spring/data/datastore/core/LazyUtil.java b/spring-cloud-gcp-data-datastore/src/main/java/com/google/cloud/spring/data/datastore/core/LazyUtil.java index 846b23609c..d8f6c1a9ed 100644 --- a/spring-cloud-gcp-data-datastore/src/main/java/com/google/cloud/spring/data/datastore/core/LazyUtil.java +++ b/spring-cloud-gcp-data-datastore/src/main/java/com/google/cloud/spring/data/datastore/core/LazyUtil.java @@ -90,6 +90,12 @@ public static boolean isLazyAndNotLoaded(Object object) { return false; } + /** + * Check if the object is a lazy loaded proxy. + * + * @param object an object + * @return true if the object is a proxy + */ static boolean isLazy(Object object) { SimpleLazyDynamicInvocationHandler handler = getProxy(object); if (handler != null) { @@ -98,9 +104,14 @@ static boolean isLazy(Object object) { return false; } + /** + * Loads the value provided by the proxy supplier. + * + * @param object an object that is a proxy + * @return value provided by the proxy supplier. + */ static Object getLazyValue(Object object) { - // validate it's object is lazy. - Assert.isTrue(isLazy(object), "A lazy loaded object is required."); + Assert.isTrue(isLazy(object), "A proxy object is required."); SimpleLazyDynamicInvocationHandler proxy = getProxy(object); return proxy.getValue(); diff --git a/spring-cloud-gcp-data-datastore/src/test/java/com/google/cloud/spring/data/datastore/it/ChildEntity.java b/spring-cloud-gcp-data-datastore/src/test/java/com/google/cloud/spring/data/datastore/it/ChildEntity.java new file mode 100644 index 0000000000..0366cea8dc --- /dev/null +++ b/spring-cloud-gcp-data-datastore/src/test/java/com/google/cloud/spring/data/datastore/it/ChildEntity.java @@ -0,0 +1,48 @@ +/* + * Copyright 2022-2022 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.cloud.spring.data.datastore.it; + +import com.google.cloud.spring.data.datastore.core.mapping.Entity; +import org.springframework.data.annotation.Id; + +@Entity +public class ChildEntity { + @Id String id; + + String value; + + public ChildEntity(String id, String value) { + this.id = id; + this.value = value; + } + + public String getId() { + return id; + } + + // public void setId(String id) { + // this.id = id; + // } + + public String getValue() { + return value; + } + + public void setValue(String value) { + this.value = value; + } +} diff --git a/spring-cloud-gcp-data-datastore/src/test/java/com/google/cloud/spring/data/datastore/it/DatastoreIntegrationTests.java b/spring-cloud-gcp-data-datastore/src/test/java/com/google/cloud/spring/data/datastore/it/DatastoreIntegrationTests.java index dff65bb05d..cdd31a1c1c 100644 --- a/spring-cloud-gcp-data-datastore/src/test/java/com/google/cloud/spring/data/datastore/it/DatastoreIntegrationTests.java +++ b/spring-cloud-gcp-data-datastore/src/test/java/com/google/cloud/spring/data/datastore/it/DatastoreIntegrationTests.java @@ -41,6 +41,7 @@ import com.google.cloud.spring.data.datastore.core.mapping.DiscriminatorField; import com.google.cloud.spring.data.datastore.core.mapping.DiscriminatorValue; import com.google.cloud.spring.data.datastore.core.mapping.Entity; +import com.google.cloud.spring.data.datastore.core.mapping.LazyReference; import com.google.cloud.spring.data.datastore.core.mapping.Unindexed; import com.google.cloud.spring.data.datastore.entities.CustomMap; import com.google.cloud.spring.data.datastore.entities.Product; @@ -145,6 +146,8 @@ void deleteAll() { this.datastoreTemplate.deleteAll(LazyEntity.class); this.datastoreTemplate.deleteAll(Product.class); this.datastoreTemplate.deleteAll(Store.class); + this.datastoreTemplate.deleteAll(ChildEntity.class); + this.datastoreTemplate.deleteAll(ParentEntityWithLazyChild.class); this.testEntityRepository.deleteAll(); if (this.keyForMap != null) { this.datastore.delete(this.keyForMap); @@ -739,6 +742,45 @@ void lazyReferenceTest() throws InterruptedException { assertThat(loadedParent).isEqualTo(lazyParentEntity); } + @Test + void lazyReferenceUpdateAndSaveParentTest() { + LazyEntity lazyParentEntity = new LazyEntity(new LazyEntity(new LazyEntity())); + this.datastoreTemplate.save(lazyParentEntity); + + LazyEntity loadedParent = + this.datastoreTemplate.findById(lazyParentEntity.id, LazyEntity.class); + + // update lazyReferenced property + LazyEntity newChild = new LazyEntity(); + loadedParent.setLazyChild(newChild); + this.datastoreTemplate.save(loadedParent); + + loadedParent = this.datastoreTemplate.findById(loadedParent.id, LazyEntity.class); + assertThat(loadedParent.getLazyChild()).isEqualTo(newChild); + } + + @Test + void lazyReferenceWithStringIdUpdateChildTest() { + + ChildEntity child = new ChildEntity("child-entity-id-string", "child value"); + // child.id = "abcdse"; + ParentEntityWithLazyChild parentEntityWithLazyChild = new ParentEntityWithLazyChild(child); + this.datastoreTemplate.save(parentEntityWithLazyChild); + + ParentEntityWithLazyChild loadedParent = this.datastoreTemplate.findById( + parentEntityWithLazyChild.id, ParentEntityWithLazyChild.class); + ChildEntity lazyChild = loadedParent.getLazyChild(); + + lazyChild.setValue("updated child"); + this.datastoreTemplate.save(lazyChild); + + loadedParent = this.datastoreTemplate.findById( + parentEntityWithLazyChild.id, ParentEntityWithLazyChild.class); + assertThat(loadedParent.getLazyChild().getId()).isEqualTo("child-entity-id-string"); + assertThat(loadedParent.getLazyChild().getValue()).isEqualTo("updated child"); + + } + @Test void singularLazyPropertyTest() { LazyEntity lazyParentEntity = new LazyEntity(new LazyEntity(new LazyEntity())); @@ -1396,6 +1438,24 @@ public String toString() { } } +@Entity +class ParentEntityWithLazyChild { + @Id Long id; + + @LazyReference + ChildEntity lazyChild; + + public ParentEntityWithLazyChild() {} + + ParentEntityWithLazyChild(ChildEntity child) { + this.lazyChild = child; + } + + ChildEntity getLazyChild() { + return this.lazyChild; + } +} + enum CommunicationChannels { EMAIL, SMS; diff --git a/spring-cloud-gcp-data-datastore/src/test/java/com/google/cloud/spring/data/datastore/it/LazyEntity.java b/spring-cloud-gcp-data-datastore/src/test/java/com/google/cloud/spring/data/datastore/it/LazyEntity.java index 02eca503dc..96bf5ae220 100644 --- a/spring-cloud-gcp-data-datastore/src/test/java/com/google/cloud/spring/data/datastore/it/LazyEntity.java +++ b/spring-cloud-gcp-data-datastore/src/test/java/com/google/cloud/spring/data/datastore/it/LazyEntity.java @@ -41,6 +41,10 @@ LazyEntity getLazyChild() { return this.lazyChild; } + public void setLazyChild(LazyEntity lazyChild) { + this.lazyChild = lazyChild; + } + @Override public boolean equals(Object o) { if (this == o) { From 7b1f62d7c2d9b07b79db4eb11f66b6a16d750cc8 Mon Sep 17 00:00:00 2001 From: Min Zhu Date: Mon, 1 Aug 2022 13:34:53 +0000 Subject: [PATCH 3/4] fix sonar bug. --- .../google/cloud/spring/data/datastore/core/LazyUtil.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/spring-cloud-gcp-data-datastore/src/main/java/com/google/cloud/spring/data/datastore/core/LazyUtil.java b/spring-cloud-gcp-data-datastore/src/main/java/com/google/cloud/spring/data/datastore/core/LazyUtil.java index d8f6c1a9ed..f6c322b0cf 100644 --- a/spring-cloud-gcp-data-datastore/src/main/java/com/google/cloud/spring/data/datastore/core/LazyUtil.java +++ b/spring-cloud-gcp-data-datastore/src/main/java/com/google/cloud/spring/data/datastore/core/LazyUtil.java @@ -113,8 +113,11 @@ static boolean isLazy(Object object) { static Object getLazyValue(Object object) { Assert.isTrue(isLazy(object), "A proxy object is required."); - SimpleLazyDynamicInvocationHandler proxy = getProxy(object); - return proxy.getValue(); + SimpleLazyDynamicInvocationHandler handler = getProxy(object); + if (handler != null) { + return handler.getValue(); + } + return null; } /** From 38226992bea1bb6b38e1b70ff28d5c339c339f4a Mon Sep 17 00:00:00 2001 From: Min Zhu Date: Tue, 9 Aug 2022 15:47:13 +0000 Subject: [PATCH 4/4] remove unused code and revert unnecessary ancestors.length != 0 change. --- .../cloud/spring/data/datastore/core/DatastoreTemplate.java | 2 +- .../google/cloud/spring/data/datastore/it/ChildEntity.java | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/spring-cloud-gcp-data-datastore/src/main/java/com/google/cloud/spring/data/datastore/core/DatastoreTemplate.java b/spring-cloud-gcp-data-datastore/src/main/java/com/google/cloud/spring/data/datastore/core/DatastoreTemplate.java index dadece7efb..fca94795b9 100644 --- a/spring-cloud-gcp-data-datastore/src/main/java/com/google/cloud/spring/data/datastore/core/DatastoreTemplate.java +++ b/spring-cloud-gcp-data-datastore/src/main/java/com/google/cloud/spring/data/datastore/core/DatastoreTemplate.java @@ -527,7 +527,7 @@ private static StructuredQuery.OrderBy createOrderBy( private List convertToEntityForSave( Object entity, Set persistedEntities, Key... ancestors) { - if (ancestors.length != 0) { + if (ancestors != null) { for (Key ancestor : ancestors) { validateKey(entity, keyToPathElement(ancestor)); } diff --git a/spring-cloud-gcp-data-datastore/src/test/java/com/google/cloud/spring/data/datastore/it/ChildEntity.java b/spring-cloud-gcp-data-datastore/src/test/java/com/google/cloud/spring/data/datastore/it/ChildEntity.java index 0366cea8dc..1203a46807 100644 --- a/spring-cloud-gcp-data-datastore/src/test/java/com/google/cloud/spring/data/datastore/it/ChildEntity.java +++ b/spring-cloud-gcp-data-datastore/src/test/java/com/google/cloud/spring/data/datastore/it/ChildEntity.java @@ -34,10 +34,6 @@ public String getId() { return id; } - // public void setId(String id) { - // this.id = id; - // } - public String getValue() { return value; }