Cleaned up methods

neo4j · Feb 14, 2018 · 9ed65b7 · 9ed65b7
1 parent 03bff73
commit 9ed65b7
Show file tree

Hide file tree

Showing 12 changed files with 104 additions and 108 deletions.
diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/SpatialKnownIndex.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/SpatialKnownIndex.java
@@ -54,7 +54,6 @@
 import org.neo4j.storageengine.api.schema.IndexReader;
 import org.neo4j.storageengine.api.schema.IndexSample;
 import org.neo4j.values.storable.CoordinateReferenceSystem;
-import org.neo4j.values.storable.Value;
 
 import static org.neo4j.helpers.collection.Iterators.asResourceIterator;
 import static org.neo4j.helpers.collection.Iterators.iterator;
@@ -116,26 +115,40 @@ public SpatialKnownIndex( IndexDirectoryStructure directoryStructure, Coordinate
         state = State.NONE;
     }
 
-    public void initIfNeeded( IndexDescriptor descriptor, IndexSamplingConfig samplingConfig )
+    /**
+     * Makes sure that the index is initialized
+     */
+    public void init( IndexDescriptor descriptor, IndexSamplingConfig samplingConfig )
     {
         if ( state == State.NONE )
         {
             initialize( descriptor, samplingConfig );
         }
     }
 
-    public void createIfNeeded() throws IOException
+    /**
+     * Makes sure that the index is ready to populate
+     */
+    public void startPopulation( IndexDescriptor descriptor, IndexSamplingConfig samplingConfig ) throws IOException
     {
+        init( descriptor, samplingConfig );
         if ( state == State.INIT )
         {
             // First add to sub-index, make sure to create
             create();
         }
+        if ( state != State.POPULATING )
+        {
+            throw new IllegalStateException( "Failed to start populating index." );
+        }
     }
 
