From b15a8cc61d3b9ff0a6780180fda2d0125af2c170 Mon Sep 17 00:00:00 2001 From: Michael Simons Date: Wed, 12 Jan 2022 17:21:51 +0100 Subject: [PATCH] GH-2463 - Apply default transaction timeout if possible. This applies the default transaction timeout of the platform transaction manager when the transaction definition is using the default. --- .../transaction/Neo4jTransactionManager.java | 2 +- .../transaction/Neo4jTransactionUtils.java | 5 ++- .../ReactiveNeo4jTransactionManager.java | 2 +- .../Neo4jTransactionUtilsTest.java | 36 +++++++++++++++++++ 4 files changed, 42 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/springframework/data/neo4j/core/transaction/Neo4jTransactionManager.java b/src/main/java/org/springframework/data/neo4j/core/transaction/Neo4jTransactionManager.java index e8f42b690..168512942 100644 --- a/src/main/java/org/springframework/data/neo4j/core/transaction/Neo4jTransactionManager.java +++ b/src/main/java/org/springframework/data/neo4j/core/transaction/Neo4jTransactionManager.java @@ -148,7 +148,7 @@ public class Neo4jTransactionManager extends AbstractPlatformTransactionManager protected void doBegin(Object transaction, TransactionDefinition definition) throws TransactionException { Neo4jTransactionObject transactionObject = extractNeo4jTransaction(transaction); - TransactionConfig transactionConfig = Neo4jTransactionUtils.createTransactionConfigFrom(definition); + TransactionConfig transactionConfig = Neo4jTransactionUtils.createTransactionConfigFrom(definition, super.getDefaultTimeout()); boolean readOnly = definition.isReadOnly(); TransactionSynchronizationManager.setCurrentTransactionReadOnly(readOnly); diff --git a/src/main/java/org/springframework/data/neo4j/core/transaction/Neo4jTransactionUtils.java b/src/main/java/org/springframework/data/neo4j/core/transaction/Neo4jTransactionUtils.java index 8026729b0..86f03f452 100644 --- a/src/main/java/org/springframework/data/neo4j/core/transaction/Neo4jTransactionUtils.java +++ b/src/main/java/org/springframework/data/neo4j/core/transaction/Neo4jTransactionUtils.java @@ -64,9 +64,10 @@ public final class Neo4jTransactionUtils { * {@link TransactionDefinition#PROPAGATION_REQUIRED propagation required} behaviour are supported. * * @param definition The transaction definition passed to a Neo4j transaction manager + * @param defaultTxManagerTimeout Default timeout from the tx manager (if available, if not, use something negative) * @return A Neo4j native transaction configuration */ - static TransactionConfig createTransactionConfigFrom(TransactionDefinition definition) { + static TransactionConfig createTransactionConfigFrom(TransactionDefinition definition, int defaultTxManagerTimeout) { if (definition.getIsolationLevel() != TransactionDefinition.ISOLATION_DEFAULT) { throw new InvalidIsolationLevelException( @@ -83,6 +84,8 @@ public final class Neo4jTransactionUtils { TransactionConfig.Builder builder = TransactionConfig.builder(); if (definition.getTimeout() > 0) { builder = builder.withTimeout(Duration.ofSeconds(definition.getTimeout())); + } else if (defaultTxManagerTimeout > 0) { + builder = builder.withTimeout(Duration.ofSeconds(defaultTxManagerTimeout)); } return builder.build(); diff --git a/src/main/java/org/springframework/data/neo4j/core/transaction/ReactiveNeo4jTransactionManager.java b/src/main/java/org/springframework/data/neo4j/core/transaction/ReactiveNeo4jTransactionManager.java index 3678a3574..938207915 100644 --- a/src/main/java/org/springframework/data/neo4j/core/transaction/ReactiveNeo4jTransactionManager.java +++ b/src/main/java/org/springframework/data/neo4j/core/transaction/ReactiveNeo4jTransactionManager.java @@ -146,7 +146,7 @@ public class ReactiveNeo4jTransactionManager extends AbstractReactiveTransaction return Mono.defer(() -> { ReactiveNeo4jTransactionObject transactionObject = extractNeo4jTransaction(transaction); - TransactionConfig transactionConfig = Neo4jTransactionUtils.createTransactionConfigFrom(transactionDefinition); + TransactionConfig transactionConfig = Neo4jTransactionUtils.createTransactionConfigFrom(transactionDefinition, -1); boolean readOnly = transactionDefinition.isReadOnly(); transactionSynchronizationManager.setCurrentTransactionReadOnly(readOnly); diff --git a/src/test/java/org/springframework/data/neo4j/core/transaction/Neo4jTransactionUtilsTest.java b/src/test/java/org/springframework/data/neo4j/core/transaction/Neo4jTransactionUtilsTest.java index 8b6980db6..ab82f520a 100644 --- a/src/test/java/org/springframework/data/neo4j/core/transaction/Neo4jTransactionUtilsTest.java +++ b/src/test/java/org/springframework/data/neo4j/core/transaction/Neo4jTransactionUtilsTest.java @@ -15,14 +15,22 @@ */ package org.springframework.data.neo4j.core.transaction; +import static org.assertj.core.api.Assertions.assertThat; + +import java.time.Duration; + import org.assertj.core.api.Assertions; import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.junit.jupiter.MockitoExtension; import org.mockito.junit.jupiter.MockitoSettings; import org.mockito.quality.Strictness; +import org.neo4j.driver.TransactionConfig; +import org.springframework.transaction.support.DefaultTransactionDefinition; /** * @author Michael J. Simons @@ -49,4 +57,32 @@ class Neo4jTransactionUtilsTest { } } + @ParameterizedTest // GH-2463 + @ValueSource(ints = { Integer.MIN_VALUE, -1, 0, DefaultTransactionDefinition.TIMEOUT_DEFAULT }) + void shouldNotApplyNegativeOrZeroTimeOuts(int value) { + + DefaultTransactionDefinition springDef = new DefaultTransactionDefinition(); + springDef.setTimeout(DefaultTransactionDefinition.TIMEOUT_DEFAULT); + TransactionConfig driverConfig = Neo4jTransactionUtils.createTransactionConfigFrom(springDef, value); + assertThat(driverConfig.timeout()).isNull(); + } + + @ParameterizedTest // GH-2463 + @ValueSource(ints = { Integer.MIN_VALUE, -1, 0 }) + void shouldPreferTxDef(int value) { + + DefaultTransactionDefinition springDef = new DefaultTransactionDefinition(); + springDef.setTimeout(2); + TransactionConfig driverConfig = Neo4jTransactionUtils.createTransactionConfigFrom(springDef, value); + assertThat(driverConfig.timeout()).isEqualTo(Duration.ofSeconds(2)); + } + + @Test // GH-2463 + void shouldFallbackToTxManagerDefault() { + + DefaultTransactionDefinition springDef = new DefaultTransactionDefinition(); + springDef.setTimeout(DefaultTransactionDefinition.TIMEOUT_DEFAULT); + TransactionConfig driverConfig = Neo4jTransactionUtils.createTransactionConfigFrom(springDef, 3); + assertThat(driverConfig.timeout()).isEqualTo(Duration.ofSeconds(3)); + } }