From 4c3e572bfe0faefe8334b5e11fbe2ee860d969c2 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Thu, 22 Aug 2024 11:18:16 +0100 Subject: [PATCH] clean up from progress (#78) * progress intermediate commit * add progress for download * remove unused code * remove batch sent callbacks * multi-threaded deserialize works * Progress for download and deserialization * Fix tests * Have less indeterminate deserialization * fix deserialization * make download faster with buffered stream * put local receive back * remove unused callback * fmt * Progress for serialization and upload * fix uploading * clean up from progress * merge fixes and fmt --- src/Speckle.Sdk/Models/Blob.cs | 2 +- src/Speckle.Sdk/Models/HashUtility.cs | 26 ++++++++ src/Speckle.Sdk/Models/Utilities.cs | 61 ------------------- .../Serialisation/BaseObjectDeserializerV2.cs | 13 ++-- .../Serialisation/BaseObjectSerializerV2.cs | 3 +- .../SpeckleDeserializeException.cs | 10 +++ .../SpeckleSerializerException.cs | 17 +----- .../CallsiteCache.cs | 2 +- .../ClosureParser.cs | 2 +- .../DeserializationWorkerThreads.cs | 6 +- .../Serialisation/Utilities/OperationTask.cs | 23 +++++++ .../ParallelOperationExecutor.cs} | 26 +------- .../TypeCache.cs} | 4 +- .../ValueConverter.cs | 2 +- .../ServerUtils/ParallelServerAPI.cs | 2 +- .../ModelPropertySupportedTypes.cs | 2 +- .../SerializationTests.cs | 1 - .../Models/UtilitiesTests.cs | 42 +------------ .../ObjectModelDeprecationTests.cs | 5 +- 19 files changed, 83 insertions(+), 166 deletions(-) create mode 100644 src/Speckle.Sdk/Models/HashUtility.cs delete mode 100644 src/Speckle.Sdk/Models/Utilities.cs create mode 100644 src/Speckle.Sdk/Serialisation/SpeckleDeserializeException.cs rename src/Speckle.Sdk/Serialisation/{SerializationUtilities => Utilities}/CallsiteCache.cs (95%) rename src/Speckle.Sdk/Serialisation/{SerializationUtilities => Utilities}/ClosureParser.cs (97%) rename src/Speckle.Sdk/Serialisation/{SerializationUtilities => Utilities}/DeserializationWorkerThreads.cs (95%) create mode 100644 src/Speckle.Sdk/Serialisation/Utilities/OperationTask.cs rename src/Speckle.Sdk/Serialisation/{SerializationUtilities/OperationTask.cs => Utilities/ParallelOperationExecutor.cs} (63%) rename src/Speckle.Sdk/Serialisation/{SerializationUtilities/BaseObjectSerializationUtilities.cs => Utilities/TypeCache.cs} (94%) rename src/Speckle.Sdk/Serialisation/{SerializationUtilities => Utilities}/ValueConverter.cs (99%) diff --git a/src/Speckle.Sdk/Models/Blob.cs b/src/Speckle.Sdk/Models/Blob.cs index c357a8aa..b9626e63 100644 --- a/src/Speckle.Sdk/Models/Blob.cs +++ b/src/Speckle.Sdk/Models/Blob.cs @@ -49,7 +49,7 @@ public string GetFileHash() { if ((_isHashExpired || _hash == null) && filePath != null) { - _hash = Utilities.HashFile(filePath); + _hash = HashUtility.HashFile(filePath); } return _hash; diff --git a/src/Speckle.Sdk/Models/HashUtility.cs b/src/Speckle.Sdk/Models/HashUtility.cs new file mode 100644 index 00000000..a129aa3b --- /dev/null +++ b/src/Speckle.Sdk/Models/HashUtility.cs @@ -0,0 +1,26 @@ +using System.Diagnostics.CodeAnalysis; +using System.Security.Cryptography; + +namespace Speckle.Sdk.Models; + +public static class HashUtility +{ + public enum HashingFunctions + { + SHA256, + MD5 + } + + public const int HASH_LENGTH = 32; + + [SuppressMessage("Security", "CA5351:Do Not Use Broken Cryptographic Algorithms")] + public static string HashFile(string filePath, HashingFunctions func = HashingFunctions.SHA256) + { + using HashAlgorithm hashAlgorithm = func == HashingFunctions.MD5 ? MD5.Create() : SHA256.Create(); + + using var stream = File.OpenRead(filePath); + + var hash = hashAlgorithm.ComputeHash(stream); + return BitConverter.ToString(hash, 0, HASH_LENGTH).Replace("-", "").ToLowerInvariant(); + } +} diff --git a/src/Speckle.Sdk/Models/Utilities.cs b/src/Speckle.Sdk/Models/Utilities.cs deleted file mode 100644 index 79b42380..00000000 --- a/src/Speckle.Sdk/Models/Utilities.cs +++ /dev/null @@ -1,61 +0,0 @@ -using System.Collections; -using System.Diagnostics.CodeAnalysis; -using System.Security.Cryptography; - -namespace Speckle.Sdk.Models; - -public static class Utilities -{ - public enum HashingFunctions - { - SHA256, - MD5 - } - - public const int HASH_LENGTH = 32; - - [SuppressMessage("Security", "CA5351:Do Not Use Broken Cryptographic Algorithms")] - public static string HashFile(string filePath, HashingFunctions func = HashingFunctions.SHA256) - { - using HashAlgorithm hashAlgorithm = func == HashingFunctions.MD5 ? MD5.Create() : SHA256.Create(); - - using var stream = File.OpenRead(filePath); - - var hash = hashAlgorithm.ComputeHash(stream); - return BitConverter.ToString(hash, 0, HASH_LENGTH).Replace("-", "").ToLowerInvariant(); - } - - /// - /// Utility function to flatten a conversion result that might have nested lists of objects. - /// This happens, for example, in the case of multiple display value fallbacks for a given object. - /// - /// - /// Assuming native objects are not inherited from IList. - /// - /// Object to flatten - /// Flattened objects after to host. - public static List FlattenToHostConversionResult(object? item) - { - List convertedList = new(); - Stack stack = new(); - stack.Push(item); - - while (stack.Count > 0) - { - object? current = stack.Pop(); - if (current is IList list) - { - foreach (object? subItem in list) - { - stack.Push(subItem); - } - } - else - { - convertedList.Add(current); - } - } - - return convertedList; - } -} diff --git a/src/Speckle.Sdk/Serialisation/BaseObjectDeserializerV2.cs b/src/Speckle.Sdk/Serialisation/BaseObjectDeserializerV2.cs index 673e8531..da7c618c 100644 --- a/src/Speckle.Sdk/Serialisation/BaseObjectDeserializerV2.cs +++ b/src/Speckle.Sdk/Serialisation/BaseObjectDeserializerV2.cs @@ -6,7 +6,7 @@ using Speckle.Sdk.Host; using Speckle.Sdk.Logging; using Speckle.Sdk.Models; -using Speckle.Sdk.Serialisation.SerializationUtilities; +using Speckle.Sdk.Serialisation.Utilities; using Speckle.Sdk.Transports; namespace Speckle.Sdk.Serialisation; @@ -15,6 +15,7 @@ public sealed class BaseObjectDeserializerV2 { private bool _isBusy; private readonly object _callbackLock = new(); + private readonly object?[] _invokeNull = [null]; // id -> Base if already deserialized or id -> Task if was handled by a bg thread private Dictionary? _deserializedObjects; @@ -37,9 +38,7 @@ public sealed class BaseObjectDeserializerV2 public string? BlobStorageFolder { get; set; } public TimeSpan Elapsed { get; private set; } - - public static int DefaultNumberThreads => Math.Min(Environment.ProcessorCount, 6); //6 threads seems the sweet spot, see performance test project - public int WorkerThreadCount { get; set; } = DefaultNumberThreads; + public int WorkerThreadCount { get; set; } = Math.Min(Environment.ProcessorCount, 6); //6 threads seems the sweet spot, see performance test project; /// The JSON string of the object to be deserialized /// A typed object deserialized from the @@ -318,7 +317,7 @@ private Base Dict2Base(Dictionary dictObj) dictObj.Remove(TYPE_DISCRIMINATOR); dictObj.Remove("__closure"); - var staticProperties = BaseObjectSerializationUtilities.GetTypeProperties(typeName); + var staticProperties = TypeCache.GetTypeProperties(typeName); foreach (var entry in dictObj) { if (staticProperties.TryGetValue(entry.Key, out PropertyInfo? value) && value.CanWrite) @@ -359,10 +358,10 @@ private Base Dict2Base(Dictionary dictObj) bb.filePath = bb.GetLocalDestinationPath(BlobStorageFolder); } - var onDeserializedCallbacks = BaseObjectSerializationUtilities.GetOnDeserializedCallbacks(typeName); + var onDeserializedCallbacks = TypeCache.GetOnDeserializedCallbacks(typeName); foreach (MethodInfo onDeserialized in onDeserializedCallbacks) { - onDeserialized.Invoke(baseObj, new object?[] { null }); + onDeserialized.Invoke(baseObj, _invokeNull); } return baseObj; diff --git a/src/Speckle.Sdk/Serialisation/BaseObjectSerializerV2.cs b/src/Speckle.Sdk/Serialisation/BaseObjectSerializerV2.cs index ba695995..25a01fc3 100644 --- a/src/Speckle.Sdk/Serialisation/BaseObjectSerializerV2.cs +++ b/src/Speckle.Sdk/Serialisation/BaseObjectSerializerV2.cs @@ -12,7 +12,6 @@ using Speckle.Sdk.Models; using Speckle.Sdk.Transports; using Constants = Speckle.Sdk.Helpers.Constants; -using Utilities = Speckle.Sdk.Models.Utilities; namespace Speckle.Sdk.Serialisation; @@ -438,7 +437,7 @@ private void UpdateParentClosures(string objectId) private static string ComputeId(IReadOnlyDictionary obj) { string serialized = JsonConvert.SerializeObject(obj); - string hash = Crypt.Sha256(serialized, length: Utilities.HASH_LENGTH); + string hash = Crypt.Sha256(serialized, length: HashUtility.HASH_LENGTH); return hash; } diff --git a/src/Speckle.Sdk/Serialisation/SpeckleDeserializeException.cs b/src/Speckle.Sdk/Serialisation/SpeckleDeserializeException.cs new file mode 100644 index 00000000..4ab2cec3 --- /dev/null +++ b/src/Speckle.Sdk/Serialisation/SpeckleDeserializeException.cs @@ -0,0 +1,10 @@ +namespace Speckle.Sdk.Serialisation; + +public class SpeckleDeserializeException : SpeckleException +{ + public SpeckleDeserializeException(string message, Exception? inner = null) + : base(message, inner) { } + + public SpeckleDeserializeException(string message) + : base(message) { } +} diff --git a/src/Speckle.Sdk/Serialisation/SpeckleSerializerException.cs b/src/Speckle.Sdk/Serialisation/SpeckleSerializerException.cs index b6fec905..68704da2 100644 --- a/src/Speckle.Sdk/Serialisation/SpeckleSerializerException.cs +++ b/src/Speckle.Sdk/Serialisation/SpeckleSerializerException.cs @@ -1,25 +1,10 @@ -using Speckle.Sdk.Logging; - -namespace Speckle.Sdk.Serialisation; +namespace Speckle.Sdk.Serialisation; public class SpeckleSerializeException : SpeckleException { - public SpeckleSerializeException() { } - public SpeckleSerializeException(string message, Exception? inner = null) : base(message, inner) { } public SpeckleSerializeException(string message) : base(message) { } } - -public class SpeckleDeserializeException : SpeckleException -{ - public SpeckleDeserializeException() { } - - public SpeckleDeserializeException(string message, Exception? inner = null) - : base(message, inner) { } - - public SpeckleDeserializeException(string message) - : base(message) { } -} diff --git a/src/Speckle.Sdk/Serialisation/SerializationUtilities/CallsiteCache.cs b/src/Speckle.Sdk/Serialisation/Utilities/CallsiteCache.cs similarity index 95% rename from src/Speckle.Sdk/Serialisation/SerializationUtilities/CallsiteCache.cs rename to src/Speckle.Sdk/Serialisation/Utilities/CallsiteCache.cs index 4b0e3f24..a6b31778 100644 --- a/src/Speckle.Sdk/Serialisation/SerializationUtilities/CallsiteCache.cs +++ b/src/Speckle.Sdk/Serialisation/Utilities/CallsiteCache.cs @@ -2,7 +2,7 @@ using System.Runtime.CompilerServices; using Microsoft.CSharp.RuntimeBinder; -namespace Speckle.Sdk.Serialisation.SerializationUtilities; +namespace Speckle.Sdk.Serialisation.Utilities; internal static class CallSiteCache { diff --git a/src/Speckle.Sdk/Serialisation/SerializationUtilities/ClosureParser.cs b/src/Speckle.Sdk/Serialisation/Utilities/ClosureParser.cs similarity index 97% rename from src/Speckle.Sdk/Serialisation/SerializationUtilities/ClosureParser.cs rename to src/Speckle.Sdk/Serialisation/Utilities/ClosureParser.cs index 1c5d0680..a586f96a 100644 --- a/src/Speckle.Sdk/Serialisation/SerializationUtilities/ClosureParser.cs +++ b/src/Speckle.Sdk/Serialisation/Utilities/ClosureParser.cs @@ -1,7 +1,7 @@ using Speckle.Newtonsoft.Json; using Speckle.Sdk.Common; -namespace Speckle.Sdk.Serialisation.SerializationUtilities; +namespace Speckle.Sdk.Serialisation.Utilities; public static class ClosureParser { diff --git a/src/Speckle.Sdk/Serialisation/SerializationUtilities/DeserializationWorkerThreads.cs b/src/Speckle.Sdk/Serialisation/Utilities/DeserializationWorkerThreads.cs similarity index 95% rename from src/Speckle.Sdk/Serialisation/SerializationUtilities/DeserializationWorkerThreads.cs rename to src/Speckle.Sdk/Serialisation/Utilities/DeserializationWorkerThreads.cs index af7d463f..33684f50 100644 --- a/src/Speckle.Sdk/Serialisation/SerializationUtilities/DeserializationWorkerThreads.cs +++ b/src/Speckle.Sdk/Serialisation/Utilities/DeserializationWorkerThreads.cs @@ -1,6 +1,6 @@ -using Speckle.Sdk.Logging; +using Speckle.Sdk.Common; -namespace Speckle.Sdk.Serialisation.SerializationUtilities; +namespace Speckle.Sdk.Serialisation.Utilities; internal enum WorkerThreadTaskType { @@ -48,7 +48,7 @@ protected override void ThreadMain() try { - (string objectJson, long? current, long? total) = ((string, long?, long?))inputValue!; + (string objectJson, long? current, long? total) = ((string, long?, long?))inputValue.NotNull(); var result = RunOperation(taskType, objectJson, current, total, _serializer); tcs.SetResult(result); } diff --git a/src/Speckle.Sdk/Serialisation/Utilities/OperationTask.cs b/src/Speckle.Sdk/Serialisation/Utilities/OperationTask.cs new file mode 100644 index 00000000..68261ce1 --- /dev/null +++ b/src/Speckle.Sdk/Serialisation/Utilities/OperationTask.cs @@ -0,0 +1,23 @@ +namespace Speckle.Sdk.Serialisation.Utilities; + +internal readonly struct OperationTask + where T : struct +{ + public readonly T OperationType; + public readonly object? InputValue; + public readonly TaskCompletionSource? Tcs; + + public OperationTask(T operationType, object? inputValue = null, TaskCompletionSource? tcs = null) + { + OperationType = operationType; + InputValue = inputValue; + Tcs = tcs; + } + + public void Deconstruct(out T operationType, out object? inputValue, out TaskCompletionSource? tcs) + { + operationType = OperationType; + inputValue = InputValue; + tcs = Tcs; + } +} diff --git a/src/Speckle.Sdk/Serialisation/SerializationUtilities/OperationTask.cs b/src/Speckle.Sdk/Serialisation/Utilities/ParallelOperationExecutor.cs similarity index 63% rename from src/Speckle.Sdk/Serialisation/SerializationUtilities/OperationTask.cs rename to src/Speckle.Sdk/Serialisation/Utilities/ParallelOperationExecutor.cs index 9cc373e7..0598e937 100644 --- a/src/Speckle.Sdk/Serialisation/SerializationUtilities/OperationTask.cs +++ b/src/Speckle.Sdk/Serialisation/Utilities/ParallelOperationExecutor.cs @@ -1,28 +1,6 @@ -using System.Collections.Concurrent; +using System.Collections.Concurrent; -namespace Speckle.Sdk.Serialisation.SerializationUtilities; - -internal readonly struct OperationTask - where T : struct -{ - public readonly T OperationType; - public readonly object? InputValue; - public readonly TaskCompletionSource? Tcs; - - public OperationTask(T operationType, object? inputValue = null, TaskCompletionSource? tcs = null) - { - OperationType = operationType; - InputValue = inputValue; - Tcs = tcs; - } - - public void Deconstruct(out T operationType, out object? inputValue, out TaskCompletionSource? tcs) - { - operationType = OperationType; - inputValue = InputValue; - tcs = Tcs; - } -} +namespace Speckle.Sdk.Serialisation.Utilities; internal abstract class ParallelOperationExecutor : IDisposable where TOperation : struct diff --git a/src/Speckle.Sdk/Serialisation/SerializationUtilities/BaseObjectSerializationUtilities.cs b/src/Speckle.Sdk/Serialisation/Utilities/TypeCache.cs similarity index 94% rename from src/Speckle.Sdk/Serialisation/SerializationUtilities/BaseObjectSerializationUtilities.cs rename to src/Speckle.Sdk/Serialisation/Utilities/TypeCache.cs index 4e51ed93..e4d65aa0 100644 --- a/src/Speckle.Sdk/Serialisation/SerializationUtilities/BaseObjectSerializationUtilities.cs +++ b/src/Speckle.Sdk/Serialisation/Utilities/TypeCache.cs @@ -3,9 +3,9 @@ using System.Runtime.Serialization; using Speckle.Sdk.Host; -namespace Speckle.Sdk.Serialisation.SerializationUtilities; +namespace Speckle.Sdk.Serialisation.Utilities; -internal static class BaseObjectSerializationUtilities +internal static class TypeCache { #region Getting Types private static ConcurrentDictionary> s_typeProperties = new(); diff --git a/src/Speckle.Sdk/Serialisation/SerializationUtilities/ValueConverter.cs b/src/Speckle.Sdk/Serialisation/Utilities/ValueConverter.cs similarity index 99% rename from src/Speckle.Sdk/Serialisation/SerializationUtilities/ValueConverter.cs rename to src/Speckle.Sdk/Serialisation/Utilities/ValueConverter.cs index 0277ebde..53692ddf 100644 --- a/src/Speckle.Sdk/Serialisation/SerializationUtilities/ValueConverter.cs +++ b/src/Speckle.Sdk/Serialisation/Utilities/ValueConverter.cs @@ -6,7 +6,7 @@ using Speckle.DoubleNumerics; using Speckle.Sdk.Logging; -namespace Speckle.Sdk.Serialisation.SerializationUtilities; +namespace Speckle.Sdk.Serialisation.Utilities; internal static class ValueConverter { diff --git a/src/Speckle.Sdk/Transports/ServerUtils/ParallelServerAPI.cs b/src/Speckle.Sdk/Transports/ServerUtils/ParallelServerAPI.cs index 34bb6bb7..b2772553 100644 --- a/src/Speckle.Sdk/Transports/ServerUtils/ParallelServerAPI.cs +++ b/src/Speckle.Sdk/Transports/ServerUtils/ParallelServerAPI.cs @@ -3,7 +3,7 @@ using Speckle.Sdk.Common; using Speckle.Sdk.Helpers; using Speckle.Sdk.Logging; -using Speckle.Sdk.Serialisation.SerializationUtilities; +using Speckle.Sdk.Serialisation.Utilities; namespace Speckle.Sdk.Transports.ServerUtils; diff --git a/tests/Speckle.Objects.Tests.Unit/ModelPropertySupportedTypes.cs b/tests/Speckle.Objects.Tests.Unit/ModelPropertySupportedTypes.cs index c647daf5..40f4ff8f 100644 --- a/tests/Speckle.Objects.Tests.Unit/ModelPropertySupportedTypes.cs +++ b/tests/Speckle.Objects.Tests.Unit/ModelPropertySupportedTypes.cs @@ -34,7 +34,7 @@ public void Setup() /// /// /// If you're tempted to add to this list, please ensure both our serializer and deserializer support properties of this type - /// Check the + /// Check the /// Check the /// (or is an interface where all concrete types are supported) /// You should also consider adding a test in SerializerNonBreakingChanges diff --git a/tests/Speckle.Sdk.Serialization.Tests/SerializationTests.cs b/tests/Speckle.Sdk.Serialization.Tests/SerializationTests.cs index d89ab2b3..202607c5 100644 --- a/tests/Speckle.Sdk.Serialization.Tests/SerializationTests.cs +++ b/tests/Speckle.Sdk.Serialization.Tests/SerializationTests.cs @@ -7,7 +7,6 @@ using Speckle.Sdk.Host; using Speckle.Sdk.Models; using Speckle.Sdk.Serialisation; -using Speckle.Sdk.Serialisation.SerializationUtilities; namespace Speckle.Sdk.Serialization.Tests; diff --git a/tests/Speckle.Sdk.Tests.Unit/Models/UtilitiesTests.cs b/tests/Speckle.Sdk.Tests.Unit/Models/UtilitiesTests.cs index 12ca7787..9ff1fffd 100644 --- a/tests/Speckle.Sdk.Tests.Unit/Models/UtilitiesTests.cs +++ b/tests/Speckle.Sdk.Tests.Unit/Models/UtilitiesTests.cs @@ -5,7 +5,7 @@ namespace Speckle.Sdk.Tests.Unit.Models; [TestFixture(TestOf = typeof(Crypt))] -public sealed class UtilitiesTests +public sealed class HashUtilityTests { [Test] [TestOf(nameof(Crypt.Md5))] @@ -29,44 +29,4 @@ public void Sha256(string input, string expected) Assert.That(lower, Is.EqualTo(expected.ToLower())); Assert.That(upper, Is.EqualTo(expected.ToUpper())); } - - [Test] - public void FlattenToNativeConversion() - { - var singleObject = new object(); - var nestedObjects = new List() - { - new List() - { - new(), // obj 1 - new() // obj 2 - }, - new() // obj 3 - }; - - var testEnum = new List() { new(), new() }.Select(o => o); - - var nestedObjectsWithEnumerableInherited = new List() - { - new List() - { - new(), // obj 1 - new(), // obj 2 - testEnum // obj 3 - }, - new() // obj 4 - }; - - var parentTestEnumFlattened = Utilities.FlattenToHostConversionResult(testEnum); - var singleObjectFlattened = Utilities.FlattenToHostConversionResult(singleObject); - var nestedObjectsFlattened = Utilities.FlattenToHostConversionResult(nestedObjects); - var nestedObjectsWithEnumerableInheritedFlattened = Utilities.FlattenToHostConversionResult( - nestedObjectsWithEnumerableInherited - ); - - Assert.That(parentTestEnumFlattened.Count, Is.EqualTo(1)); - Assert.That(singleObjectFlattened.Count, Is.EqualTo(1)); - Assert.That(nestedObjectsFlattened.Count, Is.EqualTo(3)); - Assert.That(nestedObjectsWithEnumerableInheritedFlattened.Count, Is.EqualTo(4)); - } } diff --git a/tests/Speckle.Sdk.Tests.Unit/Serialisation/ObjectModelDeprecationTests.cs b/tests/Speckle.Sdk.Tests.Unit/Serialisation/ObjectModelDeprecationTests.cs index 70b4e016..7c1437bc 100644 --- a/tests/Speckle.Sdk.Tests.Unit/Serialisation/ObjectModelDeprecationTests.cs +++ b/tests/Speckle.Sdk.Tests.Unit/Serialisation/ObjectModelDeprecationTests.cs @@ -3,13 +3,12 @@ using Speckle.Sdk.Host; using Speckle.Sdk.Models; using Speckle.Sdk.Serialisation.Deprecated; -using Speckle.Sdk.Serialisation.SerializationUtilities; namespace Speckle.Sdk.Tests.Unit.Serialisation { [TestFixture] - [TestOf(typeof(BaseObjectSerializationUtilities))] - public class ObjectModelDeprecationTests + [TestOf(typeof(TypeLoader))] + public class TypeLoaderTests { [SetUp] public void Setup()