Merge pull request #6692 from spacecowboy/2.2-importpropid

Fixed: some properties could get dropped during large imports

community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/store/BatchingIdGeneratorFactory.java

@@ -63,28 +63,22 @@ public IdGenerator get( IdType idType )
 
     private static class BatchingIdGenerator implements IdGenerator
     {
-        private long highId;
+        private final BatchingIdSequence idSequence;
         private final FileSystemAbstraction fs;
         private final File fileName;
 
         public BatchingIdGenerator( FileSystemAbstraction fs, File fileName, long highId )
         {
             this.fs = fs;
             this.fileName = fileName;
-            this.highId = highId;
+            this.idSequence = new BatchingIdSequence();
+            this.idSequence.set( highId );
         }
 
         @Override
         public long nextId()
         {
-            try
-            {
-                return highId;
-            }
-            finally
-            {
-                highId++;
-            }
+            return idSequence.nextId();
         }
 
         @Override
@@ -96,13 +90,13 @@ public IdRange nextIdBatch( int size )
         @Override
         public void setHighId( long id )
         {
-            highId = id;
+            idSequence.set( id );
         }
 
         @Override
         public long getHighId()
         {
-            return highId;
+            return idSequence.peek();
         }
 
         @Override
@@ -114,13 +108,13 @@ public void freeId( long id )
         public void close()
         {
             fs.deleteFile( fileName );
-            createGenerator( fs, fileName, highId );
+            createGenerator( fs, fileName, idSequence.peek() );
         }
 
         @Override
         public long getNumberOfIdsInUse()
         {
-            return highId;
+            return idSequence.peek();
         }
 
         @Override

community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/store/BatchingIdSequence.java

@@ -32,16 +32,27 @@ public class BatchingIdSequence implements IdSequence
     @Override
     public long nextId()
     {
-        long result = nextId++;
-        if ( result == IdGeneratorImpl.INTEGER_MINUS_ONE )
-        {
-            result = nextId++;
-        }
+        long result = peek();
+        nextId++;
         return result;
     }
 
     public void reset()
     {
         nextId = 0;
     }
+
+    public void set( long nextId )
+    {
+        this.nextId = nextId;
+    }
+
+    public long peek()
+    {
+        if ( nextId == IdGeneratorImpl.INTEGER_MINUS_ONE )
+        {
+            nextId++;
+        }
+        return nextId;
+    }
 }

community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/store/BatchingIdSequenceTest.java

@@ -0,0 +1,70 @@
+/*
+ * Copyright (c) 2002-2016 "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.unsafe.impl.batchimport.store;
+
+import org.junit.Test;
+
+import org.neo4j.kernel.impl.store.id.IdGeneratorImpl;
+
+import static org.junit.Assert.assertEquals;
+
+public class BatchingIdSequenceTest
+{
+
+    @Test
+    public void ShouldSkipNullId() throws Exception
+    {
+        BatchingIdSequence idSequence = new BatchingIdSequence();
+
+        idSequence.set( IdGeneratorImpl.INTEGER_MINUS_ONE - 1 );
+        assertEquals( IdGeneratorImpl.INTEGER_MINUS_ONE - 1, idSequence.peek() );
+
+        // The 'NULL Id' should be skipped, and never be visible anywhere.
+        // Peek should always return what nextId will return
+
+        assertEquals( IdGeneratorImpl.INTEGER_MINUS_ONE - 1, idSequence.nextId() );
+        assertEquals( IdGeneratorImpl.INTEGER_MINUS_ONE + 1, idSequence.peek() );
+        assertEquals( IdGeneratorImpl.INTEGER_MINUS_ONE + 1, idSequence.nextId() );
+
+        // And what if someone were to set it directly to the NULL id
+        idSequence.set( IdGeneratorImpl.INTEGER_MINUS_ONE );
+
+        assertEquals( IdGeneratorImpl.INTEGER_MINUS_ONE + 1, idSequence.peek() );
+        assertEquals( IdGeneratorImpl.INTEGER_MINUS_ONE + 1, idSequence.nextId() );
+    }
+
+    @Test
+    public void resetShouldSetDefault() throws Exception
+    {
+        BatchingIdSequence idSequence = new BatchingIdSequence();
+
+        idSequence.set( 99L );
+
+        assertEquals( 99L, idSequence.peek() );
+        assertEquals( 99L, idSequence.nextId() );
+        assertEquals( 100L, idSequence.peek() );
+
+        idSequence.reset();
+
+        assertEquals( 0L, idSequence.peek() );
+        assertEquals( 0L, idSequence.nextId() );
+        assertEquals( 1L, idSequence.peek() );
+    }
+}