From b4a08538b8195e08c5cde57a2f4e00f40f8451d4 Mon Sep 17 00:00:00 2001 From: Grant Lodge <6323995+thelonelyvulpes@users.noreply.github.com> Date: Mon, 4 Dec 2023 12:00:37 +0000 Subject: [PATCH 1/7] add transaction config to executable query --- .../Neo4j.Driver.Tests/AsyncSessionTests.cs | 2 +- .../Messages/BeginMessageTests.cs | 14 ++-- .../Neo4j.Driver/ExecuteQuery/QueryConfig.cs | 11 ++- .../Neo4j.Driver/Internal/AsyncSession.cs | 62 +++++--------- Neo4j.Driver/Neo4j.Driver/Internal/Driver.cs | 6 +- .../Internal/IInternalAsyncSession.cs | 4 +- .../Neo4j.Driver/TransactionConfig.cs | 83 +++++++++---------- 7 files changed, 87 insertions(+), 95 deletions(-) diff --git a/Neo4j.Driver/Neo4j.Driver.Tests/AsyncSessionTests.cs b/Neo4j.Driver/Neo4j.Driver.Tests/AsyncSessionTests.cs index eb7390b05..ef2378043 100644 --- a/Neo4j.Driver/Neo4j.Driver.Tests/AsyncSessionTests.cs +++ b/Neo4j.Driver/Neo4j.Driver.Tests/AsyncSessionTests.cs @@ -299,7 +299,7 @@ public async void PipelinedShouldBeginWithoutBlocking() false, false); - await session.PipelinedExecuteReadAsync(_ => Task.FromResult(null as EagerResult)); + await session.PipelinedExecuteReadAsync(_ => Task.FromResult(null as EagerResult), new TransactionConfig()); mockProtocol.Verify( x => diff --git a/Neo4j.Driver/Neo4j.Driver.Tests/Internal/MessageHandling/Messages/BeginMessageTests.cs b/Neo4j.Driver/Neo4j.Driver.Tests/Internal/MessageHandling/Messages/BeginMessageTests.cs index 5f4dffd1f..1bb9db4f8 100644 --- a/Neo4j.Driver/Neo4j.Driver.Tests/Internal/MessageHandling/Messages/BeginMessageTests.cs +++ b/Neo4j.Driver/Neo4j.Driver.Tests/Internal/MessageHandling/Messages/BeginMessageTests.cs @@ -56,7 +56,7 @@ public void ShouldHandleSetValues(int major, int minor) new BoltProtocolVersion(major, minor), "neo4j", bookmarks, - new TransactionConfig(txMeta, TimeSpan.FromSeconds(1)), + new TransactionConfig { Metadata = txMeta, Timeout = TimeSpan.FromSeconds(1) }, AccessMode.Read, new SessionConfig("Douglas Fir"), null); @@ -89,7 +89,7 @@ public void ShouldIgnoreNotificationConfig(int major, int minor) new BoltProtocolVersion(major, minor), "neo4j", bookmarks, - new TransactionConfig(txMeta, TimeSpan.FromSeconds(1)), + new TransactionConfig { Metadata = txMeta, Timeout = TimeSpan.FromSeconds(1) }, AccessMode.Read, new SessionConfig("Douglas Fir"), new NotificationsDisabledConfig()); @@ -122,7 +122,7 @@ public void ShouldHandleSetValuesWithNotifications(int major, int minor) new BoltProtocolVersion(major, minor), "neo4j", bookmarks, - new TransactionConfig(txMeta, TimeSpan.FromSeconds(1)), + new TransactionConfig { Metadata = txMeta, Timeout = TimeSpan.FromSeconds(1) }, AccessMode.Read, new SessionConfig("Douglas Fir"), new NotificationsConfig(Severity.Warning, new[] { Category.Generic })); @@ -153,12 +153,14 @@ public void ShouldThrowIfBoltVersionLessThan44() BoltProtocolVersion.V4_3, "neo4j", new InternalBookmarks("bm:a"), - new TransactionConfig( - new Dictionary + new TransactionConfig + { + Metadata = new Dictionary { ["a"] = "b" }, - TimeSpan.FromSeconds(1)), + Timeout = TimeSpan.FromSeconds(1) + }, AccessMode.Read, new SessionConfig("Douglas Fir"), null)) diff --git a/Neo4j.Driver/Neo4j.Driver/ExecuteQuery/QueryConfig.cs b/Neo4j.Driver/Neo4j.Driver/ExecuteQuery/QueryConfig.cs index 5e67a4cd0..c8b7d9668 100644 --- a/Neo4j.Driver/Neo4j.Driver/ExecuteQuery/QueryConfig.cs +++ b/Neo4j.Driver/Neo4j.Driver/ExecuteQuery/QueryConfig.cs @@ -40,14 +40,18 @@ public QueryConfig( string database = null, string impersonatedUser = null, IBookmarkManager bookmarkManager = null, - bool enableBookmarkManager = true) + bool enableBookmarkManager = true, + TransactionConfig transactionConfig = null) { Routing = routing; Database = database; ImpersonatedUser = impersonatedUser; BookmarkManager = bookmarkManager; EnableBookmarkManager = enableBookmarkManager; + TransactionConfig = transactionConfig ?? TransactionConfig.Default; } + /// Config for the transaction that will be used to execute the query. + public TransactionConfig TransactionConfig { get; } /// Members of the cluster the query can be processed by. public RoutingControl Routing { get; } @@ -94,8 +98,9 @@ public QueryConfig( string database = null, string impersonatedUser = null, IBookmarkManager bookmarkManager = null, - bool enableBookmarkManager = true) - : base(routing, database, impersonatedUser, bookmarkManager, enableBookmarkManager) + bool enableBookmarkManager = true, + TransactionConfig transactionConfig = null) + : base(routing, database, impersonatedUser, bookmarkManager, enableBookmarkManager, transactionConfig) { CursorProcessor = cursorProcessor ?? throw new ArgumentNullException(nameof(cursorProcessor)); } diff --git a/Neo4j.Driver/Neo4j.Driver/Internal/AsyncSession.cs b/Neo4j.Driver/Neo4j.Driver/Internal/AsyncSession.cs index 370e3641c..b74e9b153 100644 --- a/Neo4j.Driver/Neo4j.Driver/Internal/AsyncSession.cs +++ b/Neo4j.Driver/Neo4j.Driver/Internal/AsyncSession.cs @@ -141,13 +141,13 @@ public async Task BeginTransactionAsync( Action action, bool disposeUnconsumedSessionResult) { - var tx = await TryExecuteAsync( + var config = BuildTransactionConfig(action); + return await TryExecuteAsync( _logger, - () => BeginTransactionWithoutLoggingAsync(mode, action, disposeUnconsumedSessionResult, + () => BeginTransactionWithoutLoggingAsync(mode, + config, disposeUnconsumedSessionResult, new TransactionInfo(QueryApiType.UnmanagedTransaction, TelemetryEnabled, true))) .ConfigureAwait(false); - - return tx; } public Task RunAsync( @@ -195,18 +195,20 @@ public Task RunAsync( return result; } - public Task> PipelinedExecuteReadAsync(Func>> func) + public Task> PipelinedExecuteReadAsync(Func>> func, + TransactionConfig config) { return RunTransactionAsync( AccessMode.Read, func, - null, + config, new TransactionInfo(QueryApiType.DriverLevel, TelemetryEnabled, false)); } - public Task> PipelinedExecuteWriteAsync(Func>> func) + public Task> PipelinedExecuteWriteAsync(Func>> func, + TransactionConfig config) { - return RunTransactionAsync(AccessMode.Write, func, null, + return RunTransactionAsync(AccessMode.Write, func, config, new TransactionInfo(QueryApiType.DriverLevel, TelemetryEnabled, false)); } @@ -225,54 +227,54 @@ public Task ReadTransactionAsync( Func> work, Action action = null) { - return RunTransactionAsync(AccessMode.Read, work, action); + return RunTransactionAsync(AccessMode.Read, work, BuildTransactionConfig(action)); } public Task ReadTransactionAsync( Func work, Action action = null) { - return RunTransactionAsync(AccessMode.Read, work, action); + return RunTransactionAsync(AccessMode.Read, work, BuildTransactionConfig(action)); } public Task WriteTransactionAsync( Func> work, Action action = null) { - return RunTransactionAsync(AccessMode.Write, work, action); + return RunTransactionAsync(AccessMode.Write, work, BuildTransactionConfig(action)); } public Task WriteTransactionAsync( Func work, Action action = null) { - return RunTransactionAsync(AccessMode.Write, work, action); + return RunTransactionAsync(AccessMode.Write, work, BuildTransactionConfig(action)); } public Task ExecuteReadAsync(Func work, Action action = null) { - return RunTransactionAsync(AccessMode.Read, work, action); + return RunTransactionAsync(AccessMode.Read, work, BuildTransactionConfig(action)); } public Task ExecuteReadAsync( Func> work, Action action = null) { - return RunTransactionAsync(AccessMode.Read, work, action); + return RunTransactionAsync(AccessMode.Read, work, BuildTransactionConfig(action)); } public Task ExecuteWriteAsync( Func work, Action action = null) { - return RunTransactionAsync(AccessMode.Write, work, action); + return RunTransactionAsync(AccessMode.Write, work, BuildTransactionConfig(action)); } public Task ExecuteWriteAsync( Func> work, Action action = null) { - return RunTransactionAsync(AccessMode.Write, work, action); + return RunTransactionAsync(AccessMode.Write, work, BuildTransactionConfig(action)); } private async Task GetBookmarksAsync() @@ -283,26 +285,10 @@ private async Task GetBookmarksAsync() (await _bookmarkManager.GetBookmarksAsync().ConfigureAwait(false)).Concat(_initialBookmarks.Values)); } - private Task RunTransactionAsync( - AccessMode mode, - Func work, - Action action) - { - return RunTransactionAsync( - mode, - async tx => - { - await work(tx).ConfigureAwait(false); - var ignored = 1; - return ignored; - }, - action); - } - private Task RunTransactionAsync( AccessMode mode, Func work, - Action action, + TransactionConfig config, TransactionInfo transactionInfo = null) { return RunTransactionAsync( @@ -313,24 +299,23 @@ private Task RunTransactionAsync( var ignored = 1; return ignored; }, - action, + config, transactionInfo); } private Task RunTransactionAsync( AccessMode mode, Func> work, - Action action, + TransactionConfig config, TransactionInfo transactionInfo = null) { transactionInfo ??= new TransactionInfo (QueryApiType.TransactionFunction, TelemetryEnabled, true); - return TryExecuteAsync( _logger, () => _retryLogic.RetryAsync( async () => { - var tx = await BeginTransactionWithoutLoggingAsync(mode, action, true, transactionInfo).ConfigureAwait(false); + var tx = await BeginTransactionWithoutLoggingAsync(mode, config, true, transactionInfo).ConfigureAwait(false); try { var result = await work(tx).ConfigureAwait(false); @@ -355,11 +340,10 @@ private Task RunTransactionAsync( private async Task BeginTransactionWithoutLoggingAsync( AccessMode mode, - Action action, + TransactionConfig config, bool disposeUnconsumedSessionResult, TransactionInfo transactionInfo) { - var config = BuildTransactionConfig(action); await EnsureCanRunMoreQuerysAsync(disposeUnconsumedSessionResult).ConfigureAwait(false); await AcquireConnectionAndDbNameAsync(mode).ConfigureAwait(false); diff --git a/Neo4j.Driver/Neo4j.Driver/Internal/Driver.cs b/Neo4j.Driver/Neo4j.Driver/Internal/Driver.cs index 5c8236aba..6d6fab117 100644 --- a/Neo4j.Driver/Neo4j.Driver/Internal/Driver.cs +++ b/Neo4j.Driver/Neo4j.Driver/Internal/Driver.cs @@ -233,11 +233,13 @@ private async Task> ExecuteQueryAsyncInternal( { if (config.Routing == RoutingControl.Readers) { - return await session.PipelinedExecuteReadAsync(x => Work(query, x, cursorProcessor, cancellationToken)) + return await session.PipelinedExecuteReadAsync(x => Work(query, x, cursorProcessor, cancellationToken), + config.TransactionConfig) .ConfigureAwait(false); } - return await session.PipelinedExecuteWriteAsync(x => Work(query, x, cursorProcessor, cancellationToken)) + return await session.PipelinedExecuteWriteAsync(x => Work(query, x, cursorProcessor, cancellationToken), + config.TransactionConfig) .ConfigureAwait(false); } } diff --git a/Neo4j.Driver/Neo4j.Driver/Internal/IInternalAsyncSession.cs b/Neo4j.Driver/Neo4j.Driver/Internal/IInternalAsyncSession.cs index 100e5b59f..260b762a3 100644 --- a/Neo4j.Driver/Neo4j.Driver/Internal/IInternalAsyncSession.cs +++ b/Neo4j.Driver/Neo4j.Driver/Internal/IInternalAsyncSession.cs @@ -34,6 +34,6 @@ Task RunAsync( Action action, bool disposeUnconsumedSessionResult); - Task> PipelinedExecuteReadAsync(Func>> func); - Task> PipelinedExecuteWriteAsync(Func>> func); + Task> PipelinedExecuteReadAsync(Func>> func, TransactionConfig config); + Task> PipelinedExecuteWriteAsync(Func>> func, TransactionConfig config); } diff --git a/Neo4j.Driver/Neo4j.Driver/TransactionConfig.cs b/Neo4j.Driver/Neo4j.Driver/TransactionConfig.cs index b517fef3d..22508649f 100644 --- a/Neo4j.Driver/Neo4j.Driver/TransactionConfig.cs +++ b/Neo4j.Driver/Neo4j.Driver/TransactionConfig.cs @@ -16,55 +16,44 @@ using System; using System.Collections.Generic; using Neo4j.Driver.Internal; -using Neo4j.Driver.Internal.IO; namespace Neo4j.Driver; /// /// Configuration object containing settings for explicit and auto-commit transactions. Leave the fields unmodified to use /// server side transaction configurations. -/// -/// For example, the following code starts a transaction using server default transaction configurations. -/// -/// session.BeginTransaction(b=>{}); -/// /// public sealed class TransactionConfig { internal static readonly TransactionConfig Default = new(); - private IDictionary _metadata; - private TimeSpan? _timeout; - - internal TransactionConfig() - { - _timeout = null; - _metadata = PackStream.EmptyDictionary; - } - - internal TransactionConfig(IDictionary metadata, TimeSpan? timeout) - { - _metadata = metadata; - _timeout = timeout; - } + internal IDictionary _metadata = new Dictionary(); + internal TimeSpan? _timeout = null; /// - /// Transaction timeout. Transactions that execute longer than the configured timeout will be terminated - /// by the database. This functionality allows user code to limit query/transaction execution time. The specified timeout - /// overrides the default timeout configured in the database using the db.transaction.timeout setting - /// (dbms.transaction.timeout before Neo4j 5.0). Values higher than db.transaction.timeout will be - /// ignored and will fall back to the default for server versions between 4.2 and 5.2 (inclusive). Leave this field unmodified or set it - /// to null to use the default timeout configured on the server. A timeout of zero will make the transaction - /// execute indefinitely. + /// Transaction timeout. Transactions that execute longer than the configured timeout will be terminated by the + /// database. This functionality allows user code to limit query/transaction execution time. The specified timeout + /// overrides the default timeout configured in the database using the db.transaction.timeout setting ( + /// dbms.transaction.timeout before Neo4j 5.0). Values higher than db.transaction.timeout will be + /// ignored and will fall back to the default for server versions between 4.2 and 5.2 (inclusive). Leave this field + /// unmodified or set it to null to use the default timeout configured on the server. A timeout of zero will + /// make the transaction execute indefinitely. /// + /// All positive non-whole millisecond values will be rounded to the next whole 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. /// public TimeSpan? Timeout { get => _timeout; - internal set + init { - if ((value ?? TimeSpan.Zero) < TimeSpan.Zero) + if (!value.HasValue) + { + return; + } + + if (value.Value < TimeSpan.Zero) { throw new ArgumentOutOfRangeException( nameof(value), @@ -72,7 +61,14 @@ internal set "Transaction timeout should not be negative."); } - _timeout = value; + if (value.Value.Ticks % TimeSpan.TicksPerMillisecond == 0) + { + _timeout = value; + return; + } + + var result = TimeSpan.FromMilliseconds(Math.Ceiling(value.Value.TotalMilliseconds)); + _timeout = result; } } @@ -86,7 +82,7 @@ internal set public IDictionary Metadata { get => _metadata; - internal set => _metadata = + init => _metadata = value ?? throw new ArgumentNullException(nameof(value), "Transaction metadata should not be null"); } @@ -101,8 +97,8 @@ public override string ToString() /// The builder to create a public sealed class TransactionConfigBuilder { - private readonly ILogger _logger; private readonly TransactionConfig _config; + private readonly ILogger _logger; internal TransactionConfigBuilder( ILogger logger, @@ -115,28 +111,29 @@ internal TransactionConfigBuilder( /// /// Sets the transaction timeout. Transactions that execute longer than the configured timeout will be terminated /// by the database. This functionality allows user code to limit query/transaction execution time. The specified timeout - /// overrides the default timeout configured in the database using the db.transaction.timeout setting - /// (dbms.transaction.timeout before Neo4j 5.0). Values higher than db.transaction.timeout will be - /// ignored and will fall back to default for server versions between 4.2 and 5.2 (inclusive). Leave this field unmodified or - /// set it to null to use the default timeout configured on the server. A timeout of zero will make the transaction - /// execute indefinitely. + /// overrides the default timeout configured in the database using the db.transaction.timeout setting ( + /// dbms.transaction.timeout before Neo4j 5.0). Values higher than db.transaction.timeout will be + /// ignored and will fall back to default for server versions between 4.2 and 5.2 (inclusive). Leave this field unmodified + /// or set it to null to use the default timeout configured on the server. A timeout of zero will make the + /// transaction execute indefinitely. /// /// 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. public TransactionConfigBuilder WithTimeout(TimeSpan? timeout) { - _config.Timeout = FixSubmilliseconds(timeout); + _config._timeout = FixSubmilliseconds(timeout); return this; } private TimeSpan? FixSubmilliseconds(TimeSpan? timeout) { - if(timeout == null || timeout == TimeSpan.MaxValue) + if (timeout == null || timeout == TimeSpan.MaxValue) { return timeout; } @@ -164,7 +161,9 @@ public TransactionConfigBuilder WithTimeout(TimeSpan? timeout) /// this instance public TransactionConfigBuilder WithMetadata(IDictionary metadata) { - _config.Metadata = metadata; + _config._metadata = metadata ?? + throw new ArgumentNullException(nameof(metadata), "Transaction metadata should not be null"); + return this; } From ea5c5e946e09da104f10cbac27522858cbce4270 Mon Sep 17 00:00:00 2001 From: Grant Lodge <6323995+thelonelyvulpes@users.noreply.github.com> Date: Mon, 4 Dec 2023 12:25:46 +0000 Subject: [PATCH 2/7] fix tests and handling max value --- .../TransactionConfigTests.cs | 191 ++++++++++++------ .../Neo4j.Driver/TransactionConfig.cs | 41 ++-- 2 files changed, 154 insertions(+), 78 deletions(-) diff --git a/Neo4j.Driver/Neo4j.Driver.Tests/TransactionConfigTests.cs b/Neo4j.Driver/Neo4j.Driver.Tests/TransactionConfigTests.cs index a514d5833..d80cd9dfc 100644 --- a/Neo4j.Driver/Neo4j.Driver.Tests/TransactionConfigTests.cs +++ b/Neo4j.Driver/Neo4j.Driver.Tests/TransactionConfigTests.cs @@ -16,92 +16,157 @@ using System; using System.Collections.Generic; using FluentAssertions; +using Neo4j.Driver.Internal.Logging; using Xunit; -namespace Neo4j.Driver.Tests +namespace Neo4j.Driver.Tests; + +public class TransactionConfigTests { - public class TransactionConfigTests + public class TimeoutField { - public class TimeoutField + public static IEnumerable InvalidTimeSpanValues => new[] { - public static IEnumerable InvalidTimeSpanValues => new[] - { - new object[] { TimeSpan.FromSeconds(-1) }, - new object[] { TimeSpan.FromHours(-2) } - }; + new object[] { TimeSpan.FromSeconds(-1) }, + new object[] { TimeSpan.FromHours(-2) } + }; - public static IEnumerable ValidTimeSpanValues => new[] - { - new object[] { null }, - new object[] { (TimeSpan?)TimeSpan.Zero }, - new object[] { (TimeSpan?)TimeSpan.FromMilliseconds(1) }, - new object[] { (TimeSpan?)TimeSpan.FromMinutes(30) }, - new object[] { (TimeSpan?)TimeSpan.MaxValue } - }; - - [Fact] - public void ShouldReturnDefaultValueAsNull() - { - var config = new TransactionConfig(); + public static IEnumerable ValidTimeSpanValues => new[] + { + new object[] { null }, + new object[] { (TimeSpan?)TimeSpan.Zero }, + new object[] { (TimeSpan?)TimeSpan.FromMilliseconds(1) }, + new object[] { (TimeSpan?)TimeSpan.FromMinutes(30) }, + new object[] { (TimeSpan?)TimeSpan.MaxValue } + }; + + [Fact] + public void ShouldReturnDefaultValueAsNull() + { + var config = new TransactionConfig(); - config.Timeout.Should().Be(null); - } + config.Timeout.Should().Be(null); + } - [Theory] - [MemberData(nameof(ValidTimeSpanValues))] - public void ShouldAllowToSetToNewValue(TimeSpan? input) - { - var builder = new TransactionConfigBuilder(null, TransactionConfig.Default); - builder.WithTimeout(input); + [Theory] + [MemberData(nameof(ValidTimeSpanValues))] + public void ShouldAllowToSetToNewValue(TimeSpan? input) + { + var builder = new TransactionConfigBuilder(null, TransactionConfig.Default); + builder.WithTimeout(input); + + var config = builder.Build(); + + config.Timeout.Should().Be(input); + } - var config = builder.Build(); + [Theory] + [MemberData(nameof(ValidTimeSpanValues))] + public void ShouldAllowToInitToNewValue(TimeSpan? input) + { + var config = new TransactionConfig { Timeout = input }; + if (input == TimeSpan.MaxValue) + { + config.Timeout.Should().Be(TimeSpan.Zero); + } + else + { config.Timeout.Should().Be(input); } + } - [Theory] - [MemberData(nameof(InvalidTimeSpanValues))] - public void ShouldThrowExceptionIfAssigningValueLessThanZero(TimeSpan input) - { - var error = Record.Exception(() => new TransactionConfigBuilder(null, TransactionConfig.Default).WithTimeout(input)); + [Fact] + public void ShouldRoundUpWithBuilder() + { + var ts = TimeSpan.FromTicks(1); + var builder = new TransactionConfigBuilder(NullLogger.Instance, TransactionConfig.Default); + builder.WithTimeout(ts); - error.Should().BeOfType(); - error.Message.Should().Contain("not be negative"); - } + var config = builder.Build(); + + config.Timeout.Should().Be(TimeSpan.FromMilliseconds(1)); } - public class MetadataField + [Fact] + public void ShouldRoundUpWithInit() { - [Fact] - public void ShouldReturnDefaultValueEmptyDictionary() - { - var config = new TransactionConfig(); + var ts = TimeSpan.FromTicks(1); + var config = new TransactionConfig { Timeout = ts }; + config.Timeout.Should().Be(TimeSpan.FromMilliseconds(1)); + } - config.Metadata.Should().BeEmpty(); - } + [Theory] + [MemberData(nameof(InvalidTimeSpanValues))] + public void ShouldThrowExceptionIfAssigningValueLessThanZero(TimeSpan input) + { + var error = Record.Exception( + () => new TransactionConfigBuilder(null, TransactionConfig.Default).WithTimeout(input)); - [Fact] - public void ShouldAllowToSetToNewValue() - { - var builder = new TransactionConfigBuilder(null, TransactionConfig.Default) - .WithMetadata(new Dictionary { { "key", "value" } }); + error.Should().BeOfType(); + error.Message.Should().Contain("not be negative"); + } - var config = builder.Build(); + [Theory] + [MemberData(nameof(InvalidTimeSpanValues))] + public void ShouldThrowExceptionIfInitValueLessThanZero(TimeSpan input) + { + var error = Record.Exception(() => new TransactionConfig { Timeout = input }); + error.Should().BeOfType(); + error.Message.Should().Contain("not be negative"); + } + } - config.Metadata.Should() - .HaveCount(1) - .And.Contain(new KeyValuePair("key", "value")); - } + public class MetadataField + { + [Fact] + public void ShouldReturnDefaultValueEmptyDictionary() + { + var config = new TransactionConfig(); - [Fact] - public void ShouldThrowExceptionIfAssigningNull() - { - var error = Record.Exception( - () => new TransactionConfigBuilder(null, TransactionConfig.Default).WithMetadata(null)); + config.Metadata.Should().BeEmpty(); + } - error.Should().BeOfType(); - error.Message.Should().Contain("should not be null"); - } + [Fact] + public void ShouldAllowToSetToNewValue() + { + var builder = new TransactionConfigBuilder(null, TransactionConfig.Default) + .WithMetadata(new Dictionary { { "key", "value" } }); + + var config = builder.Build(); + + config.Metadata.Should() + .HaveCount(1) + .And.Contain(new KeyValuePair("key", "value")); + } + + [Fact] + public void ShouldThrowExceptionIfAssigningNull() + { + var error = Record.Exception( + () => new TransactionConfigBuilder(null, TransactionConfig.Default).WithMetadata(null)); + + error.Should().BeOfType(); + error.Message.Should().Contain("should not be null"); + } + + [Fact] + public void ShouldAllowToInitToNewValue() + { + var config = new TransactionConfig { Metadata = new Dictionary { ["key"] = "value" } }; + + config.Metadata.Should() + .HaveCount(1) + .And.Contain(new KeyValuePair("key", "value")); + } + + [Fact] + public void ShouldThrowExceptionIfInitNull() + { + var error = Record.Exception(() => new TransactionConfig { Metadata = null }); + + error.Should().BeOfType(); + error.Message.Should().Contain("should not be null"); } } } diff --git a/Neo4j.Driver/Neo4j.Driver/TransactionConfig.cs b/Neo4j.Driver/Neo4j.Driver/TransactionConfig.cs index 22508649f..772df61cb 100644 --- a/Neo4j.Driver/Neo4j.Driver/TransactionConfig.cs +++ b/Neo4j.Driver/Neo4j.Driver/TransactionConfig.cs @@ -39,6 +39,7 @@ public sealed class TransactionConfig /// make the transaction execute indefinitely. /// /// All positive non-whole millisecond values will be rounded to the next whole millisecond. + /// will be treated as and remove all timeout. /// /// If the value given to transaction timeout in milliseconds is less than /// zero. @@ -52,6 +53,12 @@ public TimeSpan? Timeout { return; } + + if (value.Value == TimeSpan.MaxValue) + { + _timeout = TimeSpan.Zero; + return; + } if (value.Value < TimeSpan.Zero) { @@ -127,27 +134,31 @@ internal TransactionConfigBuilder( /// this instance. public TransactionConfigBuilder WithTimeout(TimeSpan? timeout) { - _config._timeout = FixSubmilliseconds(timeout); - return this; - } - - private TimeSpan? FixSubmilliseconds(TimeSpan? timeout) - { - if (timeout == null || timeout == TimeSpan.MaxValue) + if (!timeout.HasValue || timeout.Value == TimeSpan.MaxValue) { - return timeout; + _config._timeout = timeout; + return this; } + if (timeout.Value < TimeSpan.Zero) + { + throw new ArgumentOutOfRangeException( + nameof(timeout), + "Transaction timeout should not be negative."); + } + if (timeout.Value.Ticks % TimeSpan.TicksPerMillisecond == 0) { - return timeout; + _config._timeout = 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}."); - - return result; + else + { + _config._timeout = TimeSpan.FromMilliseconds(Math.Ceiling(timeout.Value.TotalMilliseconds)); + _logger.Info( + $"Transaction timeout {timeout} contains sub-millisecond precision and will be rounded up to {_config._timeout}."); + } + + return this; } /// From 8d01d9b1b26dd144ace9f467337d2a5a49de438a Mon Sep 17 00:00:00 2001 From: Grant Lodge <6323995+thelonelyvulpes@users.noreply.github.com> Date: Mon, 4 Dec 2023 12:29:34 +0000 Subject: [PATCH 3/7] unify behaviour --- .../Neo4j.Driver.Tests/TransactionConfigTests.cs | 10 +--------- Neo4j.Driver/Neo4j.Driver/TransactionConfig.cs | 11 +++-------- 2 files changed, 4 insertions(+), 17 deletions(-) diff --git a/Neo4j.Driver/Neo4j.Driver.Tests/TransactionConfigTests.cs b/Neo4j.Driver/Neo4j.Driver.Tests/TransactionConfigTests.cs index d80cd9dfc..002fe495f 100644 --- a/Neo4j.Driver/Neo4j.Driver.Tests/TransactionConfigTests.cs +++ b/Neo4j.Driver/Neo4j.Driver.Tests/TransactionConfigTests.cs @@ -65,15 +65,7 @@ public void ShouldAllowToSetToNewValue(TimeSpan? input) public void ShouldAllowToInitToNewValue(TimeSpan? input) { var config = new TransactionConfig { Timeout = input }; - - if (input == TimeSpan.MaxValue) - { - config.Timeout.Should().Be(TimeSpan.Zero); - } - else - { - config.Timeout.Should().Be(input); - } + config.Timeout.Should().Be(input); } [Fact] diff --git a/Neo4j.Driver/Neo4j.Driver/TransactionConfig.cs b/Neo4j.Driver/Neo4j.Driver/TransactionConfig.cs index 772df61cb..1eee0f822 100644 --- a/Neo4j.Driver/Neo4j.Driver/TransactionConfig.cs +++ b/Neo4j.Driver/Neo4j.Driver/TransactionConfig.cs @@ -39,7 +39,7 @@ public sealed class TransactionConfig /// make the transaction execute indefinitely. /// /// All positive non-whole millisecond values will be rounded to the next whole millisecond. - /// will be treated as and remove all timeout. + /// ticks will be ignored. /// /// If the value given to transaction timeout in milliseconds is less than /// zero. @@ -49,17 +49,12 @@ public TimeSpan? Timeout get => _timeout; init { - if (!value.HasValue) + if (!value.HasValue || value.Value == TimeSpan.MaxValue) { + _timeout = value; return; } - if (value.Value == TimeSpan.MaxValue) - { - _timeout = TimeSpan.Zero; - return; - } - if (value.Value < TimeSpan.Zero) { throw new ArgumentOutOfRangeException( From 4d8be8bb05863f92d63a3a371afb6a01e10b41c4 Mon Sep 17 00:00:00 2001 From: Grant Lodge <6323995+thelonelyvulpes@users.noreply.github.com> Date: Mon, 4 Dec 2023 12:41:04 +0000 Subject: [PATCH 4/7] add testkit support for txconfig --- .../Protocol/DriverQuery/ExecuteQuery.cs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/Neo4j.Driver/Neo4j.Driver.Tests.TestBackend/Protocol/DriverQuery/ExecuteQuery.cs b/Neo4j.Driver/Neo4j.Driver.Tests.TestBackend/Protocol/DriverQuery/ExecuteQuery.cs index 7004935ba..56ded553e 100644 --- a/Neo4j.Driver/Neo4j.Driver.Tests.TestBackend/Protocol/DriverQuery/ExecuteQuery.cs +++ b/Neo4j.Driver/Neo4j.Driver.Tests.TestBackend/Protocol/DriverQuery/ExecuteQuery.cs @@ -69,12 +69,19 @@ private QueryConfig BuildConfig() } } + var transactionConfig = new TransactionConfig + { + Timeout = data.config.timeout.HasValue ? TimeSpan.FromMilliseconds(data.config.timeout.Value) : null, + Metadata = data.config.txMeta ?? new Dictionary() + }; + return new QueryConfig( routingControl, data.config.database, data.config.impersonatedUser, bookmarkManager, - enableBookmarkManager); + enableBookmarkManager, + transactionConfig); } public override string Respond() @@ -117,5 +124,7 @@ public class ExecuteQueryConfigDto public string database { get; set; } public string impersonatedUser { get; set; } public string bookmarkManagerId { get; set; } + public int? timeout { get; set; } + public Dictionary txMeta { get; set; } } } From adf5eec048419ee2e8838dcbe5548fdd67d1cf2e Mon Sep 17 00:00:00 2001 From: Grant Lodge <6323995+thelonelyvulpes@users.noreply.github.com> Date: Mon, 4 Dec 2023 12:54:08 +0000 Subject: [PATCH 5/7] fix handling testkit dictionary parameters --- .../Protocol/DriverQuery/ExecuteQuery.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Neo4j.Driver/Neo4j.Driver.Tests.TestBackend/Protocol/DriverQuery/ExecuteQuery.cs b/Neo4j.Driver/Neo4j.Driver.Tests.TestBackend/Protocol/DriverQuery/ExecuteQuery.cs index 56ded553e..5f970af72 100644 --- a/Neo4j.Driver/Neo4j.Driver.Tests.TestBackend/Protocol/DriverQuery/ExecuteQuery.cs +++ b/Neo4j.Driver/Neo4j.Driver.Tests.TestBackend/Protocol/DriverQuery/ExecuteQuery.cs @@ -72,7 +72,7 @@ private QueryConfig BuildConfig() var transactionConfig = new TransactionConfig { Timeout = data.config.timeout.HasValue ? TimeSpan.FromMilliseconds(data.config.timeout.Value) : null, - Metadata = data.config.txMeta ?? new Dictionary() + Metadata = data.config.txMeta != null ? CypherToNativeObject.ConvertDictionaryToNative(data.config.txMeta) : new Dictionary() }; return new QueryConfig( @@ -125,6 +125,8 @@ public class ExecuteQueryConfigDto public string impersonatedUser { get; set; } public string bookmarkManagerId { get; set; } public int? timeout { get; set; } - public Dictionary txMeta { get; set; } + [JsonProperty(Required = Required.AllowNull)] + [JsonConverter(typeof(QueryParameterConverter))] + public Dictionary txMeta { get; set; } } } From 28e77e21e8ca16ebf1cbefc435ec9be1e193f783 Mon Sep 17 00:00:00 2001 From: Grant Lodge <6323995+thelonelyvulpes@users.noreply.github.com> Date: Mon, 4 Dec 2023 16:09:28 +0000 Subject: [PATCH 6/7] Correct param name clause --- .../Protocol/Session/BaseSessionType.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Neo4j.Driver/Neo4j.Driver.Tests.TestBackend/Protocol/Session/BaseSessionType.cs b/Neo4j.Driver/Neo4j.Driver.Tests.TestBackend/Protocol/Session/BaseSessionType.cs index d2c5d526f..10166ab3e 100644 --- a/Neo4j.Driver/Neo4j.Driver.Tests.TestBackend/Protocol/Session/BaseSessionType.cs +++ b/Neo4j.Driver/Neo4j.Driver.Tests.TestBackend/Protocol/Session/BaseSessionType.cs @@ -45,7 +45,7 @@ public TransactionConfigBuilder ConfigureTxTimeout(TransactionConfigBuilder conf configBuilder.WithTimeout(timeout); } } - catch (ArgumentOutOfRangeException e) when ((timeout ?? 0) < 0 && e.ParamName == "value") + catch (ArgumentOutOfRangeException e) when ((timeout ?? 0) < 0 && e.ParamName == "timeout") { throw new DriverExceptionWrapper(e); } From 7bc88dda8e19dd2bcc590a37ee961c3f61d8f443 Mon Sep 17 00:00:00 2001 From: Grant Lodge <6323995+thelonelyvulpes@users.noreply.github.com> Date: Mon, 4 Dec 2023 17:12:25 +0000 Subject: [PATCH 7/7] remove accidentally copied attribute --- .../Protocol/DriverQuery/ExecuteQuery.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/Neo4j.Driver/Neo4j.Driver.Tests.TestBackend/Protocol/DriverQuery/ExecuteQuery.cs b/Neo4j.Driver/Neo4j.Driver.Tests.TestBackend/Protocol/DriverQuery/ExecuteQuery.cs index 5f970af72..a79f79cc4 100644 --- a/Neo4j.Driver/Neo4j.Driver.Tests.TestBackend/Protocol/DriverQuery/ExecuteQuery.cs +++ b/Neo4j.Driver/Neo4j.Driver.Tests.TestBackend/Protocol/DriverQuery/ExecuteQuery.cs @@ -125,7 +125,6 @@ public class ExecuteQueryConfigDto public string impersonatedUser { get; set; } public string bookmarkManagerId { get; set; } public int? timeout { get; set; } - [JsonProperty(Required = Required.AllowNull)] [JsonConverter(typeof(QueryParameterConverter))] public Dictionary txMeta { get; set; } }