From c7bf38ba6e20dfd7276509b36015f75033f9744c Mon Sep 17 00:00:00 2001 From: Pontus Melke Date: Mon, 19 Feb 2018 13:43:15 +0100 Subject: [PATCH] Support for relationshipRemoveProperty --- .../org/neo4j/internal/kernel/api/Write.java | 4 +- .../RelationshipTransactionStateTestBase.java | 244 ++++++++++++++++++ .../kernel/api/RelationshipWriteTestBase.java | 100 +++++++ .../neo4j/kernel/impl/newapi/Operations.java | 15 +- 4 files changed, 359 insertions(+), 4 deletions(-) diff --git a/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/Write.java b/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/Write.java index 855cda38b1607..3da01654dc72a 100644 --- a/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/Write.java +++ b/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/Write.java @@ -103,11 +103,11 @@ Value nodeSetProperty( long node, int propertyKey, Value value ) /** * Remove a property from a relationship - * @param node the internal relationship id + * @param relationship the internal relationship id * @param propertyKey the property key id * @return The removed value, or Values.NO_VALUE if the relationship did not have the property before */ - Value relationshipRemoveProperty( long node, int propertyKey ); + Value relationshipRemoveProperty( long relationship, int propertyKey ) throws KernelException; /** * Set a property on the graph diff --git a/community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/RelationshipTransactionStateTestBase.java b/community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/RelationshipTransactionStateTestBase.java index 421656f5421a1..f62885fb9e1b6 100644 --- a/community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/RelationshipTransactionStateTestBase.java +++ b/community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/RelationshipTransactionStateTestBase.java @@ -25,8 +25,11 @@ import java.util.List; import java.util.Map; +import org.neo4j.graphdb.Relationship; import org.neo4j.internal.kernel.api.exceptions.KernelException; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -358,6 +361,247 @@ public void shouldSeeNewRelationshipPropertyInTransaction() throws Exception } + + //start here + @Test + public void shouldSeeAddedPropertyFromExistingRelationshipWithoutPropertiesInTransaction() throws Exception + { + // Given + long relationshipId; + String propKey = "prop1"; + try ( Transaction tx = session.beginTransaction() ) + { + Write write = tx.dataWrite(); + relationshipId = write.relationshipCreate( write.nodeCreate(), + tx.tokenWrite().relationshipTypeGetOrCreateForName( "R" ), write.nodeCreate() ); + tx.success(); + } + + // When/Then + try ( Transaction tx = session.beginTransaction() ) + { + int propToken = session.token().propertyKeyGetOrCreateForName( propKey ); + assertEquals( tx.dataWrite().relationshipSetProperty( relationshipId, propToken, stringValue( "hello" ) ), NO_VALUE ); + + try ( RelationshipScanCursor relationship = cursors.allocateRelationshipScanCursor(); + PropertyCursor property = cursors.allocatePropertyCursor() ) + { + tx.dataRead().singleRelationship( relationshipId, relationship ); + assertTrue( "should access relationship", relationship.next() ); + + relationship.properties( property ); + assertTrue( property.next() ); + assertEquals( propToken, property.propertyKey() ); + assertEquals( property.propertyValue(), stringValue( "hello" ) ); + + assertFalse( "should only find one properties", property.next() ); + assertFalse( "should only find one relationship", relationship.next() ); + } + + tx.success(); + } + + try ( org.neo4j.graphdb.Transaction ignored = graphDb.beginTx() ) + { + assertThat( + graphDb.getRelationshipById( relationshipId ).getProperty( propKey ), equalTo( "hello" ) ); + } + } + + @Test + public void shouldSeeAddedPropertyFromExistingRelationshipWithPropertiesInTransaction() throws Exception + { + // Given + long relationshipId; + String propKey1 = "prop1"; + String propKey2 = "prop2"; + int propToken1; + int propToken2; + try ( Transaction tx = session.beginTransaction() ) + { + Write write = tx.dataWrite(); + relationshipId = write.relationshipCreate( write.nodeCreate(), + tx.tokenWrite().relationshipTypeGetOrCreateForName( "R" ), write.nodeCreate() ); + propToken1 = session.token().propertyKeyGetOrCreateForName( propKey1 ); + assertEquals( write.relationshipSetProperty( relationshipId, propToken1, stringValue( "hello" ) ), NO_VALUE ); + tx.success(); + } + + // When/Then + try ( Transaction tx = session.beginTransaction() ) + { + propToken2 = session.token().propertyKeyGetOrCreateForName( propKey2 ); + assertEquals( tx.dataWrite().relationshipSetProperty( relationshipId, propToken2, stringValue( "world" ) ), NO_VALUE ); + + try ( RelationshipScanCursor relationship = cursors.allocateRelationshipScanCursor(); + PropertyCursor property = cursors.allocatePropertyCursor() ) + { + tx.dataRead().singleRelationship( relationshipId, relationship ); + assertTrue( "should access relationship", relationship.next() ); + + relationship.properties( property ); + + //property 2, start with tx state + assertTrue( property.next() ); + assertEquals( propToken2, property.propertyKey() ); + assertEquals( property.propertyValue(), stringValue( "world" ) ); + + //property 1, from disk + assertTrue( property.next() ); + assertEquals( propToken1, property.propertyKey() ); + assertEquals( property.propertyValue(), stringValue( "hello" ) ); + + assertFalse( "should only find two properties", property.next() ); + assertFalse( "should only find one relationship", relationship.next() ); + } + tx.success(); + } + + try ( org.neo4j.graphdb.Transaction ignored = graphDb.beginTx() ) + { + Relationship relationship = graphDb.getRelationshipById( relationshipId ); + assertThat( relationship.getProperty( propKey1 ), equalTo( "hello" ) ); + assertThat( relationship.getProperty( propKey2 ), equalTo( "world" ) ); + } + } + + @Test + public void shouldSeeUpdatedPropertyFromExistingRelationshipWithPropertiesInTransaction() throws Exception + { + // Given + long relationshipId; + String propKey = "prop1"; + int propToken; + try ( Transaction tx = session.beginTransaction() ) + { + Write write = tx.dataWrite(); + relationshipId = write.relationshipCreate( write.nodeCreate(), + tx.tokenWrite().relationshipTypeGetOrCreateForName( "R" ), write.nodeCreate() ); + propToken = session.token().propertyKeyGetOrCreateForName( propKey ); + assertEquals( write.relationshipSetProperty( relationshipId, propToken, stringValue( "hello" ) ), NO_VALUE ); + tx.success(); + } + + // When/Then + try ( Transaction tx = session.beginTransaction() ) + { + assertEquals( tx.dataWrite().relationshipSetProperty( relationshipId, propToken, stringValue( "world" ) ), + stringValue( "hello" ) ); + try ( RelationshipScanCursor relationship = cursors.allocateRelationshipScanCursor(); + PropertyCursor property = cursors.allocatePropertyCursor() ) + { + tx.dataRead().singleRelationship( relationshipId, relationship ); + assertTrue( "should access relationship", relationship.next() ); + + relationship.properties( property ); + + assertTrue( property.next() ); + assertEquals( propToken, property.propertyKey() ); + assertEquals( property.propertyValue(), stringValue( "world" ) ); + + assertFalse( "should only find one property", property.next() ); + assertFalse( "should only find one relationship", relationship.next() ); + } + + tx.success(); + } + + try ( org.neo4j.graphdb.Transaction ignored = graphDb.beginTx() ) + { + assertThat( + graphDb.getRelationshipById( relationshipId ).getProperty( propKey ), equalTo( "world" ) ); + } + } + + @Test + public void shouldSeeRemovedPropertyInTransaction() throws Exception + { + // Given + long relationshipId; + String propKey = "prop1"; + int propToken; + try ( Transaction tx = session.beginTransaction() ) + { + Write write = tx.dataWrite(); + relationshipId = write.relationshipCreate( write.nodeCreate(), + tx.tokenWrite().relationshipTypeGetOrCreateForName( "R" ), write.nodeCreate() ); + propToken = session.token().propertyKeyGetOrCreateForName( propKey ); + assertEquals( write.relationshipSetProperty( relationshipId, propToken, stringValue( "hello" ) ), NO_VALUE ); + tx.success(); + } + + // When/Then + try ( Transaction tx = session.beginTransaction() ) + { + assertEquals( tx.dataWrite().relationshipRemoveProperty( relationshipId, propToken ), stringValue( "hello" ) ); + try ( RelationshipScanCursor relationship = cursors.allocateRelationshipScanCursor(); + PropertyCursor property = cursors.allocatePropertyCursor() ) + { + tx.dataRead().singleRelationship( relationshipId, relationship ); + assertTrue( "should access relationship", relationship.next() ); + + relationship.properties( property ); + assertFalse( "should not find any properties", property.next() ); + assertFalse( "should only find one relationship", relationship.next() ); + } + + tx.success(); + } + + try ( org.neo4j.graphdb.Transaction ignored = graphDb.beginTx() ) + { + assertFalse( + graphDb.getRelationshipById( relationshipId ).hasProperty( propKey ) ); + } + } + + @Test + public void shouldSeeRemovedThenAddedPropertyInTransaction() throws Exception + { + // Given + long relationshipId; + String propKey = "prop1"; + int propToken; + try ( Transaction tx = session.beginTransaction() ) + { + Write write = tx.dataWrite(); + relationshipId = write.relationshipCreate( write.nodeCreate(), + tx.tokenWrite().relationshipTypeGetOrCreateForName( "R" ), write.nodeCreate() ); + propToken = session.token().propertyKeyGetOrCreateForName( propKey ); + assertEquals( write.relationshipSetProperty( relationshipId, propToken, stringValue( "hello" ) ), NO_VALUE ); + tx.success(); + } + + // When/Then + try ( Transaction tx = session.beginTransaction() ) + { + assertEquals( tx.dataWrite().relationshipRemoveProperty( relationshipId, propToken ), stringValue( "hello" ) ); + assertEquals( tx.dataWrite().relationshipSetProperty( relationshipId, propToken, stringValue( "world" ) ), NO_VALUE ); + try ( RelationshipScanCursor relationship = cursors.allocateRelationshipScanCursor(); + PropertyCursor property = cursors.allocatePropertyCursor() ) + { + tx.dataRead().singleRelationship( relationshipId, relationship ); + assertTrue( "should access relationship", relationship.next() ); + + relationship.properties( property ); + assertTrue( property.next() ); + assertEquals( propToken, property.propertyKey() ); + assertEquals( property.propertyValue(), stringValue( "world" ) ); + + assertFalse( "should not find any properties", property.next() ); + assertFalse( "should only find one relationship", relationship.next() ); + } + + tx.success(); + } + + try ( org.neo4j.graphdb.Transaction ignored = graphDb.beginTx() ) + { + assertThat( + graphDb.getRelationshipById( relationshipId ).getProperty( propKey ), equalTo( "world" ) ); + } + } + private void traverseWithoutGroups( RelationshipTestSupport.StartNode start, boolean detached ) throws Exception { try ( Transaction tx = session.beginTransaction() ) diff --git a/community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/RelationshipWriteTestBase.java b/community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/RelationshipWriteTestBase.java index 9ff8bb4d467df..5426485cb6e93 100644 --- a/community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/RelationshipWriteTestBase.java +++ b/community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/RelationshipWriteTestBase.java @@ -253,6 +253,106 @@ public void shouldUpdatePropertyToRelationship() throws Exception } } + @Test + public void shouldRemovePropertyFromRelationship() throws Exception + { + // Given + long relationshipId; + String propertyKey = "prop"; + try ( org.neo4j.graphdb.Transaction tx = graphDb.beginTx() ) + { + Node node1 = graphDb.createNode(); + Node node2 = graphDb.createNode(); + + Relationship proxy = node1.createRelationshipTo( node2, RelationshipType.withName( "R" ) ); + relationshipId = proxy.getId(); + proxy.setProperty( propertyKey, 42 ); + tx.success(); + } + + // When + try ( Transaction tx = session.beginTransaction() ) + { + int token = session.token().propertyKeyGetOrCreateForName( propertyKey ); + assertThat( tx.dataWrite().relationshipRemoveProperty( relationshipId, token ), + equalTo( intValue( 42 ) ) ); + tx.success(); + } + + // Then + try ( org.neo4j.graphdb.Transaction ignore = graphDb.beginTx() ) + { + assertFalse( graphDb.getRelationshipById( relationshipId ).hasProperty( "prop" ) ); + } + } + + @Test + public void shouldRemoveNonExistingPropertyFromRelationship() throws Exception + { + // Given + long relationshipId; + String propertyKey = "prop"; + try ( org.neo4j.graphdb.Transaction tx = graphDb.beginTx() ) + { + Node node1 = graphDb.createNode(); + Node node2 = graphDb.createNode(); + + Relationship proxy = node1.createRelationshipTo( node2, RelationshipType.withName( "R" ) ); + relationshipId = proxy.getId(); + tx.success(); + } + // When + try ( Transaction tx = session.beginTransaction() ) + { + int token = session.token().propertyKeyGetOrCreateForName( propertyKey ); + assertThat( tx.dataWrite().relationshipRemoveProperty( relationshipId, token ), + equalTo( NO_VALUE ) ); + tx.success(); + } + + // Then + try ( org.neo4j.graphdb.Transaction ignore = graphDb.beginTx() ) + { + assertFalse( graphDb.getRelationshipById( relationshipId ).hasProperty( "prop" ) ); + } + } + + @Test + public void shouldRemovePropertyFromRelationshipTwice() throws Exception + { + // Given + long relationshipId; + String propertyKey = "prop"; + try ( org.neo4j.graphdb.Transaction tx = graphDb.beginTx() ) + { + Node node1 = graphDb.createNode(); + Node node2 = graphDb.createNode(); + + Relationship proxy = node1.createRelationshipTo( node2, RelationshipType.withName( "R" ) ); + relationshipId = proxy.getId(); + proxy.setProperty( propertyKey, 42 ); + tx.success(); + } + + // When + try ( Transaction tx = session.beginTransaction() ) + { + int token = session.token().propertyKeyGetOrCreateForName( propertyKey ); + assertThat( tx.dataWrite().relationshipRemoveProperty( relationshipId, token ), + equalTo( intValue( 42 ) ) ); + assertThat( tx.dataWrite().relationshipRemoveProperty( relationshipId, token ), + equalTo( NO_VALUE ) ); + tx.success(); + } + + // Then + try ( org.neo4j.graphdb.Transaction ignore = graphDb.beginTx() ) + { + assertFalse( graphDb.getRelationshipById( relationshipId ).hasProperty( "prop" ) ); + } + } + + @Test public void shouldUpdatePropertyToRelationshipInTransaction() throws Exception { 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 7a19eee4fd2e1..e15a4bc121d02 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 @@ -459,10 +459,21 @@ public Value relationshipSetProperty( long relationship, int propertyKey, Value } @Override - public Value relationshipRemoveProperty( long node, int propertyKey ) + public Value relationshipRemoveProperty( long relationship, int propertyKey ) throws KernelException { + acquireExclusiveRelationshipLock( relationship ); ktx.assertOpen(); - throw new UnsupportedOperationException(); + singleRelationship( relationship ); + Value existingValue = readRelationshipProperty( propertyKey ); + + if ( existingValue != NO_VALUE ) + { + //no existing value, we just add it + autoIndexing.relationships().propertyRemoved( this, relationship, propertyKey); + ktx.txState().relationshipDoRemoveProperty( relationship, propertyKey, existingValue); + } + + return existingValue; } @Override