From c5c43723f1cabba7449167284f6ecc7c3828e553 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mattias=20Finn=C3=A9?= Date: Fri, 9 Mar 2018 14:27:54 +0100 Subject: [PATCH] FusionSchemaIndexProvider handles native string index --- .../api/NodeValueIndexCursorTestBase.java | 2 +- .../index/schema/StringIndexProvider.java | 6 +- .../impl/index/schema/StringSchemaKey.java | 3 +- .../schema/fusion/FusionIndexAccessor.java | 60 +- .../schema/fusion/FusionIndexPopulator.java | 44 +- .../schema/fusion/FusionIndexProvider.java | 60 +- .../schema/fusion/FusionIndexReader.java | 85 ++- .../schema/fusion/FusionIndexSampler.java | 2 +- .../schema/fusion/FusionIndexUpdater.java | 37 +- .../index/schema/fusion/FusionIndexUtils.java | 63 ++- .../index/schema/fusion/FusionSelector.java | 9 +- .../fusion/FusionIndexAccessorTest.java | 525 ++++++------------ .../fusion/FusionIndexPopulatorTest.java | 431 ++++---------- .../fusion/FusionIndexProviderTest.java | 352 ++++-------- .../schema/fusion/FusionIndexReaderTest.java | 129 +++-- .../schema/fusion/FusionIndexTestHelp.java | 42 +- .../schema/fusion/FusionIndexUpdaterTest.java | 331 ++--------- ...ativeLuceneFusionIndexProviderFactory.java | 12 +- 18 files changed, 770 insertions(+), 1423 deletions(-) diff --git a/community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/NodeValueIndexCursorTestBase.java b/community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/NodeValueIndexCursorTestBase.java index 1c723cb76902e..9c4bbba3187e7 100644 --- a/community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/NodeValueIndexCursorTestBase.java +++ b/community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/NodeValueIndexCursorTestBase.java @@ -195,7 +195,7 @@ public void shouldPerformExactLookupInCompositeIndex() throws Exception PrimitiveLongSet uniqueIds = Primitive.longSet() ) { // when - IndexValueCapability valueCapability = index.valueCapability( ValueGroup.TEXT ); + IndexValueCapability valueCapability = index.valueCapability( ValueGroup.TEXT, ValueGroup.TEXT ); read.nodeIndexSeek( index, node, IndexOrder.NONE, IndexQuery.exact( firstName, "Joe" ), IndexQuery.exact( surname, "Dalton" ) ); diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/StringIndexProvider.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/StringIndexProvider.java index ebef4caca2726..19fa49490afac 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/StringIndexProvider.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/StringIndexProvider.java @@ -46,9 +46,9 @@ public class StringIndexProvider extends NativeIndexProvider accessor.force( ioLimiter ), accessors ); } @Override public void refresh() throws IOException { - numberAccessor.refresh(); - spatialAccessor.refresh(); - temporalAccessor.refresh(); - luceneAccessor.refresh(); + forAll( IndexAccessor::refresh, accessors ); } @Override public void close() throws IOException { - forAll( IndexAccessor::close, numberAccessor, spatialAccessor, temporalAccessor, luceneAccessor ); + forAll( IndexAccessor::close, accessors ); } @Override public IndexReader newReader() { return new FusionIndexReader( + stringAccessor.newReader(), numberAccessor.newReader(), spatialAccessor.newReader(), temporalAccessor.newReader(), @@ -127,6 +132,7 @@ public IndexReader newReader() @Override public BoundedIterable newAllEntriesReader() { + BoundedIterable stringAllEntries = stringAccessor.newAllEntriesReader(); BoundedIterable numberAllEntries = numberAccessor.newAllEntriesReader(); BoundedIterable spatialAllEntries = spatialAccessor.newAllEntriesReader(); BoundedIterable temporalAllEntries = temporalAccessor.newAllEntriesReader(); @@ -136,12 +142,13 @@ public BoundedIterable newAllEntriesReader() @Override public long maxCount() { + long stringMaxCount = stringAllEntries.maxCount(); long numberMaxCount = numberAllEntries.maxCount(); long spatialMaxCount = spatialAllEntries.maxCount(); long temporalMaxCount = temporalAllEntries.maxCount(); long luceneMaxCount = luceneAllEntries.maxCount(); - return existsUnknownMaxCount( numberMaxCount, spatialMaxCount, temporalMaxCount, luceneMaxCount ) ? - UNKNOWN_MAX_COUNT : numberMaxCount + spatialMaxCount + temporalMaxCount + luceneMaxCount; + return existsUnknownMaxCount( stringMaxCount, numberMaxCount, spatialMaxCount, temporalMaxCount, luceneMaxCount ) ? + UNKNOWN_MAX_COUNT : stringMaxCount + numberMaxCount + spatialMaxCount + temporalMaxCount + luceneMaxCount; } private boolean existsUnknownMaxCount( long... maxCounts ) @@ -160,13 +167,13 @@ private boolean existsUnknownMaxCount( long... maxCounts ) @Override public void close() throws Exception { - forAll( BoundedIterable::close, numberAllEntries, spatialAllEntries, temporalAllEntries, luceneAllEntries ); + forAll( BoundedIterable::close, stringAllEntries, numberAllEntries, spatialAllEntries, temporalAllEntries, luceneAllEntries ); } @Override public Iterator iterator() { - return Iterables.concat( numberAllEntries, spatialAllEntries, temporalAllEntries, luceneAllEntries ).iterator(); + return Iterables.concat( stringAllEntries, numberAllEntries, spatialAllEntries, temporalAllEntries, luceneAllEntries ).iterator(); } }; } @@ -174,26 +181,27 @@ public Iterator iterator() @Override public ResourceIterator snapshotFiles() throws IOException { - return concatResourceIterators( - asList( numberAccessor.snapshotFiles(), - spatialAccessor.snapshotFiles(), - temporalAccessor.snapshotFiles(), - luceneAccessor.snapshotFiles() ).iterator() ); + List> snapshots = new ArrayList<>(); + for ( IndexAccessor accessor : accessors ) + { + snapshots.add( accessor.snapshotFiles() ); + } + return concatResourceIterators( snapshots.iterator() ); } @Override public void verifyDeferredConstraints( PropertyAccessor propertyAccessor ) throws IndexEntryConflictException, IOException { - numberAccessor.verifyDeferredConstraints( propertyAccessor ); - spatialAccessor.verifyDeferredConstraints( propertyAccessor ); - temporalAccessor.verifyDeferredConstraints( propertyAccessor ); - luceneAccessor.verifyDeferredConstraints( propertyAccessor ); + for ( IndexAccessor accessor : accessors ) + { + accessor.verifyDeferredConstraints( propertyAccessor ); + } } @Override public boolean isDirty() { - return numberAccessor.isDirty() || spatialAccessor.isDirty() || temporalAccessor.isDirty() || luceneAccessor.isDirty(); + return Arrays.stream( accessors ).anyMatch( IndexAccessor::isDirty ); } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexPopulator.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexPopulator.java index 2a79e34b13f3f..795b56a83d5b3 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexPopulator.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexPopulator.java @@ -32,26 +32,31 @@ import org.neo4j.kernel.impl.index.schema.fusion.FusionIndexProvider.Selector; import org.neo4j.storageengine.api.schema.IndexSample; +import static org.neo4j.helpers.collection.Iterators.array; +import static org.neo4j.kernel.impl.index.schema.fusion.FusionIndexUtils.combineSamples; import static org.neo4j.kernel.impl.index.schema.fusion.FusionIndexUtils.forAll; -import static org.neo4j.kernel.impl.index.schema.fusion.FusionIndexProvider.combineSamples; class FusionIndexPopulator implements IndexPopulator { + private final IndexPopulator stringPopulator; private final IndexPopulator numberPopulator; private final IndexPopulator spatialPopulator; private final IndexPopulator temporalPopulator; private final IndexPopulator lucenePopulator; + private final IndexPopulator[] populators; private final Selector selector; private final long indexId; private final DropAction dropAction; - FusionIndexPopulator( IndexPopulator numberPopulator, IndexPopulator spatialPopulator, IndexPopulator temporalPopulator, + FusionIndexPopulator( IndexPopulator stringPopulator, IndexPopulator numberPopulator, IndexPopulator spatialPopulator, IndexPopulator temporalPopulator, IndexPopulator lucenePopulator, Selector selector, long indexId, DropAction dropAction ) { + this.stringPopulator = stringPopulator; this.numberPopulator = numberPopulator; this.spatialPopulator = spatialPopulator; this.temporalPopulator = temporalPopulator; this.lucenePopulator = lucenePopulator; + this.populators = array( stringPopulator, numberPopulator, spatialPopulator, temporalPopulator, lucenePopulator ); this.selector = selector; this.indexId = indexId; this.dropAction = dropAction; @@ -60,50 +65,50 @@ class FusionIndexPopulator implements IndexPopulator @Override public void create() throws IOException { - numberPopulator.create(); - spatialPopulator.create(); - temporalPopulator.create(); - lucenePopulator.create(); + FusionIndexUtils.forAll( IndexPopulator::create, populators ); } @Override public void drop() throws IOException { - forAll( IndexPopulator::drop, numberPopulator, spatialPopulator, temporalPopulator, lucenePopulator ); + forAll( IndexPopulator::drop, populators ); dropAction.drop( indexId ); } @Override public void add( Collection> updates ) throws IndexEntryConflictException, IOException { - Collection> luceneBatch = new ArrayList<>(); + Collection> stringBatch = new ArrayList<>(); + Collection> numberBatch = new ArrayList<>(); Collection> spatialBatch = new ArrayList<>(); Collection> temporalBatch = new ArrayList<>(); - Collection> numberBatch = new ArrayList<>(); + Collection> luceneBatch = new ArrayList<>(); for ( IndexEntryUpdate update : updates ) { - selector.select( numberBatch, spatialBatch, temporalBatch, luceneBatch, update.values() ).add( update ); + selector.select( stringBatch, numberBatch, spatialBatch, temporalBatch, luceneBatch, update.values() ).add( update ); } - lucenePopulator.add( luceneBatch ); + stringPopulator.add( stringBatch ); + numberPopulator.add( numberBatch ); spatialPopulator.add( spatialBatch ); temporalPopulator.add( temporalBatch ); - numberPopulator.add( numberBatch ); + lucenePopulator.add( luceneBatch ); } @Override public void verifyDeferredConstraints( PropertyAccessor propertyAccessor ) throws IndexEntryConflictException, IOException { - numberPopulator.verifyDeferredConstraints( propertyAccessor ); - spatialPopulator.verifyDeferredConstraints( propertyAccessor ); - temporalPopulator.verifyDeferredConstraints( propertyAccessor ); - lucenePopulator.verifyDeferredConstraints( propertyAccessor ); + for ( IndexPopulator populator : populators ) + { + populator.verifyDeferredConstraints( propertyAccessor ); + } } @Override public IndexUpdater newPopulatingUpdater( PropertyAccessor accessor ) throws IOException { return new FusionIndexUpdater( + stringPopulator.newPopulatingUpdater( accessor ), numberPopulator.newPopulatingUpdater( accessor ), spatialPopulator.newPopulatingUpdater( accessor ), temporalPopulator.newPopulatingUpdater( accessor ), @@ -113,25 +118,26 @@ public IndexUpdater newPopulatingUpdater( PropertyAccessor accessor ) throws IOE @Override public void close( boolean populationCompletedSuccessfully ) throws IOException { - forAll( populator -> populator.close( populationCompletedSuccessfully ), numberPopulator, spatialPopulator, temporalPopulator, lucenePopulator ); + forAll( populator -> populator.close( populationCompletedSuccessfully ), populators ); } @Override public void markAsFailed( String failure ) throws IOException { - forAll( populator -> populator.markAsFailed( failure ), numberPopulator, spatialPopulator, temporalPopulator, lucenePopulator ); + forAll( populator -> populator.markAsFailed( failure ), populators ); } @Override public void includeSample( IndexEntryUpdate update ) { - selector.select( numberPopulator, spatialPopulator, temporalPopulator, lucenePopulator, update.values() ).includeSample( update ); + selector.select( stringPopulator, numberPopulator, spatialPopulator, temporalPopulator, lucenePopulator, update.values() ).includeSample( update ); } @Override public IndexSample sampleResult() { return combineSamples( + stringPopulator.sampleResult(), numberPopulator.sampleResult(), spatialPopulator.sampleResult(), temporalPopulator.sampleResult(), diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexProvider.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexProvider.java index 96b1820bdb3e6..12fe849791f3b 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexProvider.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexProvider.java @@ -35,10 +35,10 @@ import org.neo4j.kernel.impl.api.index.sampling.IndexSamplingConfig; import org.neo4j.kernel.impl.newapi.UnionIndexCapability; import org.neo4j.kernel.impl.storemigration.StoreMigrationParticipant; -import org.neo4j.storageengine.api.schema.IndexSample; import org.neo4j.values.storable.Value; import org.neo4j.values.storable.ValueGroup; +import static org.neo4j.helpers.collection.Iterators.array; import static org.neo4j.internal.kernel.api.InternalIndexState.FAILED; import static org.neo4j.internal.kernel.api.InternalIndexState.POPULATING; @@ -50,31 +50,37 @@ public class FusionIndexProvider extends IndexProvider { interface Selector { - T select( T numberInstance, T spatialInstance, T temporalInstance, T luceneInstance, Value... values ); + T select( T stringInstance, T numberInstance, T spatialInstance, T temporalInstance, T luceneInstance, Value... values ); } + private final IndexProvider stringProvider; private final IndexProvider numberProvider; private final IndexProvider spatialProvider; private final IndexProvider temporalProvider; private final IndexProvider luceneProvider; + private final IndexProvider[] providers; private final Selector selector; private final DropAction dropAction; - public FusionIndexProvider( IndexProvider numberProvider, - IndexProvider spatialProvider, - IndexProvider temporalProvider, - IndexProvider luceneProvider, - Selector selector, - Descriptor descriptor, - int priority, - IndexDirectoryStructure.Factory directoryStructure, - FileSystemAbstraction fs ) + public FusionIndexProvider( + IndexProvider stringProvider, + IndexProvider numberProvider, + IndexProvider spatialProvider, + IndexProvider temporalProvider, + IndexProvider luceneProvider, + Selector selector, + Descriptor descriptor, + int priority, + IndexDirectoryStructure.Factory directoryStructure, + FileSystemAbstraction fs ) { super( descriptor, priority, directoryStructure ); + this.stringProvider = stringProvider; this.numberProvider = numberProvider; this.spatialProvider = spatialProvider; this.temporalProvider = temporalProvider; this.luceneProvider = luceneProvider; + this.providers = array( stringProvider, numberProvider, spatialProvider, temporalProvider, luceneProvider ); this.selector = selector; this.dropAction = new FileSystemDropAction( fs, directoryStructure() ); } @@ -83,6 +89,7 @@ public FusionIndexProvider( IndexProvider numberProvider, public IndexPopulator getPopulator( long indexId, SchemaIndexDescriptor descriptor, IndexSamplingConfig samplingConfig ) { return new FusionIndexPopulator( + stringProvider.getPopulator( indexId, descriptor, samplingConfig ), numberProvider.getPopulator( indexId, descriptor, samplingConfig ), spatialProvider.getPopulator( indexId, descriptor, samplingConfig ), temporalProvider.getPopulator( indexId, descriptor, samplingConfig ), @@ -94,6 +101,7 @@ public IndexAccessor getOnlineAccessor( long indexId, SchemaIndexDescriptor desc IndexSamplingConfig samplingConfig ) throws IOException { return new FusionIndexAccessor( + stringProvider.getOnlineAccessor( indexId, descriptor, samplingConfig ), numberProvider.getOnlineAccessor( indexId, descriptor, samplingConfig ), spatialProvider.getOnlineAccessor( indexId, descriptor, samplingConfig ), temporalProvider.getOnlineAccessor( indexId, descriptor, samplingConfig ), @@ -104,6 +112,7 @@ public IndexAccessor getOnlineAccessor( long indexId, SchemaIndexDescriptor desc public String getPopulationFailure( long indexId, SchemaIndexDescriptor descriptor ) throws IllegalStateException { StringBuilder builder = new StringBuilder(); + writeFailure( "string", builder, stringProvider, indexId, descriptor ); writeFailure( "number", builder, numberProvider, indexId, descriptor ); writeFailure( "spatial", builder, spatialProvider, indexId, descriptor ); writeFailure( "temporal", builder, temporalProvider, indexId, descriptor ); @@ -134,16 +143,14 @@ private void writeFailure( String indexName, StringBuilder builder, IndexProvide @Override public InternalIndexState getInitialState( long indexId, SchemaIndexDescriptor descriptor ) { - InternalIndexState numberState = numberProvider.getInitialState( indexId, descriptor ); - InternalIndexState spatialState = spatialProvider.getInitialState( indexId, descriptor ); - InternalIndexState temporalState = temporalProvider.getInitialState( indexId, descriptor ); - InternalIndexState luceneState = luceneProvider.getInitialState( indexId, descriptor ); - if ( numberState == FAILED || spatialState == FAILED || temporalState == FAILED || luceneState == FAILED ) + InternalIndexState[] states = + Arrays.stream( providers ).map( provider -> provider.getInitialState( indexId, descriptor ) ).toArray( InternalIndexState[]::new ); + if ( Arrays.stream( states ).anyMatch( state -> state == FAILED ) ) { // One of the state is FAILED, the whole state must be considered FAILED return FAILED; } - if ( numberState == POPULATING || spatialState == POPULATING || temporalState == POPULATING || luceneState == POPULATING ) + if ( Arrays.stream( states ).anyMatch( state -> state == POPULATING ) ) { // No state is FAILED and one of the state is POPULATING, the whole state must be considered POPULATING return POPULATING; @@ -155,11 +162,12 @@ public InternalIndexState getInitialState( long indexId, SchemaIndexDescriptor d @Override public IndexCapability getCapability( SchemaIndexDescriptor schemaIndexDescriptor ) { - IndexCapability numberCapability = numberProvider.getCapability( schemaIndexDescriptor ); - IndexCapability spatialCapability = spatialProvider.getCapability( schemaIndexDescriptor ); - IndexCapability temporalCapability = temporalProvider.getCapability( schemaIndexDescriptor ); - IndexCapability luceneCapability = luceneProvider.getCapability( schemaIndexDescriptor ); - return new UnionIndexCapability( numberCapability, spatialCapability, temporalCapability, luceneCapability ) + IndexCapability[] capabilities = new IndexCapability[providers.length]; + for ( int i = 0; i < providers.length; i++ ) + { + capabilities[i] = providers[i].getCapability( schemaIndexDescriptor ); + } + return new UnionIndexCapability( capabilities ) { @Override public IndexOrder[] orderCapability( ValueGroup... valueGroups ) @@ -182,14 +190,6 @@ public StoreMigrationParticipant storeMigrationParticipant( FileSystemAbstractio return StoreMigrationParticipant.NOT_PARTICIPATING; } - static IndexSample combineSamples( IndexSample... samples ) - { - long indexSize = Arrays.stream(samples).mapToLong( IndexSample::indexSize ).sum(); - long uniqueValues = Arrays.stream(samples).mapToLong( IndexSample::uniqueValues ).sum(); - long sampleSize = Arrays.stream(samples).mapToLong( IndexSample::sampleSize ).sum(); - return new IndexSample( indexSize, uniqueValues, sampleSize ); - } - /** * As an interface because this is actually dependent on whether or not an index lives on a {@link FileSystemAbstraction} * or a page cache. At the time of writing this there's only the possibility to put these on the file system, diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexReader.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexReader.java index 97399ee183fe0..d9bcf7c4d538a 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexReader.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexReader.java @@ -30,38 +30,47 @@ import org.neo4j.internal.kernel.api.IndexQuery.ExistsPredicate; import org.neo4j.internal.kernel.api.IndexQuery.GeometryRangePredicate; import org.neo4j.internal.kernel.api.IndexQuery.NumberRangePredicate; +import org.neo4j.internal.kernel.api.IndexQuery.StringRangePredicate; import org.neo4j.kernel.api.exceptions.index.IndexNotApplicableKernelException; import org.neo4j.kernel.api.schema.index.SchemaIndexDescriptor; import org.neo4j.kernel.impl.api.schema.BridgingIndexProgressor; import org.neo4j.kernel.impl.index.schema.fusion.FusionIndexProvider.Selector; + import org.neo4j.storageengine.api.schema.IndexProgressor; import org.neo4j.storageengine.api.schema.IndexReader; import org.neo4j.storageengine.api.schema.IndexSampler; import org.neo4j.values.storable.Value; import static java.lang.String.format; +import static org.neo4j.helpers.collection.Iterators.array; import static org.neo4j.kernel.impl.index.schema.fusion.FusionIndexUtils.forAll; class FusionIndexReader implements IndexReader { + private final IndexReader stringReader; private final IndexReader numberReader; private final IndexReader spatialReader; private final IndexReader temporalReader; private final IndexReader luceneReader; + private final IndexReader[] readers; private final Selector selector; private final SchemaIndexDescriptor descriptor; - FusionIndexReader( IndexReader numberReader, - IndexReader spatialReader, - IndexReader temporalReader, - IndexReader luceneReader, - Selector selector, - SchemaIndexDescriptor descriptor ) + FusionIndexReader( + IndexReader stringReader, + IndexReader numberReader, + IndexReader spatialReader, + IndexReader temporalReader, + IndexReader luceneReader, + Selector selector, + SchemaIndexDescriptor descriptor ) { + this.stringReader = stringReader; this.numberReader = numberReader; this.spatialReader = spatialReader; this.temporalReader = temporalReader; this.luceneReader = luceneReader; + this.readers = array( stringReader, numberReader, spatialReader, temporalReader, luceneReader ); this.selector = selector; this.descriptor = descriptor; } @@ -69,19 +78,21 @@ class FusionIndexReader implements IndexReader @Override public void close() { - forAll( Resource::close, numberReader, spatialReader, temporalReader, luceneReader ); + forAll( Resource::close, readers ); } @Override public long countIndexedNodes( long nodeId, Value... propertyValues ) { - return selector.select( numberReader, spatialReader, temporalReader, luceneReader, propertyValues ).countIndexedNodes( nodeId, propertyValues ); + return selector.select( stringReader, numberReader, spatialReader, temporalReader, luceneReader, propertyValues ) + .countIndexedNodes( nodeId, propertyValues ); } @Override public IndexSampler createSampler() { return new FusionIndexSampler( + stringReader.createSampler(), numberReader.createSampler(), spatialReader.createSampler(), temporalReader.createSampler(), @@ -95,19 +106,25 @@ public PrimitiveLongResourceIterator query( IndexQuery... predicates ) throws In { return luceneReader.query( predicates ); } + IndexQuery predicate = predicates[0]; - if ( predicates[0] instanceof ExactPredicate ) + if ( predicate instanceof ExactPredicate ) { ExactPredicate exactPredicate = (ExactPredicate) predicates[0]; - return selector.select( numberReader, spatialReader, temporalReader, luceneReader, exactPredicate.value() ).query( predicates ); + return selector.select( stringReader, numberReader, spatialReader, temporalReader, luceneReader, exactPredicate.value() ).query( predicates ); } - if ( predicates[0] instanceof NumberRangePredicate ) + if ( predicate instanceof NumberRangePredicate ) { return numberReader.query( predicates ); } - if ( predicates[0] instanceof GeometryRangePredicate ) + if ( predicate instanceof StringRangePredicate ) + { + return stringReader.query( predicate ); + } + + if ( predicate instanceof GeometryRangePredicate ) { return spatialReader.query( predicates ); } @@ -119,13 +136,14 @@ public PrimitiveLongResourceIterator query( IndexQuery... predicates ) throws In // } // todo: There will be no ordering of the node ids here. Is this a problem? - if ( predicates[0] instanceof ExistsPredicate ) + if ( predicate instanceof ExistsPredicate ) { - PrimitiveLongResourceIterator numberResult = numberReader.query( predicates ); - PrimitiveLongResourceIterator spatialResult = spatialReader.query( predicates ); - PrimitiveLongResourceIterator temporalResult = temporalReader.query( predicates ); - PrimitiveLongResourceIterator luceneResult = luceneReader.query( predicates ); - return PrimitiveLongResourceCollections.concat( numberResult, spatialResult, temporalResult, luceneResult ); + PrimitiveLongResourceIterator[] iterators = new PrimitiveLongResourceIterator[readers.length]; + for ( int i = 0; i < readers.length; i++ ) + { + iterators[i] = readers[i].query( predicates ); + } + return PrimitiveLongResourceCollections.concat( iterators ); } return luceneReader.query( predicates ); @@ -140,23 +158,31 @@ public void query( IndexProgressor.NodeValueClient cursor, IndexOrder indexOrder luceneReader.query( cursor, indexOrder, predicates ); return; } + IndexQuery predicate = predicates[0]; - if ( predicates[0] instanceof ExactPredicate ) + if ( predicate instanceof ExactPredicate ) { ExactPredicate exactPredicate = (ExactPredicate) predicates[0]; - selector.select( numberReader, spatialReader, temporalReader, luceneReader, exactPredicate.value() ).query( cursor, indexOrder, predicates ); + selector.select( stringReader, numberReader, spatialReader, temporalReader, luceneReader, exactPredicate.value() ) + .query( cursor, indexOrder, predicate ); return; } - if ( predicates[0] instanceof NumberRangePredicate ) + if ( predicate instanceof NumberRangePredicate ) { - numberReader.query( cursor, indexOrder, predicates ); + numberReader.query( cursor, indexOrder, predicate ); return; } - if ( predicates[0] instanceof GeometryRangePredicate ) + if ( predicate instanceof StringRangePredicate ) { - spatialReader.query( cursor, indexOrder, predicates ); + stringReader.query( cursor, indexOrder, predicate ); + return; + } + + if ( predicate instanceof GeometryRangePredicate ) + { + spatialReader.query( cursor, indexOrder, predicate ); return; } @@ -167,7 +193,7 @@ public void query( IndexProgressor.NodeValueClient cursor, IndexOrder indexOrder // } // todo: There will be no ordering of the node ids here. Is this a problem? - if ( predicates[0] instanceof ExistsPredicate ) + if ( predicate instanceof ExistsPredicate ) { if ( indexOrder != IndexOrder.NONE ) { @@ -178,10 +204,10 @@ public void query( IndexProgressor.NodeValueClient cursor, IndexOrder indexOrder BridgingIndexProgressor multiProgressor = new BridgingIndexProgressor( cursor, descriptor.schema().getPropertyIds() ); cursor.initialize( descriptor, multiProgressor, predicates ); - numberReader.query( multiProgressor, indexOrder, predicates ); - spatialReader.query( multiProgressor, indexOrder, predicates ); - temporalReader.query( multiProgressor, indexOrder, predicates ); - luceneReader.query( multiProgressor, indexOrder, predicates ); + for ( IndexReader reader : readers ) + { + reader.query( multiProgressor, indexOrder, predicate ); + } return; } @@ -201,6 +227,7 @@ public boolean hasFullValuePrecision( IndexQuery... predicates ) { Value value = ((ExactPredicate) predicate).value(); return selector.select( + stringReader.hasFullValuePrecision( predicates ), numberReader.hasFullValuePrecision( predicates ), spatialReader.hasFullValuePrecision( predicates ), temporalReader.hasFullValuePrecision( predicates ), diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexSampler.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexSampler.java index dcc6e1a22da6b..c936555668fbd 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexSampler.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexSampler.java @@ -23,7 +23,7 @@ import org.neo4j.storageengine.api.schema.IndexSample; import org.neo4j.storageengine.api.schema.IndexSampler; -import static org.neo4j.kernel.impl.index.schema.fusion.FusionIndexProvider.combineSamples; +import static org.neo4j.kernel.impl.index.schema.fusion.FusionIndexUtils.combineSamples; public class FusionIndexSampler implements IndexSampler { diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexUpdater.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexUpdater.java index b3b1c53eaa2d7..7d84c0b64c7ce 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexUpdater.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexUpdater.java @@ -20,33 +20,39 @@ package org.neo4j.kernel.impl.index.schema.fusion; import java.io.IOException; -import java.util.Arrays; import org.neo4j.kernel.api.exceptions.index.IndexEntryConflictException; import org.neo4j.kernel.api.index.IndexEntryUpdate; import org.neo4j.kernel.api.index.IndexUpdater; import org.neo4j.kernel.impl.index.schema.fusion.FusionIndexProvider.Selector; +import static org.neo4j.helpers.collection.Iterators.array; import static org.neo4j.kernel.impl.index.schema.fusion.FusionIndexUtils.forAll; class FusionIndexUpdater implements IndexUpdater { + private final IndexUpdater stringUpdater; private final IndexUpdater numberUpdater; private final IndexUpdater spatialUpdater; private final IndexUpdater temporalUpdater; private final IndexUpdater luceneUpdater; + private final IndexUpdater[] updaters; private final Selector selector; - FusionIndexUpdater( IndexUpdater numberUpdater, - IndexUpdater spatialUpdater, - IndexUpdater temporalUpdater, - IndexUpdater luceneUpdater, - Selector selector ) + FusionIndexUpdater( + IndexUpdater stringUpdater, + IndexUpdater numberUpdater, + IndexUpdater spatialUpdater, + IndexUpdater temporalUpdater, + IndexUpdater luceneUpdater, + Selector selector ) { + this.stringUpdater = stringUpdater; this.numberUpdater = numberUpdater; this.spatialUpdater = spatialUpdater; this.temporalUpdater = temporalUpdater; this.luceneUpdater = luceneUpdater; + this.updaters = array( stringUpdater, numberUpdater, spatialUpdater, temporalUpdater, luceneUpdater ); this.selector = selector; } @@ -56,14 +62,14 @@ public void process( IndexEntryUpdate update ) throws IOException, IndexEntry switch ( update.updateMode() ) { case ADDED: - selector.select( numberUpdater, spatialUpdater, temporalUpdater, luceneUpdater, update.values() ).process( update ); + selector.select( stringUpdater, numberUpdater, spatialUpdater, temporalUpdater, luceneUpdater, update.values() ).process( update ); break; case CHANGED: // Hmm, here's a little conundrum. What if we change from a value that goes into native // to a value that goes into fallback, or vice versa? We also don't want to blindly pass // all CHANGED updates to both updaters since not all values will work in them. - IndexUpdater from = selector.select( numberUpdater, spatialUpdater, temporalUpdater, luceneUpdater, update.beforeValues() ); - IndexUpdater to = selector.select( numberUpdater, spatialUpdater, temporalUpdater, luceneUpdater, update.values() ); + IndexUpdater from = selector.select( stringUpdater, numberUpdater, spatialUpdater, temporalUpdater, luceneUpdater, update.beforeValues() ); + IndexUpdater to = selector.select( stringUpdater, numberUpdater, spatialUpdater, temporalUpdater, luceneUpdater, update.values() ); // There are two cases: // - both before/after go into the same updater --> pass update into that updater if ( from == to ) @@ -73,14 +79,12 @@ public void process( IndexEntryUpdate update ) throws IOException, IndexEntry // - before go into one and after into the other --> REMOVED from one and ADDED into the other else { - from.process( IndexEntryUpdate.remove( - update.getEntityId(), update.indexKey(), update.beforeValues() ) ); - to.process( IndexEntryUpdate.add( - update.getEntityId(), update.indexKey(), update.values() ) ); + from.process( IndexEntryUpdate.remove( update.getEntityId(), update.indexKey(), update.beforeValues() ) ); + to.process( IndexEntryUpdate.add( update.getEntityId(), update.indexKey(), update.values() ) ); } break; case REMOVED: - selector.select( numberUpdater, spatialUpdater, temporalUpdater, luceneUpdater, update.values() ).process( update ); + selector.select( stringUpdater, numberUpdater, spatialUpdater, temporalUpdater, luceneUpdater, update.values() ).process( update ); break; default: throw new IllegalArgumentException( "Unknown update mode" ); @@ -92,14 +96,15 @@ public void close() throws IOException, IndexEntryConflictException { try { - forAll( IndexUpdater::close, Arrays.asList( numberUpdater, spatialUpdater, temporalUpdater, luceneUpdater ) ); + forAll( IndexUpdater::close, updaters ); } - catch ( IOException | IndexEntryConflictException e ) + catch ( IOException | IndexEntryConflictException | RuntimeException e ) { throw e; } catch ( Exception e ) { + // This catch-clause is basically only here to satisfy the compiler throw new RuntimeException( e ); } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexUtils.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexUtils.java index faa275d712c36..c4fb8b0d3d666 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexUtils.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexUtils.java @@ -20,9 +20,9 @@ package org.neo4j.kernel.impl.index.schema.fusion; import java.util.Arrays; -import java.util.Collection; import org.neo4j.function.ThrowingConsumer; +import org.neo4j.storageengine.api.schema.IndexSample; /** * Utility methods for working with multiple sub-components within the Fusion Index system @@ -30,6 +30,9 @@ public abstract class FusionIndexUtils { /** + * NOTE: duplicate of {@link #forAll(ThrowingConsumer, Iterable)} to avoid having to wrap subjects of one form into another. + * There are some real use cases for passing in an array instead of {@link Iterable} out there... + * * Method for calling a lambda function on many objects when it is expected that the function might * throw an exception. First exception will be thrown and subsequent will be suppressed. * @@ -37,8 +40,7 @@ public abstract class FusionIndexUtils * 
      *    public void drop() throws IOException
      *    {
-     *        forAll( IndexAccessor::drop, nativeAccessor, spatialAccessor, luceneAccessor );
-     *        dropAction.drop( indexId );
+     *        forAll( IndexAccessor::drop, firstAccessor, secondAccessor, thirdAccessor );
      *    }
      *
* @@ -47,7 +49,7 @@ public abstract class FusionIndexUtils * @param the type of exception anticipated, inferred from the lambda * @throws E if consumption fails with this exception */ - public static void forAll( ThrowingConsumer consumer, Iterable subjects ) throws E + public static void forAll( ThrowingConsumer consumer, T... subjects ) throws E { E exception = null; for ( T subject : subjects ) @@ -77,9 +79,58 @@ public static void forAll( ThrowingConsumer consum /** * See {@link FusionIndexUtils#forAll(ThrowingConsumer, Iterable)} + * NOTE: duplicate of {@link #forAll(ThrowingConsumer, Object[])} to avoid having to wrap subjects of one form into another. + * There are some real use cases for passing in an Iterable instead of array out there... + * + * Method for calling a lambda function on many objects when it is expected that the function might + * throw an exception. First exception will be thrown and subsequent will be suppressed. + * + * For example, in FusionIndexAccessor: + * 
+     *    public void drop() throws IOException
+     *    {
+     *        forAll( IndexAccessor::drop, firstAccessor, secondAccessor, thirdAccessor );
+     *    }
+     *
+ * + * @param consumer lambda function to call on each object passed + * @param subjects varargs array of objects to call the function on + * @param the type of exception anticipated, inferred from the lambda + * @throws E if consumption fails with this exception */ - public static void forAll( ThrowingConsumer consumer, T... subjects ) throws E + public static void forAll( ThrowingConsumer consumer, Iterable subjects ) throws E + { + E exception = null; + for ( T subject : subjects ) + { + try + { + consumer.accept( subject ); + } + catch ( Throwable t ) + { + E e = (E) t; + if ( exception == null ) + { + exception = e; + } + else + { + exception.addSuppressed( e ); + } + } + } + if ( exception != null ) + { + throw exception; + } + } + + static IndexSample combineSamples( IndexSample... samples ) { - forAll( consumer, Arrays.asList( subjects ) ); + return new IndexSample( + Arrays.stream( samples ).mapToLong( IndexSample::indexSize ).sum(), + Arrays.stream( samples ).mapToLong( IndexSample::uniqueValues ).sum(), + Arrays.stream( samples ).mapToLong( IndexSample::sampleSize ).sum() ); } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionSelector.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionSelector.java index 1145cd426fb3f..6749c5c4185e1 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionSelector.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionSelector.java @@ -26,7 +26,7 @@ public class FusionSelector implements FusionIndexProvider.Selector { @Override - public T select( T numberInstance, T spatialInstance, T temporalInstance, T luceneInstance, Value... values ) + public T select( T stringInstance, T numberInstance, T spatialInstance, T temporalInstance, T luceneInstance, Value... values ) { if ( values.length > 1 ) { @@ -35,6 +35,12 @@ public T select( T numberInstance, T spatialInstance, T temporalInstance, T } Value singleValue = values[0]; + if ( singleValue.valueGroup() == ValueGroup.TEXT ) + { + // It's a string, the native string index can handle this + return stringInstance; + } + if ( singleValue.valueGroup() == ValueGroup.NUMBER ) { // It's a number, the native index can handle this @@ -51,6 +57,7 @@ public T select( T numberInstance, T spatialInstance, T temporalInstance, T { return temporalInstance; } + return luceneInstance; } } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexAccessorTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexAccessorTest.java index ab568829f7e61..d31fa52ca274e 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexAccessorTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexAccessorTest.java @@ -24,7 +24,6 @@ import java.io.IOException; import java.util.Arrays; -import java.util.List; import java.util.Set; import org.neo4j.helpers.collection.BoundedIterable; @@ -37,40 +36,44 @@ import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.Matchers.sameInstance; import static org.hamcrest.core.AnyOf.anyOf; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.mockito.internal.verification.VerificationModeFactory.times; +import static org.neo4j.helpers.ArrayUtil.without; +import static org.neo4j.helpers.collection.Iterators.array; import static org.neo4j.kernel.impl.index.schema.fusion.FusionIndexTestHelp.verifyFusionCloseThrowIfAllThrow; import static org.neo4j.kernel.impl.index.schema.fusion.FusionIndexTestHelp.verifyFusionCloseThrowOnSingleCloseThrow; import static org.neo4j.kernel.impl.index.schema.fusion.FusionIndexTestHelp.verifyOtherIsClosedOnSingleThrow; public class FusionIndexAccessorTest { - private IndexAccessor nativeAccessor; + private IndexAccessor stringAccessor; + private IndexAccessor numberAccessor; private IndexAccessor spatialAccessor; private IndexAccessor temporalAccessor; private IndexAccessor luceneAccessor; private FusionIndexAccessor fusionIndexAccessor; private final long indexId = 10; private final DropAction dropAction = mock( DropAction.class ); - private List allAccessors; + private IndexAccessor[] allAccessors; @Before - public void setup() + public void createAccessors() { - nativeAccessor = mock( IndexAccessor.class ); + stringAccessor = mock( IndexAccessor.class ); + numberAccessor = mock( IndexAccessor.class ); spatialAccessor = mock( IndexAccessor.class ); temporalAccessor = mock( IndexAccessor.class ); luceneAccessor = mock( IndexAccessor.class ); - allAccessors = Arrays.asList(nativeAccessor, spatialAccessor, temporalAccessor, luceneAccessor); - fusionIndexAccessor = new FusionIndexAccessor( nativeAccessor, spatialAccessor, temporalAccessor, luceneAccessor, + allAccessors = array( stringAccessor, numberAccessor, spatialAccessor, temporalAccessor, luceneAccessor ); + fusionIndexAccessor = new FusionIndexAccessor( stringAccessor, numberAccessor, spatialAccessor, temporalAccessor, luceneAccessor, new FusionSelector(), indexId, mock( SchemaIndexDescriptor.class ), dropAction ); } @@ -82,6 +85,7 @@ public void dropMustDropAll() throws Exception // when // ... all drop successful fusionIndexAccessor.drop(); + // then for ( IndexAccessor accessor : allAccessors ) { @@ -91,58 +95,29 @@ public void dropMustDropAll() throws Exception } @Test - public void dropMustThrowIfDropNativeFail() throws Exception - { - // when - verifyFailOnSingleDropFailure( nativeAccessor, fusionIndexAccessor ); - } - - @Test - public void dropMustThrowIfDropSpatialFail() throws Exception - { - // when - verifyFailOnSingleDropFailure( spatialAccessor, fusionIndexAccessor ); - } - - @Test - public void dropMustThrowIfDropTemporalFail() throws Exception - { - // when - verifyFailOnSingleDropFailure( temporalAccessor, fusionIndexAccessor ); - } - - @Test - public void dropMustThrowIfDropLuceneFail() throws Exception - { - // when - verifyFailOnSingleDropFailure( luceneAccessor, fusionIndexAccessor ); - } - - @Test - public void fusionIndexIsDirtyWhenNativeIndexIsDirty() - { - when( nativeAccessor.isDirty() ).thenReturn( true ).thenReturn( false ); - - assertTrue( fusionIndexAccessor.isDirty() ); - assertFalse( fusionIndexAccessor.isDirty() ); - } - - @Test - public void fusionIndexIsDirtyWhenSpatialIndexIsDirty() + public void dropMustThrowIfDropAnyFail() throws Exception { - when( spatialAccessor.isDirty() ).thenReturn( true ).thenReturn( false ); - - assertTrue( fusionIndexAccessor.isDirty() ); - assertFalse( fusionIndexAccessor.isDirty() ); + for ( IndexAccessor accessor : allAccessors ) + { + // when + verifyFailOnSingleDropFailure( accessor, fusionIndexAccessor ); + } } @Test - public void fusionIndexIsDirtyWhenTemporalIndexIsDirty() + public void fusionIndexIsDirtyWhenAnyIsDirty() { - when( temporalAccessor.isDirty() ).thenReturn( true ).thenReturn( false ); + for ( int i = 0; i < allAccessors.length; i++ ) + { + // when + for ( int j = 0; j < allAccessors.length; j++ ) + { + when( allAccessors[j].isDirty() ).thenReturn( j == i ); + } - assertTrue( fusionIndexAccessor.isDirty() ); - assertFalse( fusionIndexAccessor.isDirty() ); + // then + assertTrue( fusionIndexAccessor.isDirty() ); + } } private void verifyFailOnSingleDropFailure( IndexAccessor failingAccessor, FusionIndexAccessor fusionIndexAccessor ) @@ -159,17 +134,20 @@ private void verifyFailOnSingleDropFailure( IndexAccessor failingAccessor, Fusio { assertSame( expectedFailure, e ); } + doAnswer( invocation -> null ).when( failingAccessor ).drop(); } @Test public void dropMustThrowIfAllFail() throws Exception { // given - IOException nativeFailure = new IOException( "native" ); + IOException stringFailure = new IOException( "string" ); + IOException numberFailure = new IOException( "number" ); IOException spatialFailure = new IOException( "spatial" ); IOException temporalFailure = new IOException( "temporal" ); IOException luceneFailure = new IOException( "lucene" ); - doThrow( nativeFailure ).when( nativeAccessor ).drop(); + doThrow( stringFailure ).when( stringAccessor ).drop(); + doThrow( numberFailure ).when( numberAccessor ).drop(); doThrow( spatialFailure ).when( spatialAccessor ).drop(); doThrow( temporalFailure ).when( temporalAccessor ).drop(); doThrow( luceneFailure ).when( luceneAccessor ).drop(); @@ -184,7 +162,8 @@ public void dropMustThrowIfAllFail() throws Exception { // then assertThat( e, anyOf( - sameInstance( nativeFailure ), + sameInstance( stringFailure ), + sameInstance( numberFailure ), sameInstance( spatialFailure ), sameInstance( temporalFailure ), sameInstance( luceneFailure ) ) ); @@ -208,57 +187,31 @@ public void closeMustCloseAll() throws Exception } @Test - public void closeMustThrowIfLuceneThrow() throws Exception - { - verifyFusionCloseThrowOnSingleCloseThrow( luceneAccessor, fusionIndexAccessor ); - } - - @Test - public void closeMustThrowIfSpatialThrow() throws Exception + public void closeMustThrowIfOneThrow() throws Exception { - verifyFusionCloseThrowOnSingleCloseThrow( spatialAccessor, fusionIndexAccessor ); - } - - @Test - public void closeMustThrowIfTemporalThrow() throws Exception - { - verifyFusionCloseThrowOnSingleCloseThrow( temporalAccessor, fusionIndexAccessor ); - } - - @Test - public void closeMustThrowIfNativeThrow() throws Exception - { - verifyFusionCloseThrowOnSingleCloseThrow( nativeAccessor, fusionIndexAccessor ); - } - - @Test - public void closeMustCloseOthersIfLuceneThrow() throws Exception - { - verifyOtherIsClosedOnSingleThrow( luceneAccessor, fusionIndexAccessor, nativeAccessor, spatialAccessor, temporalAccessor ); - } - - @Test - public void closeMustCloseOthersIfSpatialThrow() throws Exception - { - verifyOtherIsClosedOnSingleThrow( spatialAccessor, fusionIndexAccessor, nativeAccessor, temporalAccessor, luceneAccessor ); - } - - @Test - public void closeMustCloseOthersIfTemporalThrow() throws Exception - { - verifyOtherIsClosedOnSingleThrow( temporalAccessor, fusionIndexAccessor, nativeAccessor, spatialAccessor, luceneAccessor ); + for ( int i = 0; i < allAccessors.length; i++ ) + { + verifyFusionCloseThrowOnSingleCloseThrow( allAccessors[i], fusionIndexAccessor ); + createAccessors(); + } } @Test - public void closeMustCloseOthersIfNativeThrow() throws Exception + public void closeMustCloseOthersIfOneThrow() throws Exception { - verifyOtherIsClosedOnSingleThrow( nativeAccessor, fusionIndexAccessor, luceneAccessor, spatialAccessor, temporalAccessor ); + int count = allAccessors.length; + for ( int i = 0; i < count; i++ ) + { + IndexAccessor accessor = allAccessors[i]; + verifyOtherIsClosedOnSingleThrow( accessor, fusionIndexAccessor, without( allAccessors, accessor ) ); + createAccessors(); + } } @Test public void closeMustThrowIfAllFail() throws Exception { - verifyFusionCloseThrowIfAllThrow( fusionIndexAccessor, nativeAccessor, spatialAccessor, temporalAccessor, luceneAccessor ); + verifyFusionCloseThrowIfAllThrow( fusionIndexAccessor, allAccessors ); } // newAllEntriesReader @@ -267,119 +220,64 @@ public void closeMustThrowIfAllFail() throws Exception public void allEntriesReaderMustCombineResultFromAll() { // given - long[] nativeEntries = {0, 1, 6, 13, 14}; - long[] spatialEntries = {2, 5, 9}; - long[] temporalEntries = {4, 8, 11}; - long[] luceneEntries = {3, 7, 10, 12}; - mockAllEntriesReaders( nativeEntries, spatialEntries, temporalEntries, luceneEntries ); - - // when - Set result = Iterables.asSet( fusionIndexAccessor.newAllEntriesReader() ); - - // then - assertResultContainsAll( result, nativeEntries ); - assertResultContainsAll( result, spatialEntries ); - assertResultContainsAll( result, temporalEntries ); - assertResultContainsAll( result, luceneEntries ); - } - - @Test - public void allEntriesReaderMustCombineResultFromAllWithEmptyNative() - { - // given - long[] nativeEntries = new long[0]; - long[] spatialEntries = {2, 5, 9}; - long[] temporalEntries = {4, 8, 11}; - long[] luceneEntries = {3, 4, 7, 8}; - mockAllEntriesReaders( nativeEntries, spatialEntries, temporalEntries, luceneEntries ); - - // when - Set result = Iterables.asSet( fusionIndexAccessor.newAllEntriesReader() ); - - // then - assertResultContainsAll( result, nativeEntries ); - assertResultContainsAll( result, spatialEntries ); - assertResultContainsAll( result, temporalEntries ); - assertResultContainsAll( result, luceneEntries ); - } - @Test - public void allEntriesReaderMustCombineResultFromAllWithEmptySpatial() - { - // given - long[] nativeEntries = {0, 1, 6, 13, 14}; - long[] spatialEntries = new long[0]; - long[] temporalEntries = {4, 8, 11}; - long[] luceneEntries = {3, 7, 10, 12}; - mockAllEntriesReaders( nativeEntries, spatialEntries, temporalEntries, luceneEntries ); - - // when - Set result = Iterables.asSet( fusionIndexAccessor.newAllEntriesReader() ); - - // then - assertResultContainsAll( result, nativeEntries ); - assertResultContainsAll( result, spatialEntries ); - assertResultContainsAll( result, temporalEntries ); - assertResultContainsAll( result, luceneEntries ); - } - - @Test - public void allEntriesReaderMustCombineResultFromAllWithEmptyTemporal() - { - // given - long[] nativeEntries = {0, 1, 6, 13, 14}; - long[] spatialEntries = {2, 5, 9}; - long[] temporalEntries = new long[0]; - long[] luceneEntries = {3, 7, 10, 12}; - mockAllEntriesReaders( nativeEntries, spatialEntries, temporalEntries, luceneEntries ); + long[][] ids = new long[allAccessors.length][]; + long lastId = 0; + for ( int i = 0; i < ids.length; i++ ) + { + ids[i] = new long[] {lastId++, lastId++}; + } + mockAllEntriesReaders( ids ); // when Set result = Iterables.asSet( fusionIndexAccessor.newAllEntriesReader() ); // then - assertResultContainsAll( result, nativeEntries ); - assertResultContainsAll( result, spatialEntries ); - assertResultContainsAll( result, temporalEntries ); - assertResultContainsAll( result, luceneEntries ); + for ( long[] part : ids ) + { + assertResultContainsAll( result, part ); + } } @Test - public void allEntriesReaderMustCombineResultFromAllWithEmptyLucene() + public void allEntriesReaderMustCombineResultFromAllWithOneEmpty() { - // given - long[] nativeEntries = {0, 1, 2, 5, 6}; - long[] spatialEntries = {2, 5, 9}; - long[] temporalEntries = {4, 8, 11}; - long[] luceneEntries = new long[0]; - mockAllEntriesReaders( nativeEntries, spatialEntries, temporalEntries, luceneEntries ); + for ( int i = 0; i < allAccessors.length; i++ ) + { + // given + long[][] ids = new long[allAccessors.length][]; + long lastId = 0; + for ( int j = 0; j < ids.length; j++ ) + { + ids[j] = j == i ? new long[0] : new long[]{lastId++, lastId++}; + } + mockAllEntriesReaders( ids ); - // when - Set result = Iterables.asSet( fusionIndexAccessor.newAllEntriesReader() ); + // when + Set result = Iterables.asSet( fusionIndexAccessor.newAllEntriesReader() ); - // then - assertResultContainsAll( result, nativeEntries ); - assertResultContainsAll( result, spatialEntries ); - assertResultContainsAll( result, temporalEntries ); - assertResultContainsAll( result, luceneEntries ); + // then + for ( long[] part : ids ) + { + assertResultContainsAll( result, part ); + } + } } @Test public void allEntriesReaderMustCombineResultFromAllEmpty() { // given - long[] nativeEntries = new long[0]; - long[] spatialEntries = new long[0]; - long[] temporalEntries = new long[0]; - long[] luceneEntries = new long[0]; - mockAllEntriesReaders( nativeEntries, spatialEntries, temporalEntries, luceneEntries ); + long[][] ids = new long[allAccessors.length][]; + for ( int j = 0; j < ids.length; j++ ) + { + ids[j] = new long[0]; + } + mockAllEntriesReaders( ids ); // when Set result = Iterables.asSet( fusionIndexAccessor.newAllEntriesReader() ); // then - assertResultContainsAll( result, nativeEntries ); - assertResultContainsAll( result, spatialEntries ); - assertResultContainsAll( result, temporalEntries ); - assertResultContainsAll( result, luceneEntries ); assertTrue( result.isEmpty() ); } @@ -387,214 +285,101 @@ public void allEntriesReaderMustCombineResultFromAllEmpty() public void allEntriesReaderMustCloseAll() throws Exception { // given - BoundedIterable nativeAllEntriesReader = mockSingleAllEntriesReader( nativeAccessor, new long[0] ); - BoundedIterable spatialAllEntriesReader = mockSingleAllEntriesReader( spatialAccessor, new long[0] ); - BoundedIterable temporalAllEntriesReader = mockSingleAllEntriesReader( temporalAccessor, new long[0] ); - BoundedIterable luceneAllEntriesReader = mockSingleAllEntriesReader( luceneAccessor, new long[0] ); + BoundedIterable[] allEntriesReaders = Arrays.stream( allAccessors ).map( accessor -> mockSingleAllEntriesReader( accessor, new long[0] ) ) + .toArray( BoundedIterable[]::new ); // when fusionIndexAccessor.newAllEntriesReader().close(); // then - verify( nativeAllEntriesReader, times( 1 ) ).close(); - verify( spatialAllEntriesReader, times( 1 ) ).close(); - verify( temporalAllEntriesReader, times( 1 ) ).close(); - verify( luceneAllEntriesReader, times( 1 ) ).close(); - } - - @Test - public void allEntriesReaderMustCloseNativeIfLuceneThrow() throws Exception - { - // given - BoundedIterable nativeAllEntriesReader = mockSingleAllEntriesReader( nativeAccessor, new long[0] ); - BoundedIterable spatialAllEntriesReader = mockSingleAllEntriesReader( spatialAccessor, new long[0] ); - BoundedIterable temporalAllEntriesReader = mockSingleAllEntriesReader( temporalAccessor, new long[0] ); - BoundedIterable luceneAllEntriesReader = mockSingleAllEntriesReader( luceneAccessor, new long[0] ); - - // then - BoundedIterable fusionAllEntriesReader = fusionIndexAccessor.newAllEntriesReader(); - verifyOtherIsClosedOnSingleThrow( luceneAllEntriesReader, fusionAllEntriesReader, nativeAllEntriesReader, spatialAllEntriesReader, - temporalAllEntriesReader ); - } - - @Test - public void allEntriesReaderMustCloseAllIfSpatialThrow() throws Exception - { - // given - BoundedIterable nativeAllEntriesReader = mockSingleAllEntriesReader( nativeAccessor, new long[0] ); - BoundedIterable spatialAllEntriesReader = mockSingleAllEntriesReader( spatialAccessor, new long[0] ); - BoundedIterable temporalAllEntriesReader = mockSingleAllEntriesReader( temporalAccessor, new long[0] ); - BoundedIterable luceneAllEntriesReader = mockSingleAllEntriesReader( luceneAccessor, new long[0] ); - - // then - BoundedIterable fusionAllEntriesReader = fusionIndexAccessor.newAllEntriesReader(); - verifyOtherIsClosedOnSingleThrow( spatialAllEntriesReader, fusionAllEntriesReader, luceneAllEntriesReader, nativeAllEntriesReader, - temporalAllEntriesReader ); - } - - @Test - public void allEntriesReaderMustCloseAllIfTemporalThrow() throws Exception - { - // given - BoundedIterable nativeAllEntriesReader = mockSingleAllEntriesReader( nativeAccessor, new long[0] ); - BoundedIterable spatialAllEntriesReader = mockSingleAllEntriesReader( spatialAccessor, new long[0] ); - BoundedIterable temporalAllEntriesReader = mockSingleAllEntriesReader( temporalAccessor, new long[0] ); - BoundedIterable luceneAllEntriesReader = mockSingleAllEntriesReader( luceneAccessor, new long[0] ); - - // then - BoundedIterable fusionAllEntriesReader = fusionIndexAccessor.newAllEntriesReader(); - verifyOtherIsClosedOnSingleThrow( temporalAllEntriesReader, fusionAllEntriesReader, luceneAllEntriesReader, spatialAllEntriesReader, - nativeAllEntriesReader ); - } - - @Test - public void allEntriesReaderMustCloseAllIfNativeThrow() throws Exception - { - // given - BoundedIterable nativeAllEntriesReader = mockSingleAllEntriesReader( nativeAccessor, new long[0] ); - BoundedIterable spatialAllEntriesReader = mockSingleAllEntriesReader( spatialAccessor, new long[0] ); - BoundedIterable temporalAllEntriesReader = mockSingleAllEntriesReader( temporalAccessor, new long[0] ); - BoundedIterable luceneAllEntriesReader = mockSingleAllEntriesReader( luceneAccessor, new long[0] ); - - // then - BoundedIterable fusionAllEntriesReader = fusionIndexAccessor.newAllEntriesReader(); - verifyOtherIsClosedOnSingleThrow( nativeAllEntriesReader, fusionAllEntriesReader, luceneAllEntriesReader, spatialAllEntriesReader, - temporalAllEntriesReader ); - } - - @Test - public void allEntriesReaderMustThrowIfLuceneThrow() throws Exception - { - // given - mockSingleAllEntriesReader( nativeAccessor, new long[0] ); - mockSingleAllEntriesReader( spatialAccessor, new long[0] ); - mockSingleAllEntriesReader( temporalAccessor, new long[0] ); - BoundedIterable luceneAllEntriesReader = mockSingleAllEntriesReader( luceneAccessor, new long[0] ); - - // then - BoundedIterable fusionAllEntriesReader = fusionIndexAccessor.newAllEntriesReader(); - FusionIndexTestHelp.verifyFusionCloseThrowOnSingleCloseThrow( luceneAllEntriesReader, fusionAllEntriesReader ); - } - - @Test - public void allEntriesReaderMustThrowIfNativeThrow() throws Exception - { - // given - BoundedIterable nativeAllEntriesReader = mockSingleAllEntriesReader( nativeAccessor, new long[0] ); - mockSingleAllEntriesReader( spatialAccessor, new long[0] ); - mockSingleAllEntriesReader( temporalAccessor, new long[0] ); - mockSingleAllEntriesReader( luceneAccessor, new long[0] ); - - // then - BoundedIterable fusionAllEntriesReader = fusionIndexAccessor.newAllEntriesReader(); - FusionIndexTestHelp.verifyFusionCloseThrowOnSingleCloseThrow( nativeAllEntriesReader, fusionAllEntriesReader ); - - } - - @Test - public void allEntriesReaderMustThrowIfSpatialThrow() throws Exception - { - // given - mockSingleAllEntriesReader( nativeAccessor, new long[0] ); - BoundedIterable spatialAllEntriesReader = mockSingleAllEntriesReader( spatialAccessor, new long[0] ); - mockSingleAllEntriesReader( temporalAccessor, new long[0] ); - mockSingleAllEntriesReader( luceneAccessor, new long[0] ); - - // then - BoundedIterable fusionAllEntriesReader = fusionIndexAccessor.newAllEntriesReader(); - FusionIndexTestHelp.verifyFusionCloseThrowOnSingleCloseThrow( spatialAllEntriesReader, fusionAllEntriesReader ); - - } - - @Test - public void allEntriesReaderMustThrowIfTemporalThrow() throws Exception - { - // given - mockSingleAllEntriesReader( nativeAccessor, new long[0] ); - mockSingleAllEntriesReader( spatialAccessor, new long[0] ); - BoundedIterable temporalAllEntriesReader = mockSingleAllEntriesReader( temporalAccessor, new long[0] ); - mockSingleAllEntriesReader( luceneAccessor, new long[0] ); - - // then - BoundedIterable fusionAllEntriesReader = fusionIndexAccessor.newAllEntriesReader(); - FusionIndexTestHelp.verifyFusionCloseThrowOnSingleCloseThrow( temporalAllEntriesReader, fusionAllEntriesReader ); - + for ( BoundedIterable allEntriesReader : allEntriesReaders ) + { + verify( allEntriesReader, times( 1 ) ).close(); + } } @Test - public void allEntriesReaderMustReportUnknownMaxCountIfNativeReportUnknownMaxCount() + public void allEntriesReaderMustCloseOthersIfOneThrow() throws Exception { - // given - mockSingleAllEntriesReaderWithUnknownMaxCount( nativeAccessor, new long[0] ); - mockSingleAllEntriesReader( spatialAccessor, new long[0] ); - mockSingleAllEntriesReader( temporalAccessor, new long[0] ); - mockSingleAllEntriesReader( luceneAccessor, new long[0] ); - - // then - BoundedIterable fusionAllEntriesReader = fusionIndexAccessor.newAllEntriesReader(); - assertThat( fusionAllEntriesReader.maxCount(), is( BoundedIterable.UNKNOWN_MAX_COUNT ) ); - } + for ( int i = 0; i < allAccessors.length; i++ ) + { + // given + BoundedIterable[] allEntriesReaders = + Arrays.stream( allAccessors ).map( accessor -> mockSingleAllEntriesReader( accessor, new long[0] ) ).toArray( BoundedIterable[]::new ); - @Test - public void allEntriesReaderMustReportUnknownMaxCountIfSpatialReportUnknownMaxCount() throws Exception - { - // given - mockSingleAllEntriesReader( nativeAccessor, new long[0] ); - mockSingleAllEntriesReaderWithUnknownMaxCount( spatialAccessor, new long[0] ); - mockSingleAllEntriesReader( temporalAccessor, new long[0] ); - mockSingleAllEntriesReader( luceneAccessor, new long[0] ); + // then + BoundedIterable fusionAllEntriesReader = fusionIndexAccessor.newAllEntriesReader(); + verifyOtherIsClosedOnSingleThrow( allEntriesReaders[i], fusionAllEntriesReader, without( allEntriesReaders, allEntriesReaders[i] ) ); - // then - BoundedIterable fusionAllEntriesReader = fusionIndexAccessor.newAllEntriesReader(); - assertThat( fusionAllEntriesReader.maxCount(), is( BoundedIterable.UNKNOWN_MAX_COUNT ) ); + createAccessors(); + } } @Test - public void allEntriesReaderMustReportUnknownMaxCountIfTemporalReportUnknownMaxCount() throws Exception + public void allEntriesReaderMustThrowIfOneThrow() throws Exception { - // given - mockSingleAllEntriesReader( nativeAccessor, new long[0] ); - mockSingleAllEntriesReader( spatialAccessor, new long[0] ); - mockSingleAllEntriesReaderWithUnknownMaxCount( temporalAccessor, new long[0] ); - mockSingleAllEntriesReader( luceneAccessor, new long[0] ); + for ( int i = 0; i < allAccessors.length; i++ ) + { + // given + BoundedIterable allEntriesReader = null; + for ( int j = 0; j < allAccessors.length; j++ ) + { + BoundedIterable reader = mockSingleAllEntriesReader( allAccessors[j], new long[0] ); + if ( j == i ) + { + allEntriesReader = reader; + } + } - // then - BoundedIterable fusionAllEntriesReader = fusionIndexAccessor.newAllEntriesReader(); - assertThat( fusionAllEntriesReader.maxCount(), is( BoundedIterable.UNKNOWN_MAX_COUNT ) ); + // then + BoundedIterable fusionAllEntriesReader = fusionIndexAccessor.newAllEntriesReader(); + FusionIndexTestHelp.verifyFusionCloseThrowOnSingleCloseThrow( allEntriesReader, fusionAllEntriesReader ); + } } @Test - public void allEntriesReaderMustReportUnknownMaxCountIfLuceneReportUnknownMaxCount() + public void allEntriesReaderMustReportUnknownMaxCountIfAnyReportUnknownMaxCount() { - // given - mockSingleAllEntriesReader( nativeAccessor, new long[0] ); - mockSingleAllEntriesReader( spatialAccessor, new long[0] ); - mockSingleAllEntriesReader( temporalAccessor, new long[0] ); - mockSingleAllEntriesReaderWithUnknownMaxCount( luceneAccessor, new long[0] ); + for ( int i = 0; i < allAccessors.length; i++ ) + { + for ( int j = 0; j < allAccessors.length; j++ ) + { + // given + if ( j == i ) + { + mockSingleAllEntriesReaderWithUnknownMaxCount( allAccessors[j], new long[0] ); + } + else + { + mockSingleAllEntriesReader( allAccessors[j], new long[0] ); + } + } - // then - BoundedIterable fusionAllEntriesReader = fusionIndexAccessor.newAllEntriesReader(); - assertThat( fusionAllEntriesReader.maxCount(), is( BoundedIterable.UNKNOWN_MAX_COUNT ) ); + // then + BoundedIterable fusionAllEntriesReader = fusionIndexAccessor.newAllEntriesReader(); + assertThat( fusionAllEntriesReader.maxCount(), is( BoundedIterable.UNKNOWN_MAX_COUNT ) ); + } } @Test - public void allEntriesReaderMustReportFusionMaxCountOfNativeAndLucene() + public void allEntriesReaderMustReportFusionMaxCountOfAll() { - mockSingleAllEntriesReader( nativeAccessor, new long[]{1, 2} ); - mockSingleAllEntriesReader( spatialAccessor, new long[]{3, 4} ); - mockSingleAllEntriesReader( temporalAccessor, new long[]{5, 6} ); - mockSingleAllEntriesReader( luceneAccessor, new long[]{7, 8} ); + long lastId = 0; + for ( IndexAccessor accessor : allAccessors ) + { + mockSingleAllEntriesReader( accessor, new long[]{lastId++, lastId++} ); + } // then BoundedIterable fusionAllEntriesReader = fusionIndexAccessor.newAllEntriesReader(); - assertThat( fusionAllEntriesReader.maxCount(), is( 8L ) ); + assertThat( fusionAllEntriesReader.maxCount(), is( lastId ) ); } - static void assertResultContainsAll( Set result, long[] nativeEntries ) + static void assertResultContainsAll( Set result, long[] numberEntries ) { - for ( long nativeEntry : nativeEntries ) + for ( long numberEntry : numberEntries ) { - assertTrue( "Expected to contain " + nativeEntry + ", but was " + result, result.contains( nativeEntry ) ); + assertTrue( "Expected to contain " + numberEntry + ", but was " + result, result.contains( numberEntry ) ); } } @@ -629,11 +414,11 @@ static BoundedIterable mockedAllEntriesReader( boolean knownMaxCount, long return mockedAllEntriesReader; } - private void mockAllEntriesReaders( long[] numberEntries, long[] spatialEntries, long[] temporalEntries, long[] luceneEntries ) + private void mockAllEntriesReaders( long[]... entries ) { - mockSingleAllEntriesReader( nativeAccessor, numberEntries ); - mockSingleAllEntriesReader( spatialAccessor, spatialEntries ); - mockSingleAllEntriesReader( temporalAccessor, temporalEntries ); - mockSingleAllEntriesReader( luceneAccessor, luceneEntries ); + for ( int i = 0; i < entries.length; i++ ) + { + mockSingleAllEntriesReader( allAccessors[i], entries[i] ); + } } } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexPopulatorTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexPopulatorTest.java index 2d897a4302964..27b4d6ad09f03 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexPopulatorTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexPopulatorTest.java @@ -23,6 +23,7 @@ import org.junit.Test; import java.io.IOException; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -33,23 +34,25 @@ import org.neo4j.kernel.impl.index.schema.fusion.FusionIndexProvider.DropAction; import org.neo4j.values.storable.Value; -import static org.hamcrest.Matchers.sameInstance; -import static org.hamcrest.core.AnyOf.anyOf; -import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.neo4j.helpers.ArrayUtil.contains; +import static org.neo4j.helpers.ArrayUtil.without; +import static org.neo4j.helpers.collection.Iterators.array; import static org.neo4j.kernel.impl.index.schema.fusion.FusionIndexTestHelp.add; import static org.neo4j.kernel.impl.index.schema.fusion.FusionIndexTestHelp.verifyCallFail; public class FusionIndexPopulatorTest { + private IndexPopulator stringPopulator; private IndexPopulator numberPopulator; private IndexPopulator spatialPopulator; private IndexPopulator temporalPopulator; @@ -62,19 +65,20 @@ public class FusionIndexPopulatorTest @Before public void mockComponents() { + stringPopulator = mock( IndexPopulator.class ); numberPopulator = mock( IndexPopulator.class ); spatialPopulator = mock( IndexPopulator.class ); temporalPopulator = mock( IndexPopulator.class ); lucenePopulator = mock( IndexPopulator.class ); - allPopulators = new IndexPopulator[]{numberPopulator, spatialPopulator, temporalPopulator, lucenePopulator}; - fusionIndexPopulator = - new FusionIndexPopulator( numberPopulator, spatialPopulator, temporalPopulator, lucenePopulator, new FusionSelector(), indexId, dropAction ); + allPopulators = array( stringPopulator, numberPopulator, spatialPopulator, temporalPopulator, lucenePopulator ); + fusionIndexPopulator = new FusionIndexPopulator( stringPopulator, numberPopulator, spatialPopulator, temporalPopulator, lucenePopulator, + new FusionSelector(), indexId, dropAction ); } /* create */ @Test - public void createMustCreateBothNativeAndLucene() throws Exception + public void createMustCreateAll() throws Exception { // when fusionIndexPopulator.create(); @@ -87,59 +91,23 @@ public void createMustCreateBothNativeAndLucene() throws Exception } @Test - public void createMustThrowIfCreateNativeThrow() throws Exception + public void createMustThrowIfAnyThrow() throws Exception { - // given - IOException failure = new IOException( "fail" ); - doThrow( failure ).when( numberPopulator ).create(); - - verifyCallFail( failure, () -> - { - fusionIndexPopulator.create(); - return null; - } ); - } - - @Test - public void createMustThrowIfCreateSpatialThrow() throws Exception - { - // given - IOException failure = new IOException( "fail" ); - doThrow( failure ).when( spatialPopulator ).create(); - - verifyCallFail( failure, () -> - { - fusionIndexPopulator.create(); - return null; - } ); - } - - @Test - public void createMustThrowIfCreateTemporalThrow() throws Exception - { - // given - IOException failure = new IOException( "fail" ); - doThrow( failure ).when( temporalPopulator ).create(); - - verifyCallFail( failure, () -> + for ( IndexPopulator populator : allPopulators ) { - fusionIndexPopulator.create(); - return null; - } ); - } + // given + IOException failure = new IOException( "fail" ); + doThrow( failure ).when( populator ).create(); - @Test - public void createMustThrowIfCreateLuceneThrow() throws Exception - { - // given - IOException failure = new IOException( "fail" ); - doThrow( failure ).when( lucenePopulator ).create(); + verifyCallFail( failure, () -> + { + fusionIndexPopulator.create(); + return null; + } ); - verifyCallFail( failure, () -> - { - fusionIndexPopulator.create(); - return null; - } ); + // reset throw for testing of next populator + doAnswer( invocation -> null ).when( populator ).create(); + } } /* drop */ @@ -159,59 +127,23 @@ public void dropMustDropAll() throws Exception } @Test - public void dropMustThrowIfDropNativeThrow() throws Exception - { - // given - IOException failure = new IOException( "fail" ); - doThrow( failure ).when( numberPopulator ).drop(); - - verifyCallFail( failure, () -> - { - fusionIndexPopulator.drop(); - return null; - } ); - } - - @Test - public void dropMustThrowIfDropSpatialThrow() throws Exception + public void dropMustThrowIfAnyDropThrow() throws Exception { - // given - IOException failure = new IOException( "fail" ); - doThrow( failure ).when( spatialPopulator ).drop(); - - verifyCallFail( failure, () -> - { - fusionIndexPopulator.drop(); - return null; - } ); - } - - @Test - public void dropMustThrowIfDropTemporalThrow() throws Exception - { - // given - IOException failure = new IOException( "fail" ); - doThrow( failure ).when( temporalPopulator ).drop(); - - verifyCallFail( failure, () -> + for ( IndexPopulator populator : allPopulators ) { - fusionIndexPopulator.drop(); - return null; - } ); - } + // given + IOException failure = new IOException( "fail" ); + doThrow( failure ).when( populator ).drop(); - @Test - public void dropMustThrowIfDropLuceneThrow() throws Exception - { - // given - IOException failure = new IOException( "fail" ); - doThrow( failure ).when( lucenePopulator ).drop(); + verifyCallFail( failure, () -> + { + fusionIndexPopulator.drop(); + return null; + } ); - verifyCallFail( failure, () -> - { - fusionIndexPopulator.drop(); - return null; - } ); + // reset throw for testing of next populator + doAnswer( invocation -> null ).when( populator ).drop(); + } } /* add */ @@ -220,34 +152,15 @@ public void dropMustThrowIfDropLuceneThrow() throws Exception public void addMustSelectCorrectPopulator() throws Exception { // given - Value[] numberValues = FusionIndexTestHelp.valuesSupportedByNative(); - Value[] spatialValues = FusionIndexTestHelp.valuesSupportedBySpatial(); - Value[] temporalValues = FusionIndexTestHelp.valuesSupportedByTemporal(); - Value[] otherValues = FusionIndexTestHelp.valuesNotSupportedByNativeOrSpatial(); + Value[][] values = FusionIndexTestHelp.valuesByGroup(); Value[] allValues = FusionIndexTestHelp.allValues(); - // Add with native for number values - for ( Value numberValue : numberValues ) + for ( int i = 0; i < allPopulators.length; i++ ) { - verifyAddWithCorrectPopulator( numberPopulator, numberValue ); - } - - // Add with spatial for geometric values - for ( Value spatialValue : spatialValues ) - { - verifyAddWithCorrectPopulator( spatialPopulator, spatialValue ); - } - - // Add with temporal for temporal values - for ( Value temporalValue : temporalValues ) - { - verifyAddWithCorrectPopulator( temporalPopulator, temporalValue ); - } - - // Add with lucene for other values - for ( Value otherValue : otherValues ) - { - verifyAddWithCorrectPopulator( lucenePopulator, otherValue ); + for ( Value value : values[i] ) + { + verifyAddWithCorrectPopulator( allPopulators[i], value ); + } } // All composite values should go to lucene @@ -275,66 +188,30 @@ private void verifyAddWithCorrectPopulator( IndexPopulator correctPopulator, Val } } - /* verifyDeferredConstraints */ - - @Test - public void verifyDeferredConstraintsMustThrowIfNativeThrow() throws Exception - { - // given - IndexEntryConflictException failure = mock( IndexEntryConflictException.class ); - doThrow( failure ).when( numberPopulator ).verifyDeferredConstraints( any() ); - - verifyCallFail( failure, () -> - { - fusionIndexPopulator.verifyDeferredConstraints( null ); - return null; - } ); - } + /* verifyDeferredConstraints */ @Test - public void verifyDeferredConstraintsMustThrowIfSpatialThrow() throws Exception + public void verifyDeferredConstraintsMustThrowIfAnyThrow() throws Exception { - // given - IndexEntryConflictException failure = mock( IndexEntryConflictException.class ); - doThrow( failure ).when( spatialPopulator ).verifyDeferredConstraints( any() ); - - verifyCallFail( failure, () -> + for ( IndexPopulator populator : allPopulators ) { - fusionIndexPopulator.verifyDeferredConstraints( null ); - return null; - } ); - } + // given + IndexEntryConflictException failure = mock( IndexEntryConflictException.class ); + doThrow( failure ).when( populator ).verifyDeferredConstraints( any() ); - @Test - public void verifyDeferredConstraintsMustThrowIfTemporalThrow() throws Exception - { - // given - IndexEntryConflictException failure = mock( IndexEntryConflictException.class ); - doThrow( failure ).when( temporalPopulator ).verifyDeferredConstraints( any() ); + verifyCallFail( failure, () -> + { + fusionIndexPopulator.verifyDeferredConstraints( null ); + return null; + } ); - verifyCallFail( failure, () -> - { - fusionIndexPopulator.verifyDeferredConstraints( null ); - return null; - } ); + // reset throw for testing of next populator + doAnswer( invocation -> null ).when( populator ).verifyDeferredConstraints( any() ); + } } - @Test - public void verifyDeferredConstraintsMustThrowIfLuceneThrow() throws Exception - { - // given - IndexEntryConflictException failure = mock( IndexEntryConflictException.class ); - doThrow( failure ).when( lucenePopulator ).verifyDeferredConstraints( any() ); - - verifyCallFail( failure, () -> - { - fusionIndexPopulator.verifyDeferredConstraints( null ); - return null; - } ); - } /* close */ - @Test public void successfulCloseMustCloseAll() throws Exception { @@ -361,59 +238,23 @@ private void closeAndVerifyPropagation( boolean populationCompletedSuccessfully } @Test - public void closeMustThrowIfCloseNativeThrow() throws Exception - { - // given - IOException failure = new IOException( "fail" ); - doThrow( failure ).when( numberPopulator ).close( anyBoolean() ); - - verifyCallFail( failure, () -> - { - fusionIndexPopulator.close( anyBoolean() ); - return null; - } ); - } - - @Test - public void closeMustThrowIfCloseSpatialThrow() throws Exception + public void closeMustThrowIfCloseAnyThrow() throws Exception { - // given - IOException failure = new IOException( "fail" ); - doThrow( failure ).when( spatialPopulator ).close( anyBoolean() ); - - verifyCallFail( failure, () -> - { - fusionIndexPopulator.close( anyBoolean() ); - return null; - } ); - } - - @Test - public void closeMustThrowIfCloseTemporalThrow() throws Exception - { - // given - IOException failure = new IOException( "fail" ); - doThrow( failure ).when( temporalPopulator ).close( anyBoolean() ); - - verifyCallFail( failure, () -> + for ( IndexPopulator populator : allPopulators ) { - fusionIndexPopulator.close( anyBoolean() ); - return null; - } ); - } + // given + IOException failure = new IOException( "fail" ); + doThrow( failure ).when( populator ).close( anyBoolean() ); - @Test - public void closeMustThrowIfCloseLuceneThrow() throws Exception - { - // given - IOException failure = new IOException( "fail" ); - doThrow( failure ).when( lucenePopulator ).close( anyBoolean() ); + verifyCallFail( failure, () -> + { + fusionIndexPopulator.close( anyBoolean() ); + return null; + } ); - verifyCallFail( failure, () -> - { - fusionIndexPopulator.close( anyBoolean() ); - return null; - } ); + // reset throw for testing of next populator + doAnswer( invocation -> null ).when( populator ).close( anyBoolean() ); + } } private static void verifyOtherCloseOnThrow( IndexPopulator throwingPopulator, FusionIndexPopulator fusionPopulator, IndexPopulator... populators ) @@ -441,41 +282,26 @@ private static void verifyOtherCloseOnThrow( IndexPopulator throwingPopulator, F } @Test - public void closeMustCloseOthersIfLuceneThrow() throws Exception - { - verifyOtherCloseOnThrow( lucenePopulator, fusionIndexPopulator, numberPopulator, spatialPopulator, temporalPopulator ); - } - - @Test - public void closeMustCloseOthersIfSpatialThrow() throws Exception + public void closeMustCloseOthersIfAnyThrow() throws Exception { - verifyOtherCloseOnThrow( spatialPopulator, fusionIndexPopulator, numberPopulator, temporalPopulator, lucenePopulator ); - } - - @Test - public void closeMustCloseOthersIfTemporalThrow() throws Exception - { - verifyOtherCloseOnThrow( temporalPopulator, fusionIndexPopulator, numberPopulator, spatialPopulator, lucenePopulator ); - } - - @Test - public void closeMustCloseOthersIfNumberThrow() throws Exception - { - verifyOtherCloseOnThrow( numberPopulator, fusionIndexPopulator, spatialPopulator, temporalPopulator, lucenePopulator ); + for ( int i = 0; i < allPopulators.length; i++ ) + { + IndexPopulator populator = allPopulators[i]; + verifyOtherCloseOnThrow( populator, fusionIndexPopulator, without( allPopulators, populator ) ); + mockComponents(); + } } @Test public void closeMustThrowIfAllThrow() throws Exception { // given - IOException nativeFailure = new IOException( "native" ); - IOException spatialFailure = new IOException( "spatial" ); - IOException temporalFailure = new IOException( "temporal" ); - IOException luceneFailure = new IOException( "lucene" ); - doThrow( nativeFailure ).when( numberPopulator ).close( anyBoolean() ); - doThrow( spatialFailure ).when( spatialPopulator ).close( anyBoolean() ); - doThrow( temporalFailure ).when( temporalPopulator ).close( anyBoolean() ); - doThrow( luceneFailure ).when( lucenePopulator).close( anyBoolean() ); + IOException[] failures = new IOException[allPopulators.length]; + for ( int i = 0; i < allPopulators.length; i++ ) + { + failures[i] = new IOException( "FAILURE[" + i + "]" ); + doThrow( failures[i] ).when( allPopulators[i] ).close( anyBoolean() ); + } try { @@ -486,16 +312,15 @@ public void closeMustThrowIfAllThrow() throws Exception catch ( IOException e ) { // then - assertThat( e, anyOf( - sameInstance( nativeFailure ), - sameInstance( spatialFailure ), - sameInstance( temporalFailure ), - sameInstance( luceneFailure ) ) ); + if ( !contains( failures, e ) ) + { + fail( "Thrown exception didn't matchh any of the expected failures: " + Arrays.toString( failures ) ); + } } } - /* markAsFailed */ + /* markAsFailed */ @Test public void markAsFailedMustMarkAll() throws Exception { @@ -511,78 +336,36 @@ public void markAsFailedMustMarkAll() throws Exception } @Test - public void markAsFailedMustThrowIfNativeThrow() throws Exception - { - // given - IOException failure = new IOException( "fail" ); - doThrow( failure ).when( numberPopulator ).markAsFailed( anyString() ); - - // then - verifyCallFail( failure, () -> - { - fusionIndexPopulator.markAsFailed( anyString() ); - return null; - } ); - } - - @Test - public void markAsFailedMustThrowIfSpatialThrow() throws Exception + public void markAsFailedMustThrowIfAnyThrow() throws Exception { - // given - IOException failure = new IOException( "fail" ); - doThrow( failure ).when( spatialPopulator ).markAsFailed( anyString() ); - - // then - verifyCallFail( failure, () -> + for ( IndexPopulator populator : allPopulators ) { - fusionIndexPopulator.markAsFailed( anyString() ); - return null; - } ); - } + // given + IOException failure = new IOException( "fail" ); + doThrow( failure ).when( populator ).markAsFailed( anyString() ); - @Test - public void markAsFailedMustThrowIfTemporalThrow() throws Exception - { - // given - IOException failure = new IOException( "fail" ); - doThrow( failure ).when( temporalPopulator ).markAsFailed( anyString() ); + // then + verifyCallFail( failure, () -> + { + fusionIndexPopulator.markAsFailed( anyString() ); + return null; + } ); - // then - verifyCallFail( failure, () -> - { - fusionIndexPopulator.markAsFailed( anyString() ); - return null; - } ); + // reset throw for testing of next populator + doAnswer( invocation -> null ).when( populator ).markAsFailed( anyString() ); + } } @Test - public void markAsFailedMustThrowIfLuceneThrow() throws Exception + public void shouldIncludeSampleOnCorrectPopulator() { // given - IOException failure = new IOException( "fail" ); - doThrow( failure ).when( lucenePopulator ).markAsFailed( anyString() ); + Value[][] values = FusionIndexTestHelp.valuesByGroup(); - // then - verifyCallFail( failure, () -> + for ( int i = 0; i < allPopulators.length; i++ ) { - fusionIndexPopulator.markAsFailed( anyString() ); - return null; - } ); - } - - @Test - public void shouldIncludeSampleOnCorrectPopulator() - { - // given - Value[] numberValues = FusionIndexTestHelp.valuesSupportedByNative(); - Value[] spatialValues = FusionIndexTestHelp.valuesSupportedBySpatial(); - Value[] temporalValues = FusionIndexTestHelp.valuesSupportedByTemporal(); - Value[] otherValues = FusionIndexTestHelp.valuesNotSupportedByNativeOrSpatial(); - - verifySampleToCorrectPopulator( numberValues, numberPopulator ); - verifySampleToCorrectPopulator( spatialValues, spatialPopulator ); - verifySampleToCorrectPopulator( temporalValues, temporalPopulator ); - verifySampleToCorrectPopulator( otherValues, lucenePopulator ); + verifySampleToCorrectPopulator( values[i], allPopulators[i] ); + } } private void verifySampleToCorrectPopulator( Value[] values, IndexPopulator populator ) diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexProviderTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexProviderTest.java index 3ca39260ee173..cff7d855afa5e 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexProviderTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexProviderTest.java @@ -50,22 +50,27 @@ public class FusionIndexProviderTest { private static final IndexProvider.Descriptor DESCRIPTOR = new IndexProvider.Descriptor( "test-fusion", "1" ); - private IndexProvider nativeProvider; + private IndexProvider stringProvider; + private IndexProvider numberProvider; private IndexProvider spatialProvider; private IndexProvider temporalProvider; private IndexProvider luceneProvider; + private IndexProvider[] providers; @Before public void setup() { - nativeProvider = mock( IndexProvider.class ); + stringProvider = mock( IndexProvider.class ); + numberProvider = mock( IndexProvider.class ); spatialProvider = mock( IndexProvider.class ); temporalProvider = mock( IndexProvider.class ); luceneProvider = mock( IndexProvider.class ); - when( nativeProvider.getProviderDescriptor() ).thenReturn( new IndexProvider.Descriptor( "native", "1" ) ); + when( stringProvider.getProviderDescriptor() ).thenReturn( new IndexProvider.Descriptor( "string", "1" ) ); + when( numberProvider.getProviderDescriptor() ).thenReturn( new IndexProvider.Descriptor( "number", "1" ) ); when( spatialProvider.getProviderDescriptor() ).thenReturn( new IndexProvider.Descriptor( "spatial", "1" ) ); when( temporalProvider.getProviderDescriptor() ).thenReturn( new IndexProvider.Descriptor( "temporal", "1" ) ); when( luceneProvider.getProviderDescriptor() ).thenReturn( new IndexProvider.Descriptor( "lucene", "1" ) ); + providers = array( stringProvider, numberProvider, spatialProvider, temporalProvider, luceneProvider ); } @Rule @@ -75,51 +80,21 @@ public void setup() public void mustSelectCorrectTargetForAllGivenValueCombinations() { // given - Value[] numberValues = FusionIndexTestHelp.valuesSupportedByNative(); - Value[] spatialValues = FusionIndexTestHelp.valuesSupportedBySpatial(); - Value[] temporalValues = FusionIndexTestHelp.valuesSupportedByTemporal(); - Value[] otherValues = FusionIndexTestHelp.valuesNotSupportedByNativeOrSpatial(); + Value[][] values = FusionIndexTestHelp.valuesByGroup(); Value[] allValues = FusionIndexTestHelp.allValues(); - - // Number values should go to native provider Selector selector = new FusionSelector(); - for ( Value numberValue : numberValues ) - { - // when - IndexProvider selected = selector.select( nativeProvider, spatialProvider, temporalProvider, luceneProvider, numberValue ); - - // then - assertSame( nativeProvider, selected ); - } - - // Geometric values should go to spatial provider - for ( Value spatialValue : spatialValues ) - { - // when - IndexProvider selected = selector.select( nativeProvider, spatialProvider, temporalProvider, luceneProvider, spatialValue ); - - // then - assertSame( spatialProvider, selected ); - } - // Temporal values should go to temporal provider - for ( Value temporalValue : temporalValues ) + for ( int i = 0; i < values.length; i++ ) { - // when - IndexProvider selected = selector.select( nativeProvider, spatialProvider, temporalProvider, luceneProvider, temporalValue ); - - // then - assertSame( temporalProvider, selected ); - } - - // Other values should go to lucene provider - for ( Value otherValue : otherValues ) - { - // when - IndexProvider selected = selector.select( nativeProvider, spatialProvider, temporalProvider, luceneProvider, otherValue ); + Value[] group = values[i]; + for ( Value value : group ) + { + // when + IndexProvider selected = selector.select( stringProvider, numberProvider, spatialProvider, temporalProvider, luceneProvider, value ); - // then - assertSame( luceneProvider, selected ); + // then + assertSame( providers[i], selected ); + } } // All composite values should go to lucene @@ -128,8 +103,8 @@ public void mustSelectCorrectTargetForAllGivenValueCombinations() for ( Value secondValue : allValues ) { // when - IndexProvider - selected = selector.select( nativeProvider, spatialProvider, temporalProvider, luceneProvider, firstValue, secondValue ); + IndexProvider selected = selector.select( stringProvider, numberProvider, spatialProvider, temporalProvider, luceneProvider, + firstValue, secondValue ); // then assertSame( luceneProvider, selected ); @@ -141,34 +116,28 @@ public void mustSelectCorrectTargetForAllGivenValueCombinations() public void mustCombineSamples() { // given - int nativeIndexSize = random.nextInt( 0, 1_000_000 ); - int nativeUniqueValues = random.nextInt( 0, 1_000_000 ); - int nativeSampleSize = random.nextInt( 0, 1_000_000 ); - IndexSample nativeSample = new IndexSample( nativeIndexSize, nativeUniqueValues, nativeSampleSize ); - - int spatialIndexSize = random.nextInt( 0, 1_000_000 ); - int spatialUniqueValues = random.nextInt( 0, 1_000_000 ); - int spatialSampleSize = random.nextInt( 0, 1_000_000 ); - IndexSample spatialSample = new IndexSample( spatialIndexSize, spatialUniqueValues, spatialSampleSize ); - - int temporalIndexSize = random.nextInt( 0, 1_000_000 ); - int temporalUniqueValues = random.nextInt( 0, 1_000_000 ); - int temporalSampleSize = random.nextInt( 0, 1_000_000 ); - IndexSample temporalSample = new IndexSample( temporalIndexSize, temporalUniqueValues, temporalSampleSize ); - - int luceneIndexSize = random.nextInt( 0, 1_000_000 ); - int luceneUniqueValues = random.nextInt( 0, 1_000_000 ); - int luceneSampleSize = random.nextInt( 0, 1_000_000 ); - IndexSample luceneSample = new IndexSample( luceneIndexSize, luceneUniqueValues, luceneSampleSize ); + int sumIndexSize = 0; + int sumUniqueValues = 0; + int sumSampleSize = 0; + IndexSample[] samples = new IndexSample[providers.length]; + for ( int i = 0; i < samples.length; i++ ) + { + int indexSize = random.nextInt( 0, 1_000_000 ); + int uniqueValues = random.nextInt( 0, 1_000_000 ); + int sampleSize = random.nextInt( 0, 1_000_000 ); + samples[i] = new IndexSample( indexSize, uniqueValues, sampleSize ); + sumIndexSize += indexSize; + sumUniqueValues += uniqueValues; + sumSampleSize += sampleSize; + } // when - IndexSample fusionSample = FusionIndexProvider - .combineSamples( nativeSample, spatialSample, temporalSample, luceneSample ); + IndexSample fusionSample = FusionIndexUtils.combineSamples( samples ); // then - assertEquals( nativeIndexSize + spatialIndexSize + temporalIndexSize + luceneIndexSize, fusionSample.indexSize() ); - assertEquals( nativeUniqueValues + spatialUniqueValues + temporalUniqueValues + luceneUniqueValues, fusionSample.uniqueValues() ); - assertEquals( nativeSampleSize + spatialSampleSize + temporalSampleSize + luceneSampleSize, fusionSample.sampleSize() ); + assertEquals( sumIndexSize, fusionSample.indexSize() ); + assertEquals( sumUniqueValues, fusionSample.uniqueValues() ); + assertEquals( sumSampleSize, fusionSample.sampleSize() ); } @Test @@ -179,14 +148,12 @@ public void getPopulationFailureMustThrowIfNoFailure() // when // ... no failure - IllegalStateException nativeThrow = new IllegalStateException( "no native failure" ); - IllegalStateException spatialThrow = new IllegalStateException( "no spatial failure" ); - IllegalStateException temporalThrow = new IllegalStateException( "no temporal failure" ); - IllegalStateException luceneThrow = new IllegalStateException( "no lucene failure" ); - when( nativeProvider.getPopulationFailure( anyLong(), any( SchemaIndexDescriptor.class ) ) ).thenThrow( nativeThrow ); - when( spatialProvider.getPopulationFailure( anyLong(), any( SchemaIndexDescriptor.class ) ) ).thenThrow( spatialThrow ); - when( temporalProvider.getPopulationFailure( anyLong(), any( SchemaIndexDescriptor.class ) ) ).thenThrow( temporalThrow ); - when( luceneProvider.getPopulationFailure( anyLong(), any( SchemaIndexDescriptor.class ) ) ).thenThrow( luceneThrow ); + IllegalStateException failure = new IllegalStateException( "not failed" ); + for ( int i = 0; i < providers.length; i++ ) + { + when( providers[i].getPopulationFailure( anyLong(), any( SchemaIndexDescriptor.class ) ) ).thenThrow( failure ); + } + // then try { @@ -199,123 +166,54 @@ public void getPopulationFailureMustThrowIfNoFailure() } @Test - public void getPopulationFailureMustReportFailureWhenNativeFailure() + public void getPopulationFailureMustReportFailureWhenAnyFailed() { - FusionIndexProvider fusionIndexProvider = fusionProvider(); - - // when - // ... native failure - String nativeFailure = "native failure"; - IllegalStateException spatialThrow = new IllegalStateException( "no spatial failure" ); - IllegalStateException temporalThrow = new IllegalStateException( "no temporal failure" ); - IllegalStateException luceneThrow = new IllegalStateException( "no lucene failure" ); - when( nativeProvider.getPopulationFailure( anyLong(), any( SchemaIndexDescriptor.class ) ) ).thenReturn( nativeFailure ); - when( spatialProvider.getPopulationFailure( anyLong(), any( SchemaIndexDescriptor.class ) ) ).thenThrow( spatialThrow ); - when( temporalProvider.getPopulationFailure( anyLong(), any( SchemaIndexDescriptor.class ) ) ).thenThrow( temporalThrow ); - when( luceneProvider.getPopulationFailure( anyLong(), any( SchemaIndexDescriptor.class ) ) ).thenThrow( luceneThrow ); - // then - assertThat( fusionIndexProvider.getPopulationFailure( 0, forLabel( 0, 0 ) ), containsString( nativeFailure ) ); - } - - @Test - public void getPopulationFailureMustReportFailureWhenSpatialFailure() throws Exception - { - FusionIndexProvider fusionIndexProvider = fusionProvider(); - - // when - // ... spatial failure - IllegalStateException nativeThrow = new IllegalStateException( "no native failure" ); - String spatialFailure = "spatial failure"; - IllegalStateException temporalThrow = new IllegalStateException( "no temporal failure" ); - IllegalStateException luceneThrow = new IllegalStateException( "no lucene failure" ); - when( nativeProvider.getPopulationFailure( anyLong(), any( SchemaIndexDescriptor.class ) ) ).thenThrow( nativeThrow ); - when( spatialProvider.getPopulationFailure( anyLong(), any( SchemaIndexDescriptor.class ) ) ).thenReturn( spatialFailure ); - when( temporalProvider.getPopulationFailure( anyLong(), any( SchemaIndexDescriptor.class ) ) ).thenThrow( temporalThrow ); - when( luceneProvider.getPopulationFailure( anyLong(), any( SchemaIndexDescriptor.class ) ) ).thenThrow( luceneThrow ); - // then - assertThat( fusionIndexProvider.getPopulationFailure( 0, forLabel( 0, 0 ) ), containsString( spatialFailure ) ); - } - - @Test - public void getPopulationFailureMustReportFailureWhenTemporalFailure() throws Exception - { - FusionIndexProvider fusionIndexProvider = fusionProvider(); - - // when - // ... spatial failure - IllegalStateException nativeThrow = new IllegalStateException( "no native failure" ); - IllegalStateException spatialThrow = new IllegalStateException( "no spatial failure" ); - String temporalFailure = "temporal failure"; - IllegalStateException luceneThrow = new IllegalStateException( "no lucene failure" ); - when( nativeProvider.getPopulationFailure( anyLong(), any( SchemaIndexDescriptor.class ) ) ).thenThrow( nativeThrow ); - when( spatialProvider.getPopulationFailure( anyLong(), any( SchemaIndexDescriptor.class ) ) ).thenThrow( spatialThrow ); - when( temporalProvider.getPopulationFailure( anyLong(), any( SchemaIndexDescriptor.class ) ) ).thenReturn( temporalFailure ); - when( luceneProvider.getPopulationFailure( anyLong(), any( SchemaIndexDescriptor.class ) ) ).thenThrow( luceneThrow ); - // then - assertThat( fusionIndexProvider.getPopulationFailure( 0, forLabel( 0, 0 ) ), containsString( temporalFailure ) ); - } + for ( int i = 0; i < providers.length; i++ ) + { + FusionIndexProvider fusionSchemaIndexProvider = fusionProvider(); - @Test - public void getPopulationFailureMustReportFailureWhenLuceneFailure() - { - FusionIndexProvider fusionIndexProvider = fusionProvider(); + // when + String failure = "failure"; + IllegalStateException exception = new IllegalStateException( "not failed" ); + for ( int j = 0; j < providers.length; j++ ) + { + if ( j == i ) + { + when( providers[j].getPopulationFailure( anyLong(), any( SchemaIndexDescriptor.class ) ) ).thenReturn( failure ); + } + else + { + when( providers[j].getPopulationFailure( anyLong(), any( SchemaIndexDescriptor.class ) ) ).thenThrow( exception ); + } + } - // when - // ... lucene failure - IllegalStateException nativeThrow = new IllegalStateException( "no native failure" ); - IllegalStateException spatialThrow = new IllegalStateException( "no spatial failure" ); - IllegalStateException temporalThrow = new IllegalStateException( "no temporal failure" ); - String luceneFailure = "lucene failure"; - when( nativeProvider.getPopulationFailure( anyLong(), any( SchemaIndexDescriptor.class ) ) ).thenThrow( nativeThrow ); - when( spatialProvider.getPopulationFailure( anyLong(), any( SchemaIndexDescriptor.class ) ) ).thenThrow( spatialThrow ); - when( temporalProvider.getPopulationFailure( anyLong(), any( SchemaIndexDescriptor.class ) ) ).thenThrow( temporalThrow ); - when( luceneProvider.getPopulationFailure( anyLong(), any( SchemaIndexDescriptor.class ) ) ).thenReturn( luceneFailure ); - // then - assertThat( fusionIndexProvider.getPopulationFailure( 0, forLabel( 0, 0 ) ), containsString( luceneFailure ) ); + // then + assertThat( fusionSchemaIndexProvider.getPopulationFailure( 0, forLabel( 0, 0 ) ), containsString( failure ) ); + } } @Test - public void getPopulationFailureMustReportFailureWhenBothNativeAndLuceneFail() + public void getPopulationFailureMustReportFailureWhenMultipleFail() { - FusionIndexProvider fusionIndexProvider = fusionProvider(); - - // when - // ... native and lucene failure - String nativeFailure = "native failure"; - IllegalStateException spatialThrow = new IllegalStateException( "no spatial failure" ); - IllegalStateException temporalThrow = new IllegalStateException( "no temporal failure" ); - String luceneFailure = "lucene failure"; - when( nativeProvider.getPopulationFailure( anyLong(), any( SchemaIndexDescriptor.class ) ) ).thenReturn( nativeFailure ); - when( spatialProvider.getPopulationFailure( anyLong(), any( SchemaIndexDescriptor.class ) ) ).thenThrow( spatialThrow ); - when( temporalProvider.getPopulationFailure( anyLong(), any( SchemaIndexDescriptor.class ) ) ).thenThrow( temporalThrow ); - when( luceneProvider.getPopulationFailure( anyLong(), any( SchemaIndexDescriptor.class ) ) ).thenReturn( luceneFailure ); - // then - String populationFailure = fusionIndexProvider.getPopulationFailure( 0, forLabel( 0, 0 ) ); - assertThat( populationFailure, containsString( nativeFailure ) ); - assertThat( populationFailure, containsString( luceneFailure ) ); - } + for ( int i = 0; i < providers.length; i++ ) + { + FusionIndexProvider fusionSchemaIndexProvider = fusionProvider(); - @Test - public void getPopulationFailureMustReportFailureWhenAllFail() throws Exception - { - FusionIndexProvider fusionIndexProvider = fusionProvider(); + // when + String[] failures = new String[providers.length]; + for ( int j = 0; j < providers.length; j++ ) + { + failures[j] = "FAILURE[" + j + "]"; + when( providers[j].getPopulationFailure( anyLong(), any( SchemaIndexDescriptor.class ) ) ).thenReturn( failures[j] ); + } - // when - // ... native and lucene failure - String nativeFailure = "native failure"; - String spatialFailure = "spatial failure"; - String temporalFailure = "temporal failure"; - String luceneFailure = "lucene failure"; - when( nativeProvider.getPopulationFailure( anyLong(), any( SchemaIndexDescriptor.class ) ) ).thenReturn( nativeFailure ); - when( spatialProvider.getPopulationFailure( anyLong(), any( SchemaIndexDescriptor.class ) ) ).thenReturn( spatialFailure ); - when( temporalProvider.getPopulationFailure( anyLong(), any( SchemaIndexDescriptor.class ) ) ).thenReturn( temporalFailure ); - when( luceneProvider.getPopulationFailure( anyLong(), any( SchemaIndexDescriptor.class ) ) ).thenReturn( luceneFailure ); - // then - String populationFailure = fusionIndexProvider.getPopulationFailure( 0, forLabel( 0, 0 ) ); - assertThat( populationFailure, containsString( nativeFailure ) ); - assertThat( populationFailure, containsString( spatialFailure ) ); - assertThat( populationFailure, containsString( temporalFailure ) ); - assertThat( populationFailure, containsString( luceneFailure ) ); + // then + String populationFailure = fusionSchemaIndexProvider.getPopulationFailure( 0, forLabel( 0, 0 ) ); + for ( String failure : failures ) + { + assertThat( populationFailure, containsString( failure ) ); + } + } } @Test @@ -327,36 +225,18 @@ public void shouldReportFailedIfAnyIsFailed() for ( InternalIndexState state : InternalIndexState.values() ) { - // when - setInitialState( nativeProvider, InternalIndexState.FAILED ); - setInitialState( spatialProvider, state ); - setInitialState( temporalProvider, state ); - setInitialState( luceneProvider, state ); - InternalIndexState failed1 = provider.getInitialState( 0, schemaIndexDescriptor ); - - setInitialState( nativeProvider, state ); - setInitialState( spatialProvider, InternalIndexState.FAILED ); - setInitialState( temporalProvider, state ); - setInitialState( luceneProvider, state ); - InternalIndexState failed2 = provider.getInitialState( 0, schemaIndexDescriptor ); - - setInitialState( nativeProvider, state ); - setInitialState( spatialProvider, state ); - setInitialState( temporalProvider, InternalIndexState.FAILED ); - setInitialState( luceneProvider, state ); - InternalIndexState failed3 = provider.getInitialState( 0, schemaIndexDescriptor ); - - setInitialState( nativeProvider, state ); - setInitialState( spatialProvider, state ); - setInitialState( temporalProvider, state ); - setInitialState( luceneProvider, InternalIndexState.FAILED ); - InternalIndexState failed4 = provider.getInitialState( 0, schemaIndexDescriptor ); + for ( int i = 0; i < providers.length; i++ ) + { + // when + for ( int j = 0; j < providers.length; j++ ) + { + setInitialState( providers[j], i == j ? InternalIndexState.FAILED : state ); + } + InternalIndexState initialState = provider.getInitialState( 0, schemaIndexDescriptor ); - // then - assertEquals( InternalIndexState.FAILED, failed1 ); - assertEquals( InternalIndexState.FAILED, failed2 ); - assertEquals( InternalIndexState.FAILED, failed3 ); - assertEquals( InternalIndexState.FAILED, failed4 ); + // then + assertEquals( InternalIndexState.FAILED, initialState ); + } } } @@ -369,43 +249,25 @@ public void shouldReportPopulatingIfAnyIsPopulating() for ( InternalIndexState state : array( InternalIndexState.ONLINE, InternalIndexState.POPULATING ) ) { - // when - setInitialState( nativeProvider, InternalIndexState.POPULATING ); - setInitialState( spatialProvider, state ); - setInitialState( temporalProvider, state ); - setInitialState( luceneProvider, state ); - InternalIndexState failed1 = provider.getInitialState( 0, schemaIndexDescriptor ); - - setInitialState( nativeProvider, state ); - setInitialState( spatialProvider, InternalIndexState.POPULATING ); - setInitialState( temporalProvider, state ); - setInitialState( luceneProvider, state ); - InternalIndexState failed2 = provider.getInitialState( 0, schemaIndexDescriptor ); - - setInitialState( nativeProvider, state ); - setInitialState( spatialProvider, state ); - setInitialState( temporalProvider, InternalIndexState.POPULATING ); - setInitialState( luceneProvider, state ); - InternalIndexState failed3 = provider.getInitialState( 0, schemaIndexDescriptor ); - - setInitialState( nativeProvider, state ); - setInitialState( spatialProvider, state ); - setInitialState( temporalProvider, state ); - setInitialState( luceneProvider, InternalIndexState.POPULATING ); - InternalIndexState failed4 = provider.getInitialState( 0, schemaIndexDescriptor ); + for ( int i = 0; i < providers.length; i++ ) + { + // when + for ( int j = 0; j < providers.length; j++ ) + { + setInitialState( providers[j], i == j ? InternalIndexState.POPULATING : state ); + } + InternalIndexState initialState = provider.getInitialState( 0, schemaIndexDescriptor ); - // then - assertEquals( InternalIndexState.POPULATING, failed1 ); - assertEquals( InternalIndexState.POPULATING, failed2 ); - assertEquals( InternalIndexState.POPULATING, failed3 ); - assertEquals( InternalIndexState.POPULATING, failed4 ); + // then + assertEquals( InternalIndexState.POPULATING, initialState ); + } } } private FusionIndexProvider fusionProvider() { - return new FusionIndexProvider( nativeProvider, spatialProvider, temporalProvider, luceneProvider, new FusionSelector(), DESCRIPTOR, 10, NONE, - mock( FileSystemAbstraction.class ) ); + return new FusionIndexProvider( stringProvider, numberProvider, spatialProvider, temporalProvider, luceneProvider, new FusionSelector(), + DESCRIPTOR, 10, NONE, mock( FileSystemAbstraction.class ) ); } private void setInitialState( IndexProvider mockedProvider, InternalIndexState state ) diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexReaderTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexReaderTest.java index a39beb99b2c04..82b8c3f30acfe 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexReaderTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexReaderTest.java @@ -28,8 +28,8 @@ import org.neo4j.collection.primitive.PrimitiveLongResourceIterator; import org.neo4j.collection.primitive.PrimitiveLongSet; import org.neo4j.internal.kernel.api.IndexQuery; -import org.neo4j.internal.kernel.api.IndexQuery.NumberRangePredicate; import org.neo4j.internal.kernel.api.IndexQuery.GeometryRangePredicate; +import org.neo4j.internal.kernel.api.IndexQuery.NumberRangePredicate; import org.neo4j.internal.kernel.api.IndexQuery.StringContainsPredicate; import org.neo4j.internal.kernel.api.IndexQuery.StringPrefixPredicate; import org.neo4j.internal.kernel.api.IndexQuery.StringRangePredicate; @@ -49,10 +49,12 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; +import static org.neo4j.helpers.collection.Iterators.array; public class FusionIndexReaderTest { - private IndexReader nativeReader; + private IndexReader stringReader; + private IndexReader numberReader; private IndexReader spatialReader; private IndexReader luceneReader; private IndexReader temporalReader; @@ -64,12 +66,13 @@ public class FusionIndexReaderTest @Before public void setup() { - nativeReader = mock( IndexReader.class ); + stringReader = mock( IndexReader.class ); + numberReader = mock( IndexReader.class ); spatialReader = mock( IndexReader.class ); temporalReader = mock( IndexReader.class ); luceneReader = mock( IndexReader.class ); - allReaders = new IndexReader[]{nativeReader, spatialReader, temporalReader, luceneReader}; - fusionIndexReader = new FusionIndexReader( nativeReader, spatialReader, temporalReader, luceneReader, new FusionSelector(), + allReaders = array( stringReader, numberReader, spatialReader, temporalReader, luceneReader ); + fusionIndexReader = new FusionIndexReader( stringReader, numberReader, spatialReader, temporalReader, luceneReader, new FusionSelector(), SchemaIndexDescriptorFactory.forLabel( LABEL_KEY, PROP_KEY ) ); } @@ -91,26 +94,25 @@ public void closeMustCloseBothNativeAndLucene() // close iterator @Test - public void closeIteratorMustCloseNativeAndLucene() throws Exception + public void closeIteratorMustCloseAll() throws Exception { // given - PrimitiveLongResourceIterator nativeIter = mock( PrimitiveLongResourceIterator.class ); - PrimitiveLongResourceIterator spatialIter = mock( PrimitiveLongResourceIterator.class ); - PrimitiveLongResourceIterator temporalIter = mock( PrimitiveLongResourceIterator.class ); - PrimitiveLongResourceIterator luceneIter = mock( PrimitiveLongResourceIterator.class ); - when( nativeReader.query( any( IndexQuery.class ) ) ).thenReturn( nativeIter ); - when( spatialReader.query( any( IndexQuery.class ) ) ).thenReturn( spatialIter ); - when( temporalReader.query( any( IndexQuery.class ) ) ).thenReturn( temporalIter ); - when( luceneReader.query( any( IndexQuery.class ) ) ).thenReturn( luceneIter ); + PrimitiveLongResourceIterator[] iterators = new PrimitiveLongResourceIterator[allReaders.length]; + for ( int i = 0; i < allReaders.length; i++ ) + { + PrimitiveLongResourceIterator iterator = mock( PrimitiveLongResourceIterator.class ); + when( allReaders[i].query( any( IndexQuery.class ) ) ).thenReturn( iterator ); + iterators[i] = iterator; + } // when fusionIndexReader.query( IndexQuery.exists( PROP_KEY ) ).close(); // then - verify( nativeIter, times( 1 ) ).close(); - verify( spatialIter, times( 1 ) ).close(); - verify( temporalIter, times( 1 ) ).close(); - verify( luceneIter, times( 1 ) ).close(); + for ( PrimitiveLongResourceIterator iterator : iterators ) + { + verify( iterator, times( 1 ) ).close(); + } } /* countIndexedNodes */ @@ -119,34 +121,15 @@ public void closeIteratorMustCloseNativeAndLucene() throws Exception public void countIndexedNodesMustSelectCorrectReader() { // given - Value[] nativeValues = FusionIndexTestHelp.valuesSupportedByNative(); - Value[] spatialValues = FusionIndexTestHelp.valuesSupportedBySpatial(); - Value[] temporalValues = FusionIndexTestHelp.valuesSupportedByTemporal(); - Value[] otherValues = FusionIndexTestHelp.valuesNotSupportedByNativeOrSpatial(); + Value[][] values = FusionIndexTestHelp.valuesByGroup(); Value[] allValues = FusionIndexTestHelp.allValues(); - // when passing native values - for ( Value nativeValue : nativeValues ) - { - verifyCountIndexedNodesWithCorrectReader( nativeReader, nativeValue ); - } - - // when passing spatial values - for ( Value spatialValue : spatialValues ) - { - verifyCountIndexedNodesWithCorrectReader( spatialReader, spatialValue ); - } - - // when passing temporal values - for ( Value temporalValue : temporalValues ) - { - verifyCountIndexedNodesWithCorrectReader( temporalReader, temporalValue ); - } - - // when passing values not handled by others - for ( Value otherValue : otherValues ) + for ( int i = 0; i < allReaders.length; i++ ) { - verifyCountIndexedNodesWithCorrectReader( luceneReader, otherValue ); + for ( Value value : values[i] ) + { + verifyCountIndexedNodesWithCorrectReader( allReaders[i], value ); + } } // When passing composite keys, they are only handled by lucene @@ -182,15 +165,28 @@ public void mustSelectLuceneForCompositePredicate() throws Exception } @Test - public void mustSelectNativeForExactPredicateWithNumberValue() throws Exception + public void mustSelectStringForExactPredicateWithNumberValue() throws Exception + { + // given + for ( Object value : FusionIndexTestHelp.valuesSupportedByString() ) + { + IndexQuery indexQuery = IndexQuery.exact( PROP_KEY, value ); + + // then + verifyQueryWithCorrectReader( stringReader, indexQuery ); + } + } + + @Test + public void mustSelectNumberForExactPredicateWithNumberValue() throws Exception { // given - for ( Object numberValue : FusionIndexTestHelp.valuesSupportedByNative() ) + for ( Object value : FusionIndexTestHelp.valuesSupportedByNumber() ) { - IndexQuery indexQuery = IndexQuery.exact( PROP_KEY, numberValue ); + IndexQuery indexQuery = IndexQuery.exact( PROP_KEY, value ); // then - verifyQueryWithCorrectReader( nativeReader, indexQuery ); + verifyQueryWithCorrectReader( numberReader, indexQuery ); } } @@ -198,9 +194,9 @@ public void mustSelectNativeForExactPredicateWithNumberValue() throws Exception public void mustSelectSpatialForExactPredicateWithSpatialValue() throws Exception { // given - for ( Object spatialValue : FusionIndexTestHelp.valuesSupportedBySpatial() ) + for ( Object value : FusionIndexTestHelp.valuesSupportedBySpatial() ) { - IndexQuery indexQuery = IndexQuery.exact( PROP_KEY, spatialValue ); + IndexQuery indexQuery = IndexQuery.exact( PROP_KEY, value ); // then verifyQueryWithCorrectReader( spatialReader, indexQuery ); @@ -224,9 +220,9 @@ public void mustSelectTemporalForExactPredicateWithTemporalValue() throws Except public void mustSelectLuceneForExactPredicateWithOtherValue() throws Exception { // given - for ( Object nonNumberOrSpatialValue : FusionIndexTestHelp.valuesNotSupportedByNativeOrSpatial() ) + for ( Object value : FusionIndexTestHelp.valuesNotSupportedBySpecificIndex() ) { - IndexQuery indexQuery = IndexQuery.exact( PROP_KEY, nonNumberOrSpatialValue ); + IndexQuery indexQuery = IndexQuery.exact( PROP_KEY, value ); // then verifyQueryWithCorrectReader( luceneReader, indexQuery ); @@ -234,23 +230,23 @@ public void mustSelectLuceneForExactPredicateWithOtherValue() throws Exception } @Test - public void mustSelectLuceneForRangeStringPredicate() throws Exception + public void mustSelectStringForRangeStringPredicate() throws Exception { // given StringRangePredicate stringRange = IndexQuery.range( PROP_KEY, "abc", true, "def", false ); // then - verifyQueryWithCorrectReader( luceneReader, stringRange ); + verifyQueryWithCorrectReader( stringReader, stringRange ); } @Test - public void mustSelectNativeForRangeNumericPredicate() throws Exception + public void mustSelectNumberForRangeNumericPredicate() throws Exception { // given NumberRangePredicate numberRange = IndexQuery.range( PROP_KEY, 0, true, 1, false ); // then - verifyQueryWithCorrectReader( nativeReader, numberRange ); + verifyQueryWithCorrectReader( numberReader, numberRange ); } @Test @@ -266,33 +262,33 @@ public void mustSelectSpatialForRangeGeometricPredicate() throws Exception } @Test - public void mustSelectLuceneForStringPrefixPredicate() throws Exception + public void mustSelectStringForStringPrefixPredicate() throws Exception { // given StringPrefixPredicate stringPrefix = IndexQuery.stringPrefix( PROP_KEY, "abc" ); // then - verifyQueryWithCorrectReader( luceneReader, stringPrefix ); + verifyQueryWithCorrectReader( stringReader, stringPrefix ); } @Test - public void mustSelectLuceneForStringSuffixPredicate() throws Exception + public void mustSelectStringForStringSuffixPredicate() throws Exception { // given StringSuffixPredicate stringPrefix = IndexQuery.stringSuffix( PROP_KEY, "abc" ); // then - verifyQueryWithCorrectReader( luceneReader, stringPrefix ); + verifyQueryWithCorrectReader( stringReader, stringPrefix ); } @Test - public void mustSelectLuceneForStringContainsPredicate() throws Exception + public void mustSelectStringForStringContainsPredicate() throws Exception { // given StringContainsPredicate stringContains = IndexQuery.stringContains( PROP_KEY, "abc" ); // then - verifyQueryWithCorrectReader( luceneReader, stringContains ); + verifyQueryWithCorrectReader( stringReader, stringContains ); } @Test @@ -300,17 +296,18 @@ public void mustCombineResultFromExistsPredicate() throws Exception { // given IndexQuery.ExistsPredicate exists = IndexQuery.exists( PROP_KEY ); - when( nativeReader.query( exists ) ).thenReturn( PrimitiveLongResourceCollections.iterator( null, 0L, 1L, 4L, 7L ) ); - when( spatialReader.query( exists ) ).thenReturn( PrimitiveLongResourceCollections.iterator( null, 3L, 9L, 10L ) ); - when( temporalReader.query( exists ) ).thenReturn( PrimitiveLongResourceCollections.iterator( null, 8L, 11L, 12L ) ); - when( luceneReader.query( exists ) ).thenReturn( PrimitiveLongResourceCollections.iterator( null, 2L, 5L, 6L ) ); + long lastId = 0; + for ( int i = 0; i < allReaders.length; i++ ) + { + when( allReaders[i].query( exists ) ).thenReturn( PrimitiveLongResourceCollections.iterator( null, lastId++, lastId++ ) ); + } // when PrimitiveLongIterator result = fusionIndexReader.query( exists ); // then PrimitiveLongSet resultSet = PrimitiveLongCollections.asSet( result ); - for ( long i = 0L; i < 10L; i++ ) + for ( long i = 0L; i < lastId; i++ ) { assertTrue( "Expected to contain " + i + ", but was " + resultSet, resultSet.contains( i ) ); } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexTestHelp.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexTestHelp.java index ea78daeecfa29..5224cc306bfef 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexTestHelp.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexTestHelp.java @@ -19,12 +19,12 @@ */ package org.neo4j.kernel.impl.index.schema.fusion; -import org.apache.commons.lang3.ArrayUtils; import org.hamcrest.Matcher; import org.mockito.Mockito; import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.concurrent.Callable; @@ -42,6 +42,7 @@ import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.reset; import static org.mockito.Mockito.verify; class FusionIndexTestHelp @@ -49,6 +50,12 @@ class FusionIndexTestHelp private static LabelSchemaDescriptor indexKey = SchemaDescriptorFactory.forLabel( 0, 0 ); private static LabelSchemaDescriptor compositeIndexKey = SchemaDescriptorFactory.forLabel( 0, 0, 1 ); + private static final Value[] stringValues = new Value[] + { + Values.stringValue( "abc" ), + Values.stringValue( "abcdefghijklmnopqrstuvwxyzåäö" ), + Values.charValue( 'S' ), + }; private static final Value[] numberValues = new Value[] { Values.byteValue( (byte) 1 ), @@ -72,8 +79,6 @@ class FusionIndexTestHelp private static final Value[] otherValues = new Value[] { Values.booleanValue( true ), - Values.charValue( 'a' ), - Values.stringValue( "bcd" ), Values.booleanArray( new boolean[2] ), Values.byteArray( new byte[]{1, 2} ), Values.shortArray( new short[]{3, 4} ), @@ -87,7 +92,12 @@ class FusionIndexTestHelp Values.NO_VALUE }; - static Value[] valuesSupportedByNative() + static Value[] valuesSupportedByString() + { + return stringValues; + } + + static Value[] valuesSupportedByNumber() { return numberValues; } @@ -102,19 +112,31 @@ static Value[] valuesSupportedByTemporal() return temporalValues; } - private static Value[] valuesNotSupportedByNative() + static Value[] valuesNotSupportedBySpecificIndex() { - return ArrayUtils.addAll( temporalValues, ArrayUtils.addAll( pointValues, otherValues ) ); + return otherValues; } - static Value[] valuesNotSupportedByNativeOrSpatial() + static Value[] allValues() { - return otherValues; + List values = new ArrayList<>(); + for ( Value[] group : valuesByGroup() ) + { + values.addAll( Arrays.asList( group ) ); + } + return values.toArray( new Value[values.size()] ); } - static Value[] allValues() + static Value[][] valuesByGroup() { - return ArrayUtils.addAll( valuesSupportedByNative(), valuesNotSupportedByNative() ); + return new Value[][] + { + FusionIndexTestHelp.valuesSupportedByString(), + FusionIndexTestHelp.valuesSupportedByNumber(), + FusionIndexTestHelp.valuesSupportedBySpatial(), + FusionIndexTestHelp.valuesSupportedByTemporal(), + FusionIndexTestHelp.valuesNotSupportedBySpecificIndex() + }; } static void verifyCallFail( Exception expectedFailure, Callable failingCall ) diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexUpdaterTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexUpdaterTest.java index 8faadc40bb028..406e346411c7b 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexUpdaterTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexUpdaterTest.java @@ -31,16 +31,18 @@ import org.neo4j.values.storable.Value; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.reset; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.neo4j.helpers.ArrayUtil.without; +import static org.neo4j.helpers.collection.Iterators.array; import static org.neo4j.kernel.impl.index.schema.fusion.FusionIndexTestHelp.add; import static org.neo4j.kernel.impl.index.schema.fusion.FusionIndexTestHelp.change; import static org.neo4j.kernel.impl.index.schema.fusion.FusionIndexTestHelp.remove; public class FusionIndexUpdaterTest { - private IndexUpdater nativeUpdater; + private IndexUpdater stringUpdater; + private IndexUpdater numberUpdater; private IndexUpdater spatialUpdater; private IndexUpdater temporalUpdater; private IndexUpdater luceneUpdater; @@ -48,14 +50,15 @@ public class FusionIndexUpdaterTest private FusionIndexUpdater fusionIndexUpdater; @Before - public void setup() + public void mockComponents() { - nativeUpdater = mock( IndexUpdater.class ); + stringUpdater = mock( IndexUpdater.class ); + numberUpdater = mock( IndexUpdater.class ); spatialUpdater = mock( IndexUpdater.class ); temporalUpdater = mock( IndexUpdater.class ); luceneUpdater = mock( IndexUpdater.class ); - allUpdaters = new IndexUpdater[]{nativeUpdater, spatialUpdater, temporalUpdater, luceneUpdater}; - fusionIndexUpdater = new FusionIndexUpdater( nativeUpdater, spatialUpdater, temporalUpdater, luceneUpdater, new FusionSelector() ); + allUpdaters = array( stringUpdater, numberUpdater, spatialUpdater, temporalUpdater, luceneUpdater ); + fusionIndexUpdater = new FusionIndexUpdater( stringUpdater, numberUpdater, spatialUpdater, temporalUpdater, luceneUpdater, new FusionSelector() ); } /* process */ @@ -64,45 +67,19 @@ public void setup() public void processMustSelectCorrectForAdd() throws Exception { // given - Value[] supportedByNative = FusionIndexTestHelp.valuesSupportedByNative(); - Value[] supportedBySpatial = FusionIndexTestHelp.valuesSupportedBySpatial(); - Value[] supportedByTemporal = FusionIndexTestHelp.valuesSupportedByTemporal(); - Value[] notSupportedByNativeOrSpatial = FusionIndexTestHelp.valuesNotSupportedByNativeOrSpatial(); + Value[][] values = FusionIndexTestHelp.valuesByGroup(); Value[] allValues = FusionIndexTestHelp.allValues(); - // when - // ... value supported by native - for ( Value value : supportedByNative ) - { - //then - verifyAddWithCorrectUpdater( nativeUpdater, value ); - } - - // when - // ... value supported by spatial - for ( Value value : supportedBySpatial ) - { - //then - verifyAddWithCorrectUpdater( spatialUpdater, value ); - } - - // when - // ... value supported by temporal - for ( Value value : supportedByTemporal ) + for ( int i = 0; i < allUpdaters.length; i++ ) { - //then - verifyAddWithCorrectUpdater( temporalUpdater, value ); - } - - // when - // ... value not supported by native - for ( Value value : notSupportedByNativeOrSpatial ) - { - verifyAddWithCorrectUpdater( luceneUpdater, value ); + for ( Value value : values[i] ) + { + // then + verifyAddWithCorrectUpdater( allUpdaters[i], value ); + } } - // when - // ... value is composite + // when value is composite for ( Value firstValue : allValues ) { for ( Value secondValue : allValues ) @@ -116,45 +93,19 @@ public void processMustSelectCorrectForAdd() throws Exception public void processMustSelectCorrectForRemove() throws Exception { // given - Value[] supportedByNative = FusionIndexTestHelp.valuesSupportedByNative(); - Value[] supportedBySpatial = FusionIndexTestHelp.valuesSupportedBySpatial(); - Value[] supportedByTemporal = FusionIndexTestHelp.valuesSupportedByTemporal(); - Value[] notSupportedByNativeOrSpatial = FusionIndexTestHelp.valuesNotSupportedByNativeOrSpatial(); + Value[][] values = FusionIndexTestHelp.valuesByGroup(); Value[] allValues = FusionIndexTestHelp.allValues(); - // when - // ... value supported by native - for ( Value value : supportedByNative ) - { - //then - verifyRemoveWithCorrectUpdater( nativeUpdater, value ); - } - - // when - // ... value supported by spatial - for ( Value value : supportedBySpatial ) + for ( int i = 0; i < allUpdaters.length; i++ ) { - //then - verifyRemoveWithCorrectUpdater( spatialUpdater, value ); - } - - // when - // ... value supported by temporal - for ( Value value : supportedByTemporal ) - { - //then - verifyRemoveWithCorrectUpdater( temporalUpdater, value ); - } - - // when - // ... value not supported by native - for ( Value value : notSupportedByNativeOrSpatial ) - { - verifyRemoveWithCorrectUpdater( luceneUpdater, value ); + for ( Value value : values[i] ) + { + // then + verifyRemoveWithCorrectUpdater( allUpdaters[i], value ); + } } - // when - // ... value is composite + // when value is composite for ( Value firstValue : allValues ) { for ( Value secondValue : allValues ) @@ -165,175 +116,43 @@ public void processMustSelectCorrectForRemove() throws Exception } @Test - public void processMustSelectCorrectForChangeSupportedByNative() throws Exception - { - // given - Value[] supportedByNative = FusionIndexTestHelp.valuesSupportedByNative(); - - // when - // ... before - supported - // ... after - supported - for ( Value before : supportedByNative ) - { - for ( Value after : supportedByNative ) - { - verifyChangeWithCorrectUpdaterNotMixed( nativeUpdater, before, after ); - } - } - } - - @Test - public void processMustSelectCorrectForChangeSupportedBySpatial() throws Exception - { - // given - Value[] supportedBySpatial = FusionIndexTestHelp.valuesSupportedBySpatial(); - - // when - // ... before - supported - // ... after - supported - for ( Value before : supportedBySpatial ) - { - for ( Value after : supportedBySpatial ) - { - verifyChangeWithCorrectUpdaterNotMixed( spatialUpdater, before, after ); - } - } - } - - @Test - public void processMustSelectCorrectForChangeSupportedByTemporal() throws Exception + public void processMustSelectCorrectForChange() throws Exception { // given - Value[] supportedByTemporal = FusionIndexTestHelp.valuesSupportedByTemporal(); + Value[][] values = FusionIndexTestHelp.valuesByGroup(); // when - // ... before - supported - // ... after - supported - for ( Value before : supportedByTemporal ) + for ( int i = 0; i < allUpdaters.length; i++ ) { - for ( Value after : supportedByTemporal ) + for ( Value before : values[i] ) { - verifyChangeWithCorrectUpdaterNotMixed( temporalUpdater, before, after ); + for ( Value after : values[i] ) + { + verifyChangeWithCorrectUpdaterNotMixed( allUpdaters[i], before, after ); + } } } } @Test - public void processMustSelectCorrectForChangeNotSupportedByNative() throws Exception + public void processMustSelectCorrectForChangeFromOneGroupToAnother() throws Exception { - // given - Value[] notSupportedByNativeOrSpatial = FusionIndexTestHelp.valuesNotSupportedByNativeOrSpatial(); - - // when - // ... before - not supported - // ... after - not supported - for ( Value before : notSupportedByNativeOrSpatial ) + Value[][] values = FusionIndexTestHelp.valuesByGroup(); + for ( int f = 0; f < values.length; f++ ) { - for ( Value after : notSupportedByNativeOrSpatial ) + // given + for ( int t = 0; t < values.length; t++ ) { - verifyChangeWithCorrectUpdaterNotMixed( luceneUpdater, before, after ); + if ( f != t ) + { + // when + verifyChangeWithCorrectUpdaterMixed( allUpdaters[f], allUpdaters[t], values[f], values[t] ); + mockComponents(); + } } } } - @Test - public void processMustSelectCorrectForChangeFromNativeToLucene() throws Exception - { - // given - Value[] supportedByNative = FusionIndexTestHelp.valuesSupportedByNative(); - Value[] notSupportedByNativeOrSpatial = FusionIndexTestHelp.valuesNotSupportedByNativeOrSpatial(); - - // when - // ... before - supported - // ... after - not supported - verifyChangeWithCorrectUpdaterMixed( nativeUpdater, luceneUpdater, supportedByNative, notSupportedByNativeOrSpatial ); - } - - @Test - public void processMustSelectCorrectForChangeFromLuceneToNative() throws Exception - { - // given - Value[] supportedByNative = FusionIndexTestHelp.valuesSupportedByNative(); - Value[] notSupportedByNativeOrSpatial = FusionIndexTestHelp.valuesNotSupportedByNativeOrSpatial(); - - // when - // ... before - not supported - // ... after - supported - verifyChangeWithCorrectUpdaterMixed( luceneUpdater, nativeUpdater, notSupportedByNativeOrSpatial, supportedByNative ); - } - - @Test - public void processMustSelectCorrectForChangeFromNativeToSpatialOrBack() throws Exception - { - // given - Value[] supportedByNative = FusionIndexTestHelp.valuesSupportedByNative(); - Value[] supportedBySpatial = FusionIndexTestHelp.valuesSupportedBySpatial(); - - // when - // ... before - supported - // ... after - not supported - verifyChangeWithCorrectUpdaterMixed( nativeUpdater, spatialUpdater, supportedByNative, supportedBySpatial ); - verifyChangeWithCorrectUpdaterMixed( spatialUpdater, nativeUpdater, supportedBySpatial, supportedByNative ); - } - - @Test - public void processMustSelectCorrectForChangeFromNativeToTemporalOrBack() throws Exception - { - // given - Value[] supportedByNative = FusionIndexTestHelp.valuesSupportedByNative(); - Value[] supportedByTemporal = FusionIndexTestHelp.valuesSupportedByTemporal(); - - // when - // ... before - supported - // ... after - not supported - verifyChangeWithCorrectUpdaterMixed( nativeUpdater, temporalUpdater, supportedByNative, supportedByTemporal ); - verifyChangeWithCorrectUpdaterMixed( temporalUpdater, nativeUpdater, supportedByTemporal, supportedByNative ); - } - - @Test - public void processMustSelectCorrectForChangeFromSpatialToTemporalOrBack() throws Exception - { - // given - Value[] supportedBySpatial = FusionIndexTestHelp.valuesSupportedBySpatial(); - Value[] supportedByTemporal = FusionIndexTestHelp.valuesSupportedByTemporal(); - - // when - // ... before - supported - // ... after - not supported - verifyChangeWithCorrectUpdaterMixed( spatialUpdater, temporalUpdater, supportedBySpatial, supportedByTemporal ); - verifyChangeWithCorrectUpdaterMixed( temporalUpdater, spatialUpdater, supportedByTemporal, supportedBySpatial ); - } - - @Test - public void processMustSelectCorrectForChangeFromLuceneToSpatialOrBack() throws Exception - { - // given - Value[] supportedByLucene = FusionIndexTestHelp.valuesNotSupportedByNativeOrSpatial(); - Value[] supportedBySpatial = FusionIndexTestHelp.valuesSupportedBySpatial(); - - // when - // ... before - supported - // ... after - not supported - verifyChangeWithCorrectUpdaterMixed( luceneUpdater, spatialUpdater, supportedByLucene, supportedBySpatial ); - reset( luceneUpdater, spatialUpdater ); - verifyChangeWithCorrectUpdaterMixed( spatialUpdater, luceneUpdater, supportedBySpatial, supportedByLucene ); - } - - @Test - public void processMustSelectCorrectForChangeFromLuceneToTemporalOrBack() throws Exception - { - // given - Value[] supportedByLucene = FusionIndexTestHelp.valuesNotSupportedByNativeOrSpatial(); - Value[] supportedByTemporal = FusionIndexTestHelp.valuesSupportedByTemporal(); - - // when - // ... before - supported - // ... after - not supported - verifyChangeWithCorrectUpdaterMixed( luceneUpdater, temporalUpdater, supportedByLucene, supportedByTemporal ); - reset( luceneUpdater, temporalUpdater ); - verifyChangeWithCorrectUpdaterMixed( temporalUpdater, luceneUpdater, supportedByTemporal, supportedByLucene ); - } - private void verifyAddWithCorrectUpdater( IndexUpdater correctPopulator, Value... numberValues ) throws IndexEntryConflictException, IOException { @@ -390,11 +209,10 @@ private void verifyChangeWithCorrectUpdaterMixed( IndexUpdater expectRemoveFrom, Value after = afterValues[afterIndex]; IndexEntryUpdate change = change( before, after ); - IndexEntryUpdate remove = remove( before ); - IndexEntryUpdate add = add( after ); fusionIndexUpdater.process( change ); - verify( expectRemoveFrom, times( afterIndex + 1 ) ).process( remove ); - verify( expectAddTo, times( beforeIndex + 1 ) ).process( add ); + + verify( expectRemoveFrom, times( afterIndex + 1 ) ).process( remove( before ) ); + verify( expectAddTo, times( beforeIndex + 1 ) ).process( add( after ) ); } } } @@ -415,56 +233,29 @@ public void closeMustCloseAll() throws Exception } @Test - public void closeMustThrowIfLuceneThrow() throws Exception - { - FusionIndexTestHelp.verifyFusionCloseThrowOnSingleCloseThrow( luceneUpdater, fusionIndexUpdater ); - } - - @Test - public void closeMustThrowIfNativeThrow() throws Exception - { - FusionIndexTestHelp.verifyFusionCloseThrowOnSingleCloseThrow( nativeUpdater, fusionIndexUpdater ); - } - - @Test - public void closeMustThrowIfSpatialThrow() throws Exception - { - FusionIndexTestHelp.verifyFusionCloseThrowOnSingleCloseThrow( spatialUpdater, fusionIndexUpdater ); - } - - @Test - public void closeMustThrowIfTemporalThrow() throws Exception + public void closeMustThrowIfAnyThrow() throws Exception { - FusionIndexTestHelp.verifyFusionCloseThrowOnSingleCloseThrow( temporalUpdater, fusionIndexUpdater ); - } - - @Test - public void closeMustCloseOthersIfLuceneThrow() throws Exception - { - FusionIndexTestHelp.verifyOtherIsClosedOnSingleThrow( luceneUpdater, fusionIndexUpdater, nativeUpdater, spatialUpdater, temporalUpdater ); - } - - @Test - public void closeMustCloseOthersIfSpatialThrow() throws Exception - { - FusionIndexTestHelp.verifyOtherIsClosedOnSingleThrow( spatialUpdater, fusionIndexUpdater, nativeUpdater, temporalUpdater, luceneUpdater ); - } - - @Test - public void closeMustCloseOthersIfTemporalThrow() throws Exception - { - FusionIndexTestHelp.verifyOtherIsClosedOnSingleThrow( temporalUpdater, fusionIndexUpdater, nativeUpdater, spatialUpdater, luceneUpdater ); + for ( int i = 0; i < allUpdaters.length; i++ ) + { + FusionIndexTestHelp.verifyFusionCloseThrowOnSingleCloseThrow( allUpdaters[i], fusionIndexUpdater ); + mockComponents(); + } } @Test - public void closeMustCloseOthersIfNativeThrow() throws Exception + public void closeMustCloseOthersIfAnyThrow() throws Exception { - FusionIndexTestHelp.verifyOtherIsClosedOnSingleThrow( nativeUpdater, fusionIndexUpdater, spatialUpdater, temporalUpdater, luceneUpdater ); + for ( int i = 0; i < allUpdaters.length; i++ ) + { + IndexUpdater updater = allUpdaters[i]; + FusionIndexTestHelp.verifyOtherIsClosedOnSingleThrow( updater, fusionIndexUpdater, without( allUpdaters, updater ) ); + mockComponents(); + } } @Test public void closeMustThrowIfAllThrow() throws Exception { - FusionIndexTestHelp.verifyFusionCloseThrowIfAllThrow( fusionIndexUpdater, nativeUpdater, spatialUpdater, temporalUpdater, luceneUpdater ); + FusionIndexTestHelp.verifyFusionCloseThrowIfAllThrow( fusionIndexUpdater, allUpdaters ); } } diff --git a/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/schema/NativeLuceneFusionIndexProviderFactory.java b/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/schema/NativeLuceneFusionIndexProviderFactory.java index dba1b54d1813c..6630fc4337f0a 100644 --- a/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/schema/NativeLuceneFusionIndexProviderFactory.java +++ b/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/schema/NativeLuceneFusionIndexProviderFactory.java @@ -27,12 +27,13 @@ import org.neo4j.io.fs.FileSystemAbstraction; import org.neo4j.io.pagecache.PageCache; import org.neo4j.kernel.api.index.IndexDirectoryStructure; -import org.neo4j.kernel.api.index.LoggingMonitor; import org.neo4j.kernel.api.index.IndexProvider; +import org.neo4j.kernel.api.index.LoggingMonitor; import org.neo4j.kernel.configuration.Config; import org.neo4j.kernel.extension.KernelExtensionFactory; import org.neo4j.kernel.impl.factory.OperationalMode; import org.neo4j.kernel.impl.index.schema.NumberIndexProvider; +import org.neo4j.kernel.impl.index.schema.StringIndexProvider; import org.neo4j.kernel.impl.index.schema.TemporalIndexProvider; import org.neo4j.kernel.impl.index.schema.fusion.FusionIndexProvider; import org.neo4j.kernel.impl.index.schema.fusion.FusionSelector; @@ -87,18 +88,19 @@ public static FusionIndexProvider newInstance( PageCache pageCache, File storeDi { IndexDirectoryStructure.Factory childDirectoryStructure = subProviderDirectoryStructure( storeDir ); boolean readOnly = isReadOnly( config, operationalMode ); - NumberIndexProvider nativeProvider = + StringIndexProvider stringProvider = + new StringIndexProvider( pageCache, fs, childDirectoryStructure, monitor, recoveryCleanupWorkCollector, readOnly ); + NumberIndexProvider numberProvider = new NumberIndexProvider( pageCache, fs, childDirectoryStructure, monitor, recoveryCleanupWorkCollector, readOnly ); SpatialFusionIndexProvider spatialProvider = new SpatialFusionIndexProvider( pageCache, fs, childDirectoryStructure, monitor, recoveryCleanupWorkCollector, readOnly, config ); TemporalIndexProvider temporalProvider = new TemporalIndexProvider( pageCache, fs, childDirectoryStructure, monitor, recoveryCleanupWorkCollector, readOnly ); - LuceneIndexProvider - luceneProvider = LuceneIndexProviderFactory.create( fs, childDirectoryStructure, monitor, config, + LuceneIndexProvider luceneProvider = LuceneIndexProviderFactory.create( fs, childDirectoryStructure, monitor, config, operationalMode ); boolean useNativeIndex = config.get( GraphDatabaseSettings.enable_native_schema_index ); int priority = useNativeIndex ? PRIORITY : 0; - return new FusionIndexProvider( nativeProvider, + return new FusionIndexProvider( stringProvider, numberProvider, spatialProvider, temporalProvider, luceneProvider, new FusionSelector(), DESCRIPTOR, priority, directoriesByProvider( storeDir ), fs ); }