From daec64cf8e2b73a0982a4def755a66da7ba9a8d7 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Tue, 3 Dec 2024 12:48:18 +0000 Subject: [PATCH] Remove the Base cache because it's not practically useful (#183) --- .../Serialisation/V2/Send/ObjectSerializer.cs | 31 +++++-------------- .../V2/Send/ObjectSerializerFactory.cs | 5 ++- .../Serialisation/V2/Send/SerializeProcess.cs | 9 ++---- .../Program.cs | 2 +- .../DetachedTests.cs | 12 +++---- .../ExternalIdTests.cs | 27 +++------------- .../SerializationTests.cs | 2 +- 7 files changed, 23 insertions(+), 65 deletions(-) diff --git a/src/Speckle.Sdk/Serialisation/V2/Send/ObjectSerializer.cs b/src/Speckle.Sdk/Serialisation/V2/Send/ObjectSerializer.cs index b427094e..b6eada74 100644 --- a/src/Speckle.Sdk/Serialisation/V2/Send/ObjectSerializer.cs +++ b/src/Speckle.Sdk/Serialisation/V2/Send/ObjectSerializer.cs @@ -20,7 +20,6 @@ public class ObjectSerializer : IObjectSerializer { private HashSet _parentObjects = new(); private readonly Dictionary _currentClosures = new(); - private readonly IDictionary _baseCache; private readonly bool _trackDetachedChildren; private readonly IBasePropertyGatherer _propertyGatherer; @@ -41,12 +40,10 @@ public class ObjectSerializer : IObjectSerializer /// public ObjectSerializer( IBasePropertyGatherer propertyGatherer, - IDictionary baseCache, bool trackDetachedChildren = false, CancellationToken cancellationToken = default ) { - _baseCache = baseCache; _propertyGatherer = propertyGatherer; _cancellationToken = cancellationToken; _trackDetachedChildren = trackDetachedChildren; @@ -69,7 +66,6 @@ public ObjectSerializer( { throw new SpeckleSerializeException($"Failed to extract (pre-serialize) properties from the {baseObj}", ex); } - _baseCache[baseObj] = new(item.Item2, _currentClosures); yield return (item.Item1, item.Item2); foreach (var chunk in _chunks) { @@ -232,26 +228,13 @@ private void SerializeProperty(object? obj, JsonWriter writer, PropertyAttribute return null; } - Closures childClosures; - Id id; - Json json; - if (_baseCache.TryGetValue(baseObj, out var info)) - { - id = new Id(baseObj.id); - childClosures = info.Closures; - json = info.Json; - MergeClosures(_currentClosures, childClosures); - } - else - { - childClosures = isRoot || inheritedDetachInfo.IsDetachable ? _currentClosures : []; - var sb = Pools.StringBuilders.Get(); - using var writer = new StringWriter(sb); - using var jsonWriter = SpeckleObjectSerializerPool.Instance.GetJsonTextWriter(writer); - id = SerializeBaseObject(baseObj, jsonWriter, childClosures); - json = new Json(writer.ToString()); - Pools.StringBuilders.Return(sb); - } + var childClosures = isRoot || inheritedDetachInfo.IsDetachable ? _currentClosures : []; + var sb = Pools.StringBuilders.Get(); + using var writer = new StringWriter(sb); + using var jsonWriter = SpeckleObjectSerializerPool.Instance.GetJsonTextWriter(writer); + var id = SerializeBaseObject(baseObj, jsonWriter, childClosures); + var json = new Json(writer.ToString()); + Pools.StringBuilders.Return(sb); _parentObjects.Remove(baseObj); diff --git a/src/Speckle.Sdk/Serialisation/V2/Send/ObjectSerializerFactory.cs b/src/Speckle.Sdk/Serialisation/V2/Send/ObjectSerializerFactory.cs index 2fa75d05..77f3b6f9 100644 --- a/src/Speckle.Sdk/Serialisation/V2/Send/ObjectSerializerFactory.cs +++ b/src/Speckle.Sdk/Serialisation/V2/Send/ObjectSerializerFactory.cs @@ -1,11 +1,10 @@ using Speckle.InterfaceGenerator; -using Speckle.Sdk.Models; namespace Speckle.Sdk.Serialisation.V2.Send; [GenerateAutoInterface] public class ObjectSerializerFactory(IBasePropertyGatherer propertyGatherer) : IObjectSerializerFactory { - public IObjectSerializer Create(IDictionary baseCache, CancellationToken cancellationToken) => - new ObjectSerializer(propertyGatherer, baseCache, true, cancellationToken); + public IObjectSerializer Create(CancellationToken cancellationToken) => + new ObjectSerializer(propertyGatherer, true, cancellationToken); } diff --git a/src/Speckle.Sdk/Serialisation/V2/Send/SerializeProcess.cs b/src/Speckle.Sdk/Serialisation/V2/Send/SerializeProcess.cs index 40186bea..5e11205a 100644 --- a/src/Speckle.Sdk/Serialisation/V2/Send/SerializeProcess.cs +++ b/src/Speckle.Sdk/Serialisation/V2/Send/SerializeProcess.cs @@ -9,7 +9,7 @@ namespace Speckle.Sdk.Serialisation.V2.Send; -public record SerializeProcessOptions(bool SkipCacheRead, bool SkipCacheWrite, bool SkipServer, bool CacheBases); +public record SerializeProcessOptions(bool SkipCacheRead, bool SkipCacheWrite, bool SkipServer); public readonly record struct SerializeProcessResults( string RootId, @@ -26,11 +26,8 @@ public class SerializeProcess( SerializeProcessOptions? options = null ) : ChannelSaver, ISerializeProcess { - private readonly SerializeProcessOptions _options = options ?? new(false, false, false, true); + private readonly SerializeProcessOptions _options = options ?? new(false, false, false); private readonly ConcurrentDictionary _jsonCache = new(); - - private readonly IDictionary _baseCache = - options?.CacheBases ?? true ? new ConcurrentDictionary() : new EmptyDictionary(); private readonly ConcurrentDictionary _objectReferences = new(); private long _totalFound; @@ -110,7 +107,7 @@ private IEnumerable Serialise(Base obj, CancellationToken cancellation } if (id is null || !_jsonCache.TryGetValue(id.Value, out var json)) { - var serializer2 = objectSerializerFactory.Create(_baseCache, cancellationToken); + var serializer2 = objectSerializerFactory.Create(cancellationToken); var items = serializer2.Serialize(obj).ToList(); foreach (var kvp in serializer2.ObjectReferences) { diff --git a/tests/Speckle.Sdk.Serialization.Testing/Program.cs b/tests/Speckle.Sdk.Serialization.Testing/Program.cs index b807f795..5df2d34d 100644 --- a/tests/Speckle.Sdk.Serialization.Testing/Program.cs +++ b/tests/Speckle.Sdk.Serialization.Testing/Program.cs @@ -61,7 +61,7 @@ streamId, token, progress, - new SerializeProcessOptions(skipCacheSendCheck, skipCacheSendSave, true, true) + new SerializeProcessOptions(skipCacheSendCheck, skipCacheSendSave, true) ); await process2.Serialize(@base, default).ConfigureAwait(false); Console.WriteLine("Detach"); diff --git a/tests/Speckle.Sdk.Serialization.Tests/DetachedTests.cs b/tests/Speckle.Sdk.Serialization.Tests/DetachedTests.cs index dabdda56..54316cd2 100644 --- a/tests/Speckle.Sdk.Serialization.Tests/DetachedTests.cs +++ b/tests/Speckle.Sdk.Serialization.Tests/DetachedTests.cs @@ -26,9 +26,7 @@ public void Setup() } [Test(Description = "Checks that all typed properties (including obsolete ones) are returned")] - [TestCase(true)] - [TestCase(false)] - public async Task CanSerialize_New_Detached(bool cacheBases) + public async Task CanSerialize_New_Detached() { var expectedJson = """ { @@ -76,7 +74,7 @@ public async Task CanSerialize_New_Detached(bool cacheBases) new DummyServerObjectManager(), new BaseChildFinder(new BasePropertyGatherer()), new ObjectSerializerFactory(new BasePropertyGatherer()), - new SerializeProcessOptions(false, false, true, cacheBases) + new SerializeProcessOptions(false, false, true) ); await process2.Serialize(@base, default).ConfigureAwait(false); @@ -193,9 +191,7 @@ public void GetPropertiesExpected_All() } [Test(Description = "Checks that all typed properties (including obsolete ones) are returned")] - [TestCase(true)] - [TestCase(false)] - public async Task CanSerialize_New_Detached2(bool cacheBases) + public async Task CanSerialize_New_Detached2() { var root = """ @@ -270,7 +266,7 @@ public async Task CanSerialize_New_Detached2(bool cacheBases) new DummyServerObjectManager(), new BaseChildFinder(new BasePropertyGatherer()), new ObjectSerializerFactory(new BasePropertyGatherer()), - new SerializeProcessOptions(false, false, true, cacheBases) + new SerializeProcessOptions(false, false, true) ); var results = await process2.Serialize(@base, default).ConfigureAwait(false); diff --git a/tests/Speckle.Sdk.Serialization.Tests/ExternalIdTests.cs b/tests/Speckle.Sdk.Serialization.Tests/ExternalIdTests.cs index c9a110e3..cde6aac0 100644 --- a/tests/Speckle.Sdk.Serialization.Tests/ExternalIdTests.cs +++ b/tests/Speckle.Sdk.Serialization.Tests/ExternalIdTests.cs @@ -1,5 +1,4 @@ -using System.Collections.Concurrent; -using NUnit.Framework; +using NUnit.Framework; using Shouldly; using Speckle.Newtonsoft.Json.Linq; using Speckle.Objects.Geometry; @@ -27,11 +26,7 @@ public void Setup() public void ExternalIdTest_Detached(string lineId, string valueId) { var p = new Polyline() { units = "cm", value = [1, 2] }; - var serializer = new ObjectSerializer( - new BasePropertyGatherer(), - new ConcurrentDictionary(), - true - ); + var serializer = new ObjectSerializer(new BasePropertyGatherer(), true); var list = serializer.Serialize(p).ToDictionary(x => x.Item1, x => x.Item2); list.ContainsKey(new Id(lineId)).ShouldBeTrue(); var json = list[new Id(lineId)]; @@ -58,11 +53,7 @@ public void ExternalIdTest_Detached_Nested(string lineId, string valueId) knots = [], weights = [], }; - var serializer = new ObjectSerializer( - new BasePropertyGatherer(), - new ConcurrentDictionary(), - true - ); + var serializer = new ObjectSerializer(new BasePropertyGatherer(), true); var list = serializer.Serialize(curve).ToDictionary(x => x.Item1, x => x.Item2); list.ContainsKey(new Id(lineId)).ShouldBeTrue(); var json = list[new Id(lineId)]; @@ -90,11 +81,7 @@ public void ExternalIdTest_Detached_Nested_More(string lineId, string valueId) weights = [], }; var polycurve = new Polycurve() { segments = [curve], units = "cm" }; - var serializer = new ObjectSerializer( - new BasePropertyGatherer(), - new ConcurrentDictionary(), - true - ); + var serializer = new ObjectSerializer(new BasePropertyGatherer(), true); var list = serializer.Serialize(polycurve).ToDictionary(x => x.Item1, x => x.Item2); list.ContainsKey(new Id(lineId)).ShouldBeTrue(); var json = list[new Id(lineId)]; @@ -124,11 +111,7 @@ public void ExternalIdTest_Detached_Nested_More_Too(string lineId, string valueI var polycurve = new Polycurve() { segments = [curve], units = "cm" }; var @base = new Base(); @base.SetDetachedProp("profile", polycurve); - var serializer = new ObjectSerializer( - new BasePropertyGatherer(), - new ConcurrentDictionary(), - true - ); + var serializer = new ObjectSerializer(new BasePropertyGatherer(), true); var list = serializer.Serialize(@base).ToDictionary(x => x.Item1, x => x.Item2); list.ContainsKey(new Id(lineId)).ShouldBeTrue(); var json = list[new Id(lineId)]; diff --git a/tests/Speckle.Sdk.Serialization.Tests/SerializationTests.cs b/tests/Speckle.Sdk.Serialization.Tests/SerializationTests.cs index 70cd8ba4..cfd18f63 100644 --- a/tests/Speckle.Sdk.Serialization.Tests/SerializationTests.cs +++ b/tests/Speckle.Sdk.Serialization.Tests/SerializationTests.cs @@ -263,7 +263,7 @@ public async Task Roundtrip_Test_New(string fileName, string rootId, int oldCoun new DummySendServerObjectManager(newIdToJson), new BaseChildFinder(new BasePropertyGatherer()), new ObjectSerializerFactory(new BasePropertyGatherer()), - new SerializeProcessOptions(true, true, false, true) + new SerializeProcessOptions(true, true, false) ); var (rootId2, _) = await serializeProcess.Serialize(root, default);