diff --git a/Neo4j.Driver/Neo4j.Driver.Tests.TestBackend/TestBlackList.cs b/Neo4j.Driver/Neo4j.Driver.Tests.TestBackend/TestBlackList.cs index 28934cfcc..399e138ce 100644 --- a/Neo4j.Driver/Neo4j.Driver.Tests.TestBackend/TestBlackList.cs +++ b/Neo4j.Driver/Neo4j.Driver.Tests.TestBackend/TestBlackList.cs @@ -88,6 +88,8 @@ private static readonly (string Name, string Reason)[] BlackListNames = "driver does not report errors on RUN before consuming results"), ("tests.stub.tx_run.test_tx_run.TestTxRun.test_should_prevent_pull_after_tx_termination_on_run", "driver does not report errors on RUN before consuming results"), + ("stub.tx_run.test_tx_run.TestTxRun.test_should_prevent_commit_after_tx_termination", + "driver does not report errors on RUN before consuming results"), //TODO: ("stub.tx_run.test_tx_run.TestTxRun.test_should_prevent_pull_after_tx_termination_on_pull", diff --git a/Neo4j.Driver/Neo4j.Driver.Tests/Internal/Util/ConfigBuildersTests.cs b/Neo4j.Driver/Neo4j.Driver.Tests/Internal/Util/ConfigBuildersTests.cs index eb25218af..46bcc6237 100644 --- a/Neo4j.Driver/Neo4j.Driver.Tests/Internal/Util/ConfigBuildersTests.cs +++ b/Neo4j.Driver/Neo4j.Driver.Tests/Internal/Util/ConfigBuildersTests.cs @@ -23,30 +23,6 @@ namespace Neo4j.Driver.Internal.Util { public class ConfigBuildersTests { - public class BuildTransactionOptions - { - [Fact] - public void ShouldReturnEmptyTxOptionsWhenBuilderIsNull() - { - var options = ConfigBuilders.BuildTransactionConfig(null); - options.Should().Be(TransactionConfig.Default); - } - - [Fact] - public void ShouldReturnNewTxOptions() - { - var options1 = ConfigBuilders.BuildTransactionConfig(o => o.WithTimeout(TimeSpan.FromSeconds(5))); - var options2 = ConfigBuilders.BuildTransactionConfig(o => o.WithTimeout(TimeSpan.FromSeconds(30))); - options1.Timeout.Should().Be(TimeSpan.FromSeconds(5)); - options2.Timeout.Should().Be(TimeSpan.FromSeconds(30)); - - // When I reset to another value - options1.Timeout = TimeSpan.FromMinutes(1); - options1.Timeout.Should().Be(TimeSpan.FromMinutes(1)); - options2.Timeout.Should().Be(TimeSpan.FromSeconds(30)); - } - } - public class BuildSessionOptions { [Fact] diff --git a/Neo4j.Driver/Neo4j.Driver.Tests/Internal/Util/TransactionTimeoutTests.cs b/Neo4j.Driver/Neo4j.Driver.Tests/Internal/Util/TransactionTimeoutTests.cs new file mode 100644 index 000000000..3630ec2fb --- /dev/null +++ b/Neo4j.Driver/Neo4j.Driver.Tests/Internal/Util/TransactionTimeoutTests.cs @@ -0,0 +1,72 @@ +// Copyright (c) "Neo4j" +// Neo4j Sweden AB [http://neo4j.com] +// +// This file is part of Neo4j. +// +// Licensed under the Apache License, Version 2.0 (the "License"). +// You may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +using System; +using FluentAssertions; +using Moq; +using Moq.AutoMock; +using Xunit; + +namespace Neo4j.Driver.Internal.Util +{ + public class TransactionTimeoutTests + { + [Theory] + [InlineData(0, 0, 0)] + [InlineData(0, 1, 1)] + [InlineData(0.1, 0, 1)] + [InlineData(1, 0, 1)] + [InlineData(1, 1, 2)] + [InlineData(1.23, 0, 2)] + [InlineData(598, 1, 599)] + [InlineData(598.2, 0, 599)] + [InlineData(2000, 96, 2001)] + [InlineData(2000.8, 0, 2001)] + public void ShouldRoundSubmillisecondTimeoutsToMilliseconds( + double milliseconds, + int ticks, + int expectedMilliseconds) + { + var inputMilliseconds = TimeSpan.FromMilliseconds(milliseconds); + var inputTicks = TimeSpan.FromTicks(ticks); + var totalInput = inputMilliseconds + inputTicks; + var expectedTimeout = TimeSpan.FromMilliseconds(expectedMilliseconds); + + var autoMocker = new AutoMocker(MockBehavior.Strict); + if (expectedTimeout != inputMilliseconds) + { + // only logs if it changes the timeout + autoMocker.GetMock() + .Setup( + x => x.Info( + It.Is(s => s.Contains("rounded up")), + It.IsAny())) + .Verifiable(); + } + + var sut = + new TransactionConfigBuilder(autoMocker.GetMock().Object, TransactionConfig.Default) + .WithTimeout(totalInput); + + var result = sut.Build().Timeout; + + result.Should().Be(expectedTimeout); + (result?.Ticks % TimeSpan.TicksPerMillisecond).Should().Be(0); + autoMocker.VerifyAll(); + } + } +} diff --git a/Neo4j.Driver/Neo4j.Driver.Tests/TransactionConfigTests.cs b/Neo4j.Driver/Neo4j.Driver.Tests/TransactionConfigTests.cs index dee078f25..4cb0f4d51 100644 --- a/Neo4j.Driver/Neo4j.Driver.Tests/TransactionConfigTests.cs +++ b/Neo4j.Driver/Neo4j.Driver.Tests/TransactionConfigTests.cs @@ -36,7 +36,7 @@ public class TimeoutField { new object[] { null }, new object[] { (TimeSpan?)TimeSpan.Zero }, - new object[] { (TimeSpan?)TimeSpan.FromMilliseconds(0.1) }, + new object[] { (TimeSpan?)TimeSpan.FromMilliseconds(1) }, new object[] { (TimeSpan?)TimeSpan.FromMinutes(30) }, new object[] { (TimeSpan?)TimeSpan.MaxValue } }; @@ -53,7 +53,7 @@ public void ShouldReturnDefaultValueAsNull() [MemberData(nameof(ValidTimeSpanValues))] public void ShouldAllowToSetToNewValue(TimeSpan? input) { - var builder = TransactionConfig.Builder; + var builder = new TransactionConfigBuilder(null, TransactionConfig.Default); builder.WithTimeout(input); var config = builder.Build(); @@ -65,7 +65,7 @@ public void ShouldAllowToSetToNewValue(TimeSpan? input) [MemberData(nameof(InvalidTimeSpanValues))] public void ShouldThrowExceptionIfAssigningValueLessThanZero(TimeSpan input) { - var error = Record.Exception(() => TransactionConfig.Builder.WithTimeout(input)); + var error = Record.Exception(() => new TransactionConfigBuilder(null, TransactionConfig.Default).WithTimeout(input)); error.Should().BeOfType(); error.Message.Should().Contain("not be negative"); @@ -85,8 +85,8 @@ public void ShouldReturnDefaultValueEmptyDictionary() [Fact] public void ShouldAllowToSetToNewValue() { - var builder = TransactionConfig.Builder; - builder.WithMetadata(new Dictionary { { "key", "value" } }); + var builder = new TransactionConfigBuilder(null, TransactionConfig.Default) + .WithMetadata(new Dictionary { { "key", "value" } }); var config = builder.Build(); @@ -98,7 +98,8 @@ public void ShouldAllowToSetToNewValue() [Fact] public void ShouldThrowExceptionIfAssigningNull() { - var error = Record.Exception(() => TransactionConfig.Builder.WithMetadata(null)); + var error = Record.Exception( + () => new TransactionConfigBuilder(null, TransactionConfig.Default).WithMetadata(null)); error.Should().BeOfType(); error.Message.Should().Contain("should not be null"); diff --git a/Neo4j.Driver/Neo4j.Driver/Internal/AsyncSession.cs b/Neo4j.Driver/Neo4j.Driver/Internal/AsyncSession.cs index 3b7162a46..940fd6f61 100644 --- a/Neo4j.Driver/Neo4j.Driver/Internal/AsyncSession.cs +++ b/Neo4j.Driver/Neo4j.Driver/Internal/AsyncSession.cs @@ -193,6 +193,17 @@ public Task RunAsync( return result; } + private TransactionConfig BuildTransactionConfig(Action action) + { + if (action == null) + { + return TransactionConfig.Default; + } + var builder = new TransactionConfigBuilder(_logger, new TransactionConfig()); + action.Invoke(builder); + return builder.Build(); + } + public Task ReadTransactionAsync( Func> work, Action action = null) diff --git a/Neo4j.Driver/Neo4j.Driver/Internal/Util/ConfigBuilders.cs b/Neo4j.Driver/Neo4j.Driver/Internal/Util/ConfigBuilders.cs index ec06ad879..2ab52ada1 100644 --- a/Neo4j.Driver/Neo4j.Driver/Internal/Util/ConfigBuilders.cs +++ b/Neo4j.Driver/Neo4j.Driver/Internal/Util/ConfigBuilders.cs @@ -21,23 +21,6 @@ namespace Neo4j.Driver.Internal.Util; internal static class ConfigBuilders { - public static TransactionConfig BuildTransactionConfig(Action action) - { - TransactionConfig config; - if (action == null) - { - config = TransactionConfig.Default; - } - else - { - var builder = TransactionConfig.Builder; - action.Invoke(builder); - config = builder.Build(); - } - - return config; - } - public static SessionConfig BuildSessionConfig(Action action) { SessionConfig config; diff --git a/Neo4j.Driver/Neo4j.Driver/TransactionConfig.cs b/Neo4j.Driver/Neo4j.Driver/TransactionConfig.cs index caead02b7..182442308 100644 --- a/Neo4j.Driver/Neo4j.Driver/TransactionConfig.cs +++ b/Neo4j.Driver/Neo4j.Driver/TransactionConfig.cs @@ -49,8 +49,6 @@ internal TransactionConfig(IDictionary metadata, TimeSpan? timeo _timeout = timeout; } - internal static TransactionConfigBuilder Builder => new(new TransactionConfig()); - /// /// Transaction timeout. Transactions that execute longer than the configured timeout will be terminated by the /// database. This functionality allows to limit query/transaction execution time. Specified timeout overrides the default @@ -103,10 +101,14 @@ public override string ToString() /// The builder to create a public sealed class TransactionConfigBuilder { + private readonly ILogger _logger; private readonly TransactionConfig _config; - internal TransactionConfigBuilder(TransactionConfig config) + internal TransactionConfigBuilder( + ILogger logger, + TransactionConfig config) { + _logger = logger; _config = config; } @@ -115,19 +117,41 @@ internal TransactionConfigBuilder(TransactionConfig config) /// by the database. This functionality allows to limit query/transaction execution time. Specified timeout overrides the /// default timeout configured in the database using dbms.transaction.timeout setting. Leave this field /// unmodified to use default timeout configured on database. Setting a zero timeout will result in no timeout. + /// + /// If the timeout is not an exact number of milliseconds, it will be rounded up to the next millisecond. /// /// - /// If the value given to transaction timeout in milliseconds is less than - /// zero + /// If the value given to transaction timeout in milliseconds is less than zero. /// - /// the new timeout - /// this instance + /// The new timeout. + /// this instance. public TransactionConfigBuilder WithTimeout(TimeSpan? timeout) { - _config.Timeout = timeout; + _config.Timeout = FixSubmilliseconds(timeout); return this; } + private TimeSpan? FixSubmilliseconds(TimeSpan? timeout) + { + if(timeout == null || timeout == TimeSpan.MaxValue) + { + return timeout; + } + + if (timeout.Value.Ticks % TimeSpan.TicksPerMillisecond == 0) + { + return timeout; + } + + var result = TimeSpan.FromMilliseconds(Math.Ceiling(timeout.Value.TotalMilliseconds)); + _logger?.Info( + "Transaction timeout {timeout} contains sub-millisecond precision and will be rounded up to {result}.", + timeout, + result); + + return result; + } + /// /// The transaction metadata. Specified metadata will be attached to the executing transaction and visible in the /// output of dbms.listQueries and dbms.listTransactions procedures. It will also get logged to