Skip to content

Commit

Permalink
HSEARCH-3170 Close container value extractor bean holders as appropriate
Browse files Browse the repository at this point in the history
  • Loading branch information
yrodiere committed Dec 11, 2018
1 parent a446b40 commit a85ca06
Show file tree
Hide file tree
Showing 10 changed files with 73 additions and 51 deletions.
Expand Up @@ -15,10 +15,10 @@

import org.hibernate.search.mapper.pojo.dirtiness.ReindexOnUpdate;
import org.hibernate.search.mapper.pojo.dirtiness.impl.PojoImplicitReindexingResolver;
import org.hibernate.search.mapper.pojo.extractor.ContainerValueExtractor;
import org.hibernate.search.mapper.pojo.extractor.ContainerValueExtractorPath;
import org.hibernate.search.mapper.pojo.extractor.impl.BoundContainerValueExtractorPath;
import org.hibernate.search.mapper.pojo.extractor.impl.ContainerValueExtractorBinder;
import org.hibernate.search.mapper.pojo.extractor.impl.ContainerValueExtractorHolder;
import org.hibernate.search.mapper.pojo.model.additionalmetadata.building.impl.PojoTypeAdditionalMetadataProvider;
import org.hibernate.search.mapper.pojo.model.additionalmetadata.impl.PojoTypeAdditionalMetadata;
import org.hibernate.search.mapper.pojo.model.path.PojoModelPathValueNode;
Expand Down Expand Up @@ -129,7 +129,7 @@ <T> PojoImplicitReindexingResolverBuilder<T> getOrCreateResolverBuilder(
return extractorBinder.bindPath( typeModel, extractorPath );
}

