From dd66288ada5276a7b7c6c68155956c7336ed69e2 Mon Sep 17 00:00:00 2001 From: Scott Murphy Date: Sun, 19 May 2019 17:20:36 -0400 Subject: [PATCH 1/5] Cloud Client Library Link/BlobKey behaviour compatibility --- .../gaelyk/datastore/DatastoreEntity.java | 3 ++ .../datastore/DatastoreEntityCoercion.java | 27 +++++++++--- .../datastore/EntityTransformation.groovy | 43 ++++++++++++++++++- .../datastore/ReflectionEntityCoercion.groovy | 33 +++++++++++++- 4 files changed, 96 insertions(+), 10 deletions(-) diff --git a/core/src/main/groovyx/gaelyk/datastore/DatastoreEntity.java b/core/src/main/groovyx/gaelyk/datastore/DatastoreEntity.java index 2575e1d9..875bea16 100644 --- a/core/src/main/groovyx/gaelyk/datastore/DatastoreEntity.java +++ b/core/src/main/groovyx/gaelyk/datastore/DatastoreEntity.java @@ -1,6 +1,7 @@ package groovyx.gaelyk.datastore; import java.util.List; +import org.codehaus.groovy.ast.PropertyNode; /** * Helper interface to speed up Object to Entity coercion skipping unnecessary reflection which @@ -127,4 +128,6 @@ public interface DatastoreEntity { */ void setProperty(String propertyName, Object newValue); +// List getDatastoreIndexedPropertyNodes(); +// List getDatastoreUnindexedPropertyNodes(); } diff --git a/core/src/main/groovyx/gaelyk/datastore/DatastoreEntityCoercion.java b/core/src/main/groovyx/gaelyk/datastore/DatastoreEntityCoercion.java index e31082e6..2949b10f 100755 --- a/core/src/main/groovyx/gaelyk/datastore/DatastoreEntityCoercion.java +++ b/core/src/main/groovyx/gaelyk/datastore/DatastoreEntityCoercion.java @@ -3,9 +3,11 @@ import com.google.appengine.api.datastore.Entities; import com.google.appengine.api.datastore.Entity; import com.google.appengine.api.datastore.Text; +import org.codehaus.groovy.ast.PropertyNode; import groovy.lang.GString; import groovyx.gaelyk.extensions.DatastoreExtensions; +import groovyx.gaelyk.datastore.ReflectionEntityCoercion; import static java.nio.charset.StandardCharsets.UTF_8; @@ -113,12 +115,18 @@ private static Object transformValueForStorage(Object value) { dsEntity.setDatastoreVersion(0); } } - for(String propertyName : dsEntity.getDatastoreIndexedProperties()){ - setEntityProperty(en, dsEntity, propertyName); + for(String property : dsEntity.getDatastoreIndexedProperties()){ + setEntityProperty(en, dsEntity, property); } - for(String propertyName : dsEntity.getDatastoreUnindexedProperties()){ - setEntityProperty(en, dsEntity, propertyName); + for(String property : dsEntity.getDatastoreIndexedProperties()){ + setEntityProperty(en, dsEntity, property); } +// for(PropertyNode propertyNode : dsEntity.getDatastoreIndexedPropertyNodes()){ +// setEntityProperty(en, dsEntity, propertyNode); +// } +// for(PropertyNode propertyNode : dsEntity.getDatastoreUnindexedPropertyNodes()){ +// setEntityProperty(en, dsEntity, propertyNode); +// } return dsEntity; } @@ -129,11 +137,16 @@ private static > void setEntityProperty(Entity en, return; } Object value = en.getProperty(propertyName); - if (value instanceof Text) { - value = ((Text) value).getValue(); + + Class type = null; + for (Class clazz = dsEntity.getClass(); type == null && clazz != null; clazz = clazz.getSuperclass()) { + try { + type = clazz.getDeclaredField(propertyName).getType(); + } catch (NoSuchFieldException nfe) {} } + try { - dsEntity.setProperty(propertyName, value); + dsEntity.setProperty(propertyName, ReflectionEntityCoercion.transformValueForRetrieval(value, type)); } catch (Exception e) { throw new IllegalArgumentException("Problem setting value '" + value + "' to property '" + propertyName + "' of entity " + dsEntity.getClass().getSimpleName(), e); } diff --git a/core/src/main/groovyx/gaelyk/datastore/EntityTransformation.groovy b/core/src/main/groovyx/gaelyk/datastore/EntityTransformation.groovy index 00e29ddc..2b107563 100644 --- a/core/src/main/groovyx/gaelyk/datastore/EntityTransformation.groovy +++ b/core/src/main/groovyx/gaelyk/datastore/EntityTransformation.groovy @@ -362,6 +362,8 @@ class EntityTransformation extends AbstractASTTransformation { List indexed = [] List unindexed = [] + List indexedNodes = [] + List unindexedNodes = [] eachPropertyIncludingSuper(parent) { PropertyNode prop -> if(Modifier.isStatic(prop.modifiers) || Modifier.isFinal(prop.modifiers)) { @@ -382,6 +384,7 @@ class EntityTransformation extends AbstractASTTransformation { } if(hasUnindexedAnno){ unindexed << prop.name + unindexedNodes << prop return } boolean hasIndexedAnno = annos.any { AnnotationNode a -> @@ -389,17 +392,19 @@ class EntityTransformation extends AbstractASTTransformation { } if(hasIndexedAnno){ indexed << prop.name + indexedNodes << prop return } if(defaultIndexed){ indexed << prop.name + indexedNodes << prop } else { unindexed << prop.name + unindexedNodes << prop } } parent.addField new FieldNode('DATASTORE_INDEXED_PROPERTIES', Modifier.PRIVATE | Modifier.STATIC | Modifier.FINAL, getBoundListNode(ClassHelper.STRING_TYPE), parent, buildList(indexed)) - parent.addMethod new MethodNode( 'getDatastoreIndexedProperties', Modifier.PUBLIC, @@ -426,6 +431,34 @@ class EntityTransformation extends AbstractASTTransformation { self.columnNumber = 1 self } + + // parent.addField new FieldNode('DATASTORE_INDEXED_PROPERTY_NODES', Modifier.PRIVATE | Modifier.STATIC | Modifier.FINAL, getBoundListNode(ClassHelper.make(PropertyNode).plainNodeReference), parent, buildNodeList(indexedNodes)) + // parent.addMethod new MethodNode( + // 'getDatastoreIndexedPropertyNodes', + // Modifier.PUBLIC, + // getBoundListNode(ClassHelper.make(PropertyNode).plainNodeReference), + // Parameter.EMPTY_ARRAY, + // ClassNode.EMPTY_ARRAY, + // new ReturnStatement(new VariableExpression('DATASTORE_INDEXED_PROPERTY_NODES')) + // ).with { MethodNode self -> + // self.lineNumber = 10015 + // self.columnNumber = 1 + // self + // } + + // parent.addField new FieldNode('DATASTORE_UNINDEXED_PROPERTY_NODES', Modifier.PRIVATE | Modifier.STATIC | Modifier.FINAL, getBoundListNode(ClassHelper.make(PropertyNode).plainNodeReference), parent, buildList(unindexedNodes)) + // parent.addMethod new MethodNode( + // 'getDatastoreUnindexedPropertyNodes', + // Modifier.PUBLIC, + // getBoundListNode(ClassHelper.make(PropertyNode).plainNodeReference), + // Parameter.EMPTY_ARRAY, + // ClassNode.EMPTY_ARRAY, + // new ReturnStatement(new VariableExpression('DATASTORE_UNINDEXED_PROPERTY_NODES')) + // ).with { MethodNode self -> + // self.lineNumber = 10016 + // self.columnNumber = 1 + // self + // } } private void eachPropertyIncludingSuper(ClassNode parent, Closure iterator, List alreadyProcessed = []){ @@ -457,6 +490,14 @@ class EntityTransformation extends AbstractASTTransformation { list } + // private Expression buildNodeList(List values) { + // ListExpression list = new ListExpression() + // for (PropertyNode value in values) { + // list.addExpression(new ConstantExpression(value)) + // } + // list + // } + private MethodNode addDelegatedMethod(String name, ClassNode returnType = ClassHelper.DYNAMIC_TYPE) { def helper = ClassHelper.make(EntityTransformationHelper).plainNodeReference diff --git a/core/src/main/groovyx/gaelyk/datastore/ReflectionEntityCoercion.groovy b/core/src/main/groovyx/gaelyk/datastore/ReflectionEntityCoercion.groovy index a1193a84..1d39d638 100755 --- a/core/src/main/groovyx/gaelyk/datastore/ReflectionEntityCoercion.groovy +++ b/core/src/main/groovyx/gaelyk/datastore/ReflectionEntityCoercion.groovy @@ -25,6 +25,8 @@ import java.lang.reflect.Modifier import com.google.appengine.api.datastore.Entities import com.google.appengine.api.datastore.Entity import com.google.appengine.api.datastore.EntityNotFoundException +import com.google.appengine.api.datastore.Link +import com.google.appengine.api.blobstore.BlobKey import static groovyx.gaelyk.extensions.DatastoreExtensions.transformValueForRetrieval import static groovyx.gaelyk.extensions.DatastoreExtensions.transformValueForStorage @@ -193,6 +195,32 @@ class ReflectionEntityCoercion { return entity } + public static Object transformValueForRetrieval(Object value, Class type) { + // Google Client Libraries do not support Link and BlobKey. + // They automatically map them to String. If a properties type is String, + // consistant behaviour is expected. + // https://googleapis.dev/java/google-cloud-clients/latest/com/google/cloud/datastore/Value.html + switch (type) { // setting to + case BlobKey: + switch (value) { // from + case String: return new BlobKey((String) value) + } + break + case Link: + switch (value) { // from + case String: return new Link((String) value) + } + break + case String: + switch (value) { // from + case BlobKey: return ((BlobKey) value).getKeyString() + case Link: return value.toString() + } + } // otherwise normal behaviour + return transformValueForRetrieval(value) + } + + /** * Convert an entity into an object * @@ -213,9 +241,10 @@ class ReflectionEntityCoercion { } } else { entityProps.each { String k, v -> - if (o.metaClass.hasProperty(o, k)) { + def prop = o.metaClass.hasProperty(o, k) + if (prop) { try { - o[k] = transformValueForRetrieval(v) + o[k] = transformValueForRetrieval(v, prop.type) } catch(ReadOnlyPropertyException rope){ // cannot set read only property! } From 1bb0bce824895221a16d0445cada919194ad755c Mon Sep 17 00:00:00 2001 From: Scott Murphy Date: Sun, 19 May 2019 20:24:06 -0400 Subject: [PATCH 2/5] typo --- .../main/groovyx/gaelyk/datastore/DatastoreEntityCoercion.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/groovyx/gaelyk/datastore/DatastoreEntityCoercion.java b/core/src/main/groovyx/gaelyk/datastore/DatastoreEntityCoercion.java index 2949b10f..a7b43e42 100755 --- a/core/src/main/groovyx/gaelyk/datastore/DatastoreEntityCoercion.java +++ b/core/src/main/groovyx/gaelyk/datastore/DatastoreEntityCoercion.java @@ -118,7 +118,7 @@ private static Object transformValueForStorage(Object value) { for(String property : dsEntity.getDatastoreIndexedProperties()){ setEntityProperty(en, dsEntity, property); } - for(String property : dsEntity.getDatastoreIndexedProperties()){ + for(String property : dsEntity.getDatastoreUnindexedProperties()){ setEntityProperty(en, dsEntity, property); } // for(PropertyNode propertyNode : dsEntity.getDatastoreIndexedPropertyNodes()){ From 2e8467be69fc4266fe4e052c3b29942234ef37c7 Mon Sep 17 00:00:00 2001 From: Scott Murphy Date: Mon, 20 May 2019 22:09:15 -0400 Subject: [PATCH 3/5] Make line numbers unique --- .../main/groovyx/gaelyk/datastore/EntityTransformation.groovy | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/groovyx/gaelyk/datastore/EntityTransformation.groovy b/core/src/main/groovyx/gaelyk/datastore/EntityTransformation.groovy index 2bdba5f2..9e1e2102 100644 --- a/core/src/main/groovyx/gaelyk/datastore/EntityTransformation.groovy +++ b/core/src/main/groovyx/gaelyk/datastore/EntityTransformation.groovy @@ -524,7 +524,7 @@ class EntityTransformation extends AbstractASTTransformation { ClassNode.EMPTY_ARRAY, block ).with { MethodNode self -> - self.lineNumber = 10013 + self.lineNumber = 10015 self.columnNumber = 1 self } @@ -561,7 +561,7 @@ class EntityTransformation extends AbstractASTTransformation { ClassNode.EMPTY_ARRAY, block ).with { MethodNode self -> - self.lineNumber = 10014 + self.lineNumber = 10016 self.columnNumber = 1 self } From f4925e294845238d40496c51e213bd024e76bcae Mon Sep 17 00:00:00 2001 From: Scott Murphy Date: Mon, 20 May 2019 22:09:34 -0400 Subject: [PATCH 4/5] Replicate exception in phase 'canonicalization' bug --- .../gaelyk/datastore/EntityTransformationSpec.groovy | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/core/src/test/groovyx/gaelyk/datastore/EntityTransformationSpec.groovy b/core/src/test/groovyx/gaelyk/datastore/EntityTransformationSpec.groovy index 0656977b..631edbf2 100644 --- a/core/src/test/groovyx/gaelyk/datastore/EntityTransformationSpec.groovy +++ b/core/src/test/groovyx/gaelyk/datastore/EntityTransformationSpec.groovy @@ -422,12 +422,16 @@ class EntityTransformationSpec extends Specification { import groovyx.gaelyk.datastore.Key import groovyx.gaelyk.datastore.Entity as GE import groovyx.gaelyk.datastore.Indexed + import groovyx.gaelyk.datastore.Ignore + import groovyx.gaelyk.datastore.Order + import groovy.transform.Canonical import com.google.appengine.api.datastore.* - @GE @groovy.transform.CompileStatic + @GE @Canonical @groovy.transform.CompileStatic class Person { @Key long id @Indexed String name + @Ignore Order order } def key = new Person(name: 'test').save() From 08a2e2772b9f9a99b41a78fd19d2bc235e495bc4 Mon Sep 17 00:00:00 2001 From: Scott Murphy Date: Mon, 20 May 2019 22:22:41 -0400 Subject: [PATCH 5/5] Create separate test for AST error --- .../datastore/EntityTransformationSpec.groovy | 24 +++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/core/src/test/groovyx/gaelyk/datastore/EntityTransformationSpec.groovy b/core/src/test/groovyx/gaelyk/datastore/EntityTransformationSpec.groovy index 631edbf2..13859d64 100644 --- a/core/src/test/groovyx/gaelyk/datastore/EntityTransformationSpec.groovy +++ b/core/src/test/groovyx/gaelyk/datastore/EntityTransformationSpec.groovy @@ -422,16 +422,12 @@ class EntityTransformationSpec extends Specification { import groovyx.gaelyk.datastore.Key import groovyx.gaelyk.datastore.Entity as GE import groovyx.gaelyk.datastore.Indexed - import groovyx.gaelyk.datastore.Ignore - import groovyx.gaelyk.datastore.Order - import groovy.transform.Canonical import com.google.appengine.api.datastore.* - @GE @Canonical @groovy.transform.CompileStatic + @GE @groovy.transform.CompileStatic class Person { @Key long id @Indexed String name - @Ignore Order order } def key = new Person(name: 'test').save() @@ -444,6 +440,24 @@ class EntityTransformationSpec extends Specification { obj.pogo.id == obj.key.id } + def "AST Error test"(){ + def obj = newShell().evaluate ''' + import groovyx.gaelyk.datastore.Order + import groovyx.gaelyk.datastore.Entity as GE + import groovyx.gaelyk.datastore.Indexed + import groovyx.gaelyk.datastore.Ignore + import groovy.transform.Canonical + + @GE @Canonical + class Person { + @Ignore Order order + } + true + ''' + expect: + obj == true + } + /*@spock.lang.Ignore*/ def "Id is set 2"(){ DatastoreEntity obj = new Order()