From 62d77d6d6558d1323140a6ce2722f0e0c8f7fbf4 Mon Sep 17 00:00:00 2001 From: Michael Simons Date: Tue, 8 Jan 2019 18:03:31 +0100 Subject: [PATCH] DATAGRAPH-1174 - Fix class loading leak with custom QueryResults. * Optimize CustomResultConverter. * Remove checks for instantiator * Instantiate instantiator only once * Use generated name for config bean. This is actually a bug fix. We need a config bean for each session factory, not only one. --- .../Neo4jOgmEntityInstantiatorAdapter.java | 17 ++++------ .../neo4j/mapping/Neo4jMappingContext.java | 12 +++++++ ...gmEntityInstantiatorConfigurationBean.java | 2 +- ...Neo4jRepositoryConfigurationExtension.java | 20 ++++-------- .../query/CustomResultConverter.java | 32 ++++--------------- .../query/GraphRepositoryQuery.java | 14 +++++--- 6 files changed, 41 insertions(+), 56 deletions(-) diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/conversion/Neo4jOgmEntityInstantiatorAdapter.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/conversion/Neo4jOgmEntityInstantiatorAdapter.java index 29ec1be9f..c0451eb7c 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/conversion/Neo4jOgmEntityInstantiatorAdapter.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/conversion/Neo4jOgmEntityInstantiatorAdapter.java @@ -15,12 +15,12 @@ package org.springframework.data.neo4j.conversion; import java.util.Map; -import org.neo4j.ogm.session.EntityInstantiator; import org.springframework.core.convert.ConversionService; -import org.springframework.data.convert.EntityInstantiators; +import org.springframework.data.convert.EntityInstantiator; import org.springframework.data.mapping.PreferredConstructor; import org.springframework.data.mapping.context.MappingContext; import org.springframework.data.mapping.model.ParameterValueProvider; +import org.springframework.data.neo4j.mapping.Neo4jMappingContext; import org.springframework.data.neo4j.mapping.Neo4jPersistentEntity; import org.springframework.data.neo4j.mapping.Neo4jPersistentProperty; import org.springframework.lang.Nullable; @@ -32,19 +32,17 @@ import org.springframework.util.Assert; * @author Nicolas Mervaillie * @author Michael J. Simons */ -public class Neo4jOgmEntityInstantiatorAdapter implements EntityInstantiator { +public class Neo4jOgmEntityInstantiatorAdapter implements org.neo4j.ogm.session.EntityInstantiator { - private final MappingContext, Neo4jPersistentProperty> context; + private final Neo4jMappingContext context; private ConversionService conversionService; - private final EntityInstantiators instantiators; public Neo4jOgmEntityInstantiatorAdapter(MappingContext, Neo4jPersistentProperty> context, @Nullable ConversionService conversionService) { Assert.notNull(context, "MappingContext cannot be null"); - this.context = context; + this.context = (Neo4jMappingContext) context; this.conversionService = conversionService; - instantiators = new EntityInstantiators(); } @SuppressWarnings({ "unchecked" }) @@ -52,10 +50,9 @@ public class Neo4jOgmEntityInstantiatorAdapter implements EntityInstantiator { public T createInstance(Class clazz, Map propertyValues) { Neo4jPersistentEntity persistentEntity = (Neo4jPersistentEntity) context.getRequiredPersistentEntity(clazz); - org.springframework.data.convert.EntityInstantiator instantiator = instantiators - .getInstantiatorFor(persistentEntity); + EntityInstantiator sdnInstantiator = context.getInstantiatorFor(persistentEntity); - return instantiator.createInstance(persistentEntity, getParameterProvider(propertyValues, conversionService)); + return sdnInstantiator.createInstance(persistentEntity, getParameterProvider(propertyValues, conversionService)); } private ParameterValueProvider getParameterProvider(Map propertyValues, diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/mapping/Neo4jMappingContext.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/mapping/Neo4jMappingContext.java index e4a31ad55..d15f96f4d 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/mapping/Neo4jMappingContext.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/mapping/Neo4jMappingContext.java @@ -22,6 +22,9 @@ import org.neo4j.ogm.metadata.FieldInfo; import org.neo4j.ogm.metadata.MetaData; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.data.convert.EntityInstantiator; +import org.springframework.data.convert.EntityInstantiators; +import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.context.AbstractMappingContext; import org.springframework.data.mapping.model.Property; import org.springframework.data.mapping.model.SimpleTypeHolder; @@ -41,6 +44,11 @@ public class Neo4jMappingContext extends AbstractMappingContext entity) { + return instantiators.getInstantiatorFor(entity); + } + private SimpleTypeHolder updateSimpleTypeHolder(SimpleTypeHolder currentSimpleTypeHolder, Field field) { if (field == null) { return currentSimpleTypeHolder; diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/config/Neo4jOgmEntityInstantiatorConfigurationBean.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/config/Neo4jOgmEntityInstantiatorConfigurationBean.java index 31dca3899..6e671067d 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/config/Neo4jOgmEntityInstantiatorConfigurationBean.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/config/Neo4jOgmEntityInstantiatorConfigurationBean.java @@ -29,7 +29,7 @@ import org.springframework.data.neo4j.mapping.Neo4jMappingContext; * @author Gerrit Meier * @author Michael J. Simons */ -public class Neo4jOgmEntityInstantiatorConfigurationBean { +class Neo4jOgmEntityInstantiatorConfigurationBean { public Neo4jOgmEntityInstantiatorConfigurationBean(SessionFactory sessionFactory, Neo4jMappingContext mappingContext, ObjectProvider conversionServiceObjectProvider) { diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/config/Neo4jRepositoryConfigurationExtension.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/config/Neo4jRepositoryConfigurationExtension.java index 358fb402a..7a34839c0 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/config/Neo4jRepositoryConfigurationExtension.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/config/Neo4jRepositoryConfigurationExtension.java @@ -17,7 +17,6 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Optional; -import java.util.function.Supplier; import org.neo4j.ogm.annotation.NodeEntity; import org.neo4j.ogm.annotation.RelationshipEntity; @@ -61,15 +60,11 @@ public class Neo4jRepositoryConfigurationExtension extends RepositoryConfigurati static final String DEFAULT_SESSION_FACTORY_BEAN_NAME = "sessionFactory"; static final String DEFAULT_TRANSACTION_MANAGER_BEAN_NAME = "transactionManager"; - private static final String ENTITY_INSTANTIATOR_CONFIGURATION_BEAN_NAME = "neo4jOgmEntityInstantiatorConfigurationBean"; private static final String ENABLE_DEFAULT_TRANSACTIONS_ATTRIBUTE = "enableDefaultTransactions"; private static final String NEO4J_PERSISTENCE_EXCEPTION_TRANSLATOR_NAME = "neo4jPersistenceExceptionTranslator"; private static final String MODULE_PREFIX = "neo4j"; private static final String MODULE_NAME = "Neo4j"; - public static final boolean HAS_ENTITY_INSTANTIATOR_FEATURE = ClassUtils.isPresent( - "org.neo4j.ogm.session.EntityInstantiator", Neo4jRepositoryConfigurationExtension.class.getClassLoader()); - /** * We use a generated name for every pair of {@code SessionFactory} and {@code Session} unless the user configures a * session bean name with {@code @EnableNeo4jRepositories(sessionBeanName="someName")}. @@ -204,15 +199,12 @@ public class Neo4jRepositoryConfigurationExtension extends RepositoryConfigurati registerIfNotAlreadyRegistered(() -> new RootBeanDefinition(Neo4jPersistenceExceptionTranslator.class), registry, NEO4J_PERSISTENCE_EXCEPTION_TRANSLATOR_NAME, source); - if (HAS_ENTITY_INSTANTIATOR_FEATURE) { - - Supplier rootBeanDefinition = BeanDefinitionBuilder - .rootBeanDefinition(Neo4jOgmEntityInstantiatorConfigurationBean.class) - .setAutowireMode(AbstractBeanDefinition.AUTOWIRE_BY_TYPE) - .addConstructorArgReference(getSessionFactoryBeanName(config)) - .addConstructorArgReference(this.neo4jMappingContextBeanName)::getBeanDefinition; - registerIfNotAlreadyRegistered(rootBeanDefinition, registry, ENTITY_INSTANTIATOR_CONFIGURATION_BEAN_NAME, source); - } + AbstractBeanDefinition rootBeanDefinition = BeanDefinitionBuilder + .rootBeanDefinition(Neo4jOgmEntityInstantiatorConfigurationBean.class) + .setAutowireMode(AbstractBeanDefinition.AUTOWIRE_BY_TYPE) + .addConstructorArgReference(getSessionFactoryBeanName(config)) + .addConstructorArgReference(this.neo4jMappingContextBeanName).getBeanDefinition(); + registerWithGeneratedNameOrUseConfigured(rootBeanDefinition, registry, GENERATE_BEAN_NAME, source); } /** diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/CustomResultConverter.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/CustomResultConverter.java index 0fadbcbba..fa186bde7 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/CustomResultConverter.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/CustomResultConverter.java @@ -14,24 +14,13 @@ package org.springframework.data.neo4j.repository.query; import static java.lang.reflect.Proxy.*; -import static org.springframework.data.neo4j.repository.config.Neo4jRepositoryConfigurationExtension.*; -import java.lang.reflect.Constructor; import java.util.Map; -import java.util.Optional; import org.neo4j.ogm.context.SingleUseEntityMapper; import org.neo4j.ogm.metadata.MetaData; -import org.neo4j.ogm.metadata.reflect.EntityFactory; -import org.neo4j.ogm.session.EntityInstantiator; -import org.springframework.beans.BeanUtils; import org.springframework.core.convert.converter.Converter; -import org.springframework.data.mapping.context.MappingContext; import org.springframework.data.neo4j.annotation.QueryResult; -import org.springframework.data.neo4j.mapping.Neo4jPersistentEntity; -import org.springframework.data.neo4j.mapping.Neo4jPersistentProperty; -import org.springframework.data.util.ReflectionUtils; -import org.springframework.lang.Nullable; /** * Convert OGM special {@link QueryResult} annotated types into SD understandable type. @@ -44,19 +33,20 @@ class CustomResultConverter implements Converter { private final MetaData metaData; private final Class returnedType; - private final MappingContext, Neo4jPersistentProperty> mappingContext; - CustomResultConverter(MetaData metaData, Class returnedType, - @Nullable MappingContext, Neo4jPersistentProperty> mappingContext) { + private final QueryResultInstantiator entityInstantiator; + + CustomResultConverter(MetaData metaData, Class returnedType, QueryResultInstantiator entityInstantiator) { this.metaData = metaData; this.returnedType = returnedType; - this.mappingContext = mappingContext; + this.entityInstantiator = entityInstantiator; } @Override @SuppressWarnings("unchecked") public Object convert(Object source) { + if (returnedType.getAnnotation(QueryResult.class) == null) { return source; } @@ -66,17 +56,7 @@ class CustomResultConverter implements Converter { new QueryResultProxy((Map) source)); } - SingleUseEntityMapper mapper; - if (HAS_ENTITY_INSTANTIATOR_FEATURE) { - EntityInstantiator entityInstantiator = new QueryResultInstantiator(metaData, mappingContext); - Optional> optionalConstructor = ReflectionUtils.findConstructor(SingleUseEntityMapper.class, - metaData, entityInstantiator); - // the constructor must exist - Constructor constructor = optionalConstructor.get(); - mapper = (SingleUseEntityMapper) BeanUtils.instantiateClass(constructor, metaData, entityInstantiator); - } else { - mapper = new SingleUseEntityMapper(metaData, new EntityFactory(metaData)); - } + SingleUseEntityMapper mapper = new SingleUseEntityMapper(metaData, entityInstantiator); return mapper.map(returnedType, (Map) source); } } diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/GraphRepositoryQuery.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/GraphRepositoryQuery.java index 2970daf2b..09e942ddc 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/GraphRepositoryQuery.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/GraphRepositoryQuery.java @@ -43,6 +43,8 @@ public class GraphRepositoryQuery extends AbstractGraphRepositoryQuery { private static final Logger LOG = LoggerFactory.getLogger(GraphRepositoryQuery.class); private final QueryMethodEvaluationContextProvider evaluationContextProvider; + private final QueryResultInstantiator entityInstantiator; + private ParameterizedQuery parameterizedQuery; GraphRepositoryQuery(GraphQueryMethod graphQueryMethod, MetaData metaData, Session session, @@ -52,6 +54,7 @@ public class GraphRepositoryQuery extends AbstractGraphRepositoryQuery { super(graphQueryMethod, metaData, session); this.evaluationContextProvider = evaluationContextProvider; + this.entityInstantiator = new QueryResultInstantiator(metaData, queryMethod.getMappingContext()); } protected Object doExecute(Query query, Object[] parameters) { @@ -61,15 +64,16 @@ public class GraphRepositoryQuery extends AbstractGraphRepositoryQuery { } GraphParameterAccessor accessor = new GraphParametersParameterAccessor(queryMethod, parameters); - Class returnType = queryMethod.getMethod().getReturnType(); - ResultProcessor processor = queryMethod.getResultProcessor().withDynamicProjection(accessor); - Object result = getExecution(accessor).execute(query, processor.getReturnedType().getReturnedType()); + Class methodReturnType = queryMethod.getMethod().getReturnType(); + Class processorReturnType = processor.getReturnedType().getReturnedType(); - return Result.class.equals(returnType) ? result + Object result = getExecution(accessor).execute(query, processorReturnType); + + return Result.class.equals(methodReturnType) ? result : processor.processResult(result, new CustomResultConverter(metaData, - processor.getReturnedType().getReturnedType(), queryMethod.getMappingContext())); + processorReturnType, entityInstantiator)); } protected Query getQuery(Object[] parameters) {