From b16e32d1ff1b9c3ed17bfaed41868bca1ce0bd3a Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Fri, 12 Sep 2025 11:30:47 +0100 Subject: [PATCH] fix: Correctly pass options to object saver (#387) * Correctly pass options to object saver * formatting --- .../Serialisation/V2/Send/ObjectSaver.cs | 4 ++-- .../Serialisation/V2/Send/SerializeProcess.cs | 15 +++++++-------- .../Serialisation/V2/SerializeProcessFactory.cs | 5 +++-- .../DataObjectTests.cs | 2 +- .../DetachedTests.cs | 12 ++++++------ .../SerializeProcessRecordExceptionTests.cs | 3 +++ 6 files changed, 22 insertions(+), 19 deletions(-) diff --git a/src/Speckle.Sdk/Serialisation/V2/Send/ObjectSaver.cs b/src/Speckle.Sdk/Serialisation/V2/Send/ObjectSaver.cs index 0e26a1bc..19ccca3d 100644 --- a/src/Speckle.Sdk/Serialisation/V2/Send/ObjectSaver.cs +++ b/src/Speckle.Sdk/Serialisation/V2/Send/ObjectSaver.cs @@ -20,10 +20,10 @@ public sealed class ObjectSaver( ISqLiteJsonCacheManager sqLiteJsonCacheManager, IServerObjectManager serverObjectManager, ILogger logger, - CancellationToken cancellationToken, + SerializeProcessOptions options, + CancellationToken cancellationToken #pragma warning disable CS9107 #pragma warning disable CA2254 - SerializeProcessOptions? options = null ) : ChannelSaver, IObjectSaver #pragma warning restore CA2254 #pragma warning restore CS9107 diff --git a/src/Speckle.Sdk/Serialisation/V2/Send/SerializeProcess.cs b/src/Speckle.Sdk/Serialisation/V2/Send/SerializeProcess.cs index 16aeef10..c92e96a8 100644 --- a/src/Speckle.Sdk/Serialisation/V2/Send/SerializeProcess.cs +++ b/src/Speckle.Sdk/Serialisation/V2/Send/SerializeProcess.cs @@ -37,8 +37,8 @@ public sealed class SerializeProcess( IBaseChildFinder baseChildFinder, IBaseSerializer baseSerializer, ILoggerFactory loggerFactory, - CancellationToken cancellationToken, - SerializeProcessOptions? options = null + SerializeProcessOptions options, + CancellationToken cancellationToken ) : ISerializeProcess { private static readonly Dictionary EMPTY_CLOSURES = new(); @@ -64,7 +64,6 @@ public sealed class SerializeProcess( ThreadPriority.BelowNormal, Environment.ProcessorCount * 2 ); - private readonly SerializeProcessOptions _options = options ?? new(); private readonly Pool> _currentClosurePool = Pools.CreateDictionaryPool(); private readonly Pool> _childClosurePool = Pools.CreateConcurrentDictionaryPool< @@ -113,13 +112,13 @@ public sealed class SerializeProcess( try { var channelTask = objectSaver.Start( - options?.MaxParallelism, - options?.MaxHttpSendBatchSize, - options?.MaxCacheBatchSize, + options.MaxParallelism, + options.MaxHttpSendBatchSize, + options.MaxCacheBatchSize, _processSource.Token ); var findTotalObjectsTask = Task.CompletedTask; - if (!_options.SkipFindTotalObjects) + if (!options.SkipFindTotalObjects) { ThrowIfFailed(); findTotalObjectsTask = Task.Factory.StartNew( @@ -249,7 +248,7 @@ public sealed class SerializeProcess( return EMPTY_CLOSURES; } - var items = baseSerializer.Serialise(obj, childClosures, _options.SkipCacheRead, _processSource.Token); + var items = baseSerializer.Serialise(obj, childClosures, options.SkipCacheRead, _processSource.Token); if (IsCancelled()) { diff --git a/src/Speckle.Sdk/Serialisation/V2/SerializeProcessFactory.cs b/src/Speckle.Sdk/Serialisation/V2/SerializeProcessFactory.cs index c1428bf9..b708f4d4 100644 --- a/src/Speckle.Sdk/Serialisation/V2/SerializeProcessFactory.cs +++ b/src/Speckle.Sdk/Serialisation/V2/SerializeProcessFactory.cs @@ -44,13 +44,14 @@ public class SerializeProcessFactory( sqLiteJsonCacheManager, serverObjectManager, loggerFactory.CreateLogger(), + options ?? new SerializeProcessOptions(), cancellationToken ), baseChildFinder, new BaseSerializer(sqLiteJsonCacheManager, objectSerializerFactory), loggerFactory, - cancellationToken, - options + options ?? new SerializeProcessOptions(), + cancellationToken ); public ISerializeProcess CreateSerializeProcess( diff --git a/tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.cs b/tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.cs index da6fdef8..998fa055 100644 --- a/tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.cs +++ b/tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.cs @@ -41,7 +41,7 @@ public class DataObjectTests new DummyServerObjectManager(), null, default, - new SerializeProcessOptions(true, true, false, true) + new SerializeProcessOptions(false, false, true, true) ); await serializeProcess.Serialize(x); await VerifyJson(json.Single().Value.Value).UseParameters(type); diff --git a/tests/Speckle.Sdk.Serialization.Tests/DetachedTests.cs b/tests/Speckle.Sdk.Serialization.Tests/DetachedTests.cs index a8664172..f41a5c10 100644 --- a/tests/Speckle.Sdk.Serialization.Tests/DetachedTests.cs +++ b/tests/Speckle.Sdk.Serialization.Tests/DetachedTests.cs @@ -41,7 +41,7 @@ public class DetachedTests objects, null, default, - new SerializeProcessOptions(false, false, true, true) + new SerializeProcessOptions(true, true, false, true) ); await serializeProcess.Serialize(@base); @@ -123,7 +123,7 @@ public class DetachedTests objects, null, default, - new SerializeProcessOptions(false, false, true, true) { MaxParallelism = 1, MaxHttpSendBatchSize = 1 } + new SerializeProcessOptions(true, true, false, true) { MaxParallelism = 1, MaxHttpSendBatchSize = 1 } ); var results = await serializeProcess.Serialize(@base); @@ -150,7 +150,7 @@ public class DetachedTests objects, null, default, - new SerializeProcessOptions(false, false, true, true) { MaxParallelism = 1, MaxHttpSendBatchSize = 1 } + new SerializeProcessOptions(true, true, false, true) { MaxParallelism = 1, MaxHttpSendBatchSize = 1 } ); var results = await serializeProcess.Serialize(@base); @@ -172,7 +172,7 @@ public class DetachedTests objects, null, default, - new SerializeProcessOptions(false, false, true, true) { MaxParallelism = 1, MaxHttpSendBatchSize = 1 } + new SerializeProcessOptions(true, true, false, true) { MaxParallelism = 1, MaxHttpSendBatchSize = 1 } ); var results = await serializeProcess.Serialize(@base); @@ -239,7 +239,7 @@ public class DetachedTests objects, null, default, - new SerializeProcessOptions(false, false, true, true) + new SerializeProcessOptions(true, true, false, true) ); var results = await serializeProcess.Serialize(@base); @@ -272,7 +272,7 @@ public class DetachedTests objects, null, default, - new SerializeProcessOptions(false, false, true, true) + new SerializeProcessOptions(true, true, false, true) ); var results = await serializeProcess.Serialize(@base); await VerifyJsonDictionary(objects); diff --git a/tests/Speckle.Sdk.Tests.Unit/Serialisation/SerializeProcessRecordExceptionTests.cs b/tests/Speckle.Sdk.Tests.Unit/Serialisation/SerializeProcessRecordExceptionTests.cs index 1e855dd7..dbe994c6 100644 --- a/tests/Speckle.Sdk.Tests.Unit/Serialisation/SerializeProcessRecordExceptionTests.cs +++ b/tests/Speckle.Sdk.Tests.Unit/Serialisation/SerializeProcessRecordExceptionTests.cs @@ -31,6 +31,7 @@ public class SerializeProcessRecordExceptionTests : MoqTest baseChildFinderMock.Object, baseSerializerMock.Object, loggerFactoryMock.Object, + new(), cts.Token ); var ex = new Exception("Test error"); @@ -67,6 +68,7 @@ public class SerializeProcessRecordExceptionTests : MoqTest baseChildFinderMock.Object, baseSerializerMock.Object, loggerFactoryMock.Object, + new(), cts.Token ); var ex = new OperationCanceledException(); @@ -98,6 +100,7 @@ public class SerializeProcessRecordExceptionTests : MoqTest baseChildFinderMock.Object, baseSerializerMock.Object, loggerFactoryMock.Object, + new(), cts.Token ); var ex = new AggregateException(new OperationCanceledException());