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.
This commit is contained in:
Michael Simons
2019-01-08 18:03:31 +01:00
committed by Gerrit Meier
parent b487148107
commit 62d77d6d65
6 changed files with 41 additions and 56 deletions

View File

@@ -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<Neo4jPersistentEntity<?>, Neo4jPersistentProperty> context;
private final Neo4jMappingContext context;
private ConversionService conversionService;
private final EntityInstantiators instantiators;
public Neo4jOgmEntityInstantiatorAdapter(MappingContext<Neo4jPersistentEntity<?>, 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> T createInstance(Class<T> clazz, Map<String, Object> propertyValues) {
Neo4jPersistentEntity<T> persistentEntity = (Neo4jPersistentEntity<T>) 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<Neo4jPersistentProperty> getParameterProvider(Map<String, Object> propertyValues,

View File

@@ -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<Neo4jPersistentE
private static final Logger logger = LoggerFactory.getLogger(Neo4jMappingContext.class);
// Instantiation of the EntityInstantiators must happen only once, otherwise
// the org.springframework.data.convert.ClassGeneratingEntityInstantiator will
// be created every time an instantiator is requested and thus the whole thing
// will dynamically create classes like there is no tomorrow.
private final EntityInstantiators instantiators = new EntityInstantiators();
private final MetaData metaData;
/**
@@ -87,6 +95,10 @@ public class Neo4jMappingContext extends AbstractMappingContext<Neo4jPersistentE
updateSimpleTypeHolder(simpleTypeHolder, propertyField));
}
public EntityInstantiator getInstantiatorFor(PersistentEntity<?, ?> entity) {
return instantiators.getInstantiatorFor(entity);
}
private SimpleTypeHolder updateSimpleTypeHolder(SimpleTypeHolder currentSimpleTypeHolder, Field field) {
if (field == null) {
return currentSimpleTypeHolder;

View File

@@ -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<ConversionService> conversionServiceObjectProvider) {

View File

@@ -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<AbstractBeanDefinition> 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);
}
/**

View File

@@ -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<Object, Object> {
private final MetaData metaData;
private final Class returnedType;
private final MappingContext<Neo4jPersistentEntity<?>, Neo4jPersistentProperty> mappingContext;
CustomResultConverter(MetaData metaData, Class<?> returnedType,
@Nullable MappingContext<Neo4jPersistentEntity<?>, 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<Object, Object> {
new QueryResultProxy((Map<String, Object>) source));
}
SingleUseEntityMapper mapper;
if (HAS_ENTITY_INSTANTIATOR_FEATURE) {
EntityInstantiator entityInstantiator = new QueryResultInstantiator(metaData, mappingContext);
Optional<Constructor<?>> 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<String, Object>) source);
}
}

View File

@@ -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) {