+    /**
+     * Makes sure that the index is online
+     */
     public void takeOnline( IndexDescriptor descriptor, IndexSamplingConfig samplingConfig ) throws IOException
     {
-        initIfNeeded( descriptor, samplingConfig );
+        init( descriptor, samplingConfig );
         if ( !indexExists() )
         {
             throw new IOException( "Index file does not exist." );
@@ -169,7 +182,7 @@ public IndexUpdater updaterWithCreate( IndexDescriptor descriptor, IndexSampling
                 // sub-index didn't exist, create and make it online
                 initialize( descriptor, samplingConfig );
                 create();
-                close( true );
+                finishPopulation( true );
                 online();
             }
             return newUpdater();
@@ -220,7 +233,7 @@ private IndexUpdater newUpdater()
 
     // POPULATING
 
-    public synchronized void close( boolean populationCompletedSuccessfully ) throws IOException
+    public synchronized void finishPopulation( boolean populationCompletedSuccessfully ) throws IOException
     {
         assert state == State.POPULATING;
         closeWriter();
@@ -233,7 +246,7 @@ public synchronized void close( boolean populationCompletedSuccessfully ) throws
         {
             if ( populationCompletedSuccessfully )
             {
-                assertPopulatorOpen();
+                schemaIndex.assertOpen();
                 markTreeAsOnline();
                 state = State.POPULATED;
             }
@@ -449,14 +462,6 @@ private void applyWithWorkSync( Collection<? extends IndexEntryUpdate<?>> update
         }
     }
 
-    private void assertPopulatorOpen()
-    {
-        if ( schemaIndex.tree == null )
-        {
-            throw new IllegalStateException( "Populator has already been closed." );
-        }
-    }
-
     private void closeWriter() throws IOException
     {
         singleTreeWriter = schemaIndex.closeIfPresent( singleTreeWriter );
@@ -531,8 +536,6 @@ private enum State
 
     public interface Factory
     {
-        SpatialKnownIndex selectAndCreate( Map<CoordinateReferenceSystem,SpatialKnownIndex> indexMap, long indexId, Value value );
-
         SpatialKnownIndex selectAndCreate( Map<CoordinateReferenceSystem,SpatialKnownIndex> indexMap, long indexId,
                 CoordinateReferenceSystem crs );
     }

diff --git a/.../src/main/java/org/neo4j/kernel/impl/index/schema/fusion/SpatialFusionIndexPopulator.java b/.../src/main/java/org/neo4j/kernel/impl/index/schema/fusion/SpatialFusionIndexPopulator.java
@@ -87,8 +87,8 @@ public void add( Collection<? extends IndexEntryUpdate<?>> updates ) throws Inde
         }
         for ( CoordinateReferenceSystem crs : batchMap.keySet() )
         {
-            SpatialKnownIndex index = getOrCreateInitializedIndex( crs );
+            SpatialKnownIndex index = indexFactory.selectAndCreate( indexMap, indexId, crs );
-            index.createIfNeeded();
+            index.startPopulation( descriptor, samplingConfig );
             index.add( batchMap.get( crs ) );
         }
     }
@@ -116,7 +116,7 @@ public IndexUpdater newPopulatingUpdater( PropertyAccessor accessor )
     @Override
     public void close( boolean populationCompletedSuccessfully ) throws IOException
     {
-        forAll( entry -> ((SpatialKnownIndex) entry).close( populationCompletedSuccessfully ), indexMap.values().toArray() );
+        forAll( entry -> ((SpatialKnownIndex) entry).finishPopulation( populationCompletedSuccessfully ), indexMap.values().toArray() );
     }
 
     @Override
@@ -131,15 +131,9 @@ public void includeSample( IndexEntryUpdate<?> update )
         Value[] values = update.values();
         assert values.length == 1;
         CoordinateReferenceSystem crs = ((PointValue) values[0]).getCoordinateReferenceSystem();
-        SpatialKnownIndex index = getOrCreateInitializedIndex( crs );
-        index.includeSample( update );
-    }
-
-    private SpatialKnownIndex getOrCreateInitializedIndex( CoordinateReferenceSystem crs )
-    {
         SpatialKnownIndex index = indexFactory.selectAndCreate( indexMap, indexId, crs );
-        index.initIfNeeded( descriptor, samplingConfig );
+        index.init( descriptor, samplingConfig );
-        return index;
+        index.includeSample( update );
     }
 
     @Override

diff --git a/...main/java/org/neo4j/kernel/impl/index/schema/fusion/SpatialFusionSchemaIndexProvider.java b/...main/java/org/neo4j/kernel/impl/index/schema/fusion/SpatialFusionSchemaIndexProvider.java
@@ -123,31 +123,24 @@ public InternalIndexState getInitialState( long indexId, IndexDescriptor descrip
         // loop through all files, check if file exists, then check state
         // if any have failed, return failed, else if any are populating return populating, else online
         InternalIndexState state = InternalIndexState.ONLINE;
-        if ( !indexes.containsKey( indexId ) )
+        for ( SpatialKnownIndex index : indexesFor( indexId ).values() )
         {
-            return InternalIndexState.ONLINE;
+            try
-        }
-        for ( SpatialKnownIndex index : indexes.get( indexId ).values() )
-        {
-            if ( index.indexExists() )
             {
-                try
+                switch ( index.readState( descriptor ) )
-                {
-                    switch ( index.readState( descriptor ) )
-                    {
-                    case FAILED:
-                        return InternalIndexState.FAILED;
-                    case POPULATING:
-                        state = InternalIndexState.POPULATING;
-                    default:
-                    }
-                }
-                catch ( IOException e )
                 {
-                    monitor.failedToOpenIndex( indexId, descriptor, "Requesting re-population.", e );
+                case FAILED:
+                    return InternalIndexState.FAILED;
+                case POPULATING:
                     state = InternalIndexState.POPULATING;
+                default:
                 }
             }
+            catch ( IOException e )
+            {
+                monitor.failedToOpenIndex( indexId, descriptor, "Requesting re-population.", e );
+                state = InternalIndexState.POPULATING;
+            }
         }
         return state;
     }
@@ -167,19 +160,11 @@ public StoreMigrationParticipant storeMigrationParticipant( FileSystemAbstractio
         return StoreMigrationParticipant.NOT_PARTICIPATING;
     }
 
-    @Override
-    public SpatialKnownIndex selectAndCreate( Map<CoordinateReferenceSystem,SpatialKnownIndex> indexMap, long indexId, Value value )
-    {
-        PointValue pointValue = (PointValue) value;
-        CoordinateReferenceSystem crs = pointValue.getCoordinateReferenceSystem();
-        return selectAndCreate( indexMap, indexId, crs );
-    }
-
     @Override
     public SpatialKnownIndex selectAndCreate( Map<CoordinateReferenceSystem,SpatialKnownIndex> indexMap, long indexId, CoordinateReferenceSystem crs )
     {
         return indexMap.computeIfAbsent( crs,
-                k -> new SpatialKnownIndex( directoryStructure(), crs, indexId, pageCache, fs, monitor, recoveryCleanupWorkCollector ) );
+                crsKey -> new SpatialKnownIndex( directoryStructure(), crsKey, indexId, pageCache, fs, monitor, recoveryCleanupWorkCollector ) );
     }
 
     /**

diff --git a/community/kernel/src/test/java/org/neo4j/graphdb/IndexingAcceptanceTest.java b/community/kernel/src/test/java/org/neo4j/graphdb/IndexingAcceptanceTest.java
@@ -359,8 +359,8 @@ public void shouldBeAbleToQuerySupportedPropertyTypes()
         assertCanCreateAndFind( db, LABEL1, property, 12L );
         assertCanCreateAndFind( db, LABEL1, property, (float)12. );
         assertCanCreateAndFind( db, LABEL1, property, 12. );
-        assertCanCreateAndFind( db, LABEL1, property, new SpatialMocks.MockPoint( 12.3, 45.6, mockWGS84() ) );
+        assertCanCreateAndFind( db, LABEL1, property, SpatialMocks.mockPoint( 12.3, 45.6, mockWGS84() ) );
-        assertCanCreateAndFind( db, LABEL1, property, new SpatialMocks.MockPoint( 123, 456, mockCartesian() ) );
+        assertCanCreateAndFind( db, LABEL1, property, SpatialMocks.mockPoint( 123, 456, mockCartesian() ) );
         assertCanCreateAndFind( db, LABEL1, property, Values.pointValue( CoordinateReferenceSystem.WGS84, 12.3, 45.6 ) );
         assertCanCreateAndFind( db, LABEL1, property, Values.pointValue( CoordinateReferenceSystem.Cartesian, 123, 456 ) );
 
@@ -381,8 +381,8 @@ public void shouldBeAbleToQuerySupportedPropertyTypes()
         assertCanCreateAndFind( db, LABEL1, property, new Float[]{(float)19.} );
         assertCanCreateAndFind( db, LABEL1, property, new double[]{20.} );
         assertCanCreateAndFind( db, LABEL1, property, new Double[]{21.} );
-        assertCanCreateAndFind( db, LABEL1, property, new Point[]{new SpatialMocks.MockPoint( 12.3, 45.6, mockWGS84() )} );
+        assertCanCreateAndFind( db, LABEL1, property, new Point[]{SpatialMocks.mockPoint( 12.3, 45.6, mockWGS84() )} );
-        assertCanCreateAndFind( db, LABEL1, property, new Point[]{new SpatialMocks.MockPoint( 123, 456, mockCartesian() )} );
+        assertCanCreateAndFind( db, LABEL1, property, new Point[]{SpatialMocks.mockPoint( 123, 456, mockCartesian() )} );
         assertCanCreateAndFind( db, LABEL1, property, new PointValue[]{Values.pointValue( CoordinateReferenceSystem.WGS84, 12.3, 45.6 )} );
         assertCanCreateAndFind( db, LABEL1, property, new PointValue[]{Values.pointValue( CoordinateReferenceSystem.Cartesian, 123, 456 )} );
     }

diff --git a/community/kernel/src/test/java/org/neo4j/graphdb/SpatialMocks.java b/community/kernel/src/test/java/org/neo4j/graphdb/SpatialMocks.java
@@ -29,6 +29,15 @@
 
 public class SpatialMocks
 {
+    public static MockPoint mockPoint( double x, double y, CRS crs )
+    {
+        return new MockPoint( x, y, crs );
+    }
+
+    public static MockGeometry mockGeometry( String geometryType, List<Coordinate> coordinates, CRS crs )
+    {
+        return new MockGeometry( geometryType, coordinates, crs );
+    }
 
     public static CRS mockWGS84()
     {
@@ -40,7 +49,7 @@ public static CRS mockCartesian()
         return mockCRS( 7203, "cartesian", "http://spatialreference.org/ref/sr-org/7203/" );
     }
 
-    public static CRS mockCRS( final int code, final String type, final String href )
+    private static CRS mockCRS( final int code, final String type, final String href )
     {
         return new CRS()
         {
@@ -61,25 +70,25 @@ public String getHref()
         };
     }
 
-    public static class MockPoint extends MockGeometry implements Point
+    private static class MockPoint extends MockGeometry implements Point
     {
         private final Coordinate coordinate;
 
-        public MockPoint( final double x, final double y, final CRS crs )
+        private MockPoint( final double x, final double y, final CRS crs )
         {
             super( "Point", new ArrayList<>(), crs );
             this.coordinate = new Coordinate( x, y );
             this.coordinates.add( this.coordinate );
         }
     }
 
-    public static class MockGeometry implements Geometry
+    private static class MockGeometry implements Geometry
     {
-        protected final String geometryType;
+        final String geometryType;
+        final List<Coordinate> coordinates;
         protected final CRS crs;
-        protected final List<Coordinate> coordinates;
 
-        public MockGeometry( String geometryType, final List<Coordinate> coordinates, final CRS crs )
+        private MockGeometry( String geometryType, final List<Coordinate> coordinates, final CRS crs )
         {
             this.geometryType = geometryType;
             this.coordinates = coordinates;

diff --git a/community/kernel/src/test/java/org/neo4j/kernel/api/index/NodeUpdatesTest.java b/community/kernel/src/test/java/org/neo4j/kernel/api/index/NodeUpdatesTest.java
@@ -83,7 +83,7 @@ public void shouldNotGenerateUpdateForMultipleExistingPropertiesAndLabels()
         NodeUpdates updates = NodeUpdates.forNode( nodeId, labels )
                 .existing( propertyKeyId1, Values.of( "Neo" ) )
                 .existing( propertyKeyId2, Values.of( 100L ) )
-                .existing( propertyKeyId3, Values.of( Values.pointValue( CoordinateReferenceSystem.WGS84, 12.3, 45.6 ) ) )
+                .existing( propertyKeyId3, Values.pointValue( CoordinateReferenceSystem.WGS84, 12.3, 45.6 ) )
                 .build();
 
         // Then
@@ -122,7 +122,7 @@ public void shouldGenerateUpdatesForLabelAdditionWithExistingProperties()
                 NodeUpdates.forNode( nodeId, empty, labels )
                         .existing( propertyKeyId1, Values.of( "Neo" ) )
                         .existing( propertyKeyId2, Values.of( 100L ) )
-                        .existing( propertyKeyId3, Values.of( Values.pointValue( CoordinateReferenceSystem.WGS84, 12.3, 45.6 ) ) )
+                        .existing( propertyKeyId3, Values.pointValue( CoordinateReferenceSystem.WGS84, 12.3, 45.6 ) )
                         .build();
 
         // Then

diff --git a/...ity/kernel/src/test/java/org/neo4j/kernel/api/index/SimpleIndexAccessorCompatibility.java b/...ity/kernel/src/test/java/org/neo4j/kernel/api/index/SimpleIndexAccessorCompatibility.java
@@ -27,9 +27,6 @@
 import org.neo4j.internal.kernel.api.IndexQuery;
 import org.neo4j.kernel.api.schema.index.IndexDescriptor;
 import org.neo4j.kernel.api.schema.index.IndexDescriptorFactory;
-import org.neo4j.values.storable.CoordinateReferenceSystem;
-import org.neo4j.values.storable.PointValue;
-import org.neo4j.values.storable.Values;
 
 import static java.util.Arrays.asList;
 import static java.util.Collections.EMPTY_LIST;