diff --git a/community/bolt/src/test/java/org/neo4j/bolt/v1/runtime/integration/SessionIT.java b/community/bolt/src/test/java/org/neo4j/bolt/v1/runtime/integration/SessionIT.java index 8dd29b81ad3f2..1b41f2f795da1 100644 --- a/community/bolt/src/test/java/org/neo4j/bolt/v1/runtime/integration/SessionIT.java +++ b/community/bolt/src/test/java/org/neo4j/bolt/v1/runtime/integration/SessionIT.java @@ -240,7 +240,7 @@ public void shouldHandleImplicitCommitFailure() throws Throwable assertThat( responses.next(), success() ); // But the stop should have failed, since it implicitly triggers commit and thus triggers a failure - assertThat( discarding.next(), failedWith( Status.Transaction.ValidationFailed ) ); + assertThat( discarding.next(), failedWith( Status.Schema.ConstraintViolation ) ); } @Test diff --git a/community/cypher/acceptance/src/test/scala/org/neo4j/internal/cypher/acceptance/DeleteAcceptanceTest.scala b/community/cypher/acceptance/src/test/scala/org/neo4j/internal/cypher/acceptance/DeleteAcceptanceTest.scala index 061764375418d..25cf7fd612d71 100644 --- a/community/cypher/acceptance/src/test/scala/org/neo4j/internal/cypher/acceptance/DeleteAcceptanceTest.scala +++ b/community/cypher/acceptance/src/test/scala/org/neo4j/internal/cypher/acceptance/DeleteAcceptanceTest.scala @@ -19,7 +19,7 @@ */ package org.neo4j.internal.cypher.acceptance -import org.neo4j.cypher.{NodeStillHasRelationshipsException, NewPlannerTestSupport, ExecutionEngineFunSuite, QueryStatisticsTestSupport} +import org.neo4j.cypher._ class DeleteAcceptanceTest extends ExecutionEngineFunSuite with QueryStatisticsTestSupport with NewPlannerTestSupport { test("should be able to delete nodes") { @@ -61,7 +61,7 @@ class DeleteAcceptanceTest extends ExecutionEngineFunSuite with QueryStatisticsT relate(x, createNode()) relate(x, createNode()) - a [NodeStillHasRelationshipsException] should be thrownBy + a [ConstraintValidationException] should be thrownBy executeWithCostPlannerOnly(s"match (n:X) delete n") } diff --git a/community/cypher/cypher-compiler-3.0/src/main/scala/org/neo4j/cypher/internal/compiler/v3_0/ResultIterator.scala b/community/cypher/cypher-compiler-3.0/src/main/scala/org/neo4j/cypher/internal/compiler/v3_0/ResultIterator.scala index b00ef8be20a95..1f55847376aec 100644 --- a/community/cypher/cypher-compiler-3.0/src/main/scala/org/neo4j/cypher/internal/compiler/v3_0/ResultIterator.scala +++ b/community/cypher/cypher-compiler-3.0/src/main/scala/org/neo4j/cypher/internal/compiler/v3_0/ResultIterator.scala @@ -91,29 +91,8 @@ class ClosingIterator(inner: Iterator[collection.Map[String, Any]], close(success = true) } - def close(success: Boolean) = translateException { + def close(success: Boolean) = decoratedCypherException({ closer.close(success) - } - - private def translateException[U](f: => U): U = decoratedCypherException({ - try { - f - } catch { - case e: TransactionFailureException => - e.getCause match { - case exception: org.neo4j.kernel.api.exceptions.TransactionFailureException => - val status = exception.status() - if (status == Status.Transaction.ValidationFailed) { - exception.getMessage match { - case still_has_relationships(id) => throw new NodeStillHasRelationshipsException(id.toLong, e) - case _ => throw e - } - } - case _ => - } - - throw e - } }) private def failIfThrows[U](f: => U): U = decoratedCypherException({ diff --git a/community/cypher/cypher/src/test/scala/org/neo4j/cypher/MutatingIntegrationTest.scala b/community/cypher/cypher/src/test/scala/org/neo4j/cypher/MutatingIntegrationTest.scala index 8db13e6169ac4..21f8f3f02531c 100644 --- a/community/cypher/cypher/src/test/scala/org/neo4j/cypher/MutatingIntegrationTest.scala +++ b/community/cypher/cypher/src/test/scala/org/neo4j/cypher/MutatingIntegrationTest.scala @@ -180,7 +180,7 @@ class MutatingIntegrationTest extends ExecutionEngineFunSuite with Assertions wi relate(a, b, "HATES") relate(a, b, "LOVES") - intercept[NodeStillHasRelationshipsException](executeWithRulePlanner("match (n) where id(n) = 0 match (n)-[r:HATES]->() delete n,r")) + intercept[ConstraintValidationException](executeWithRulePlanner("match (n) where id(n) = 0 match (n)-[r:HATES]->() delete n,r")) } test("delete and return") { diff --git a/community/kernel/src/main/java/org/neo4j/kernel/TopLevelTransaction.java b/community/kernel/src/main/java/org/neo4j/kernel/TopLevelTransaction.java index b99bed494bdf6..c1134082fb675 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/TopLevelTransaction.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/TopLevelTransaction.java @@ -27,7 +27,6 @@ import org.neo4j.graphdb.TransientFailureException; import org.neo4j.graphdb.TransientTransactionFailureException; import org.neo4j.kernel.api.KernelTransaction; -import org.neo4j.kernel.api.KernelTransaction.CloseListener; import org.neo4j.kernel.api.exceptions.ConstraintViolationTransactionFailureException; import org.neo4j.kernel.api.exceptions.KernelException; import org.neo4j.kernel.api.exceptions.Status.Classification; @@ -51,14 +50,7 @@ public TopLevelTransaction( KernelTransaction transaction, { this.transaction = transaction; this.stmtProvider = stmtProvider; - this.transaction.registerCloseListener( new CloseListener() - { - @Override - public void notify( boolean success ) - { - stmtProvider.unbindTransactionFromCurrentThread(); - } - } ); + this.transaction.registerCloseListener( success -> stmtProvider.unbindTransactionFromCurrentThread() ); } @Override diff --git a/community/kernel/src/main/java/org/neo4j/kernel/api/exceptions/ConstraintViolationTransactionFailureException.java b/community/kernel/src/main/java/org/neo4j/kernel/api/exceptions/ConstraintViolationTransactionFailureException.java index 0df8b8d4b509d..f14368647e7e4 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/api/exceptions/ConstraintViolationTransactionFailureException.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/api/exceptions/ConstraintViolationTransactionFailureException.java @@ -30,4 +30,9 @@ public ConstraintViolationTransactionFailureException( String msg, KernelExcepti { super( Status.Schema.ConstraintViolation, msg, cause ); } + + public ConstraintViolationTransactionFailureException( String msg ) + { + this( msg, null ); + } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/state/IntegrityValidator.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/state/IntegrityValidator.java index 8a8f2148c7393..4f45c810c3260 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/state/IntegrityValidator.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/state/IntegrityValidator.java @@ -19,6 +19,7 @@ */ package org.neo4j.kernel.impl.transaction.state; +import org.neo4j.kernel.api.exceptions.ConstraintViolationTransactionFailureException; import org.neo4j.kernel.api.exceptions.Status; import org.neo4j.kernel.api.exceptions.TransactionFailureException; import org.neo4j.kernel.api.exceptions.index.IndexNotFoundKernelException; @@ -49,8 +50,9 @@ public void validateNodeRecord( NodeRecord record ) throws TransactionFailureExc { if ( !record.inUse() && record.getNextRel() != Record.NO_NEXT_RELATIONSHIP.intValue() ) { - throw new TransactionFailureException( Status.Transaction.ValidationFailed, - "Node record " + record + " still has relationships" ); + throw new ConstraintViolationTransactionFailureException( + "Cannot delete node<" + record.getId() + ">, because it still has relationships. " + + "To delete this node, you must first delete its relationships." ); } } diff --git a/community/kernel/src/test/java/org/neo4j/graphdb/DeleteNodeWithRelsIT.java b/community/kernel/src/test/java/org/neo4j/graphdb/DeleteNodeWithRelsIT.java new file mode 100644 index 0000000000000..7bfc7e83f1e0b --- /dev/null +++ b/community/kernel/src/test/java/org/neo4j/graphdb/DeleteNodeWithRelsIT.java @@ -0,0 +1,64 @@ +/* + * Copyright (c) 2002-2015 "Neo Technology," + * Network Engine for Objects in Lund AB [http://neotechnology.com] + * + * This file is part of Neo4j. + * + * Neo4j is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.neo4j.graphdb; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + +import org.neo4j.test.ImpermanentDatabaseRule; + +public class DeleteNodeWithRelsIT +{ + @Rule + public ImpermanentDatabaseRule db = new ImpermanentDatabaseRule(); + + @Rule + public ExpectedException exception = ExpectedException.none(); + + @Test + public void shouldGiveHelpfulExceptionWhenDeletingNodeWithRels() throws Exception + { + // Given + GraphDatabaseService db = this.db.getGraphDatabaseService(); + + Node node; + try( Transaction tx = db.beginTx() ) + { + node = db.createNode(); + node.createRelationshipTo( db.createNode(), DynamicRelationshipType.withName( "MAYOR_OF" ) ); + tx.success(); + } + + // And given a transaction deleting just the node + Transaction tx = db.beginTx(); + node.delete(); + tx.success(); + + // Expect + exception.expect( ConstraintViolationException.class ); + exception.expectMessage( "Cannot delete node<"+node.getId()+">, because it still has relationships. " + + "To delete this node, you must first delete its relationships." ); + + // When I commit + tx.close(); + } + +} diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/core/NodeTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/core/NodeTest.java index ff709f785e391..8a5a5d69620c7 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/core/NodeTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/core/NodeTest.java @@ -19,23 +19,23 @@ */ package org.neo4j.kernel.impl.core; +import org.junit.ClassRule; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + import java.lang.Thread.State; import java.util.Iterator; import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; -import org.junit.Assert; -import org.junit.ClassRule; -import org.junit.Rule; -import org.junit.Test; - +import org.neo4j.graphdb.ConstraintViolationException; import org.neo4j.graphdb.GraphDatabaseService; import org.neo4j.graphdb.Node; import org.neo4j.graphdb.NotFoundException; import org.neo4j.graphdb.PropertyContainer; import org.neo4j.graphdb.RelationshipType; import org.neo4j.graphdb.Transaction; -import org.neo4j.graphdb.TransactionFailureException; import org.neo4j.test.DatabaseRule; import org.neo4j.test.GraphTransactionRule; import org.neo4j.test.ImpermanentDatabaseRule; @@ -44,7 +44,6 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; - import static org.neo4j.helpers.Exceptions.launderedException; public class NodeTest @@ -55,31 +54,31 @@ public class NodeTest @Rule public GraphTransactionRule tx = new GraphTransactionRule( db ); + @Rule + public ExpectedException exception = ExpectedException.none(); + @Test - public void givenNodeWithRelationshipWhenDeleteNodeThenThrowExceptionOnCommit() throws Exception + public void shouldGiveHelpfulExceptionWhenDeletingNodeWithRels() throws Exception { // Given - Node node1 = getGraphDb().createNode(); - Node node2 = getGraphDb().createNode(); - node1.createRelationshipTo( node2, RelationshipType.withName( "KNOWS" ) ); + Node node; + node = db.createNode(); + Node node2 = db.createNode(); + node.createRelationshipTo( node2, RelationshipType.withName( "MAYOR_OF" ) ); tx.success(); - // When + // And given a transaction deleting just the node tx.begin(); - node1.delete(); + node.delete(); - try - { - tx.success(); + // Expect + exception.expect( ConstraintViolationException.class ); + exception.expectMessage( "Cannot delete node<"+node.getId()+">, because it still has relationships. " + + "To delete this node, you must first delete its relationships." ); - // Then - Assert.fail(); - } - catch ( TransactionFailureException e ) - { - Assert.assertNotEquals( e.getCause().getMessage().indexOf( "still has relationships" ), -1 ); - } + // When I commit + tx.success(); } @Test diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/core/TestLoopRelationships.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/core/TestLoopRelationships.java index ea5f82d78136f..278d479c650e3 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/core/TestLoopRelationships.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/core/TestLoopRelationships.java @@ -19,31 +19,37 @@ */ package org.neo4j.kernel.impl.core; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + import java.util.Arrays; import java.util.HashSet; import java.util.Iterator; import java.util.Set; -import org.junit.Test; - +import org.neo4j.graphdb.ConstraintViolationException; import org.neo4j.graphdb.Direction; +import org.neo4j.graphdb.DynamicRelationshipType; +import org.neo4j.graphdb.GraphDatabaseService; import org.neo4j.graphdb.Node; import org.neo4j.graphdb.NotFoundException; import org.neo4j.graphdb.Relationship; -import org.neo4j.graphdb.TransactionFailureException; +import org.neo4j.graphdb.Transaction; import org.neo4j.helpers.collection.PrefetchingIterator; import org.neo4j.kernel.impl.AbstractNeo4jTestCase; import static java.util.Arrays.asList; - import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; - import static org.neo4j.kernel.impl.MyRelTypes.TEST; public class TestLoopRelationships extends AbstractNeo4jTestCase { + @Rule + public ExpectedException exception = ExpectedException.none(); + @Test public void canCreateRelationshipBetweenTwoNodesWithLoopsThenDeleteOneOfTheNodesAndItsRelationships() throws Exception @@ -192,18 +198,28 @@ public void getSingleRelationshipOnNodeWithOneLoopOnly() throws Exception @Test public void cannotDeleteNodeWithLoopStillAttached() throws Exception { - Node node = getGraphDb().createNode(); - node.createRelationshipTo( node, TEST ); - newTransaction(); - node.delete(); - try + // Given + GraphDatabaseService db = getGraphDb(); + Node node; + try( Transaction tx = db.beginTx() ) { - commit(); - fail( "Shouldn't be able to delete a node which still has a loop relationship attached" ); - } - catch ( TransactionFailureException e ) - { // Good + node = db.createNode(); + node.createRelationshipTo( node, DynamicRelationshipType.withName( "MAYOR_OF" ) ); + tx.success(); } + + // And given a transaction deleting just the node + Transaction tx = newTransaction(); + node.delete(); + tx.success(); + + // Expect + exception.expect( ConstraintViolationException.class ); + exception.expectMessage( "Cannot delete node<"+node.getId()+">, because it still has relationships. " + + "To delete this node, you must first delete its relationships." ); + + // When I commit + tx.close(); } @Test diff --git a/enterprise/ha/src/test/java/org/neo4j/ha/TransactionConstraintsIT.java b/enterprise/ha/src/test/java/org/neo4j/ha/TransactionConstraintsIT.java index 9957ea7666c27..eb1e23b2989e4 100644 --- a/enterprise/ha/src/test/java/org/neo4j/ha/TransactionConstraintsIT.java +++ b/enterprise/ha/src/test/java/org/neo4j/ha/TransactionConstraintsIT.java @@ -23,10 +23,12 @@ import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import java.util.concurrent.Callable; import java.util.concurrent.Future; +import org.neo4j.graphdb.ConstraintViolationException; import org.neo4j.graphdb.GraphDatabaseService; import org.neo4j.graphdb.Label; import org.neo4j.graphdb.Lock; @@ -67,6 +69,9 @@ public class TransactionConstraintsIT public final ClusterRule clusterRule = new ClusterRule( getClass() ) .withSharedSetting( HaSettings.pull_interval, "0" ); + @Rule + public ExpectedException exception = ExpectedException.none(); + protected ClusterManager.ManagedCluster cluster; @Before @@ -147,19 +152,16 @@ public void slaveShouldNotBeAbleToProduceAnInvalidTransaction() throws Exception HighlyAvailableGraphDatabase aSlave = cluster.getAnySlave(); Node node = createMiniTree( aSlave ); - // WHEN Transaction tx = aSlave.beginTx(); - try - { - // Deleting this node isn't allowed since it still has relationships - node.delete(); - tx.success(); - } - finally - { - // THEN - assertFinishGetsTransactionFailure( tx ); - } + // Deleting this node isn't allowed since it still has relationships + node.delete(); + tx.success(); + + // EXPECT + exception.expect( ConstraintViolationException.class ); + + // WHEN + tx.close(); } @Test @@ -169,19 +171,16 @@ public void masterShouldNotBeAbleToProduceAnInvalidTransaction() throws Exceptio HighlyAvailableGraphDatabase master = cluster.getMaster(); Node node = createMiniTree( master ); - // WHEN Transaction tx = master.beginTx(); - try - { - // Deleting this node isn't allowed since it still has relationships - node.delete(); - tx.success(); - } - finally - { - // THEN - assertFinishGetsTransactionFailure( tx ); - } + // Deleting this node isn't allowed since it still has relationships + node.delete(); + tx.success(); + + // EXPECT + exception.expect( ConstraintViolationException.class ); + + // WHEN + tx.close(); } @Test