From f0e680500ae4edae4933146dc90fa9e10d1a776e Mon Sep 17 00:00:00 2001 From: Anton Persson Date: Tue, 28 Jun 2016 14:30:22 +0200 Subject: [PATCH] Clean up around TransactionTerminatedException --- .../TransactionTerminatedException.java | 21 ++++++--- .../org/neo4j/kernel/TopLevelTransaction.java | 31 ++++++++----- .../neo4j/kernel/api/exceptions/Status.java | 5 ++- .../kernel/impl/api/KernelStatement.java | 5 +-- .../api/KernelTransactionImplementation.java | 3 +- .../impl/api/TransactionTermination.java | 44 ------------------- .../core/ThreadToStatementContextBridge.java | 5 +-- .../locking/LockClientStoppedException.java | 5 +-- .../graphdb/GraphDatabaseShutdownTest.java | 2 +- .../neo4j/kernel/TopLevelTransactionTest.java | 6 +-- .../impl/api/integrationtest/KernelIT.java | 15 ++++--- 11 files changed, 58 insertions(+), 84 deletions(-) delete mode 100644 community/kernel/src/main/java/org/neo4j/kernel/impl/api/TransactionTermination.java diff --git a/community/kernel/src/main/java/org/neo4j/graphdb/TransactionTerminatedException.java b/community/kernel/src/main/java/org/neo4j/graphdb/TransactionTerminatedException.java index e3f030517e28e..a097d89b15ff9 100644 --- a/community/kernel/src/main/java/org/neo4j/graphdb/TransactionTerminatedException.java +++ b/community/kernel/src/main/java/org/neo4j/graphdb/TransactionTerminatedException.java @@ -19,21 +19,30 @@ */ package org.neo4j.graphdb; -import static java.util.Objects.requireNonNull; +import org.neo4j.kernel.api.exceptions.Status; /** * Signals that the transaction within which the failed operations ran * has been terminated with {@link Transaction#terminate()}. */ -public class TransactionTerminatedException extends TransactionFailureException +public class TransactionTerminatedException extends TransactionFailureException implements Status.HasStatus { - public TransactionTerminatedException() + private final Status status; + + public TransactionTerminatedException( Status status ) + { + this( status, "" ); + } + + protected TransactionTerminatedException( Status status, String additionalInfo ) { - this( "" ); + super( "The transaction has been terminated. " + status.code().description() + " " + additionalInfo ); + this.status = status; } - public TransactionTerminatedException( String info ) + @Override + public Status status() { - super( "The transaction has been terminated. " + requireNonNull( info ) ); + return status; } } 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 c93be052f2bfe..d2f84dd970fe2 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/TopLevelTransaction.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/TopLevelTransaction.java @@ -24,6 +24,7 @@ import org.neo4j.graphdb.PropertyContainer; import org.neo4j.graphdb.Transaction; import org.neo4j.graphdb.TransactionFailureException; +import org.neo4j.graphdb.TransactionTerminatedException; import org.neo4j.graphdb.TransientFailureException; import org.neo4j.graphdb.TransientTransactionFailureException; import org.neo4j.kernel.api.KernelTransaction; @@ -86,7 +87,7 @@ public final void finish() @Override public final void terminate() { - this.transaction.markForTermination( Status.Transaction.MarkedAsFailed ); + this.transaction.markForTermination( Status.Transaction.Terminated ); } @Override @@ -110,23 +111,29 @@ public void close() { throw new ConstraintViolationException( e.getMessage(), e ); } - catch ( Exception e ) + catch ( KernelException | TransactionTerminatedException e ) { - String userMessage = successCalled - ? "Transaction was marked as successful, but unable to commit transaction so rolled back." - : "Unable to rollback transaction"; - if ( e instanceof KernelException ) + Code statusCode = e.status().code(); + if ( statusCode.classification() == Classification.TransientError ) { - Code statusCode = ((KernelException) e).status().code(); - if ( statusCode.classification() == Classification.TransientError ) - { - throw new TransientTransactionFailureException( userMessage + ": " + statusCode.description(), e ); - } + throw new TransientTransactionFailureException( + closeFailureMessage() + ": " + statusCode.description(), e ); } - throw new TransactionFailureException( userMessage, e ); + throw new TransactionFailureException( closeFailureMessage(), e ); + } + catch ( Exception e ) + { + throw new TransactionFailureException( closeFailureMessage(), e ); } } + private String closeFailureMessage() + { + return successCalled + ? "Transaction was marked as successful, but unable to commit transaction so rolled back." + : "Unable to rollback transaction"; + } + @Override public Lock acquireWriteLock( PropertyContainer entity ) { diff --git a/community/kernel/src/main/java/org/neo4j/kernel/api/exceptions/Status.java b/community/kernel/src/main/java/org/neo4j/kernel/api/exceptions/Status.java index e5901d472925d..e9380bfe7ac42 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/api/exceptions/Status.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/api/exceptions/Status.java @@ -24,7 +24,6 @@ import java.util.Collections; import static java.lang.String.format; - import static org.neo4j.kernel.api.exceptions.Status.Classification.ClientError; import static org.neo4j.kernel.api.exceptions.Status.Classification.ClientNotification; import static org.neo4j.kernel.api.exceptions.Status.Classification.DatabaseError; @@ -123,7 +122,9 @@ enum Transaction implements Status " and so this transaction was rolled back although it may have looked like it was going to be " + "committed" ), Outdated( TransientError, "Transaction has seen state which has been invalidated by applied updates while " + - "transaction was active. Transaction should succeed if retried" ) + "transaction was active. Transaction should succeed if retried." ), + LockClientStopped( TransientError, "Lock client is stopped, no more locks can be acquired." ), + Terminated( TransientError, "Explicitly terminated by the user." ) ; diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelStatement.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelStatement.java index 85daa48d942a0..7e4d5edda33ae 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelStatement.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelStatement.java @@ -20,6 +20,7 @@ package org.neo4j.kernel.impl.api; import org.neo4j.graphdb.NotInTransactionException; +import org.neo4j.graphdb.TransactionTerminatedException; import org.neo4j.kernel.api.DataWriteOperations; import org.neo4j.kernel.api.ReadOperations; import org.neo4j.kernel.api.SchemaWriteOperations; @@ -38,8 +39,6 @@ import org.neo4j.kernel.impl.api.store.StoreStatement; import org.neo4j.kernel.impl.locking.Locks; -import static org.neo4j.kernel.impl.api.TransactionTermination.throwCorrectExceptionBasedOnTerminationReason; - public class KernelStatement implements TxStateHolder, Statement { protected final Locks.Client locks; @@ -132,7 +131,7 @@ void assertOpen() Status terminationReason = transaction.shouldBeTerminated(); if ( terminationReason != null ) { - throwCorrectExceptionBasedOnTerminationReason( terminationReason ); + throw new TransactionTerminatedException( terminationReason ); } } 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 5fc7b3157b297..797bf632885ab 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 @@ -544,8 +544,7 @@ private void failOnNonExplicitRollbackIfNeeded() throws TransactionFailureExcept { if ( success && terminated ) { - String terminationMessage = terminationReason == null ? "" : terminationReason.code().description(); - throw new TransactionTerminatedException( terminationMessage ); + throw new TransactionTerminatedException( terminationReason ); } if ( success ) { diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/TransactionTermination.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/TransactionTermination.java deleted file mode 100644 index 572dcd4822c87..0000000000000 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/TransactionTermination.java +++ /dev/null @@ -1,44 +0,0 @@ -/* - * Copyright (c) 2002-2016 "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.kernel.impl.api; - -import org.neo4j.graphdb.TransactionTerminatedException; -import org.neo4j.graphdb.TransientTransactionFailureException; -import org.neo4j.kernel.api.exceptions.Status; - -public class TransactionTermination -{ - /** - * Throws different user facing exception depending on {@code reason} of transaction termination. - * - * @param reason the {@link Status} given as reason for terminating a transaction. - */ - public static void throwCorrectExceptionBasedOnTerminationReason( Status reason ) - { - if ( reason != null ) - { - if ( reason.code().classification() == Status.Classification.TransientError ) - { - throw new TransientTransactionFailureException( reason.code().description() ); - } - throw new TransactionTerminatedException(); - } - } -} diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/core/ThreadToStatementContextBridge.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/core/ThreadToStatementContextBridge.java index 7fd81602bc2ed..ba9a95fca00e8 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/core/ThreadToStatementContextBridge.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/core/ThreadToStatementContextBridge.java @@ -22,14 +22,13 @@ import org.neo4j.function.Supplier; import org.neo4j.graphdb.DatabaseShutdownException; import org.neo4j.graphdb.NotInTransactionException; +import org.neo4j.graphdb.TransactionTerminatedException; import org.neo4j.kernel.TopLevelTransaction; import org.neo4j.kernel.api.KernelTransaction; import org.neo4j.kernel.api.Statement; import org.neo4j.kernel.api.exceptions.Status; import org.neo4j.kernel.lifecycle.LifecycleAdapter; -import static org.neo4j.kernel.impl.api.TransactionTermination.throwCorrectExceptionBasedOnTerminationReason; - /** * This is meant to serve as the bridge that makes the Beans API tie transactions to threads. The Beans API * will use this to get the appropriate {@link Statement} when it performs operations. @@ -75,7 +74,7 @@ private void assertInUnterminatedTransaction( TopLevelTransaction transaction ) Status terminationReason = transaction.getTransaction().shouldBeTerminated(); if ( terminationReason != null ) { - throwCorrectExceptionBasedOnTerminationReason( terminationReason ); + throw new TransactionTerminatedException( terminationReason ); } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/LockClientStoppedException.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/LockClientStoppedException.java index 505074f7c5558..abe00b155e8a5 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/LockClientStoppedException.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/LockClientStoppedException.java @@ -20,8 +20,7 @@ package org.neo4j.kernel.impl.locking; import org.neo4j.graphdb.TransactionTerminatedException; - -import static java.util.Objects.requireNonNull; +import org.neo4j.kernel.api.exceptions.Status; /** * Exception thrown when stopped {@link Locks.Client} used to acquire locks. @@ -30,6 +29,6 @@ public class LockClientStoppedException extends TransactionTerminatedException { public LockClientStoppedException( Locks.Client client ) { - super( requireNonNull( client ) + " is stopped" ); + super( Status.Transaction.LockClientStopped, String.valueOf( client ) ); } } diff --git a/community/kernel/src/test/java/org/neo4j/graphdb/GraphDatabaseShutdownTest.java b/community/kernel/src/test/java/org/neo4j/graphdb/GraphDatabaseShutdownTest.java index a43e4219d3d08..0ad4a3f855681 100644 --- a/community/kernel/src/test/java/org/neo4j/graphdb/GraphDatabaseShutdownTest.java +++ b/community/kernel/src/test/java/org/neo4j/graphdb/GraphDatabaseShutdownTest.java @@ -133,7 +133,7 @@ public Void call() throws Exception } catch ( Exception e ) { - assertThat( rootCause( e ), instanceOf( TransientTransactionFailureException.class ) ); + assertThat( rootCause( e ), instanceOf( TransactionTerminatedException.class ) ); } } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/TopLevelTransactionTest.java b/community/kernel/src/test/java/org/neo4j/kernel/TopLevelTransactionTest.java index 192b32088b0f5..5f7f7f443d728 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/TopLevelTransactionTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/TopLevelTransactionTest.java @@ -109,11 +109,11 @@ public void shouldLetThroughTransientFailureException() throws Exception } @Test - public void shouldLetThroughTransactionTerminatedException() throws Exception + public void shouldShowTransactionTerminatedExceptionAsTransient() throws Exception { KernelTransaction kernelTransaction = mock( KernelTransaction.class ); doReturn( true ).when( kernelTransaction ).isOpen(); - RuntimeException error = new TransactionTerminatedException(); + RuntimeException error = new TransactionTerminatedException( Status.Transaction.Terminated ); doThrow( error ).when( kernelTransaction ).close(); ThreadToStatementContextBridge bridge = new ThreadToStatementContextBridge(); TopLevelTransaction transaction = new TopLevelTransaction( kernelTransaction, bridge ); @@ -126,7 +126,7 @@ public void shouldLetThroughTransactionTerminatedException() throws Exception } catch ( Exception e ) { - assertThat( e, instanceOf( org.neo4j.graphdb.TransactionFailureException.class ) ); + assertThat( e, instanceOf( TransientTransactionFailureException.class ) ); assertSame( error, e.getCause() ); } } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/integrationtest/KernelIT.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/integrationtest/KernelIT.java index ec0e71e70abfc..cacd6235a4938 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/integrationtest/KernelIT.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/integrationtest/KernelIT.java @@ -19,12 +19,13 @@ */ package org.neo4j.kernel.impl.api.integrationtest; +import org.junit.Assert; +import org.junit.Test; + import java.util.Collections; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; -import org.junit.Assert; -import org.junit.Test; import org.neo4j.collection.primitive.PrimitiveIntIterator; import org.neo4j.collection.primitive.PrimitiveLongIterator; import org.neo4j.function.Function; @@ -32,7 +33,7 @@ import org.neo4j.graphdb.Node; import org.neo4j.graphdb.Transaction; import org.neo4j.graphdb.TransactionFailureException; -import org.neo4j.graphdb.TransientFailureException; +import org.neo4j.graphdb.TransactionTerminatedException; import org.neo4j.kernel.api.KernelTransaction; import org.neo4j.kernel.api.SchemaWriteOperations; import org.neo4j.kernel.api.Statement; @@ -44,7 +45,11 @@ import static java.util.concurrent.TimeUnit.SECONDS; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import static org.junit.Assume.assumeThat; import static org.neo4j.graphdb.DynamicLabel.label; import static org.neo4j.helpers.collection.IteratorUtil.asSet; @@ -545,7 +550,7 @@ public void shouldKillTransactionsOnShutdown() throws Throwable tx.acquireStatement().readOperations().nodeExists( 0l ); fail("Should have been terminated."); } - catch( TransientFailureException e ) + catch( TransactionTerminatedException e ) { // Success }