Couple fixes after the code review

Simplified StorePropertyCursor construction, added javadocs.
neo4j · May 12, 2016 · 18f7802 · 18f7802
1 parent 9fb0c46
commit 18f7802
Show file tree

Hide file tree

Showing 8 changed files with 33 additions and 30 deletions.
diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StoreAbstractNodeCursor.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StoreAbstractNodeCursor.java
@@ -116,17 +116,15 @@ protected StoreNodeRelationshipCursor create()
             @Override
             protected StoreSinglePropertyCursor create()
             {
-                return new StoreSinglePropertyCursor( cursors.property(), cursors.propertyString(),
-                        cursors.propertyArray(), this );
+                return new StoreSinglePropertyCursor( cursors, this );
             }
         };
         allPropertyCursor = new InstanceCache<StorePropertyCursor>()
         {
             @Override
             protected StorePropertyCursor create()
             {
-                return new StorePropertyCursor( cursors.property(), cursors.propertyString(), cursors.propertyArray(),
-                        allPropertyCursor );
+                return new StorePropertyCursor( cursors, allPropertyCursor );
             }
         };
     }

diff --git a/...kernel/src/main/java/org/neo4j/kernel/impl/api/store/StoreAbstractRelationshipCursor.java b/...kernel/src/main/java/org/neo4j/kernel/impl/api/store/StoreAbstractRelationshipCursor.java
@@ -59,17 +59,15 @@ public StoreAbstractRelationshipCursor( RelationshipRecord relationshipRecord, L
             @Override
             protected StoreSinglePropertyCursor create()
             {
-                return new StoreSinglePropertyCursor( cursors.property(),
-                        cursors.propertyString(), cursors.propertyArray(), this );
+                return new StoreSinglePropertyCursor( cursors, this );
             }
         };
         allPropertyCursor = new InstanceCache<StorePropertyCursor>()
         {
             @Override
             protected StorePropertyCursor create()
             {
-                return new StorePropertyCursor( cursors.property(),
-                        cursors.propertyString(), cursors.propertyArray(), this );
+                return new StorePropertyCursor( cursors, this );
             }
         };
     }

diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StorePropertyCursor.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StorePropertyCursor.java
@@ -24,7 +24,7 @@
 import org.neo4j.cursor.Cursor;
 import org.neo4j.kernel.impl.locking.Lock;
 import org.neo4j.kernel.impl.store.RecordCursor;
-import org.neo4j.kernel.impl.store.record.DynamicRecord;
+import org.neo4j.kernel.impl.store.RecordCursors;
 import org.neo4j.kernel.impl.store.record.PropertyRecord;
 import org.neo4j.kernel.impl.store.record.Record;
 import org.neo4j.storageengine.api.PropertyItem;
@@ -42,15 +42,11 @@ public class StorePropertyCursor implements Cursor<PropertyItem>, PropertyItem
 
     private Lock lock;
 
-    public StorePropertyCursor(
-            RecordCursor<PropertyRecord> recordCursor,
-            RecordCursor<DynamicRecord> stringRecordCursor,
-            RecordCursor<DynamicRecord> arrayRecordCursor,
-            Consumer<StorePropertyCursor> instanceCache )
+    public StorePropertyCursor( RecordCursors cursors, Consumer<StorePropertyCursor> instanceCache )
     {
         this.instanceCache = instanceCache;
-        this.payload = new StorePropertyPayloadCursor( stringRecordCursor, arrayRecordCursor );
-        this.recordCursor = recordCursor;
+        this.payload = new StorePropertyPayloadCursor( cursors.propertyString(), cursors.propertyArray() );
+        this.recordCursor = cursors.property();
     }
 
     public StorePropertyCursor init( long firstPropertyId, Lock lock )

diff --git a/...unity/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StoreSinglePropertyCursor.java b/...unity/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StoreSinglePropertyCursor.java
@@ -23,9 +23,7 @@
 
 import org.neo4j.kernel.api.StatementConstants;
 import org.neo4j.kernel.impl.locking.Lock;