<V, T> ContainerValueExtractor<? super T, V> createExtractors(
<V, T> ContainerValueExtractorHolder<T, V> createExtractors(
BoundContainerValueExtractorPath<T, V> boundExtractorPath) {
return extractorBinder.create( boundExtractorPath );
}
Expand Down
Expand Up @@ -12,7 +12,7 @@

import org.hibernate.search.mapper.pojo.dirtiness.impl.PojoImplicitReindexingResolverNode;
import org.hibernate.search.mapper.pojo.dirtiness.impl.PojoImplicitReindexingResolverContainerElementNode;
import org.hibernate.search.mapper.pojo.extractor.ContainerValueExtractor;
import org.hibernate.search.mapper.pojo.extractor.impl.ContainerValueExtractorHolder;
import org.hibernate.search.mapper.pojo.model.path.PojoModelPathValueNode;
import org.hibernate.search.mapper.pojo.model.path.impl.BoundPojoModelPathValueNode;
import org.hibernate.search.mapper.pojo.model.path.spi.PojoPathFilterFactory;
Expand All @@ -22,15 +22,15 @@ class PojoImplicitReindexingResolverContainerElementNodeBuilder<C, V>
extends AbstractPojoImplicitReindexingResolverNodeBuilder<C> {

private final BoundPojoModelPathValueNode<?, ? extends C, V> modelPath;
private final ContainerValueExtractor<C, V> extractor;
private final ContainerValueExtractorHolder<C, V> extractorHolder;
private final PojoImplicitReindexingResolverValueNodeBuilderDelegate<V> valueBuilderDelegate;

PojoImplicitReindexingResolverContainerElementNodeBuilder(BoundPojoModelPathValueNode<?, ? extends C, V> modelPath,
ContainerValueExtractor<C, V> extractor,
ContainerValueExtractorHolder<C, V> extractorHolder,
PojoImplicitReindexingResolverBuildingHelper buildingHelper) {
super( buildingHelper );
this.modelPath = modelPath;
this.extractor = extractor;
this.extractorHolder = extractorHolder;
this.valueBuilderDelegate =
new PojoImplicitReindexingResolverValueNodeBuilderDelegate<>( modelPath, buildingHelper );
}
Expand All @@ -43,7 +43,7 @@ class PojoImplicitReindexingResolverContainerElementNodeBuilder<C, V>
@Override
void closeOnFailure() {
try ( Closer<RuntimeException> closer = new Closer<>() ) {
// TODO HSEARCH-3170 release the extractor beans
closer.push( ContainerValueExtractorHolder::close, extractorHolder );
closer.push( PojoImplicitReindexingResolverValueNodeBuilderDelegate::closeOnFailure, valueBuilderDelegate );
}
}
Expand Down Expand Up @@ -73,7 +73,7 @@ <S> Optional<PojoImplicitReindexingResolverNode<C, S>> doBuild(PojoPathFilterFac
}
else {
return Optional.of( new PojoImplicitReindexingResolverContainerElementNode<>(
extractor, valueTypeNodes
extractorHolder, valueTypeNodes
) );
}
}
Expand Down
Expand Up @@ -16,9 +16,9 @@

import org.hibernate.search.mapper.pojo.dirtiness.impl.PojoImplicitReindexingResolverNode;
import org.hibernate.search.mapper.pojo.dirtiness.impl.PojoImplicitReindexingResolverPropertyNode;
import org.hibernate.search.mapper.pojo.extractor.ContainerValueExtractor;
import org.hibernate.search.mapper.pojo.extractor.ContainerValueExtractorPath;
import org.hibernate.search.mapper.pojo.extractor.impl.BoundContainerValueExtractorPath;
import org.hibernate.search.mapper.pojo.extractor.impl.ContainerValueExtractorHolder;
import org.hibernate.search.mapper.pojo.model.path.PojoModelPathValueNode;
import org.hibernate.search.mapper.pojo.model.path.impl.BoundPojoModelPathPropertyNode;
import org.hibernate.search.mapper.pojo.model.path.impl.BoundPojoModelPathValueNode;
Expand Down Expand Up @@ -141,11 +141,11 @@ <S> Optional<PojoImplicitReindexingResolverNode<T, S>> doBuild(PojoPathFilterFac
*/
private <V> PojoImplicitReindexingResolverContainerElementNodeBuilder<? super P, V>
createContainerBuilder(BoundContainerValueExtractorPath<P, V> boundExtractorPath) {
ContainerValueExtractor<? super P, V> extractor =
ContainerValueExtractorHolder<P, V> extractorHolder =
buildingHelper.createExtractors( boundExtractorPath );
BoundPojoModelPathValueNode<T, P, V> containerElementPath = modelPath.value( boundExtractorPath );
return new PojoImplicitReindexingResolverContainerElementNodeBuilder<>(
containerElementPath, extractor, buildingHelper
containerElementPath, extractorHolder, buildingHelper
);
}

Expand Down
Expand Up @@ -9,7 +9,7 @@
import java.util.Collection;
import java.util.stream.Stream;

import org.hibernate.search.mapper.pojo.extractor.ContainerValueExtractor;
import org.hibernate.search.mapper.pojo.extractor.impl.ContainerValueExtractorHolder;
import org.hibernate.search.mapper.pojo.model.spi.PojoRuntimeIntrospector;
import org.hibernate.search.util.impl.common.Closer;
import org.hibernate.search.util.impl.common.ToStringTreeBuilder;
Expand All @@ -30,27 +30,27 @@
public class PojoImplicitReindexingResolverContainerElementNode<C, S, V>
extends PojoImplicitReindexingResolverNode<C, S> {

private final ContainerValueExtractor<C, V> extractor;
private final ContainerValueExtractorHolder<C, V> extractorHolder;
private final Collection<PojoImplicitReindexingResolverNode<V, S>> nestedNodes;

public PojoImplicitReindexingResolverContainerElementNode(ContainerValueExtractor<C, V> extractor,
public PojoImplicitReindexingResolverContainerElementNode(ContainerValueExtractorHolder<C, V> extractorHolder,
Collection<PojoImplicitReindexingResolverNode<V, S>> nestedNodes) {
this.extractor = extractor;
this.extractorHolder = extractorHolder;
this.nestedNodes = nestedNodes;
}

@Override
public void close() {
try ( Closer<RuntimeException> closer = new Closer<>() ) {
// TODO HSEARCH-3170 release the extractor beans
closer.push( ContainerValueExtractorHolder::close, extractorHolder );
closer.pushAll( PojoImplicitReindexingResolverNode::close, nestedNodes );
}
}

@Override
public void appendTo(ToStringTreeBuilder builder) {
builder.attribute( "class", getClass().getSimpleName() );
builder.attribute( "extractor", extractor );
builder.attribute( "extractor", extractorHolder.get() );
builder.startList( "nestedNodes" );
for ( PojoImplicitReindexingResolverNode<?, ?> nestedNode : nestedNodes ) {
builder.value( nestedNode );
Expand All @@ -61,7 +61,7 @@ public void appendTo(ToStringTreeBuilder builder) {
@Override
public void resolveEntitiesToReindex(PojoReindexingCollector collector,
PojoRuntimeIntrospector runtimeIntrospector, C dirty, S dirtinessState) {
try ( Stream<V> stream = extractor.extract( dirty ) ) {
try ( Stream<V> stream = extractorHolder.get().extract( dirty ) ) {
stream.forEach( containerElement -> resolveEntitiesToReindexForContainerElement(
collector, runtimeIntrospector, containerElement, dirtinessState
) );
Expand Down
Expand Up @@ -14,6 +14,7 @@
import java.util.Map;
import java.util.Optional;

import org.hibernate.search.engine.environment.bean.BeanHolder;
import org.hibernate.search.engine.environment.bean.BeanProvider;
import org.hibernate.search.engine.mapper.mapping.spi.MappingBuildContext;
import org.hibernate.search.mapper.pojo.extractor.ContainerValueExtractor;
Expand All @@ -33,6 +34,7 @@
import org.hibernate.search.mapper.pojo.util.impl.GenericTypeContext;
import org.hibernate.search.util.AssertionFailure;
import org.hibernate.search.util.impl.common.LoggerFactory;
import org.hibernate.search.util.impl.common.SuppressingCloser;

/**
* Binds {@link ContainerValueExtractorPath}s to a given input type,
Expand Down Expand Up @@ -165,28 +167,37 @@ public ContainerValueExtractorBinder(MappingBuildContext buildContext,
*/
// Checks are performed using reflection when building the resolved path
@SuppressWarnings( {"rawtypes", "unchecked"} )
public <C, V> ContainerValueExtractor<? super C, V> create(BoundContainerValueExtractorPath<C, V> boundPath) {
public <C, V> ContainerValueExtractorHolder<C, V> create(BoundContainerValueExtractorPath<C, V> boundPath) {
if ( boundPath.getExtractorPath().isEmpty() ) {
throw new AssertionFailure(
"Received a request to create extractors, but the extractor path was empty."
+ " There is probably a bug in Hibernate Search."
);
}
ContainerValueExtractor<? super C, ?> extractor = null;
for ( Class<? extends ContainerValueExtractor> extractorClass :
boundPath.getExtractorPath().getExplicitExtractorClasses() ) {
// TODO HSEARCH-3170 properly handle the release of container value extractor beans
ContainerValueExtractor<?, ?> newExtractor =
beanProvider.getBean( extractorClass ).get();
if ( extractor == null ) {
// First extractor: must be able to process type C
extractor = (ContainerValueExtractor<? super C, ?>) newExtractor;
}
else {
extractor = new ChainingContainerValueExtractor( extractor, newExtractor );
List<BeanHolder<?>> beanHolders = new ArrayList<>();
try {
for ( Class<? extends ContainerValueExtractor> extractorClass :
boundPath.getExtractorPath().getExplicitExtractorClasses() ) {
BeanHolder<? extends ContainerValueExtractor> newExtractorHolder =
beanProvider.getBean( extractorClass );
beanHolders.add( newExtractorHolder );
if ( extractor == null ) {
// First extractor: must be able to process type C
extractor = (ContainerValueExtractor<? super C, ?>) newExtractorHolder.get();
}
else {
extractor = new ChainingContainerValueExtractor( extractor, newExtractorHolder.get() );
}
}
return new ContainerValueExtractorHolder<>(
(ContainerValueExtractor<? super C, V>) extractor, beanHolders
);
}
catch (RuntimeException e) {
new SuppressingCloser( e ).pushAll( BeanHolder::close, beanHolders );
throw e;
}
return (ContainerValueExtractor<C, V>) extractor;
}

public boolean isDefaultExtractorPath(PojoGenericTypeModel<?> sourceType, ContainerValueExtractorPath extractorPath) {
Expand Down
Expand Up @@ -23,6 +23,7 @@
import org.hibernate.search.mapper.pojo.extractor.ContainerValueExtractor;
import org.hibernate.search.mapper.pojo.extractor.ContainerValueExtractorPath;
import org.hibernate.search.mapper.pojo.extractor.impl.BoundContainerValueExtractorPath;
import org.hibernate.search.mapper.pojo.extractor.impl.ContainerValueExtractorHolder;
import org.hibernate.search.mapper.pojo.model.path.impl.BoundPojoModelPathPropertyNode;
import org.hibernate.search.mapper.pojo.model.path.impl.BoundPojoModelPathTypeNode;
import org.hibernate.search.mapper.pojo.model.path.impl.BoundPojoModelPathValueNode;
Expand All @@ -47,7 +48,7 @@ public interface PojoIndexModelBinder {
<C> BoundContainerValueExtractorPath<C, ?> bindExtractorPath(
PojoGenericTypeModel<C> pojoGenericTypeModel, ContainerValueExtractorPath extractorPath);

<C, V> ContainerValueExtractor<? super C, V> createExtractors(
<C, V> ContainerValueExtractorHolder<C, V> createExtractors(
BoundContainerValueExtractorPath<C, V> boundExtractorPath);

<I> BeanHolder<? extends IdentifierBridge<I>> addIdentifierBridge(BoundPojoModelPathPropertyNode<?, I> modelPath,
Expand Down
Expand Up @@ -31,10 +31,10 @@
import org.hibernate.search.mapper.pojo.bridge.impl.BridgeResolver;
import org.hibernate.search.mapper.pojo.bridge.mapping.BridgeBuildContext;
import org.hibernate.search.mapper.pojo.bridge.mapping.BridgeBuilder;
import org.hibernate.search.mapper.pojo.extractor.ContainerValueExtractor;
import org.hibernate.search.mapper.pojo.extractor.ContainerValueExtractorPath;
import org.hibernate.search.mapper.pojo.extractor.impl.BoundContainerValueExtractorPath;
import org.hibernate.search.mapper.pojo.extractor.impl.ContainerValueExtractorBinder;
import org.hibernate.search.mapper.pojo.extractor.impl.ContainerValueExtractorHolder;
import org.hibernate.search.mapper.pojo.logging.impl.Log;
import org.hibernate.search.mapper.pojo.model.additionalmetadata.building.impl.PojoTypeAdditionalMetadataProvider;
import org.hibernate.search.mapper.pojo.model.impl.PojoModelPropertyRootElement;
Expand Down Expand Up @@ -78,7 +78,7 @@ public class PojoIndexModelBinderImpl implements PojoIndexModelBinder {
}

@Override
public <C, V> ContainerValueExtractor<? super C, V> createExtractors(
public <C, V> ContainerValueExtractorHolder<C, V> createExtractors(
BoundContainerValueExtractorPath<C, V> boundExtractorPath) {
return extractorBinder.create( boundExtractorPath );
}
Expand Down
Expand Up @@ -11,12 +11,13 @@

import org.hibernate.search.engine.mapper.mapping.building.spi.IndexModelBindingContext;
import org.hibernate.search.mapper.pojo.dirtiness.building.impl.PojoIndexingDependencyCollectorPropertyNode;
import org.hibernate.search.mapper.pojo.extractor.ContainerValueExtractor;
import org.hibernate.search.mapper.pojo.extractor.impl.ContainerValueExtractorHolder;
import org.hibernate.search.mapper.pojo.mapping.building.spi.PojoMappingCollectorValueNode;
import org.hibernate.search.mapper.pojo.mapping.building.impl.PojoMappingHelper;
import org.hibernate.search.mapper.pojo.model.path.impl.BoundPojoModelPathValueNode;
import org.hibernate.search.mapper.pojo.processing.impl.PojoIndexingProcessor;
import org.hibernate.search.mapper.pojo.processing.impl.PojoIndexingProcessorContainerElementNode;
import org.hibernate.search.util.impl.common.Closer;

/**
* A builder of {@link PojoIndexingProcessorContainerElementNode}.
Expand All @@ -28,16 +29,16 @@
class PojoIndexingProcessorContainerElementNodeBuilder<P extends C, C, V> extends AbstractPojoProcessorNodeBuilder {

private final BoundPojoModelPathValueNode<?, P, V> modelPath;
private final ContainerValueExtractor<C, V> extractor;
private final ContainerValueExtractorHolder<C, V> extractorHolder;

private final PojoIndexingProcessorValueNodeBuilderDelegate<P, V> valueNodeProcessorCollectionBuilder;

PojoIndexingProcessorContainerElementNodeBuilder(BoundPojoModelPathValueNode<?, P, V> modelPath,
ContainerValueExtractor<C, V> extractor,
ContainerValueExtractorHolder<C, V> extractorHolder,
PojoMappingHelper mappingHelper, IndexModelBindingContext bindingContext) {
super( mappingHelper, bindingContext );
this.modelPath = modelPath;
this.extractor = extractor;
this.extractorHolder = extractorHolder;

valueNodeProcessorCollectionBuilder = new PojoIndexingProcessorValueNodeBuilderDelegate<>(
modelPath,
Expand All @@ -56,7 +57,13 @@ public PojoMappingCollectorValueNode value() {

@Override
void closeOnFailure() {
valueNodeProcessorCollectionBuilder.closeOnFailure();
try ( Closer<RuntimeException> closer = new Closer<>() ) {
closer.pushAll( ContainerValueExtractorHolder::close, extractorHolder );
closer.pushAll(
PojoIndexingProcessorValueNodeBuilderDelegate::closeOnFailure,
valueNodeProcessorCollectionBuilder
);
}
}

Optional<PojoIndexingProcessorContainerElementNode<C, V>> build(
Expand All @@ -78,13 +85,15 @@ private Optional<PojoIndexingProcessorContainerElementNode<C, V>> doBuild(
if ( immutableNestedProcessors.isEmpty() ) {
/*
* If this processor doesn't have any bridge, nor any nested processor,
* it is useless and we don't need to build it
* it is useless and we don't need to build it.
* Release the other resources (the container value extractors) and return.
*/
extractorHolder.close();
return Optional.empty();
}
else {
return Optional.of( new PojoIndexingProcessorContainerElementNode<>(
extractor, immutableNestedProcessors
extractorHolder, immutableNestedProcessors
) );
}
}
Expand Down
Expand Up @@ -21,9 +21,9 @@
import org.hibernate.search.mapper.pojo.bridge.mapping.BridgeBuilder;
import org.hibernate.search.mapper.pojo.dirtiness.building.impl.PojoIndexingDependencyCollectorPropertyNode;
import org.hibernate.search.mapper.pojo.dirtiness.building.impl.PojoIndexingDependencyCollectorTypeNode;
import org.hibernate.search.mapper.pojo.extractor.ContainerValueExtractor;
import org.hibernate.search.mapper.pojo.extractor.ContainerValueExtractorPath;
import org.hibernate.search.mapper.pojo.extractor.impl.BoundContainerValueExtractorPath;
import org.hibernate.search.mapper.pojo.extractor.impl.ContainerValueExtractorHolder;
import org.hibernate.search.mapper.pojo.mapping.building.impl.BoundPropertyBridge;
import org.hibernate.search.mapper.pojo.mapping.building.impl.PojoIdentityMappingCollector;
import org.hibernate.search.mapper.pojo.mapping.building.impl.PojoMappingHelper;
Expand Down Expand Up @@ -124,11 +124,11 @@ public PojoMappingCollectorValueNode value(ContainerValueExtractorPath extractor
*/
private <V> PojoIndexingProcessorContainerElementNodeBuilder<P, ? super P, V> createContainerElementNodeBuilder(
BoundContainerValueExtractorPath<P, V> boundExtractorPath) {
ContainerValueExtractor<? super P, V> extractor =
mappingHelper.getIndexModelBinder().createExtractors( boundExtractorPath );
BoundPojoModelPathValueNode<T, P, V> containerElementPath = modelPath.value( boundExtractorPath );
ContainerValueExtractorHolder<P, V> extractorHolder =
mappingHelper.getIndexModelBinder().createExtractors( boundExtractorPath );
return new PojoIndexingProcessorContainerElementNodeBuilder<>(
containerElementPath, extractor,
containerElementPath, extractorHolder,
mappingHelper, bindingContext
);
}
Expand Down
Expand Up @@ -10,7 +10,7 @@
import java.util.stream.Stream;

import org.hibernate.search.engine.backend.document.DocumentElement;
import org.hibernate.search.mapper.pojo.extractor.ContainerValueExtractor;
import org.hibernate.search.mapper.pojo.extractor.impl.ContainerValueExtractorHolder;
import org.hibernate.search.mapper.pojo.session.context.spi.AbstractPojoSessionContextImplementor;
import org.hibernate.search.util.impl.common.Closer;
import org.hibernate.search.util.impl.common.ToStringTreeBuilder;
Expand All @@ -24,26 +24,27 @@
*/
public class PojoIndexingProcessorContainerElementNode<C, V> extends PojoIndexingProcessor<C> {

private final ContainerValueExtractor<C, V> extractor;
private final ContainerValueExtractorHolder<C, V> extractorHolder;
private final Collection<PojoIndexingProcessor<? super V>> nestedNodes;

public PojoIndexingProcessorContainerElementNode(ContainerValueExtractor<C, V> extractor,
public PojoIndexingProcessorContainerElementNode(ContainerValueExtractorHolder<C, V> extractorHolder,
Collection<PojoIndexingProcessor<? super V>> nestedNodes) {
this.extractor = extractor;
this.extractorHolder = extractorHolder;
this.nestedNodes = nestedNodes;
}

@Override
public void close() {
try ( Closer<RuntimeException> closer = new Closer<>() ) {
closer.push( ContainerValueExtractorHolder::close, extractorHolder );
closer.pushAll( PojoIndexingProcessor::close, nestedNodes );
}
}

@Override
public void appendTo(ToStringTreeBuilder builder) {
builder.attribute( "class", getClass().getSimpleName() );
builder.attribute( "extractor", extractor );
builder.attribute( "extractor", extractorHolder.get() );
builder.startList( "nestedNodes" );
for ( PojoIndexingProcessor<?> nestedNode : nestedNodes ) {
builder.value( nestedNode );
Expand All @@ -53,7 +54,7 @@ public void appendTo(ToStringTreeBuilder builder) {

@Override
public final void process(DocumentElement target, C source, AbstractPojoSessionContextImplementor sessionContext) {
try ( Stream<V> stream = extractor.extract( source ) ) {
try ( Stream<V> stream = extractorHolder.get().extract( source ) ) {
stream.forEach( sourceItem -> processItem( target, sourceItem, sessionContext ) );
}
}
Expand Down

0 comments on commit a85ca06

Please sign in to comment.