From 18f7802689685bf9528b587a4e2251cb6a8d9526 Mon Sep 17 00:00:00 2001 From: lutovich Date: Thu, 12 May 2016 22:29:17 +0200 Subject: [PATCH] Couple fixes after the code review Simplified StorePropertyCursor construction, added javadocs. --- .../impl/api/store/StoreAbstractNodeCursor.java | 6 ++---- .../api/store/StoreAbstractRelationshipCursor.java | 6 ++---- .../kernel/impl/api/store/StorePropertyCursor.java | 12 ++++-------- .../impl/api/store/StoreSinglePropertyCursor.java | 13 +++---------- .../org/neo4j/kernel/impl/store/RecordCursor.java | 8 ++++++++ .../org/neo4j/kernel/impl/store/RecordCursors.java | 3 +++ .../storemigration/participant/StoreMigrator.java | 3 +-- .../impl/api/store/StorePropertyCursorTest.java | 12 ++++++++++-- 8 files changed, 33 insertions(+), 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 index a375a775e45c..be5427d5b72f 100644 --- 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,8 +116,7 @@ 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() @@ -125,8 +124,7 @@ protected StoreSinglePropertyCursor create() @Override protected StorePropertyCursor create() { - return new StorePropertyCursor( cursors.property(), cursors.propertyString(), cursors.propertyArray(), - allPropertyCursor ); + return new StorePropertyCursor( cursors, allPropertyCursor ); } }; } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StoreAbstractRelationshipCursor.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StoreAbstractRelationshipCursor.java index ca763cb54cd3..d0be9f5144cd 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StoreAbstractRelationshipCursor.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StoreAbstractRelationshipCursor.java @@ -59,8 +59,7 @@ 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() @@ -68,8 +67,7 @@ protected StoreSinglePropertyCursor create() @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 index efd4b8ae6c22..420c3534b481 100644 --- 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 private Lock lock; - public StorePropertyCursor( - RecordCursor recordCursor, - RecordCursor stringRecordCursor, - RecordCursor arrayRecordCursor, - Consumer instanceCache ) + public StorePropertyCursor( RecordCursors cursors, Consumer 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/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StoreSinglePropertyCursor.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StoreSinglePropertyCursor.java index 6ef653d2a1db..fea14d385284 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StoreSinglePropertyCursor.java +++ b/community/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 propertyRecordCursor, - RecordCursor propertyStringRecordCursor, - RecordCursor propertyArrayRecordCursor, - Consumer instanceCache ) + public StoreSinglePropertyCursor( RecordCursors cursors, Consumer 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 index 98c1e3837b95..81a56bd1a058 100644 --- 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 extends Cursor */ 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 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 index 30c987541f41..fc03e8468a5a 100644 --- 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 node; diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/StoreMigrator.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/StoreMigrator.java index c38e464e800f..c04ef53d4d53 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/StoreMigrator.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/StoreMigrator.java @@ -539,8 +539,7 @@ private BiConsumer {}; } - final StorePropertyCursor cursor = new StorePropertyCursor( cursors.property(), - cursors.propertyString(), cursors.propertyArray(), (ignored) -> {} ); + final StorePropertyCursor cursor = new StorePropertyCursor( cursors, ignored -> {} ); final List 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 index aea2e382392d..c32fad5f5305 100644 --- 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 cache ) { RecordCursor propertyRecordCursor = propertyStore.newRecordCursor( propertyStore.newRecord() ); + propertyRecordCursor.acquire( 0, NORMAL ); DynamicStringStore stringStore = propertyStore.getStringStore(); RecordCursor dynamicStringCursor = stringStore.newRecordCursor( stringStore.newRecord() ); + dynamicStringCursor.acquire( 0, NORMAL ); DynamicArrayStore arrayStore = propertyStore.getArrayStore(); RecordCursor 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 createPropertyChain( PropertyStore store, int firstRecordId, int keyId,