-import org.neo4j.kernel.impl.store.RecordCursor;
-import org.neo4j.kernel.impl.store.record.DynamicRecord;
-import org.neo4j.kernel.impl.store.record.PropertyRecord;
+import org.neo4j.kernel.impl.store.RecordCursors;
 
 /**
  * Cursor for a specific property on a node or relationship.
@@ -34,14 +32,9 @@ public class StoreSinglePropertyCursor extends StorePropertyCursor
 {
     private int propertyKeyId;
 
-    public StoreSinglePropertyCursor(
-            RecordCursor<PropertyRecord> propertyRecordCursor,
-            RecordCursor<DynamicRecord> propertyStringRecordCursor,
-            RecordCursor<DynamicRecord> propertyArrayRecordCursor,
-            Consumer<StoreSinglePropertyCursor> instanceCache )
+    public StoreSinglePropertyCursor( RecordCursors cursors, Consumer<StoreSinglePropertyCursor> instanceCache )
     {
-        //noinspection unchecked
-        super( propertyRecordCursor, propertyStringRecordCursor, propertyArrayRecordCursor, (Consumer) instanceCache );
+        super( cursors, (Consumer) instanceCache );
     }
 
     public StoreSinglePropertyCursor init( long firstPropertyId, int propertyKeyId, Lock lock )

diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/RecordCursor.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/RecordCursor.java
@@ -91,6 +91,14 @@ public interface RecordCursor<R extends AbstractBaseRecord> extends Cursor<R>
       */
     boolean next( long id, R record, RecordLoad mode );
 
+    /**
+     * Read all records in the chain starting from the id this cursor is positioned at using either
+     * {@link #acquire(long, RecordLoad)} or {@link #placeAt(long, RecordLoad)}. Each next record in the chain is
+     * determined by {@link RecordStore#getNextRecordReference(AbstractBaseRecord)}. Each record placed in the
+     * resulting list is a clone of the reused record.
+     *
+     * @return records of the chain in list.
+     */
     @SuppressWarnings( "unchecked" )
     default List<R> getAll()
     {

diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/RecordCursors.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/RecordCursors.java
@@ -30,6 +30,9 @@
 
 import static org.neo4j.kernel.impl.store.record.RecordLoad.NORMAL;
 
+/**
+ * Container for {@link RecordCursor}s for different stores. Intended to be reused by pooled transactions.
+ */
 public class RecordCursors implements AutoCloseable
 {
     private final RecordCursor<NodeRecord> node;

diff --git a/.../kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/StoreMigrator.java b/.../kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/StoreMigrator.java
@@ -539,8 +539,7 @@ private <ENTITY extends InputEntity,RECORD extends PrimitiveRecord> BiConsumer<E
             return (a, b) -> {};
         }
 
-        final StorePropertyCursor cursor = new StorePropertyCursor( cursors.property(),
-                cursors.propertyString(), cursors.propertyArray(), (ignored) -> {} );
+        final StorePropertyCursor cursor = new StorePropertyCursor( cursors, ignored -> {} );
         final List<Object> scratch = new ArrayList<>();
         return (ENTITY entity, RECORD record) -> {
             cursor.init( record.getNextProp(), LockService.NO_LOCK );

diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/StorePropertyCursorTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/StorePropertyCursorTest.java
@@ -47,6 +47,7 @@
 import org.neo4j.kernel.impl.store.PropertyStore;
 import org.neo4j.kernel.impl.store.PropertyType;
 import org.neo4j.kernel.impl.store.RecordCursor;
+import org.neo4j.kernel.impl.store.RecordCursors;
 import org.neo4j.kernel.impl.store.RecordStore;
 import org.neo4j.kernel.impl.store.StoreFactory;
 import org.neo4j.kernel.impl.store.format.standard.PropertyRecordFormat;
@@ -786,15 +787,22 @@ private static StorePropertyCursor newStorePropertyCursor( PropertyStore propert
             Consumer<StorePropertyCursor> cache )
     {
         RecordCursor<PropertyRecord> propertyRecordCursor = propertyStore.newRecordCursor( propertyStore.newRecord() );
+        propertyRecordCursor.acquire( 0, NORMAL );
 
         DynamicStringStore stringStore = propertyStore.getStringStore();
         RecordCursor<DynamicRecord> dynamicStringCursor = stringStore.newRecordCursor( stringStore.newRecord() );
+        dynamicStringCursor.acquire( 0, NORMAL );
 
         DynamicArrayStore arrayStore = propertyStore.getArrayStore();
         RecordCursor<DynamicRecord> dynamicArrayCursor = arrayStore.newRecordCursor( arrayStore.newRecord() );
+        dynamicArrayCursor.acquire( 0, NORMAL );
 
-        return new StorePropertyCursor( propertyRecordCursor.acquire( 0, NORMAL ),
-                dynamicStringCursor.acquire( 0, NORMAL ), dynamicArrayCursor.acquire( 0, NORMAL ), cache );
+        RecordCursors cursors = mock( RecordCursors.class );
+        when( cursors.property() ).thenReturn( propertyRecordCursor );
+        when( cursors.propertyString() ).thenReturn( dynamicStringCursor );
+        when( cursors.propertyArray() ).thenReturn( dynamicArrayCursor );
+
+        return new StorePropertyCursor( cursors, cache );
     }
 
     private static List<PropertyRecord> createPropertyChain( PropertyStore store, int firstRecordId, int keyId,