Skip to content

Commit

Permalink
Fix dispose PageCursor used for reading property records in PropertyC…
Browse files Browse the repository at this point in the history
…ursor
  • Loading branch information
davidegrohmann committed May 8, 2017
1 parent b3bf382 commit 32d5a00
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 19 deletions.
Expand Up @@ -197,6 +197,7 @@ public final void close()
@Override
public void dispose()
{
cursor.close();
payload.dispose();
}
}
Expand Up @@ -235,9 +235,9 @@ private void readFromStore( CommonAbstractStore<DynamicRecord,?> store, PageCurs

private void readRecord( CommonAbstractStore<DynamicRecord,?> store, PageCursor cursor, long blockId )
{
record.clear();
try
{
record.clear();
store.readIntoRecord( blockId, record, FORCE, cursor );
}
catch ( IOException e )
Expand Down
Expand Up @@ -38,11 +38,13 @@
import org.neo4j.cursor.Cursor;
import org.neo4j.graphdb.mockfs.EphemeralFileSystemAbstraction;
import org.neo4j.io.pagecache.PageCache;
import org.neo4j.io.pagecache.PageCursor;
import org.neo4j.kernel.impl.store.DynamicArrayStore;
import org.neo4j.kernel.impl.store.DynamicRecordAllocator;
import org.neo4j.kernel.impl.store.DynamicStringStore;
import org.neo4j.kernel.impl.store.NeoStores;
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.RecordStore;
import org.neo4j.kernel.impl.store.StoreFactory;
import org.neo4j.kernel.impl.store.format.standard.PropertyRecordFormat;
Expand All @@ -52,7 +54,6 @@
import org.neo4j.kernel.impl.store.record.PropertyRecord;
import org.neo4j.logging.NullLogProvider;
import org.neo4j.storageengine.api.PropertyItem;
import org.neo4j.test.MockedNeoStores;
import org.neo4j.test.rule.PageCacheRule;
import org.neo4j.test.rule.fs.EphemeralFileSystemRule;

Expand All @@ -64,7 +65,6 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.Matchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
Expand Down Expand Up @@ -224,22 +224,16 @@ private static Object actualValue( Object parameter )
return parameter instanceof BigProperty ? ((BigProperty)parameter).value() : parameter;
}

public static class ErrorTest
public static class CloseDisposeTest
{
private final NeoStores neoStores = MockedNeoStores.basicMockedNeoStores();
private final PropertyStore propertyStore = neoStores.getPropertyStore();
private final PropertyStore propertyStore = mock( PropertyStore.class );
private final DynamicStringStore stringStore = mock( DynamicStringStore.class );
private final DynamicArrayStore arrayStore = mock( DynamicArrayStore.class );
{
RecordCursor<PropertyRecord> recordCursor = MockedNeoStores.mockedRecordCursor();
try
{
when( recordCursor.next() ).thenReturn( true );
}
catch ( Exception e )
{
throw new RuntimeException( e );
}
when( recordCursor.get() ).thenReturn( new PropertyRecord( 42 ) );
when( propertyStore.newRecordCursor( any( PropertyRecord.class ) ) ).thenReturn( recordCursor );
when( stringStore.newRecord() ).thenReturn( mock( DynamicRecord.class ) );
when( propertyStore.getStringStore() ).thenReturn( stringStore );
when( arrayStore.newRecord() ).thenReturn( mock( DynamicRecord.class ) );
when( propertyStore.getArrayStore() ).thenReturn( arrayStore );
}

@Test
Expand All @@ -258,6 +252,27 @@ public void shouldReturnTheCursorToTheCacheOnClose() throws Throwable
// then
verify( cache, times( 1 ) ).accept( storePropertyCursor );
}

@Test
public void shouldClosePageCursorsOnDispose() throws Throwable
{
// given
PageCursor propertyCursor = mock( PageCursor.class );
when( propertyStore.newPageCursor() ).thenReturn( propertyCursor );
PageCursor stringCursor = mock( PageCursor.class );
when( stringStore.newPageCursor() ).thenReturn( stringCursor );
PageCursor arrayCursor = mock( PageCursor.class );
when( arrayStore.newPageCursor() ).thenReturn( arrayCursor );
StorePropertyCursor storePropertyCursor = new StorePropertyCursor( propertyStore, c -> {} );

// when
storePropertyCursor.dispose();

// then
verify( propertyCursor ).close();
verify( stringCursor ).close();
verify( arrayCursor ).close();
}
}

public static class PropertyStoreBasedTestSupport
Expand Down
Expand Up @@ -530,6 +530,7 @@ private static long[] asBlocks( Param... params )
private static <S extends AbstractDynamicStore> S newDynamicStoreMock( Class<S> clazz ) throws IOException
{
S store = mock( clazz );
when( store.newRecord() ).thenReturn( new DynamicRecord( DynamicRecord.NO_ID ) );
doAnswer( invocationOnMock ->
{
DynamicRecord record = (DynamicRecord) invocationOnMock.getArguments()[1];
Expand All @@ -538,7 +539,6 @@ private static <S extends AbstractDynamicStore> S newDynamicStoreMock( Class<S>
return null;
} ).when( store ).readIntoRecord( anyLong(), any( DynamicRecord.class ), any( RecordLoad.class ),
any( PageCursor.class ) );

return store;
}

Expand Down

0 comments on commit 32d5a00

Please sign in to comment.