From 3a9a633d30e86e9511dcc3db37ee6c5c203b7355 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Fri, 9 May 2025 15:51:03 +0100 Subject: [PATCH 1/2] Add empty DataObject tests (#301) * Add empty DataObject tests * format --- ...le.Objects.Data.ArcgisObject.verified.json | 10 ++++ ....Objects.Data.ArchicadObject.verified.json | 11 +++++ ...e.Objects.Data.Civil3dObject.verified.json | 12 +++++ ...ckle.Objects.Data.DataObject.verified.json | 8 +++ ...kle.Objects.Data.EtabsObject.verified.json | 11 +++++ ...bjects.Data.NavisworksObject.verified.json | 9 ++++ ...kle.Objects.Data.RevitObject.verified.json | 15 ++++++ ...kle.Objects.Data.TeklaObject.verified.json | 11 +++++ .../DataObjectTests.cs | 49 +++++++++++++++++++ 9 files changed, 136 insertions(+) create mode 100644 tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.ValidateDataObject_type=Speckle.Objects.Data.ArcgisObject.verified.json create mode 100644 tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.ValidateDataObject_type=Speckle.Objects.Data.ArchicadObject.verified.json create mode 100644 tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.ValidateDataObject_type=Speckle.Objects.Data.Civil3dObject.verified.json create mode 100644 tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.ValidateDataObject_type=Speckle.Objects.Data.DataObject.verified.json create mode 100644 tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.ValidateDataObject_type=Speckle.Objects.Data.EtabsObject.verified.json create mode 100644 tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.ValidateDataObject_type=Speckle.Objects.Data.NavisworksObject.verified.json create mode 100644 tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.ValidateDataObject_type=Speckle.Objects.Data.RevitObject.verified.json create mode 100644 tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.ValidateDataObject_type=Speckle.Objects.Data.TeklaObject.verified.json create mode 100644 tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.cs diff --git a/tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.ValidateDataObject_type=Speckle.Objects.Data.ArcgisObject.verified.json b/tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.ValidateDataObject_type=Speckle.Objects.Data.ArcgisObject.verified.json new file mode 100644 index 00000000..e6693371 --- /dev/null +++ b/tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.ValidateDataObject_type=Speckle.Objects.Data.ArcgisObject.verified.json @@ -0,0 +1,10 @@ +{ + "applicationId": null, + "displayValue": null, + "id": "15168a13ce3f336dee9aa1807cbf375c", + "name": null, + "properties": null, + "speckle_type": "Objects.Data.DataObject:Objects.Data.ArcgisObject", + "type": null, + "units": null +} diff --git a/tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.ValidateDataObject_type=Speckle.Objects.Data.ArchicadObject.verified.json b/tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.ValidateDataObject_type=Speckle.Objects.Data.ArchicadObject.verified.json new file mode 100644 index 00000000..36f3f9c7 --- /dev/null +++ b/tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.ValidateDataObject_type=Speckle.Objects.Data.ArchicadObject.verified.json @@ -0,0 +1,11 @@ +{ + "applicationId": null, + "displayValue": null, + "elements": null, + "id": "bf80e8a10eca2264f11c39bae42538da", + "level": null, + "name": null, + "properties": null, + "speckle_type": "Objects.Data.DataObject:Objects.Data.ArchicadObject", + "type": null +} diff --git a/tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.ValidateDataObject_type=Speckle.Objects.Data.Civil3dObject.verified.json b/tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.ValidateDataObject_type=Speckle.Objects.Data.Civil3dObject.verified.json new file mode 100644 index 00000000..df8662b5 --- /dev/null +++ b/tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.ValidateDataObject_type=Speckle.Objects.Data.Civil3dObject.verified.json @@ -0,0 +1,12 @@ +{ + "applicationId": null, + "baseCurves": null, + "displayValue": null, + "elements": null, + "id": "76b7634117981a9fb9d3cffca5464f26", + "name": null, + "properties": null, + "speckle_type": "Objects.Data.DataObject:Objects.Data.Civil3dObject", + "type": null, + "units": null +} diff --git a/tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.ValidateDataObject_type=Speckle.Objects.Data.DataObject.verified.json b/tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.ValidateDataObject_type=Speckle.Objects.Data.DataObject.verified.json new file mode 100644 index 00000000..cc54e976 --- /dev/null +++ b/tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.ValidateDataObject_type=Speckle.Objects.Data.DataObject.verified.json @@ -0,0 +1,8 @@ +{ + "applicationId": null, + "displayValue": null, + "id": "a1567a1cf10417294c93b70bf5ca97c1", + "name": null, + "properties": null, + "speckle_type": "Objects.Data.DataObject" +} diff --git a/tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.ValidateDataObject_type=Speckle.Objects.Data.EtabsObject.verified.json b/tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.ValidateDataObject_type=Speckle.Objects.Data.EtabsObject.verified.json new file mode 100644 index 00000000..5dbf9a53 --- /dev/null +++ b/tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.ValidateDataObject_type=Speckle.Objects.Data.EtabsObject.verified.json @@ -0,0 +1,11 @@ +{ + "applicationId": null, + "displayValue": null, + "elements": null, + "id": "39d4deaa7cd20e7004812304f41a68d5", + "name": null, + "properties": null, + "speckle_type": "Objects.Data.DataObject:Objects.Data.EtabsObject", + "type": null, + "units": null +} diff --git a/tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.ValidateDataObject_type=Speckle.Objects.Data.NavisworksObject.verified.json b/tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.ValidateDataObject_type=Speckle.Objects.Data.NavisworksObject.verified.json new file mode 100644 index 00000000..3521ef35 --- /dev/null +++ b/tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.ValidateDataObject_type=Speckle.Objects.Data.NavisworksObject.verified.json @@ -0,0 +1,9 @@ +{ + "applicationId": null, + "displayValue": null, + "id": "fbda4ea7bb1b3722ca28e97573742a4e", + "name": null, + "properties": null, + "speckle_type": "Objects.Data.DataObject:Objects.Data.NavisworksObject", + "units": null +} diff --git a/tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.ValidateDataObject_type=Speckle.Objects.Data.RevitObject.verified.json b/tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.ValidateDataObject_type=Speckle.Objects.Data.RevitObject.verified.json new file mode 100644 index 00000000..9fa48788 --- /dev/null +++ b/tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.ValidateDataObject_type=Speckle.Objects.Data.RevitObject.verified.json @@ -0,0 +1,15 @@ +{ + "applicationId": null, + "category": null, + "displayValue": null, + "elements": null, + "family": null, + "id": "7e285508a71c55589bbc053451f687d2", + "level": null, + "location": null, + "name": null, + "properties": null, + "speckle_type": "Objects.Data.DataObject:Objects.Data.RevitObject", + "type": null, + "units": null +} diff --git a/tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.ValidateDataObject_type=Speckle.Objects.Data.TeklaObject.verified.json b/tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.ValidateDataObject_type=Speckle.Objects.Data.TeklaObject.verified.json new file mode 100644 index 00000000..8fe467f9 --- /dev/null +++ b/tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.ValidateDataObject_type=Speckle.Objects.Data.TeklaObject.verified.json @@ -0,0 +1,11 @@ +{ + "applicationId": null, + "displayValue": null, + "elements": null, + "id": "c07f15678d8b4a48a7ecab9eb30e69a8", + "name": null, + "properties": null, + "speckle_type": "Objects.Data.DataObject:Objects.Data.TeklaObject", + "type": null, + "units": null +} diff --git a/tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.cs b/tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.cs new file mode 100644 index 00000000..da6fdef8 --- /dev/null +++ b/tests/Speckle.Sdk.Serialization.Tests/DataObjectTests.cs @@ -0,0 +1,49 @@ +using System.Collections.Concurrent; +using Microsoft.Extensions.DependencyInjection; +using Speckle.Objects.Data; +using Speckle.Objects.Geometry; +using Speckle.Sdk.Models; +using Speckle.Sdk.Serialisation; +using Speckle.Sdk.Serialisation.V2; +using Speckle.Sdk.Serialisation.V2.Send; + +namespace Speckle.Sdk.Serialization.Tests; + +public class DataObjectTests +{ + private readonly ISerializeProcessFactory _factory; + + public DataObjectTests() + { + var serviceCollection = new ServiceCollection(); + serviceCollection.AddSpeckleSdk(new("Tests", "test"), "v3", typeof(TestClass).Assembly, typeof(Polyline).Assembly); + var serviceProvider = serviceCollection.BuildServiceProvider(); + + _factory = serviceProvider.GetRequiredService(); + } + + [Theory] + [InlineData(typeof(ArcgisObject))] + [InlineData(typeof(ArchicadObject))] + [InlineData(typeof(Civil3dObject))] + [InlineData(typeof(DataObject))] + [InlineData(typeof(EtabsObject))] + [InlineData(typeof(NavisworksObject))] + [InlineData(typeof(RevitObject))] + [InlineData(typeof(TeklaObject))] + public async Task ValidateDataObject(Type type) + { + Base x = (Base)(Activator.CreateInstance(type) ?? throw new Exception("Could not create instance of " + type.Name)); + + var json = new ConcurrentDictionary(); + await using var serializeProcess = _factory.CreateSerializeProcess( + new MemoryJsonCacheManager(json), + new DummyServerObjectManager(), + null, + default, + new SerializeProcessOptions(true, true, false, true) + ); + await serializeProcess.Serialize(x); + await VerifyJson(json.Single().Value.Value).UseParameters(type); + } +} From 21851c06d2ee8e7e0e38b88299e125c7f19d5931 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Tue, 13 May 2025 10:00:53 +0100 Subject: [PATCH 2/2] Use WhenAll instead of WhenAny and avoid manual task exception processing (#300) * No reason to process exceptions manually * formatting * use a pool for gathering child task results * Use a smaller whenany with a cancellation * formatting --- .../Serialisation/V2/Send/SerializeProcess.cs | 39 ++++++++++--------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/src/Speckle.Sdk/Serialisation/V2/Send/SerializeProcess.cs b/src/Speckle.Sdk/Serialisation/V2/Send/SerializeProcess.cs index 28fae019..ab7dc7bb 100644 --- a/src/Speckle.Sdk/Serialisation/V2/Send/SerializeProcess.cs +++ b/src/Speckle.Sdk/Serialisation/V2/Send/SerializeProcess.cs @@ -69,6 +69,10 @@ public sealed class SerializeProcess( NodeInfo >(); + private readonly Pool>>> _taskResultPool = Pools.CreateListPool< + Task> + >(); + private long _objectCount; private long _objectsFound; @@ -163,7 +167,7 @@ public sealed class SerializeProcess( try { - var tasks = new List>>(); + var tasks = _taskResultPool.Get(); foreach (var child in baseChildFinder.GetChildren(obj)) { // tmp is necessary because of the way closures close over loop variables @@ -190,30 +194,27 @@ public sealed class SerializeProcess( return EMPTY_CLOSURES; } - List> taskClosures = new(); + Dictionary[] taskClosures = []; if (tasks.Count > 0) { - var currentTasks = tasks.ToList(); - do + //get child results + var childTask = Task.WhenAll(tasks); + await Task.WhenAny(childTask, Task.Delay(Timeout.InfiniteTimeSpan, _processSource.Token)).ConfigureAwait(false); + if (childTask.IsFaulted) { - //grab when any Task is done and see if we're cancelling - var t = await Task.WhenAny(currentTasks).ConfigureAwait(false); - if (t.IsCanceled) + if (childTask.Exception is not null) { - return EMPTY_CLOSURES; + RecordException(childTask.Exception); } - if (t.IsFaulted) - { - if (t.Exception is not null) - { - RecordException(t.Exception); - } - return EMPTY_CLOSURES; - } - taskClosures.Add(t.Result); - currentTasks.Remove(t); - } while (currentTasks.Count > 0); + return EMPTY_CLOSURES; + } + if (!childTask.IsCompleted) + { + return EMPTY_CLOSURES; + } + taskClosures = childTask.Result; } + _taskResultPool.Return(tasks); if (_processSource.Token.IsCancellationRequested) {