Added filtering iterator testing and some renaming and cleanup

community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/FilteringNumberHitIterator.java → community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/FilteringNativeHitIterator.java

@@ -27,11 +27,11 @@
 import org.neo4j.internal.kernel.api.IndexQuery;
 import org.neo4j.values.storable.Value;
 
-class FilteringNumberHitIterator<KEY extends NativeSchemaKey, VALUE extends NativeSchemaValue> extends NumberHitIterator<KEY,VALUE>
+class FilteringNativeHitIterator<KEY extends NativeSchemaKey, VALUE extends NativeSchemaValue> extends NativeHitIterator<KEY,VALUE>
 {
     private final IndexQuery[] filters;
 
-    FilteringNumberHitIterator( RawCursor<Hit<KEY,VALUE>,IOException> seeker,
+    FilteringNativeHitIterator( RawCursor<Hit<KEY,VALUE>,IOException> seeker,
             Collection<RawCursor<Hit<KEY,VALUE>,IOException>> toRemoveFromWhenExhausted, IndexQuery[] filters )
     {
         super( seeker, toRemoveFromWhenExhausted );

community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/NumberHitIterator.java → community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/NativeHitIterator.java

@@ -36,15 +36,15 @@
  * @param <KEY> type of {@link NumberSchemaKey}.
  * @param <VALUE> type of {@link NativeSchemaValue}.
  */
-public class NumberHitIterator<KEY extends NativeSchemaKey, VALUE extends NativeSchemaValue>
+public class NativeHitIterator<KEY extends NativeSchemaKey, VALUE extends NativeSchemaValue>
         extends PrimitiveLongCollections.PrimitiveLongBaseIterator
         implements PrimitiveLongResourceIterator
 {
     private final RawCursor<Hit<KEY,VALUE>,IOException> seeker;
     private final Collection<RawCursor<Hit<KEY,VALUE>,IOException>> toRemoveFromWhenExhausted;
     private boolean closed;
 
-    NumberHitIterator( RawCursor<Hit<KEY,VALUE>,IOException> seeker,
+    NativeHitIterator( RawCursor<Hit<KEY,VALUE>,IOException> seeker,
             Collection<RawCursor<Hit<KEY,VALUE>,IOException>> toRemoveFromWhenExhausted )
     {
         this.seeker = seeker;
@@ -58,9 +58,10 @@ protected boolean fetchNext()
         {
             while ( seeker.next() )
             {
-                if ( acceptValue( seeker.get().key().asValue() ) )
+                KEY key = seeker.get().key();
+                if ( acceptValue( key.asValue() ) )
                 {
-                    return next( seeker.get().key().getEntityId() );
+                    return next( key.getEntityId() );
                 }
             }
             return false;

community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/NativeSchemaIndexReader.java

@@ -123,24 +123,18 @@ public PrimitiveLongResourceIterator query( IndexQuery... predicates )
         {
             RawCursor<Hit<KEY,VALUE>,IOException> seeker = tree.seek( treeKeyFrom, treeKeyTo );
             openSeekers.add( seeker );
-            return getNumberHitIterator( seeker, needFilter, predicates );
+            return getHitIterator( seeker, needFilter, predicates );
         }
         catch ( IOException e )
         {
             throw new UncheckedIOException( e );
         }
     }
 
-    private PrimitiveLongResourceIterator getNumberHitIterator( RawCursor<Hit<KEY,VALUE>,IOException> seeker, boolean needFilter, IndexQuery[] predicates )
+    private PrimitiveLongResourceIterator getHitIterator( RawCursor<Hit<KEY,VALUE>,IOException> seeker, boolean needFilter, IndexQuery[] predicates )
     {
-        if ( needFilter )
-        {
-            return new FilteringNumberHitIterator<>( seeker, openSeekers, predicates );
-        }
-        else
-        {
-            return new NumberHitIterator<>( seeker, openSeekers );
-        }
+        return needFilter ? new FilteringNativeHitIterator<KEY,VALUE>( seeker, openSeekers, predicates )
+                          : new NativeHitIterator<KEY,VALUE>( seeker, openSeekers );
     }
 
     @Override
@@ -194,14 +188,8 @@ RawCursor<Hit<KEY,VALUE>,IOException> makeIndexSeeker( KEY treeKeyFrom, KEY tree
     private IndexProgressor getIndexProgressor( RawCursor<Hit<KEY,VALUE>,IOException> seeker, IndexProgressor.NodeValueClient client, boolean needFilter,
             IndexQuery[] query )
     {
-        if ( needFilter )
-        {
-            return new FilteringNativeHitIndexProgressor<>( seeker, client, openSeekers, query );
-        }
-        else
-        {
-            return new NativeHitIndexProgressor<>( seeker, client, openSeekers );
-        }
+        return needFilter ? new FilteringNativeHitIndexProgressor<KEY,VALUE>( seeker, client, openSeekers, query )
+                          : new NativeHitIndexProgressor<KEY,VALUE>( seeker, client, openSeekers );
     }
 
     private boolean isBackwardsSeek( KEY treeKeyFrom, KEY treeKeyTo )

community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/StringSchemaKey.java

@@ -173,7 +173,6 @@ public void writeString( String value )
     @Override
     public void writeString( char value )
     {
-        // TODO more efficient impl
         writeString( String.valueOf( value ) );
     }
 }

community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexBase.java

@@ -34,7 +34,6 @@
 public abstract class FusionIndexBase<T>
 {
     private static final int INSTANCE_COUNT = 5;
-    private static final String[] NAMES = { "string", "number", "spatial", "temporal", "lucene" };
 
     static final int STRING = 0;
     static final int NUMBER = 1;
@@ -67,11 +66,6 @@ static <T,R,E extends Exception> R[] instancesAs( T[] instances, Class<R> cls, T
         return result;
     }
 
-    static String nameOf( int slot )
-    {
-        return NAMES[slot];
-    }
-
     /**
      * 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...

community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexProvider.java

@@ -33,6 +33,9 @@
 import org.neo4j.kernel.api.index.IndexProvider;
 import org.neo4j.kernel.api.schema.index.SchemaIndexDescriptor;
 import org.neo4j.kernel.impl.api.index.sampling.IndexSamplingConfig;
+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.newapi.UnionIndexCapability;
 import org.neo4j.kernel.impl.storemigration.StoreMigrationParticipant;
 import org.neo4j.values.storable.Value;
@@ -42,7 +45,6 @@
 import static org.neo4j.internal.kernel.api.InternalIndexState.FAILED;
 import static org.neo4j.internal.kernel.api.InternalIndexState.POPULATING;
 import static org.neo4j.kernel.impl.index.schema.fusion.FusionIndexBase.instancesAs;
-import static org.neo4j.kernel.impl.index.schema.fusion.FusionIndexBase.nameOf;
 
 /**
  * This {@link IndexProvider index provider} act as one logical index but is backed by four physical
@@ -66,10 +68,10 @@ default <T> T select( T[] instances, Value... values )
 
     public FusionIndexProvider(
             // good to be strict with specific providers here since this is dev facing
-            IndexProvider stringProvider,
-            IndexProvider numberProvider,
-            IndexProvider spatialProvider,
-            IndexProvider temporalProvider,
+            StringIndexProvider stringProvider,
+            NumberIndexProvider numberProvider,
+            SpatialFusionIndexProvider spatialProvider,
+            TemporalIndexProvider temporalProvider,
             IndexProvider luceneProvider,
             Selector selector,
             Descriptor descriptor,
@@ -115,6 +117,15 @@ public String getPopulationFailure( long indexId, SchemaIndexDescriptor descript
         throw new IllegalStateException( "None of the indexes were in a failed state" );
     }
 
+    /**
+     * @param subProviderIndex the index into the providers array to get the name of.
+     * @return some name distinguishing the provider of this subProviderIndex from other providers.
+     */
+    private String nameOf( int subProviderIndex )
+    {
+        return providers[subProviderIndex].getClass().getSimpleName();
+    }
+
     private void writeFailure( String indexName, StringBuilder builder, IndexProvider provider, long indexId, SchemaIndexDescriptor descriptor )
     {
         try

community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/FilteringNativeHitIndexProgressorTest.java

@@ -0,0 +1,92 @@
+/*
+ * Copyright (c) 2002-2018 "Neo Technology,"
+ * Network Engine for Objects in Lund AB [http://neotechnology.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Neo4j is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.neo4j.kernel.impl.index.schema;
+
+import org.junit.Rule;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.function.Predicate;
+
+import org.neo4j.cursor.RawCursor;
+import org.neo4j.index.internal.gbptree.Hit;
+import org.neo4j.internal.kernel.api.IndexQuery;
+import org.neo4j.kernel.api.schema.index.SchemaIndexDescriptorFactory;
+import org.neo4j.test.rule.RandomRule;
+import org.neo4j.values.storable.TextValue;
+import org.neo4j.values.storable.Value;
+
+import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+public class FilteringNativeHitIndexProgressorTest
+{
+    @Rule
+    public final RandomRule random = new RandomRule();
+
+    @Test
+    public void shouldFilterResults()
+    {
+        // given
+        List<String> keys = new ArrayList<>();
+        for ( int i = 0; i < 100; i++ )
+        {
+            // duplicates are fine
+            keys.add( random.string() );
+        }
+
+        RawCursor<Hit<StringSchemaKey,NativeSchemaValue>,IOException> cursor = new ResultCursor( keys.iterator() );
+        NodeValueIterator valueClient = new NodeValueIterator()
+        {
+            @Override
+            public boolean needsValues()
+            {
+                return true;
+            }
+        };
+        IndexQuery[] predicates = new IndexQuery[]{mock( IndexQuery.class )};
+        Predicate<String> filter = string -> string.contains( "a" );
+        when( predicates[0].acceptsValue( any( Value.class ) ) ).then( invocation -> filter.test( ((TextValue)invocation.getArgument( 0 )).stringValue() ) );
+        FilteringNativeHitIndexProgressor<StringSchemaKey,NativeSchemaValue> progressor = new FilteringNativeHitIndexProgressor<>( cursor, valueClient,
+                new ArrayList<>(), predicates );
+        valueClient.initialize( SchemaIndexDescriptorFactory.forLabel( 0, 0 ), progressor, predicates );
+        List<Long> result = new ArrayList<>();
+
+        // when
+        while ( valueClient.hasNext() )
+        {
+            result.add( valueClient.next() );
+        }
+
+        // then
+        for ( int i = 0; i < keys.size(); i++ )
+        {
+            if ( filter.test( keys.get( i ) ) )
+            {
+                assertTrue( result.remove( (long) i ) );
+            }
+        }
+        assertTrue( result.isEmpty() );
+    }
+}

community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/FilteringNativeHitIteratorTest.java

@@ -0,0 +1,81 @@
+/*
+ * Copyright (c) 2002-2018 "Neo Technology,"
+ * Network Engine for Objects in Lund AB [http://neotechnology.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Neo4j is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.neo4j.kernel.impl.index.schema;
+
+import org.junit.Rule;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.function.Predicate;
+
+import org.neo4j.cursor.RawCursor;
+import org.neo4j.index.internal.gbptree.Hit;
+import org.neo4j.internal.kernel.api.IndexQuery;
+import org.neo4j.test.rule.RandomRule;
+import org.neo4j.values.storable.TextValue;
+import org.neo4j.values.storable.Value;
+
+import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+public class FilteringNativeHitIteratorTest
+{
+    @Rule
+    public final RandomRule random = new RandomRule();
+
+    @Test
+    public void shouldFilterResults()
+    {
+        // given
+        List<String> keys = new ArrayList<>();
+        for ( int i = 0; i < 100; i++ )
+        {
+            // duplicates are fine
+            keys.add( random.string() );
+        }
+
+        RawCursor<Hit<StringSchemaKey,NativeSchemaValue>,IOException> cursor = new ResultCursor( keys.iterator() );
+        IndexQuery[] predicates = new IndexQuery[]{mock( IndexQuery.class )};
+        Predicate<String> filter = string -> string.contains( "a" );
+        when( predicates[0].acceptsValue( any( Value.class ) ) ).then( invocation -> filter.test( ((TextValue)invocation.getArgument( 0 )).stringValue() ) );
+        FilteringNativeHitIterator<StringSchemaKey,NativeSchemaValue> iterator = new FilteringNativeHitIterator<>( cursor, new ArrayList<>(), predicates );
+        List<Long> result = new ArrayList<>();
+
+        // when
+        while ( iterator.hasNext() )
+        {
+            result.add( iterator.next() );
+        }
+
+        // then
+        for ( int i = 0; i < keys.size(); i++ )
+        {
+            if ( filter.test( keys.get( i ) ) )
+            {
+                assertTrue( result.remove( (long) i ) );
+            }
+        }
+        assertTrue( result.isEmpty() );
+    }
+}