From d218e55c8cbd51d43c24d9c5ab6d4b617e609663 Mon Sep 17 00:00:00 2001 From: Michael Hunger Date: Mon, 30 Jun 2014 09:44:55 +0200 Subject: [PATCH] DATAGRAPH-487 - Handle unique fields that have to be converted correctly --- .../data/neo4j/config/Neo4jConfiguration.java | 2 +- ...rtingNodePropertyFieldAccessorFactory.java | 8 +- ...nericNodePropertyFieldAccessorFactory.java | 5 +- .../neo4j/fieldaccess/PropertyConverter.java | 13 ++- .../MappingInfrastructureFactoryBean.java | 4 +- .../support/mapping/EntityStateHandler.java | 19 +++- .../mapping/Neo4jPersistentTestBase.java | 2 +- .../unique/UniqueConvertedValueTests.java | 97 +++++++++++++++++++ 8 files changed, 132 insertions(+), 18 deletions(-) create mode 100644 spring-data-neo4j/src/test/java/org/springframework/data/neo4j/unique/UniqueConvertedValueTests.java diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/config/Neo4jConfiguration.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/config/Neo4jConfiguration.java index 674ce96a1..01be1b9b4 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/config/Neo4jConfiguration.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/config/Neo4jConfiguration.java @@ -152,7 +152,7 @@ public abstract class Neo4jConfiguration { @Bean public EntityStateHandler entityStateHandler() throws Exception { - return new EntityStateHandler(neo4jMappingContext(),graphDatabase()); + return new EntityStateHandler(neo4jMappingContext(),graphDatabase(),neo4jConversionService()); } diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/ConvertingNodePropertyFieldAccessorFactory.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/ConvertingNodePropertyFieldAccessorFactory.java index a72d49d62..3f85c82a2 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/ConvertingNodePropertyFieldAccessorFactory.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/ConvertingNodePropertyFieldAccessorFactory.java @@ -63,18 +63,14 @@ public class ConvertingNodePropertyFieldAccessorFactory implements FieldAccessor @Override public Object setValue(final Object entity, final Object newVal, MappingPolicy mappingPolicy) { - Object value = propertyConverter.isObjectOrSupportedType(newVal, this.property) ? newVal : propertyConverter.serializePropertyValue(newVal); + Object value = propertyConverter.serializeIfNotBuiltIn(newVal); super.setValue(entity, value, mappingPolicy); return newVal; } @Override public Object doGetValue(final Object entity) { - Object ret = super.doGetValue(entity); - if (propertyConverter.isObjectOrSupportedType(ret, this.property)) { - return ret; - } - return propertyConverter.deserializePropertyValue(ret); + return propertyConverter.deserializeIfNotBuiltIn(super.doGetValue(entity)); } @Override diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/GenericNodePropertyFieldAccessorFactory.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/GenericNodePropertyFieldAccessorFactory.java index 73d633ee5..8a2f6c2c6 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/GenericNodePropertyFieldAccessorFactory.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/GenericNodePropertyFieldAccessorFactory.java @@ -17,7 +17,6 @@ package org.springframework.data.neo4j.fieldaccess; import org.springframework.beans.factory.annotation.Configurable; -import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.support.GenericConversionService; import org.springframework.data.neo4j.mapping.MappingPolicy; import org.springframework.data.neo4j.mapping.Neo4jPersistentProperty; @@ -61,7 +60,7 @@ public class GenericNodePropertyFieldAccessorFactory implements FieldAccessorFac @Override public Object setValue(final Object entity, final Object newVal, MappingPolicy mappingPolicy) { - Object value = propertyConverter.isObjectOrSupportedType(newVal, this.property) ? newVal : propertyConverter.serializePropertyValue(newVal); + Object value = propertyConverter.isObjectOrSupportedType(newVal) ? newVal : propertyConverter.serializePropertyValue(newVal); super.setValue(entity, value, mappingPolicy); return newVal; } @@ -69,7 +68,7 @@ public class GenericNodePropertyFieldAccessorFactory implements FieldAccessorFac @Override public Object doGetValue(final Object entity) { Object ret = super.doGetValue(entity); - if (propertyConverter.isObjectOrSupportedType(ret, this.property)) { + if (propertyConverter.isObjectOrSupportedType(ret)) { return ret; } return propertyConverter.deserializePropertyValue(ret); diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/PropertyConverter.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/PropertyConverter.java index edea85e75..578b6143d 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/PropertyConverter.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/PropertyConverter.java @@ -95,7 +95,18 @@ public class PropertyConverter { } } - boolean isObjectOrSupportedType(final Object value, Neo4jPersistentProperty property) { + boolean isObjectOrSupportedType(final Object value) { return property.getType().equals(Object.class) && property.isNeo4jPropertyValue(value); } + + public Object deserializeIfNotBuiltIn(Object ret) { + if (isObjectOrSupportedType(ret)) { + return ret; + } + return deserializePropertyValue(ret); + } + + public Object serializeIfNotBuiltIn(Object newVal) { + return isObjectOrSupportedType(newVal) ? newVal : serializePropertyValue(newVal); + } } diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/MappingInfrastructureFactoryBean.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/MappingInfrastructureFactoryBean.java index 66c31838e..7f72da10b 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/MappingInfrastructureFactoryBean.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/MappingInfrastructureFactoryBean.java @@ -123,7 +123,7 @@ public class MappingInfrastructureFactoryBean implements FactoryBean nodeEntityTools = new EntityTools(nodeTypeRepresentationStrategy, nodeEntityStateFactory, nodeEntityInstantiator, mappingContext); EntityTools relationshipEntityTools = new EntityTools(relationshipTypeRepresentationStrategy, relationshipEntityStateFactory, relationshipEntityInstantiator, mappingContext); this.entityPersister = new Neo4jEntityPersister(conversionService, nodeEntityTools, relationshipEntityTools, mappingContext, entityStateHandler); diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/mapping/EntityStateHandler.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/mapping/EntityStateHandler.java index 1480fa29f..a47d2f7ea 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/mapping/EntityStateHandler.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/mapping/EntityStateHandler.java @@ -21,8 +21,10 @@ import org.neo4j.graphdb.Node; import org.neo4j.graphdb.PropertyContainer; import org.neo4j.graphdb.Relationship; import org.neo4j.graphdb.RelationshipType; +import org.springframework.core.convert.ConversionService; import org.springframework.data.mapping.model.MappingException; import org.springframework.data.neo4j.core.GraphDatabase; +import org.springframework.data.neo4j.fieldaccess.PropertyConverter; import org.springframework.data.neo4j.mapping.IndexInfo; import org.springframework.data.neo4j.mapping.ManagedEntity; import org.springframework.data.neo4j.mapping.MappingPolicy; @@ -43,12 +45,14 @@ import static org.neo4j.helpers.collection.MapUtil.map; */ public class EntityStateHandler { - private Neo4jMappingContext mappingContext; + private final Neo4jMappingContext mappingContext; private final GraphDatabase graphDatabase; + private final ConversionService conversionService; - public EntityStateHandler(Neo4jMappingContext mappingContext, GraphDatabase graphDatabase) { + public EntityStateHandler(Neo4jMappingContext mappingContext, GraphDatabase graphDatabase, ConversionService conversionService) { this.mappingContext = mappingContext; this.graphDatabase = graphDatabase; + this.conversionService = conversionService; } @SuppressWarnings("unchecked") @@ -149,9 +153,9 @@ public class EntityStateHandler { private Node createUniqueNode(Neo4jPersistentEntityImpl persistentEntity, Object entity) { Neo4jPersistentProperty uniqueProperty = persistentEntity.getUniqueProperty(); - final IndexInfo indexInfo = uniqueProperty.getIndexInfo(); - final Object value = uniqueProperty.getValueFromEntity(entity, MappingPolicy.MAP_FIELD_DIRECT_POLICY); + final Object value = getSerializedUniqueValue(entity, uniqueProperty); if (value==null) throw new MappingException("Error creating "+uniqueProperty.getOwner().getName()+" with "+entity+" unique property "+uniqueProperty.getName()+" has null value"); + final IndexInfo indexInfo = uniqueProperty.getIndexInfo(); if (indexInfo.isLabelBased()) { return (indexInfo.isFailOnDuplicate()) ? graphDatabase.createNode(map(uniqueProperty.getName(),value),persistentEntity.getAllLabels()) @@ -161,6 +165,13 @@ public class EntityStateHandler { } } + private Object getSerializedUniqueValue(Object entity, Neo4jPersistentProperty uniqueProperty) { + final Object value = uniqueProperty.getValueFromEntity(entity, MappingPolicy.MAP_FIELD_DIRECT_POLICY); + if (uniqueProperty.isSerializablePropertyField(conversionService)) + return new PropertyConverter(conversionService,uniqueProperty).serializeIfNotBuiltIn(value); + return value; + } + @SuppressWarnings("unchecked") private S getOrCreateRelationship( Object entity, Neo4jPersistentEntity persistentEntity, RelationshipType annotationProvidedRelationshipType ) { final RelationshipProperties relationshipProperties = persistentEntity.getRelationshipProperties(); diff --git a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/mapping/Neo4jPersistentTestBase.java b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/mapping/Neo4jPersistentTestBase.java index 26930d49b..e7a64c8c6 100644 --- a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/mapping/Neo4jPersistentTestBase.java +++ b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/mapping/Neo4jPersistentTestBase.java @@ -112,7 +112,7 @@ public class Neo4jPersistentTestBase { final DelegatingGraphDatabase graphDatabase = new DelegatingGraphDatabase(gdb); factoryBean.setGraphDatabase(graphDatabase); factoryBean.setMappingContext(mappingContext); - final EntityStateHandler entityStateHandler = new EntityStateHandler(mappingContext, graphDatabase); + final EntityStateHandler entityStateHandler = new EntityStateHandler(mappingContext, graphDatabase, conversionService); final NoopNodeTypeRepresentationStrategy nodeTypeRepresentationStrategy = new NoopNodeTypeRepresentationStrategy(); factoryBean.setNodeTypeRepresentationStrategy(nodeTypeRepresentationStrategy); final NoopRelationshipTypeRepresentationStrategy relationshipTypeRepresentationStrategy = new NoopRelationshipTypeRepresentationStrategy(); diff --git a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/unique/UniqueConvertedValueTests.java b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/unique/UniqueConvertedValueTests.java new file mode 100644 index 000000000..ec2780869 --- /dev/null +++ b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/unique/UniqueConvertedValueTests.java @@ -0,0 +1,97 @@ +package org.springframework.data.neo4j.unique; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.neo4j.graphdb.GraphDatabaseService; +import org.neo4j.test.TestGraphDatabaseFactory; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.data.neo4j.annotation.GraphId; +import org.springframework.data.neo4j.annotation.Indexed; +import org.springframework.data.neo4j.annotation.NodeEntity; +import org.springframework.data.neo4j.config.EnableNeo4jRepositories; +import org.springframework.data.neo4j.config.Neo4jConfiguration; +import org.springframework.data.neo4j.support.Neo4jTemplate; +import org.springframework.data.neo4j.support.index.IndexType; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.transaction.annotation.Transactional; + +import static org.junit.Assert.assertEquals; + +/** + * @author mh + * @since 30.06.14 + */ + +@RunWith(SpringJUnit4ClassRunner.class) +@ContextConfiguration +public class UniqueConvertedValueTests { + + @Configuration + @EnableNeo4jRepositories + static class TestConfig extends Neo4jConfiguration { + TestConfig() { + setBasePackage("org.springframework.data.neo4j.unique"); + } + + @Bean + GraphDatabaseService graphDatabaseService() { + return new TestGraphDatabaseFactory().newImpermanentDatabase(); + } + } + + @Autowired + Neo4jTemplate template; + + static enum Letter { + A, B, C + } + + @NodeEntity + static class Word { + @GraphId + Long id; + @Indexed(unique = true, indexType = IndexType.LABEL) + Letter letter; + String word; + + Word() { } + + Word(Letter letter, String word) { + this.letter = letter; + this.word = word; + } + } + @NodeEntity + static class Word2 { + @GraphId + Long id; + @Indexed(unique = true, indexType = IndexType.SIMPLE) + Letter letter; + String word; + + Word2() { } + + Word2(Letter letter, String word) { + this.letter = letter; + this.word = word; + } + } + + @Test + @Transactional + public void testPersistUniqueEnumValue() throws Exception { + Word word = template.save(new Word(Letter.A, "Afternoon")); + assertEquals(Letter.A, word.letter); + assertEquals("Afternoon", word.word); + } + @Test + @Transactional + public void testPersistUniqueEnumValueLegacyIndex() throws Exception { + Word word = template.save(new Word(Letter.A, "Afternoon")); + assertEquals(Letter.A, word.letter); + assertEquals("Afternoon", word.word); + } +}