From 1b7075d8ba720ba11bbc1c9548cd7ca92938bab8 Mon Sep 17 00:00:00 2001 From: Pontus Melke Date: Mon, 19 Feb 2018 14:42:44 +0100 Subject: [PATCH] Port relationship property operations to kernel API --- .../neo4j/kernel/api/KernelTransaction.java | 3 + .../api/KernelTransactionImplementation.java | 7 + .../kernel/impl/core/RelationshipProxy.java | 276 +++++++++++------- .../neo4j/kernel/impl/newapi/Operations.java | 5 + .../builtinprocs/StubKernelTransaction.java | 8 +- .../ConstraintIndexCreatorTest.java | 7 + .../builtinprocs/StubKernelTransaction.java | 8 +- 7 files changed, 210 insertions(+), 104 deletions(-) diff --git a/community/kernel/src/main/java/org/neo4j/kernel/api/KernelTransaction.java b/community/kernel/src/main/java/org/neo4j/kernel/api/KernelTransaction.java index 73ce247d61e5f..6552b4a7a98e3 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/api/KernelTransaction.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/api/KernelTransaction.java @@ -23,6 +23,7 @@ import org.neo4j.internal.kernel.api.NodeCursor; import org.neo4j.internal.kernel.api.PropertyCursor; +import org.neo4j.internal.kernel.api.RelationshipScanCursor; import org.neo4j.internal.kernel.api.Transaction; import org.neo4j.internal.kernel.api.security.LoginContext; import org.neo4j.internal.kernel.api.security.SecurityContext; @@ -206,6 +207,8 @@ default void close() throws TransactionFailureException NodeCursor nodeCursor(); + RelationshipScanCursor relationshipCursor(); + PropertyCursor propertyCursor(); @FunctionalInterface diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelTransactionImplementation.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelTransactionImplementation.java index ff5c50d6b9d8e..9ed1a77155afc 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelTransactionImplementation.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelTransactionImplementation.java @@ -41,6 +41,7 @@ import org.neo4j.internal.kernel.api.NodeCursor; import org.neo4j.internal.kernel.api.PropertyCursor; import org.neo4j.internal.kernel.api.Read; +import org.neo4j.internal.kernel.api.RelationshipScanCursor; import org.neo4j.internal.kernel.api.SchemaRead; import org.neo4j.internal.kernel.api.SchemaWrite; import org.neo4j.internal.kernel.api.TokenRead; @@ -1072,6 +1073,12 @@ public NodeCursor nodeCursor() return operations.nodeCursor(); } + @Override + public RelationshipScanCursor relationshipCursor() + { + return operations.relationshipCursor(); + } + @Override public PropertyCursor propertyCursor() { diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/core/RelationshipProxy.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/core/RelationshipProxy.java index 13a608035772e..3464683245f11 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/core/RelationshipProxy.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/core/RelationshipProxy.java @@ -24,9 +24,8 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; -import org.neo4j.collection.primitive.PrimitiveIntIterator; -import org.neo4j.cursor.Cursor; import org.neo4j.graphdb.ConstraintViolationException; import org.neo4j.graphdb.DatabaseShutdownException; import org.neo4j.graphdb.GraphDatabaseService; @@ -35,20 +34,25 @@ import org.neo4j.graphdb.NotInTransactionException; import org.neo4j.graphdb.Relationship; import org.neo4j.graphdb.RelationshipType; +import org.neo4j.graphdb.TransactionTerminatedException; +import org.neo4j.internal.kernel.api.PropertyCursor; +import org.neo4j.internal.kernel.api.RelationshipScanCursor; +import org.neo4j.internal.kernel.api.TokenRead; import org.neo4j.internal.kernel.api.exceptions.InvalidTransactionTypeKernelException; +import org.neo4j.internal.kernel.api.exceptions.KernelException; import org.neo4j.internal.kernel.api.exceptions.PropertyKeyIdNotFoundKernelException; import org.neo4j.internal.kernel.api.exceptions.explicitindex.AutoIndexingKernelException; +import org.neo4j.internal.kernel.api.exceptions.schema.ConstraintValidationException; import org.neo4j.internal.kernel.api.exceptions.schema.IllegalTokenNameException; +import org.neo4j.kernel.api.KernelTransaction; +import org.neo4j.kernel.api.SilentTokenNameLookup; import org.neo4j.kernel.api.Statement; -import org.neo4j.kernel.api.StatementTokenNameLookup; import org.neo4j.kernel.api.exceptions.EntityNotFoundException; -import org.neo4j.kernel.api.exceptions.PropertyNotFoundException; +import org.neo4j.kernel.api.exceptions.Status; import org.neo4j.kernel.impl.api.RelationshipVisitor; import org.neo4j.kernel.impl.api.operations.KeyReadOperations; import org.neo4j.kernel.impl.store.record.AbstractBaseRecord; import org.neo4j.storageengine.api.EntityType; -import org.neo4j.storageengine.api.PropertyItem; -import org.neo4j.storageengine.api.RelationshipItem; import org.neo4j.values.storable.Value; import org.neo4j.values.storable.Values; @@ -132,19 +136,20 @@ public GraphDatabaseService getGraphDatabase() @Override public void delete() { - try ( Statement statement = actions.statement() ) + KernelTransaction transaction = safeAcquireTransaction(); + try { - statement.dataWriteOperations().relationshipDelete( getId() ); + boolean deleted = transaction.dataWrite().relationshipDelete( id ); + if ( !deleted ) + { + throw new NotFoundException( "Unable to delete Relationship[" + id + + "] since it has already been deleted." ); + } } catch ( InvalidTransactionTypeKernelException e ) { throw new ConstraintViolationException( e.getMessage(), e ); } - catch ( EntityNotFoundException e ) - { - throw new NotFoundException( "Unable to delete relationship[" + - getId() + "] since it is already deleted." ); - } catch ( AutoIndexingKernelException e ) { throw new IllegalStateException( "Auto indexing encountered a failure while deleting the relationship: " @@ -220,88 +225,103 @@ public RelationshipType getType() @Override public Iterable getPropertyKeys() { - try ( Statement statement = actions.statement() ) + KernelTransaction transaction = safeAcquireTransaction(); + List keys = new ArrayList<>(); + try { - List keys = new ArrayList<>(); - PrimitiveIntIterator properties = statement.readOperations().relationshipGetPropertyKeys( getId() ); - while ( properties.hasNext() ) + RelationshipScanCursor relationships = transaction.relationshipCursor(); + PropertyCursor properties = transaction.propertyCursor(); + singleRelationship( transaction, relationships ); + TokenRead token = transaction.tokenRead(); + relationships.properties( properties ); + while ( properties.next() ) { - keys.add( statement.readOperations().propertyKeyGetName( properties.next() ) ); + keys.add( token.propertyKeyName( properties.propertyKey() )); } - return keys; - } - catch ( EntityNotFoundException e ) - { - throw new NotFoundException( "Relationship not found", e ); } catch ( PropertyKeyIdNotFoundKernelException e ) { - throw new IllegalStateException( "Property key retrieved through kernel API should exist." ); + throw new IllegalStateException( "Property key retrieved through kernel API should exist.", e ); } + return keys; } @Override public Map getProperties( String... keys ) { - if ( keys == null ) - { - throw new NullPointerException( "keys" ); - } + Objects.requireNonNull( keys, "Properties keys should be not null array." ); if ( keys.length == 0 ) { return Collections.emptyMap(); } - try ( Statement statement = actions.statement() ) + KernelTransaction transaction = safeAcquireTransaction(); + + int itemsToReturn = keys.length; + Map properties = new HashMap<>( itemsToReturn ); + TokenRead token = transaction.tokenRead(); + + //Find ids, note we are betting on that the number of keys + //is small enough not to use a set here. + int[] propertyIds = new int[itemsToReturn]; + for ( int i = 0; i < itemsToReturn; i++ ) { - try ( Cursor relationship = statement.readOperations().relationshipCursorById( getId() ) ) + String key = keys[i]; + if ( key == null ) { - try ( Cursor propertyCursor = statement.readOperations() - .relationshipGetProperties( relationship.get() ) ) - { - return PropertyContainerProxyHelper.getProperties( statement, propertyCursor, keys ); - } + throw new NullPointerException( String.format( "Key %d was null", i ) ); } - catch ( EntityNotFoundException e ) + propertyIds[i] = token.propertyKey( key ); + } + + RelationshipScanCursor relationships = transaction.relationshipCursor(); + PropertyCursor propertyCursor = transaction.propertyCursor(); + singleRelationship( transaction, relationships ); + relationships.properties( propertyCursor ); + int propertiesToFind = itemsToReturn; + while ( propertiesToFind > 0 && propertyCursor.next() ) + { + //Do a linear check if this is a property we are interested in. + int currentKey = propertyCursor.propertyKey(); + for ( int i = 0; i < itemsToReturn; i++ ) { - throw new NotFoundException( "Relationship not found", e ); + if ( propertyIds[i] == currentKey ) + { + properties.put( keys[i], + propertyCursor.propertyValue().asObjectCopy() ); + propertiesToFind--; + break; + } } } + return properties; } @Override public Map getAllProperties() { - try ( Statement statement = actions.statement() ) - { - try ( Cursor relationship = statement.readOperations().relationshipCursorById( getId() ) ) - { - try ( Cursor propertyCursor = statement.readOperations() - .relationshipGetProperties( relationship.get() ) ) - { - Map properties = new HashMap<>(); - - // Get all properties - while ( propertyCursor.next() ) - { - String name = - statement.readOperations().propertyKeyGetName( propertyCursor.get().propertyKeyId() ); - properties.put( name, propertyCursor.get().value().asObjectCopy() ); - } + KernelTransaction transaction = safeAcquireTransaction(); + Map properties = new HashMap<>(); - return properties; - } - } - catch ( EntityNotFoundException e ) + try + { + RelationshipScanCursor relationships = transaction.relationshipCursor(); + PropertyCursor propertyCursor = transaction.propertyCursor(); + TokenRead token = transaction.tokenRead(); + singleRelationship( transaction, relationships ); + relationships.properties( propertyCursor ); + while ( propertyCursor.next() ) { - throw new NotFoundException( "Relationship not found", e ); + properties.put( token.propertyKeyName( propertyCursor.propertyKey() ), + propertyCursor.propertyValue().asObjectCopy() ); } } catch ( PropertyKeyIdNotFoundKernelException e ) { throw new IllegalStateException( "Property key retrieved through kernel API should exist.", e ); } + return properties; } @Override @@ -311,32 +331,30 @@ public Object getProperty( String key ) { throw new IllegalArgumentException( "(null) property key is not allowed" ); } + KernelTransaction transaction = safeAcquireTransaction(); + int propertyKey = transaction.tokenRead().propertyKey( key ); + if ( propertyKey == KeyReadOperations.NO_SUCH_PROPERTY_KEY ) + { + throw new NotFoundException( format( "No such property, '%s'.", key ) ); + } - try ( Statement statement = actions.statement() ) + RelationshipScanCursor relationships = transaction.relationshipCursor(); + PropertyCursor properties = transaction.propertyCursor(); + singleRelationship( transaction, relationships ); + relationships.properties( properties ); + while ( properties.next() ) { - try + if ( propertyKey == properties.propertyKey() ) { - int propertyId = statement.readOperations().propertyKeyGetForName( key ); - if ( propertyId == KeyReadOperations.NO_SUCH_PROPERTY_KEY ) - { - throw new NotFoundException( String.format( "No such property, '%s'.", key ) ); - } - - Value value = statement.readOperations().relationshipGetProperty( getId(), propertyId ); - + Value value = properties.propertyValue(); if ( value == Values.NO_VALUE ) { - throw new PropertyNotFoundException( propertyId, EntityType.RELATIONSHIP, getId() ); + throw new NotFoundException( format( "No such property, '%s'.", key ) ); } - return value.asObjectCopy(); } - catch ( EntityNotFoundException | PropertyNotFoundException e ) - { - throw new NotFoundException( - e.getUserMessage( new StatementTokenNameLookup( statement.readOperations() ) ), e ); - } } + throw new NotFoundException( format( "No such property, '%s'.", key ) ); } @Override @@ -346,17 +364,25 @@ public Object getProperty( String key, Object defaultValue ) { throw new IllegalArgumentException( "(null) property key is not allowed" ); } - - try ( Statement statement = actions.statement() ) + KernelTransaction transaction = safeAcquireTransaction(); + RelationshipScanCursor relationships = transaction.relationshipCursor(); + PropertyCursor properties = transaction.propertyCursor(); + int propertyKey = transaction.tokenRead().propertyKey( key ); + if ( propertyKey == KeyReadOperations.NO_SUCH_PROPERTY_KEY ) { - int propertyId = statement.readOperations().propertyKeyGetForName( key ); - Value value = statement.readOperations().relationshipGetProperty( getId(), propertyId ); - return value == Values.NO_VALUE ? defaultValue : value.asObjectCopy(); + return defaultValue; } - catch ( EntityNotFoundException e ) + singleRelationship( transaction, relationships ); + relationships.properties( properties ); + while ( properties.next() ) { - throw new NotFoundException( e ); + if ( propertyKey == properties.propertyKey() ) + { + Value value = properties.propertyValue(); + return value == Values.NO_VALUE ? defaultValue : value.asObjectCopy(); + } } + return defaultValue; } @Override @@ -367,26 +393,49 @@ public boolean hasProperty( String key ) return false; } - try ( Statement statement = actions.statement() ) + KernelTransaction transaction = safeAcquireTransaction(); + int propertyKey = transaction.tokenRead().propertyKey( key ); + if ( propertyKey == KeyReadOperations.NO_SUCH_PROPERTY_KEY ) { - int propertyId = statement.readOperations().propertyKeyGetForName( key ); - return propertyId != KeyReadOperations.NO_SUCH_PROPERTY_KEY && - statement.readOperations().relationshipHasProperty( getId(), propertyId ); + return false; } - catch ( EntityNotFoundException e ) + + RelationshipScanCursor relationships = transaction.relationshipCursor(); + PropertyCursor properties = transaction.propertyCursor(); + singleRelationship( transaction, relationships ); + relationships.properties( properties ); + while ( properties.next() ) { - throw new NotFoundException( e ); + if ( propertyKey == properties.propertyKey() ) + { + return true; + } } + return false; } @Override public void setProperty( String key, Object value ) { - try ( Statement statement = actions.statement() ) + KernelTransaction transaction = actions.kernelTransaction(); + int propertyKeyId; + try + { + propertyKeyId = transaction.tokenWrite().propertyKeyGetOrCreateForName( key ); + } + catch ( IllegalTokenNameException e ) + { + throw new IllegalArgumentException( format( "Invalid property key '%s'.", key ), e ); + } + + try ( Statement ignore = transaction.acquireStatement() ) + { + transaction.dataWrite().relationshipSetProperty( id, propertyKeyId, Values.of( value, false ) ); + } + catch ( ConstraintValidationException e ) { - int propertyKeyId = statement.tokenWriteOperations().propertyKeyGetOrCreateForName( key ); - statement.dataWriteOperations() - .relationshipSetProperty( getId(), propertyKeyId, Values.of( value, false ) ); + throw new ConstraintViolationException( + e.getUserMessage( new SilentTokenNameLookup( transaction.tokenRead() ) ), e ); } catch ( IllegalArgumentException e ) { @@ -398,11 +447,6 @@ public void setProperty( String key, Object value ) { throw new NotFoundException( e ); } - catch ( IllegalTokenNameException e ) - { - // TODO: Maybe throw more context-specific error than just IllegalArgument - throw new IllegalArgumentException( e ); - } catch ( InvalidTransactionTypeKernelException e ) { throw new ConstraintViolationException( e.getMessage(), e ); @@ -412,15 +456,20 @@ public void setProperty( String key, Object value ) throw new IllegalStateException( "Auto indexing encountered a failure while setting property: " + e.getMessage(), e ); } + catch ( KernelException e ) + { + throw new ConstraintViolationException( e.getMessage(), e ); + } } @Override public Object removeProperty( String key ) { - try ( Statement statement = actions.statement() ) + KernelTransaction transaction = actions.kernelTransaction(); + try ( Statement ignore = transaction.acquireStatement() ) { - int propertyId = statement.tokenWriteOperations().propertyKeyGetOrCreateForName( key ); - return statement.dataWriteOperations().relationshipRemoveProperty( getId(), propertyId ).asObjectCopy(); + int propertyKeyId = transaction.tokenWrite().propertyKeyGetOrCreateForName( key ); + return transaction.dataWrite().relationshipRemoveProperty( id, propertyKeyId ).asObjectCopy(); } catch ( EntityNotFoundException e ) { @@ -428,8 +477,7 @@ public Object removeProperty( String key ) } catch ( IllegalTokenNameException e ) { - // TODO: Maybe throw more context-specific error than just IllegalArgument - throw new IllegalArgumentException( e ); + throw new IllegalArgumentException( format( "Invalid property key '%s'.", key ), e ); } catch ( InvalidTransactionTypeKernelException e ) { @@ -440,6 +488,10 @@ public Object removeProperty( String key ) throw new IllegalStateException( "Auto indexing encountered a failure while removing property: " + e.getMessage(), e ); } + catch ( KernelException e ) + { + throw new ConstraintViolationException( e.getMessage(), e ); + } } @Override @@ -486,6 +538,26 @@ public String toString() return format( "(?)-[%s,%d]->(?)", relType, getId() ); } + private KernelTransaction safeAcquireTransaction() + { + KernelTransaction transaction = actions.kernelTransaction(); + if ( transaction.isTerminated() ) + { + Status terminationReason = transaction.getReasonIfTerminated().orElse( Status.Transaction.Terminated ); + throw new TransactionTerminatedException( terminationReason ); + } + return transaction; + } + + private void singleRelationship( KernelTransaction transaction, RelationshipScanCursor relationships ) + { + transaction.dataRead().singleRelationship( id, relationships ); + if ( !relationships.next() ) + { + throw new NotFoundException( new EntityNotFoundException( EntityType.RELATIONSHIP, id ) ); + } + } + private void assertInUnterminatedTransaction() { actions.assertInUnterminatedTransaction(); diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/Operations.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/Operations.java index 80150c43227ed..5e68f5bdff69a 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/Operations.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/Operations.java @@ -720,6 +720,11 @@ public DefaultNodeCursor nodeCursor() return nodeCursor; } + public DefaultRelationshipScanCursor relationshipCursor() + { + return relationshipCursor; + } + public DefaultPropertyCursor propertyCursor() { return propertyCursor; diff --git a/community/kernel/src/test/java/org/neo4j/kernel/builtinprocs/StubKernelTransaction.java b/community/kernel/src/test/java/org/neo4j/kernel/builtinprocs/StubKernelTransaction.java index 9388a0db7a756..2d59e205bdefe 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/builtinprocs/StubKernelTransaction.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/builtinprocs/StubKernelTransaction.java @@ -28,6 +28,7 @@ import org.neo4j.internal.kernel.api.NodeCursor; import org.neo4j.internal.kernel.api.PropertyCursor; import org.neo4j.internal.kernel.api.Read; +import org.neo4j.internal.kernel.api.RelationshipScanCursor; import org.neo4j.internal.kernel.api.SchemaRead; import org.neo4j.internal.kernel.api.SchemaWrite; import org.neo4j.internal.kernel.api.TokenRead; @@ -38,7 +39,6 @@ import org.neo4j.kernel.api.ReadOperations; import org.neo4j.kernel.api.Statement; import org.neo4j.kernel.api.exceptions.Status; -import org.neo4j.kernel.api.exceptions.TransactionFailureException; public class StubKernelTransaction implements KernelTransaction { @@ -235,6 +235,12 @@ public NodeCursor nodeCursor() throw new UnsupportedOperationException( "not implemented" ); } + @Override + public RelationshipScanCursor relationshipCursor() + { + throw new UnsupportedOperationException( "not implemented" ); + } + @Override public PropertyCursor propertyCursor() { diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/constraints/ConstraintIndexCreatorTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/constraints/ConstraintIndexCreatorTest.java index 7ede793ebf3d2..f5be8ce4f3562 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/constraints/ConstraintIndexCreatorTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/constraints/ConstraintIndexCreatorTest.java @@ -33,6 +33,7 @@ import org.neo4j.internal.kernel.api.NodeCursor; import org.neo4j.internal.kernel.api.PropertyCursor; import org.neo4j.internal.kernel.api.Read; +import org.neo4j.internal.kernel.api.RelationshipScanCursor; import org.neo4j.internal.kernel.api.SchemaRead; import org.neo4j.internal.kernel.api.SchemaWrite; import org.neo4j.internal.kernel.api.Session; @@ -578,6 +579,12 @@ public NodeCursor nodeCursor() throw new UnsupportedOperationException( "not implemented" ); } + @Override + public RelationshipScanCursor relationshipCursor() + { + throw new UnsupportedOperationException( "not implemented" ); + } + @Override public PropertyCursor propertyCursor() { diff --git a/enterprise/kernel/src/test/java/org/neo4j/kernel/enterprise/builtinprocs/StubKernelTransaction.java b/enterprise/kernel/src/test/java/org/neo4j/kernel/enterprise/builtinprocs/StubKernelTransaction.java index 514d201f3201e..9c975b89a4e32 100644 --- a/enterprise/kernel/src/test/java/org/neo4j/kernel/enterprise/builtinprocs/StubKernelTransaction.java +++ b/enterprise/kernel/src/test/java/org/neo4j/kernel/enterprise/builtinprocs/StubKernelTransaction.java @@ -30,6 +30,7 @@ import org.neo4j.internal.kernel.api.NodeCursor; import org.neo4j.internal.kernel.api.PropertyCursor; import org.neo4j.internal.kernel.api.Read; +import org.neo4j.internal.kernel.api.RelationshipScanCursor; import org.neo4j.internal.kernel.api.SchemaRead; import org.neo4j.internal.kernel.api.SchemaWrite; import org.neo4j.internal.kernel.api.TokenRead; @@ -39,7 +40,6 @@ import org.neo4j.kernel.api.KernelTransaction; import org.neo4j.kernel.api.Statement; import org.neo4j.kernel.api.exceptions.Status; -import org.neo4j.kernel.api.exceptions.TransactionFailureException; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -230,6 +230,12 @@ public NodeCursor nodeCursor() throw new UnsupportedOperationException( "not implemented" ); } + @Override + public RelationshipScanCursor relationshipCursor() + { + throw new UnsupportedOperationException( "not implemented" ); + } + @Override public PropertyCursor propertyCursor() {