From bfd3d70847f0547e64fb45facd17d4f166416b2f Mon Sep 17 00:00:00 2001 From: fickludd Date: Tue, 14 Feb 2017 11:21:38 +0100 Subject: [PATCH] Clean-up NodeUpdate building in two tests --- .../state/TransactionRecordStateTest.java | 119 ++++++++++-------- .../batchinsert/internal/BatchInsertTest.java | 11 +- ...ltiIndexPopulationConcurrentUpdatesIT.java | 17 ++- 3 files changed, 84 insertions(+), 63 deletions(-) diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/state/TransactionRecordStateTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/state/TransactionRecordStateTest.java index 18dad3182108d..9784e2217c8d1 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/state/TransactionRecordStateTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/state/TransactionRecordStateTest.java @@ -96,7 +96,6 @@ import static org.mockito.Mockito.reset; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; -import static org.neo4j.collection.primitive.PrimitiveLongCollections.EMPTY_LONG_ARRAY; import static org.neo4j.graphdb.Direction.INCOMING; import static org.neo4j.graphdb.Direction.OUTGOING; import static org.neo4j.helpers.collection.Iterables.count; @@ -113,8 +112,14 @@ public class TransactionRecordStateTest { private static final String LONG_STRING = "string value long enough not to be stored as a short string"; + private static final int propertyId1 = 1; + private static final int propertyId2 = 2; private static final String value1 = "first"; private static final int value2 = 4; + private static final long[] noLabels = new long[0]; + private long[] oneLabelId = new long[]{3}; + private long[] secondLabelId = new long[]{4}; + private long[] bothLabelIds = new long[]{3, 4}; public static void assertRelationshipGroupDoesNotExist( RecordChangeSet recordChangeSet, NodeRecord node, int type ) @@ -293,23 +298,22 @@ public void shouldConvertLabelAdditionToNodePropertyUpdates() throws Exception NeoStores neoStores = neoStoresRule.open(); long nodeId = 0; TransactionRecordState recordState = newTransactionRecordState( neoStores ); - int propertyKey1 = 1, propertyKey2 = 2, labelId = 3; Object value1 = LONG_STRING, value2 = LONG_STRING.getBytes(); recordState.nodeCreate( nodeId ); - recordState.nodeAddProperty( nodeId, propertyKey1, value1 ); - recordState.nodeAddProperty( nodeId, propertyKey2, value2 ); + recordState.nodeAddProperty( nodeId, propertyId1, value1 ); + recordState.nodeAddProperty( nodeId, propertyId2, value2 ); apply( neoStores, recordState ); // WHEN recordState = newTransactionRecordState( neoStores ); - recordState.addLabelToNode( labelId, nodeId ); + addLabelsToNode( recordState, nodeId, oneLabelId ); Iterable indexUpdates = indexUpdatesOf( neoStores, recordState ); // THEN - NodeUpdates expected = NodeUpdates.forNode( nodeId, new long[0], new long[]{labelId} ) + NodeUpdates expected = NodeUpdates.forNode( nodeId, noLabels, oneLabelId ) .buildWithExistingProperties( - Property.stringProperty( propertyKey1, LONG_STRING ), - Property.byteArrayProperty( propertyKey2, LONG_STRING.getBytes() ) ); + Property.stringProperty( propertyId1, LONG_STRING ), + Property.byteArrayProperty( propertyId2, LONG_STRING.getBytes() ) ); assertEquals( expected, Iterables.single( indexUpdates ) ); } @@ -320,23 +324,22 @@ public void shouldConvertMixedLabelAdditionAndSetPropertyToNodePropertyUpdates() NeoStores neoStores = neoStoresRule.open(); long nodeId = 0; TransactionRecordState recordState = newTransactionRecordState( neoStores ); - int propertyKey1 = 1, propertyKey2 = 2, labelId1 = 3, labelId2 = 4; recordState.nodeCreate( nodeId ); - recordState.nodeAddProperty( nodeId, propertyKey1, value1 ); - recordState.addLabelToNode( labelId1, nodeId ); + recordState.nodeAddProperty( nodeId, propertyId1, value1 ); + addLabelsToNode( recordState, nodeId, oneLabelId ); apply( neoStores, recordState ); // WHEN recordState = newTransactionRecordState( neoStores ); - recordState.nodeAddProperty( nodeId, propertyKey2, value2 ); - recordState.addLabelToNode( labelId2, nodeId ); + recordState.nodeAddProperty( nodeId, propertyId2, value2 ); + addLabelsToNode( recordState, nodeId, secondLabelId ); Iterable indexUpdates = indexUpdatesOf( neoStores, recordState ); // THEN NodeUpdates expected = - NodeUpdates.forNode( nodeId, new long[]{labelId1}, new long[]{labelId1, labelId2} ) - .added( propertyKey2, value2 ) - .buildWithExistingProperties( Property.stringProperty( propertyKey1, value1 ) ); + NodeUpdates.forNode( nodeId, oneLabelId, bothLabelIds ) + .added( propertyId2, value2 ) + .buildWithExistingProperties( Property.stringProperty( propertyId1, value1 ) ); assertEquals( expected, Iterables.single( indexUpdates ) ); } @@ -347,23 +350,22 @@ public void shouldConvertLabelRemovalToNodePropertyUpdates() throws Exception NeoStores neoStores = neoStoresRule.open(); long nodeId = 0; TransactionRecordState recordState = newTransactionRecordState( neoStores ); - int propertyKey1 = 1, propertyKey2 = 2, labelId = 3; recordState.nodeCreate( nodeId ); - recordState.nodeAddProperty( nodeId, propertyKey1, value1 ); - recordState.nodeAddProperty( nodeId, propertyKey2, value2 ); - recordState.addLabelToNode( labelId, nodeId ); + recordState.nodeAddProperty( nodeId, propertyId1, value1 ); + recordState.nodeAddProperty( nodeId, propertyId2, value2 ); + addLabelsToNode( recordState, nodeId, oneLabelId ); apply( neoStores, recordState ); // WHEN recordState = newTransactionRecordState( neoStores ); - recordState.removeLabelFromNode( labelId, nodeId ); + removeLabelsFromNode( recordState, nodeId, oneLabelId ); Iterable indexUpdates = indexUpdatesOf( neoStores, recordState ); // THEN - NodeUpdates expected = NodeUpdates.forNode( nodeId, new long[]{labelId}, EMPTY_LONG_ARRAY ) + NodeUpdates expected = NodeUpdates.forNode( nodeId, oneLabelId, noLabels ) .buildWithExistingProperties( - Property.stringProperty( propertyKey1, value1 ), - Property.intProperty( propertyKey2, value2 ) ); + Property.stringProperty( propertyId1, value1 ), + Property.intProperty( propertyId2, value2 ) ); assertEquals( expected, Iterables.single( indexUpdates ) ); } @@ -374,23 +376,21 @@ public void shouldConvertMixedLabelRemovalAndRemovePropertyToNodePropertyUpdates NeoStores neoStores = neoStoresRule.open(); long nodeId = 0; TransactionRecordState recordState = newTransactionRecordState( neoStores ); - int propertyKey1 = 1, propertyKey2 = 2, labelId1 = 3, labelId2 = 4; recordState.nodeCreate( nodeId ); - DefinedProperty property1 = recordState.nodeAddProperty( nodeId, propertyKey1, value1 ); - DefinedProperty property2 = recordState.nodeAddProperty( nodeId, propertyKey2, value2 ); - recordState.addLabelToNode( labelId1, nodeId ); - recordState.addLabelToNode( labelId2, nodeId ); + DefinedProperty property1 = recordState.nodeAddProperty( nodeId, propertyId1, value1 ); + DefinedProperty property2 = recordState.nodeAddProperty( nodeId, propertyId2, value2 ); + addLabelsToNode( recordState, nodeId, bothLabelIds ); apply( neoStores, recordState ); // WHEN recordState = newTransactionRecordState( neoStores ); recordState.nodeRemoveProperty( nodeId, property1.propertyKeyId() ); - recordState.removeLabelFromNode( labelId2, nodeId ); + removeLabelsFromNode( recordState, nodeId, secondLabelId ); Iterable indexUpdates = indexUpdatesOf( neoStores, recordState ); // THEN NodeUpdates expected = - NodeUpdates.forNode( nodeId, new long[]{labelId1, labelId2}, new long[]{labelId1} ) + NodeUpdates.forNode( nodeId, bothLabelIds, oneLabelId ) .removed( property1.propertyKeyId(), property1.value() ) .buildWithExistingProperties( property2 ); assertEquals( expected, Iterables.single( indexUpdates ) ); @@ -403,22 +403,20 @@ public void shouldConvertMixedLabelRemovalAndAddPropertyToNodePropertyUpdates() NeoStores neoStores = neoStoresRule.open(); long nodeId = 0; TransactionRecordState recordState = newTransactionRecordState( neoStores ); - int propertyKey1 = 1, propertyKey2 = 2, labelId1 = 3, labelId2 = 4; recordState.nodeCreate( nodeId ); - DefinedProperty property1 = recordState.nodeAddProperty( nodeId, propertyKey1, value1 ); - recordState.addLabelToNode( labelId1, nodeId ); - recordState.addLabelToNode( labelId2, nodeId ); + DefinedProperty property1 = recordState.nodeAddProperty( nodeId, propertyId1, value1 ); + addLabelsToNode( recordState, nodeId, bothLabelIds ); apply( neoStores, recordState ); // WHEN recordState = newTransactionRecordState( neoStores ); - DefinedProperty property2 = recordState.nodeAddProperty( nodeId, propertyKey2, value2 ); - recordState.removeLabelFromNode( labelId2, nodeId ); + DefinedProperty property2 = recordState.nodeAddProperty( nodeId, propertyId2, value2 ); + removeLabelsFromNode( recordState, nodeId, secondLabelId ); Iterable indexUpdates = indexUpdatesOf( neoStores, recordState ); // THEN NodeUpdates expected = - NodeUpdates.forNode( nodeId, new long[]{labelId1, labelId2}, new long[]{labelId1} ) + NodeUpdates.forNode( nodeId, bothLabelIds, oneLabelId ) .added( property2.propertyKeyId(), property2.value() ) .buildWithExistingProperties( property1, property2 ); assertEquals( expected, Iterables.single( indexUpdates ) ); @@ -431,10 +429,9 @@ public void shouldConvertChangedPropertyToNodePropertyUpdates() throws Exception NeoStores neoStores = neoStoresRule.open(); int nodeId = 0; TransactionRecordState recordState = newTransactionRecordState( neoStores ); - int propertyKey1 = 1, propertyKey2 = 2; recordState.nodeCreate( nodeId ); - DefinedProperty property1 = recordState.nodeAddProperty( nodeId, propertyKey1, value1 ); - DefinedProperty property2 = recordState.nodeAddProperty( nodeId, propertyKey2, value2 ); + DefinedProperty property1 = recordState.nodeAddProperty( nodeId, propertyId1, value1 ); + DefinedProperty property2 = recordState.nodeAddProperty( nodeId, propertyId2, value2 ); apply( neoStores, transactionRepresentationOf( recordState ) ); // WHEN @@ -460,12 +457,10 @@ public void shouldConvertRemovedPropertyToNodePropertyUpdates() throws Exception NeoStores neoStores = neoStoresRule.open(); int nodeId = 0; TransactionRecordState recordState = newTransactionRecordState( neoStores ); - int propertyKey1 = 1, propertyKey2 = 2; - int labelId = 3; recordState.nodeCreate( nodeId ); - recordState.addLabelToNode( labelId, nodeId ); - DefinedProperty property1 = recordState.nodeAddProperty( nodeId, propertyKey1, value1 ); - DefinedProperty property2 = recordState.nodeAddProperty( nodeId, propertyKey2, value2 ); + addLabelsToNode( recordState, nodeId, oneLabelId ); + DefinedProperty property1 = recordState.nodeAddProperty( nodeId, propertyId1, value1 ); + DefinedProperty property2 = recordState.nodeAddProperty( nodeId, propertyId2, value2 ); apply( neoStores, transactionRepresentationOf( recordState ) ); // WHEN @@ -476,7 +471,7 @@ public void shouldConvertRemovedPropertyToNodePropertyUpdates() throws Exception // THEN NodeUpdates expected = - NodeUpdates.forNode( nodeId, new long[]{labelId} ) + NodeUpdates.forNode( nodeId, oneLabelId ) .removed( property1.propertyKeyId(), property1.value() ) .removed( property2.propertyKeyId(), property2.value() ) .build(); @@ -727,21 +722,19 @@ public void shouldConvertAddedPropertyToNodePropertyUpdates() throws Exception NeoStores neoStores = neoStoresRule.open(); long nodeId = 0; TransactionRecordState recordState = newTransactionRecordState( neoStores ); - int labelId = 3; - int propertyKey1 = 1, propertyKey2 = 2; // WHEN recordState.nodeCreate( nodeId ); - recordState.addLabelToNode( labelId, nodeId ); - recordState.nodeAddProperty( nodeId, propertyKey1, value1 ); - recordState.nodeAddProperty( nodeId, propertyKey2, value2 ); + addLabelsToNode( recordState, nodeId, oneLabelId ); + recordState.nodeAddProperty( nodeId, propertyId1, value1 ); + recordState.nodeAddProperty( nodeId, propertyId2, value2 ); Iterable updates = indexUpdatesOf( neoStores, recordState ); // THEN NodeUpdates expected = - NodeUpdates.forNode( nodeId, new long[0], new long[]{labelId} ) - .added( propertyKey1, value1 ) - .added( propertyKey2, value2 ).build(); + NodeUpdates.forNode( nodeId, noLabels, oneLabelId ) + .added( propertyId1, value1 ) + .added( propertyId2, value2 ).build(); assertEquals( expected, Iterables.single( updates ) ); } @@ -1167,6 +1160,22 @@ else if ( command instanceof RelationshipGroupCommand ) assertEquals( 1, groups ); } + private void addLabelsToNode( TransactionRecordState recordState, long nodeId, long[] labelIds ) + { + for ( long labelId : labelIds ) + { + recordState.addLabelToNode( (int)labelId, nodeId ); + } + } + + private void removeLabelsFromNode( TransactionRecordState recordState, long nodeId, long[] labelIds ) + { + for ( long labelId : labelIds ) + { + recordState.removeLabelFromNode( (int)labelId, nodeId ); + } + } + private long[] createRelationships( NeoStores neoStores, TransactionRecordState tx, long nodeId, int type, Direction direction, int count ) { diff --git a/community/kernel/src/test/java/org/neo4j/unsafe/batchinsert/internal/BatchInsertTest.java b/community/kernel/src/test/java/org/neo4j/unsafe/batchinsert/internal/BatchInsertTest.java index 5f8b0e40817d9..49f7a2059def9 100644 --- a/community/kernel/src/test/java/org/neo4j/unsafe/batchinsert/internal/BatchInsertTest.java +++ b/community/kernel/src/test/java/org/neo4j/unsafe/batchinsert/internal/BatchInsertTest.java @@ -19,6 +19,7 @@ */ package org.neo4j.unsafe.batchinsert.internal; +import org.hamcrest.Matchers; import org.junit.After; import org.junit.AfterClass; import org.junit.BeforeClass; @@ -70,6 +71,8 @@ import org.neo4j.kernel.api.labelscan.LabelScanStore; import org.neo4j.kernel.api.labelscan.LabelScanWriter; import org.neo4j.kernel.api.labelscan.NodeLabelUpdate; +import org.neo4j.kernel.api.schema_new.index.NewIndexDescriptor; +import org.neo4j.kernel.api.schema_new.index.NewIndexDescriptorFactory; import org.neo4j.kernel.extension.KernelExtensionFactory; import org.neo4j.kernel.impl.MyRelTypes; import org.neo4j.kernel.impl.api.index.inmemory.InMemoryIndexProviderFactory; @@ -105,6 +108,7 @@ import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.Matchers.arrayContaining; +import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.emptyArray; import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertArrayEquals; @@ -1000,13 +1004,16 @@ public void shouldRepopulatePreexistingIndexed() throws Throwable inserter.shutdown(); // THEN + // This is the assumed internal index descriptor based on knowledge of what ids get assigned + NewIndexDescriptor internalIndex = NewIndexDescriptorFactory.forLabel( 0, 0 ); + verify( provider ).init(); verify( provider ).start(); verify( provider ).getPopulator( anyLong(), any( IndexDescriptor.class ), any( IndexConfiguration.class ), any( IndexSamplingConfig.class ) ); verify( populator ).create(); - verify( populator ).add( singletonList( IndexEntryUpdate.add( jakewins, any(), "Jakewins" ) ) ); - verify( populator ).add( singletonList( IndexEntryUpdate.add( boggle, any(), "b0ggl3" ) ) ); + verify( populator ).add( singletonList( IndexEntryUpdate.add( jakewins, internalIndex, "Jakewins" ) ) ); + verify( populator ).add( singletonList( IndexEntryUpdate.add( boggle, internalIndex, "b0ggl3" ) ) ); verify( populator ).verifyDeferredConstraints( any( PropertyAccessor.class ) ); verify( populator ).close( true ); verify( provider ).stop(); diff --git a/community/neo4j/src/test/java/schema/MultiIndexPopulationConcurrentUpdatesIT.java b/community/neo4j/src/test/java/schema/MultiIndexPopulationConcurrentUpdatesIT.java index ad42d5fb5e7bf..0dabc911f7bd1 100644 --- a/community/neo4j/src/test/java/schema/MultiIndexPopulationConcurrentUpdatesIT.java +++ b/community/neo4j/src/test/java/schema/MultiIndexPopulationConcurrentUpdatesIT.java @@ -127,9 +127,9 @@ public void setUp() public void applyConcurrentDeletesToPopulatedIndex() throws Throwable { List updates = new ArrayList<>( 2 ); - updates.add( NodeUpdates.forNode( 0, new long[]{labelsNameIdMap.get( COUNTRY_LABEL )} ) + updates.add( NodeUpdates.forNode( 0, id( COUNTRY_LABEL ) ) .removed( propertyId, "Sweden" ).build() ); - updates.add( NodeUpdates.forNode( 3, new long[]{labelsNameIdMap.get( COLOR_LABEL )} ) + updates.add( NodeUpdates.forNode( 3, id( COLOR_LABEL ) ) .removed( propertyId, "green" ).build() ); launchCustomIndexPopulation( labelsNameIdMap, propertyId, updates ); @@ -157,9 +157,9 @@ public void applyConcurrentDeletesToPopulatedIndex() throws Throwable public void applyConcurrentAddsToPopulatedIndex() throws Throwable { List updates = new ArrayList<>( 2 ); - updates.add( NodeUpdates.forNode( 6, new long[]{labelsNameIdMap.get( COUNTRY_LABEL )} ) + updates.add( NodeUpdates.forNode( 6, id( COUNTRY_LABEL ) ) .added( propertyId, "Denmark" ).build() ); - updates.add( NodeUpdates.forNode( 7, new long[]{labelsNameIdMap.get( CAR_LABEL )} ) + updates.add( NodeUpdates.forNode( 7, id( CAR_LABEL ) ) .added( propertyId, "BMW" ).build() ); launchCustomIndexPopulation( labelsNameIdMap, propertyId, updates ); @@ -185,9 +185,9 @@ public void applyConcurrentAddsToPopulatedIndex() throws Throwable public void applyConcurrentChangesToPopulatedIndex() throws Exception { List updates = new ArrayList<>( 2 ); - updates.add( NodeUpdates.forNode( 3, new long[]{labelsNameIdMap.get( COLOR_LABEL )} ) + updates.add( NodeUpdates.forNode( 3, id( COLOR_LABEL ) ) .changed( propertyId, "green", "pink" ).build() ); - updates.add( NodeUpdates.forNode( 5, new long[]{labelsNameIdMap.get( CAR_LABEL )} ) + updates.add( NodeUpdates.forNode( 5, id( CAR_LABEL ) ) .changed( propertyId, "Ford", "SAAB" ).build() ); launchCustomIndexPopulation( labelsNameIdMap, propertyId, updates ); @@ -213,6 +213,11 @@ public void applyConcurrentChangesToPopulatedIndex() throws Exception } } + private long[] id( String label ) + { + return new long[]{labelsNameIdMap.get( label )}; + } + private IndexReader getIndexReader( int propertyId, Integer countryLabelId ) throws IndexNotFoundKernelException { return indexService.getIndexProxy( IndexDescriptorFactory.of( countryLabelId, propertyId ) ).newReader();