From 1039e75d0c7331bbe1c8a37f1ed21784825abcdd Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Tue, 27 May 2025 14:05:10 +0100 Subject: [PATCH] Calculate closures correctly (#309) * Maybe really fixes closures * fornat * add ai generated tests * fix tests * fix tests * added test with correct number of closures? * closures are self contained. don't increment on attached properties * format * MergeClosure should reuse if exists, not just set * add not null on a method --- .github/copilot-instructions.md | 14 ++ .github/git-commit-instructions.md | 22 +++ Speckle.Sdk.slnx | 4 +- src/Speckle.Sdk/Common/NotNullExtensions.cs | 18 ++ .../SQLite/SQLiteJsonCacheManager.cs | 53 ++++-- .../Serialisation/V2/Send/ClosureMath.cs | 58 +++++++ .../Serialisation/V2/Send/ObjectSerializer.cs | 154 +++++++++--------- ...peckle.Objects.Geometry.Mesh.verified.json | 6 +- ...Tests.Cancellation_Traversal.verified.json | 15 ++ .../AdditionalCancellationTests.cs | 96 +++++++++++ ...dTests.CanSerialize_Attached.verified.json | 65 ++++++++ ...ests.CanSerialize_Attached_2.verified.json | 21 +++ ...ts.CanSerialize_New_Detached.verified.json | 2 +- ...s.CanSerialize_New_Detached2.verified.json | 36 ++-- ...ew_Detached_With_DataChunks2.verified.json | 4 +- .../DetachedTests.cs | 55 ++++++- ...ests.ExternalIdTest_Detached.verified.json | 2 +- ...ternalIdTest_Detached_Nested.verified.json | 6 +- ...lIdTest_Detached_Nested_More.verified.json | 6 +- ...est_Detached_Nested_More_Too.verified.json | 12 +- .../Common/NotNullTests.cs | 40 +++++ .../SQLite/SQLiteJsonCacheManagerTests.cs | 70 +++++++- 22 files changed, 619 insertions(+), 140 deletions(-) create mode 100644 .github/copilot-instructions.md create mode 100644 .github/git-commit-instructions.md create mode 100644 src/Speckle.Sdk/Serialisation/V2/Send/ClosureMath.cs create mode 100644 tests/Speckle.Sdk.Serialization.Tests/AdditionalCancellationTests.Cancellation_Traversal.verified.json create mode 100644 tests/Speckle.Sdk.Serialization.Tests/AdditionalCancellationTests.cs create mode 100644 tests/Speckle.Sdk.Serialization.Tests/DetachedTests.CanSerialize_Attached.verified.json create mode 100644 tests/Speckle.Sdk.Serialization.Tests/DetachedTests.CanSerialize_Attached_2.verified.json diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 00000000..542428eb --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,14 @@ +# Coding standards, domain knowledge, and preferences that AI should follow + +## C# Coding Standards + +- Use the csharpier formatter for formatting C# code. +- Use the .editorconfig file for code style settings. +- Always use `var` when the type is obvious from the right side of the assignment. +- Always add braces for `if`, `else`, `for`, `foreach`, `while`, and `do` statements, even if they are single-line statements. + +## Testing + +- Use xUnit for unit testing. +- Use FluentAssertions for assertions in tests. +- Use Moq for mocking dependencies in tests. diff --git a/.github/git-commit-instructions.md b/.github/git-commit-instructions.md new file mode 100644 index 00000000..048149c4 --- /dev/null +++ b/.github/git-commit-instructions.md @@ -0,0 +1,22 @@ +# Git Commit Instructions + +To ensure high-quality and consistent commits, please follow these guidelines: + +1. **Format your code** + - Run the `csharpier` formatter on all C# files before committing. + - Ensure your code adheres to the `.editorconfig` settings. + +2. **Write clear commit messages** + - Use the present tense ("Add feature" not "Added feature"). + - Start with a short summary (max 72 characters), followed by a blank line and a detailed description if necessary. + +3. **Test your changes** + - Run all unit tests before committing. + - Add or update xUnit tests as needed. + - Use FluentAssertions for assertions and Moq for mocking in tests. + +4. **Review your changes** + - Double-check for accidental debug code or commented-out code. + - Ensure only relevant files are staged. + +Thank you for helping maintain code quality! diff --git a/Speckle.Sdk.slnx b/Speckle.Sdk.slnx index 722954ee..f5111e37 100644 --- a/Speckle.Sdk.slnx +++ b/Speckle.Sdk.slnx @@ -14,6 +14,8 @@ + + @@ -35,4 +37,4 @@ - + \ No newline at end of file diff --git a/src/Speckle.Sdk/Common/NotNullExtensions.cs b/src/Speckle.Sdk/Common/NotNullExtensions.cs index 7d07c704..b98216d5 100644 --- a/src/Speckle.Sdk/Common/NotNullExtensions.cs +++ b/src/Speckle.Sdk/Common/NotNullExtensions.cs @@ -95,4 +95,22 @@ public static class NotNullExtensions } return obj; } + + public static string NotNullOrWhiteSpace( + [NotNull] this string? value, + [CallerArgumentExpression(nameof(value))] string? paramName = null + ) + { + if (value is null) + { + throw new ArgumentNullException(paramName ?? "Value is null"); + } + + if (string.IsNullOrWhiteSpace(value)) + { + throw new ArgumentException("Value cannot be empty or whitespace.", paramName); + } + + return value; + } } diff --git a/src/Speckle.Sdk/SQLite/SQLiteJsonCacheManager.cs b/src/Speckle.Sdk/SQLite/SQLiteJsonCacheManager.cs index 58710224..a2a39c4b 100644 --- a/src/Speckle.Sdk/SQLite/SQLiteJsonCacheManager.cs +++ b/src/Speckle.Sdk/SQLite/SQLiteJsonCacheManager.cs @@ -1,6 +1,7 @@ using System.Text; using Microsoft.Data.Sqlite; using Speckle.InterfaceGenerator; +using Speckle.Sdk.Common; using Speckle.Sdk.Dependencies; namespace Speckle.Sdk.SQLite; @@ -120,7 +121,10 @@ public sealed class SqLiteJsonCacheManager : ISqLiteJsonCacheManager ); //This does an insert or ignores if already exists - public void SaveObject(string id, string json) => + public void SaveObject(string id, string json) + { + id.NotNullOrWhiteSpace(); + json.NotNullOrWhiteSpace(); _pool.Use( CacheOperation.InsertOrIgnore, command => @@ -130,6 +134,7 @@ public sealed class SqLiteJsonCacheManager : ISqLiteJsonCacheManager command.ExecuteNonQuery(); } ); + } //This does an insert or replaces if already exists public void UpdateObject(string id, string json) => @@ -148,29 +153,45 @@ public sealed class SqLiteJsonCacheManager : ISqLiteJsonCacheManager CacheOperation.BulkInsertOrIgnore, cmd => { - CreateBulkInsert(cmd, items); - return cmd.ExecuteNonQuery(); + if (CreateBulkInsert(cmd, items)) + { + cmd.ExecuteNonQuery(); + } } ); - private void CreateBulkInsert(SqliteCommand cmd, IEnumerable<(string id, string json)> items) + private bool CreateBulkInsert(SqliteCommand cmd, IEnumerable<(string id, string json)> items) { StringBuilder sb = Pools.StringBuilders.Get(); - sb.AppendLine(CacheDbCommands.Commands[(int)CacheOperation.BulkInsertOrIgnore]); - int i = 0; - foreach (var (id, json) in items) + try { - sb.Append($"(@key{i}, @value{i}),"); - cmd.Parameters.AddWithValue($"@key{i}", id); - cmd.Parameters.AddWithValue($"@value{i}", json); - i++; - } - sb.Remove(sb.Length - 1, 1); - sb.Append(';'); + sb.AppendLine(CacheDbCommands.Commands[(int)CacheOperation.BulkInsertOrIgnore]); + int i = 0; + foreach (var (id, json) in items) + { + sb.Append($"(@key{i}, @value{i}),"); + cmd.Parameters.AddWithValue($"@key{i}", id); + cmd.Parameters.AddWithValue($"@value{i}", json); + i++; + } + + if (i == 0) + { + return false; + } + + sb.Remove(sb.Length - 1, 1); + sb.Append(';'); #pragma warning disable CA2100 - cmd.CommandText = sb.ToString(); + cmd.CommandText = sb.ToString(); #pragma warning restore CA2100 - Pools.StringBuilders.Return(sb); + } + finally + { + Pools.StringBuilders.Return(sb); + } + + return true; } public bool HasObject(string objectId) => diff --git a/src/Speckle.Sdk/Serialisation/V2/Send/ClosureMath.cs b/src/Speckle.Sdk/Serialisation/V2/Send/ClosureMath.cs new file mode 100644 index 00000000..fd72eaa7 --- /dev/null +++ b/src/Speckle.Sdk/Serialisation/V2/Send/ClosureMath.cs @@ -0,0 +1,58 @@ +namespace Speckle.Sdk.Serialisation.V2.Send; + +public static class ClosureMath +{ + public static void IncrementClosures(this Dictionary current, IEnumerable> child) + { + foreach (var closure in child) + { + if (current.TryGetValue(closure.Key, out var count)) + { + current[closure.Key] = Math.Max(closure.Value, count) + 1; + } + else + { + current[closure.Key] = closure.Value + 1; + } + } + } + + public static void MergeClosures(this Dictionary current, IEnumerable> child) + { + foreach (var closure in child) + { + if (current.TryGetValue(closure.Key, out var count)) + { + current[closure.Key] = Math.Max(closure.Value, count); + } + else + { + current[closure.Key] = closure.Value; + } + } + } + + public static void IncrementClosure(this Dictionary current, Id id) + { + if (current.TryGetValue(id, out var count)) + { + current[id] = count + 1; + } + else + { + current[id] = 1; + } + } + + public static void MergeClosure(this Dictionary current, Id id) + { + if (current.TryGetValue(id, out var count)) + { + current[id] = count; + } + else + { + current[id] = 1; + } + } +} diff --git a/src/Speckle.Sdk/Serialisation/V2/Send/ObjectSerializer.cs b/src/Speckle.Sdk/Serialisation/V2/Send/ObjectSerializer.cs index 5f85353a..6d3e091d 100644 --- a/src/Speckle.Sdk/Serialisation/V2/Send/ObjectSerializer.cs +++ b/src/Speckle.Sdk/Serialisation/V2/Send/ObjectSerializer.cs @@ -25,7 +25,6 @@ public partial interface IObjectSerializer : IDisposable; public sealed class ObjectSerializer : IObjectSerializer { private HashSet _parentObjects = new(); - private readonly Dictionary _currentClosures = new(); private readonly IReadOnlyDictionary _childCache; @@ -92,15 +91,16 @@ public sealed class ObjectSerializer : IObjectSerializer try { (Id, Json) item; + Closures closures = []; try { - item = SerializeBase(baseObj, true, default).NotNull(); + item = SerializeBase(baseObj, true, closures, default).NotNull(); } catch (Exception ex) when (!ex.IsFatal() && ex is not OperationCanceledException) { throw new SpeckleSerializeException($"Failed to extract (pre-serialize) properties from the {baseObj}", ex); } - yield return (item.Item1, item.Item2, _currentClosures); + yield return (item.Item1, item.Item2, closures); foreach (var chunk in _chunks) { yield return chunk; @@ -114,7 +114,12 @@ public sealed class ObjectSerializer : IObjectSerializer // `Preserialize` means transforming all objects into the final form that will appear in json, with basic .net objects // (primitives, lists and dictionaries with string keys) - private void SerializeProperty(object? obj, JsonWriter writer, PropertyAttributeInfo propertyAttributeInfo) + private void SerializeProperty( + object? obj, + JsonWriter writer, + Closures closures, + PropertyAttributeInfo propertyAttributeInfo + ) { _cancellationToken.ThrowIfCancellationRequested(); @@ -161,17 +166,14 @@ public sealed class ObjectSerializer : IObjectSerializer ["referencedId"] = r.referencedId, ["__closure"] = r.closure, }; + closures.IncrementClosure(new(r.referencedId)); //references can be externally provided and need to know the ids in the closure and reference here - //AddClosure can take the same value twice - foreach (var kvp in r.closure.Empty()) - { - AddClosure(new(kvp.Key)); - } - AddClosure(new(r.referencedId)); - SerializeProperty(ret, writer, default); + closures.IncrementClosures(r.closure.Empty().Select(x => new KeyValuePair(new Id(x.Key), x.Value))); + + SerializeProperty(ret, writer, closures, default); break; case Base b: - var result = SerializeBase(b, false, propertyAttributeInfo); + var result = SerializeBase(b, false, closures, propertyAttributeInfo); if (result is not null) { writer.WriteRawValue(result.Value.Item2.Value); @@ -196,7 +198,7 @@ public sealed class ObjectSerializer : IObjectSerializer } writer.WritePropertyName(key); - SerializeProperty(kvp.Value, writer, propertyAttributeInfo); + SerializeProperty(kvp.Value, writer, closures, propertyAttributeInfo); } writer.WriteEndObject(); } @@ -206,7 +208,7 @@ public sealed class ObjectSerializer : IObjectSerializer writer.WriteStartArray(); foreach (object? element in e) { - SerializeProperty(element, writer, propertyAttributeInfo); + SerializeProperty(element, writer, closures, propertyAttributeInfo); } writer.WriteEndArray(); } @@ -253,7 +255,12 @@ public sealed class ObjectSerializer : IObjectSerializer } } - private (Id, Json)? SerializeBase(Base baseObj, bool isRoot, PropertyAttributeInfo inheritedDetachInfo) + private (Id, Json)? SerializeBase( + Base baseObj, + bool isRequestedObject, + Closures closures, + PropertyAttributeInfo inheritedDetachInfo + ) { // handle circular references bool alreadySerialized = !_parentObjects.Add(baseObj); @@ -272,67 +279,65 @@ public sealed class ObjectSerializer : IObjectSerializer return new(json, id);*/ } - var isDataChunk = baseObj is DataChunk; - if (inheritedDetachInfo.IsDetachable) { - Closures childClosures; - Id id; - Json json; - //avoid multiple serialization to get closures - if (baseObj.id != null && _childCache.TryGetValue(new(baseObj.id), out var info)) - { - id = new Id(baseObj.id); - childClosures = info.GetClosures(_cancellationToken); - json = info.Json; - MergeClosures(_currentClosures, childClosures); - } - else - { - if (isDataChunk) //datachunks never have child closures - { - 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 json2 = ReferenceGenerator.CreateReference(id); - AddClosure(id); - // add to obj refs to return - if (baseObj.applicationId != null) // && baseObj is not DataChunk && baseObj is not Abstract) // not needed, as data chunks will never have application ids, and abstract objs are not really used. - { - ObjectReferences[new(baseObj.applicationId)] = new ObjectReference() - { - referencedId = id.Value, - applicationId = baseObj.applicationId, - closure = childClosures.ToDictionary(x => x.Key.Value, x => x.Value), - }; - } - _chunks.Add(new(id, json, [])); - return new(id, json2); + return SerializeDetachedBase(baseObj, closures); + } + + //do attached + Closures childClosures = []; + var sb = Pools.StringBuilders.Get(); + using var writer = new StringWriter(sb); + using var jsonWriter = SpeckleObjectSerializerPool.Instance.GetJsonTextWriter(writer); + var id = SerializeBaseWithClosures(baseObj, jsonWriter, childClosures, isRequestedObject); + //don't increment attached objects + closures.MergeClosures(childClosures); + var json = new Json(writer.ToString()); + Pools.StringBuilders.Return(sb); + return new(id, json); + } + + private (Id, Json)? SerializeDetachedBase(Base baseObj, Closures closures) + { + Closures childClosures; + Id id; + Json json; + //avoid multiple serialization to get closures + if (baseObj.id != null && _childCache.TryGetValue(new(baseObj.id), out var info)) + { + id = new Id(baseObj.id); + childClosures = info.GetClosures(_cancellationToken); + json = info.Json; + closures.IncrementClosures(childClosures); } else { - var childClosures = isRoot || inheritedDetachInfo.IsDetachable ? _currentClosures : []; + childClosures = []; 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()); + id = SerializeBaseWithClosures(baseObj, jsonWriter, childClosures, true); + closures.IncrementClosures(childClosures); + json = new Json(writer.ToString()); Pools.StringBuilders.Return(sb); - return new(id, json); } + var json2 = ReferenceGenerator.CreateReference(id); + closures.MergeClosure(id); + // add to obj refs to return + if (baseObj.applicationId != null) // && baseObj is not DataChunk && baseObj is not Abstract) // not needed, as data chunks will never have application ids, and abstract objs are not really used. + { + ObjectReferences[new(baseObj.applicationId)] = new ObjectReference() + { + referencedId = id.Value, + applicationId = baseObj.applicationId, + closure = childClosures.ToDictionary(x => x.Key.Value, x => x.Value), + }; + } + _chunks.Add(new(id, json, [])); + return new(id, json2); } - private Id SerializeBaseObject(Base baseObj, JsonWriter writer, Closures closure) + private Id SerializeBaseWithClosures(Base baseObj, JsonWriter writer, Closures closures, bool writeClosures) { if (baseObj is not Blob) { @@ -349,7 +354,7 @@ public sealed class ObjectSerializer : IObjectSerializer } writer.WritePropertyName(prop.Name); - SerializeOrChunkProperty(prop.Value, writer, prop.PropertyAttributeInfo); + SerializeOrChunkProperty(prop.Value, writer, closures, prop.PropertyAttributeInfo); } Id id; @@ -366,11 +371,11 @@ public sealed class ObjectSerializer : IObjectSerializer writer.WriteValue(id.Value); baseObj.id = id.Value; - if (closure.Count > 0) + if (writeClosures && closures.Count > 0) { writer.WritePropertyName("__closure"); writer.WriteStartObject(); - foreach (var c in closure) + foreach (var c in closures) { writer.WritePropertyName(c.Key.Value); writer.WriteValue(c.Value); @@ -392,6 +397,7 @@ public sealed class ObjectSerializer : IObjectSerializer private void SerializeOrChunkProperty( object? baseValue, JsonWriter jsonWriter, + Closures closures, PropertyAttributeInfo propertyAttributeInfo ) { @@ -417,20 +423,10 @@ public sealed class ObjectSerializer : IObjectSerializer chunks.Add(crtChunk); } - SerializeProperty(chunks, jsonWriter, new PropertyAttributeInfo(true, false, 0, null)); + SerializeProperty(chunks, jsonWriter, closures, new PropertyAttributeInfo(true, false, 0, null)); return; } - SerializeProperty(baseValue, jsonWriter, propertyAttributeInfo); + SerializeProperty(baseValue, jsonWriter, closures, propertyAttributeInfo); } - - private static void MergeClosures(Dictionary current, Closures child) - { - foreach (var closure in child) - { - current[closure.Key] = 100; - } - } - - private void AddClosure(Id id) => _currentClosures[id] = 100; } diff --git a/tests/Speckle.Objects.Tests.Unit/Serialization/ObjectsSerializationTest.SerializeAndVerify_testCase=Speckle.Objects.Geometry.Mesh.verified.json b/tests/Speckle.Objects.Tests.Unit/Serialization/ObjectsSerializationTest.SerializeAndVerify_testCase=Speckle.Objects.Geometry.Mesh.verified.json index 19724719..ce8d4083 100644 --- a/tests/Speckle.Objects.Tests.Unit/Serialization/ObjectsSerializationTest.SerializeAndVerify_testCase=Speckle.Objects.Geometry.Mesh.verified.json +++ b/tests/Speckle.Objects.Tests.Unit/Serialization/ObjectsSerializationTest.SerializeAndVerify_testCase=Speckle.Objects.Geometry.Mesh.verified.json @@ -12,9 +12,9 @@ }, { "__closure": { - "13da8e855141b835e68bba721411046e": 100, - "cc9d42395b317ed25947b1285bbd5103": 100, - "ede4848fc3abda9275a19fee3447ffbd": 100 + "13da8e855141b835e68bba721411046e": 1, + "cc9d42395b317ed25947b1285bbd5103": 1, + "ede4848fc3abda9275a19fee3447ffbd": 1 }, "applicationId": "asdfasdf", "area": 42.0, diff --git a/tests/Speckle.Sdk.Serialization.Tests/AdditionalCancellationTests.Cancellation_Traversal.verified.json b/tests/Speckle.Sdk.Serialization.Tests/AdditionalCancellationTests.Cancellation_Traversal.verified.json new file mode 100644 index 00000000..cdc627d2 --- /dev/null +++ b/tests/Speckle.Sdk.Serialization.Tests/AdditionalCancellationTests.Cancellation_Traversal.verified.json @@ -0,0 +1,15 @@ +{ + "CancellationToken": { + "IsCancellationRequested": true, + "CanBeCanceled": true, + "WaitHandle": { + "SafeWaitHandle": { + "IsClosed": false, + "IsInvalid": false + } + } + }, + "Data": {}, + "Message": "The operation was canceled.", + "Type": "OperationCanceledException" +} diff --git a/tests/Speckle.Sdk.Serialization.Tests/AdditionalCancellationTests.cs b/tests/Speckle.Sdk.Serialization.Tests/AdditionalCancellationTests.cs new file mode 100644 index 00000000..74933f88 --- /dev/null +++ b/tests/Speckle.Sdk.Serialization.Tests/AdditionalCancellationTests.cs @@ -0,0 +1,96 @@ +using System.Collections.Concurrent; +using FluentAssertions; +using Microsoft.Extensions.DependencyInjection; +using Speckle.Sdk.Serialisation; +using Speckle.Sdk.Serialisation.V2; +using Speckle.Sdk.Serialisation.V2.Send; + +namespace Speckle.Sdk.Serialization.Tests; + +public class AdditionalCancellationTests +{ + private readonly ISerializeProcessFactory _factory; + + public AdditionalCancellationTests() + { + var serviceCollection = new ServiceCollection(); + serviceCollection.AddSpeckleSdk(new("Tests", "test"), "v3", typeof(TestClass).Assembly); + var serviceProvider = serviceCollection.BuildServiceProvider(); + + _factory = serviceProvider.GetRequiredService(); + } + + [Fact] + public async Task Cancellation_Traversal() + { + var testClass = new TestClass() { RegularProperty = "Hello" }; + + using var cancellationSource = new CancellationTokenSource(); + + await using var serializeProcess = _factory.CreateSerializeProcess( + new ConcurrentDictionary(), + new ConcurrentDictionary(), + null, + cancellationSource.Token, + new SerializeProcessOptions(true, true, false, true) + ); + + var task = serializeProcess.Serialize(testClass); + cancellationSource.Cancel(); + + var ex = await Assert.ThrowsAsync(async () => await task); + await Verify(ex); + cancellationSource.IsCancellationRequested.Should().BeTrue(); + } + + [Fact] + public async Task Cancellation_MultipleConcurrent() + { + var testClass1 = new TestClass() { RegularProperty = "Hello" }; + var testClass2 = new TestClass() { RegularProperty = "World" }; + + using var cancellationSource = new CancellationTokenSource(); + + var tasks = new List(); + for (int i = 0; i < 2; i++) + { + var serializeProcess = _factory.CreateSerializeProcess( + new ConcurrentDictionary(), + new ConcurrentDictionary(), + null, + cancellationSource.Token, + new SerializeProcessOptions(true, true, false, true) + ); + tasks.Add(serializeProcess.Serialize(i % 2 == 0 ? testClass1 : testClass2)); + } + + cancellationSource.Cancel(); + + foreach (var task in tasks) + { + await Assert.ThrowsAsync(async () => await task); + } + cancellationSource.IsCancellationRequested.Should().BeTrue(); + } + + [Fact] + public async Task Cancellation_AfterCompletion() + { + var testClass = new TestClass() { RegularProperty = "Hello" }; + + using var cancellationSource = new CancellationTokenSource(); + + await using var serializeProcess = _factory.CreateSerializeProcess( + new ConcurrentDictionary(), + new ConcurrentDictionary(), + null, + cancellationSource.Token, + new SerializeProcessOptions(true, true, false, true) + ); + + await serializeProcess.Serialize(testClass); + cancellationSource.Cancel(); // Cancel after completion + + cancellationSource.IsCancellationRequested.Should().BeTrue(); + } +} diff --git a/tests/Speckle.Sdk.Serialization.Tests/DetachedTests.CanSerialize_Attached.verified.json b/tests/Speckle.Sdk.Serialization.Tests/DetachedTests.CanSerialize_Attached.verified.json new file mode 100644 index 00000000..c45eade4 --- /dev/null +++ b/tests/Speckle.Sdk.Serialization.Tests/DetachedTests.CanSerialize_Attached.verified.json @@ -0,0 +1,65 @@ +{ + "027a7c5ffcf8d8efe432899c729a954c": { + "__closure": { + "5b86b66b61c556ead500915b05852875": 1 + }, + "applicationId": null, + "area": 0.0, + "bbox": null, + "closed": false, + "domain": { + "applicationId": null, + "end": 1.0, + "id": "4d47df83148d8bad7bd2657bd9158b4c", + "speckle_type": "Objects.Primitive.Interval", + "start": 0.0 + }, + "id": "027a7c5ffcf8d8efe432899c729a954c", + "length": 0.0, + "speckle_type": "Objects.Geometry.Polyline", + "units": "test", + "value": [ + { + "__closure": null, + "referencedId": "5b86b66b61c556ead500915b05852875", + "speckle_type": "reference" + } + ] + }, + "5b86b66b61c556ead500915b05852875": { + "applicationId": null, + "data": [ + 3.0, + 4.0 + ], + "id": "5b86b66b61c556ead500915b05852875", + "speckle_type": "Speckle.Core.Models.DataChunk" + }, + "d71fd05503f7ec3346f9a8e1e28f55cf": { + "__closure": { + "027a7c5ffcf8d8efe432899c729a954c": 1, + "5b86b66b61c556ead500915b05852875": 2 + }, + "applicationId": "1", + "arr": null, + "attachedProp": { + "applicationId": "4", + "id": "c5dd540ee1299c0349829d045c04ef2d", + "line": { + "__closure": null, + "referencedId": "027a7c5ffcf8d8efe432899c729a954c", + "speckle_type": "reference" + }, + "name": "attachedProp", + "speckle_type": "Speckle.Core.Tests.Unit.Models.BaseTests+SamplePropBase2" + }, + "crazyProp": null, + "detachedProp": null, + "detachedProp2": null, + "dynamicProp": 123, + "id": "d71fd05503f7ec3346f9a8e1e28f55cf", + "list": [], + "list2": null, + "speckle_type": "Speckle.Core.Tests.Unit.Models.BaseTests+SampleObjectBase2" + } +} diff --git a/tests/Speckle.Sdk.Serialization.Tests/DetachedTests.CanSerialize_Attached_2.verified.json b/tests/Speckle.Sdk.Serialization.Tests/DetachedTests.CanSerialize_Attached_2.verified.json new file mode 100644 index 00000000..10b97b41 --- /dev/null +++ b/tests/Speckle.Sdk.Serialization.Tests/DetachedTests.CanSerialize_Attached_2.verified.json @@ -0,0 +1,21 @@ +{ + "3f4e76fdb759a8663bf61016548982b7": { + "applicationId": "1", + "arr": null, + "attachedProp": { + "applicationId": "4", + "id": "4afd58037ebe6816c6018705e911ee6d", + "line": null, + "name": "attachedProp", + "speckle_type": "Speckle.Core.Tests.Unit.Models.BaseTests+SamplePropBase2" + }, + "crazyProp": null, + "detachedProp": null, + "detachedProp2": null, + "dynamicProp": 123, + "id": "3f4e76fdb759a8663bf61016548982b7", + "list": [], + "list2": null, + "speckle_type": "Speckle.Core.Tests.Unit.Models.BaseTests+SampleObjectBase2" + } +} diff --git a/tests/Speckle.Sdk.Serialization.Tests/DetachedTests.CanSerialize_New_Detached.verified.json b/tests/Speckle.Sdk.Serialization.Tests/DetachedTests.CanSerialize_New_Detached.verified.json index 77406846..f10c987a 100644 --- a/tests/Speckle.Sdk.Serialization.Tests/DetachedTests.CanSerialize_New_Detached.verified.json +++ b/tests/Speckle.Sdk.Serialization.Tests/DetachedTests.CanSerialize_New_Detached.verified.json @@ -1,7 +1,7 @@ { "9ff8efb13c62fa80f3d1c4519376ba13": { "__closure": { - "d3dd4621b2f68c3058c2b9c023a9de19": 100 + "d3dd4621b2f68c3058c2b9c023a9de19": 1 }, "applicationId": null, "arr": null, diff --git a/tests/Speckle.Sdk.Serialization.Tests/DetachedTests.CanSerialize_New_Detached2.verified.json b/tests/Speckle.Sdk.Serialization.Tests/DetachedTests.CanSerialize_New_Detached2.verified.json index be3c516f..0c48cc61 100644 --- a/tests/Speckle.Sdk.Serialization.Tests/DetachedTests.CanSerialize_New_Detached2.verified.json +++ b/tests/Speckle.Sdk.Serialization.Tests/DetachedTests.CanSerialize_New_Detached2.verified.json @@ -1,13 +1,7 @@ { "027a7c5ffcf8d8efe432899c729a954c": { "__closure": { - "045cbee36837d589b17f9d8483c90763": 100, - "1afc694774efa5913d0077302cd37888": 100, - "32a385e7ddeda810e037b21ab26381b7": 100, - "4ba53b5e84e956fb076bc8b0a03ca879": 100, - "5b86b66b61c556ead500915b05852875": 100, - "8d27f5c7fac36d985d89bb6d6d8acddc": 100, - "c3858f47dd3e7a308a1b465375f1645f": 100 + "5b86b66b61c556ead500915b05852875": 1 }, "applicationId": null, "area": 0.0, @@ -34,7 +28,7 @@ }, "045cbee36837d589b17f9d8483c90763": { "__closure": { - "1afc694774efa5913d0077302cd37888": 100 + "1afc694774efa5913d0077302cd37888": 1 }, "applicationId": null, "area": 0.0, @@ -70,14 +64,14 @@ }, "2ebfd4f317754fce14cadd001151441e": { "__closure": { - "027a7c5ffcf8d8efe432899c729a954c": 100, - "045cbee36837d589b17f9d8483c90763": 100, - "1afc694774efa5913d0077302cd37888": 100, - "32a385e7ddeda810e037b21ab26381b7": 100, - "4ba53b5e84e956fb076bc8b0a03ca879": 100, - "5b86b66b61c556ead500915b05852875": 100, - "8d27f5c7fac36d985d89bb6d6d8acddc": 100, - "c3858f47dd3e7a308a1b465375f1645f": 100 + "027a7c5ffcf8d8efe432899c729a954c": 1, + "045cbee36837d589b17f9d8483c90763": 2, + "1afc694774efa5913d0077302cd37888": 3, + "32a385e7ddeda810e037b21ab26381b7": 1, + "4ba53b5e84e956fb076bc8b0a03ca879": 2, + "5b86b66b61c556ead500915b05852875": 2, + "8d27f5c7fac36d985d89bb6d6d8acddc": 3, + "c3858f47dd3e7a308a1b465375f1645f": 1 }, "applicationId": "1", "arr": null, @@ -111,8 +105,8 @@ }, "32a385e7ddeda810e037b21ab26381b7": { "__closure": { - "4ba53b5e84e956fb076bc8b0a03ca879": 100, - "8d27f5c7fac36d985d89bb6d6d8acddc": 100 + "4ba53b5e84e956fb076bc8b0a03ca879": 1, + "8d27f5c7fac36d985d89bb6d6d8acddc": 2 }, "applicationId": "2", "id": "32a385e7ddeda810e037b21ab26381b7", @@ -126,7 +120,7 @@ }, "4ba53b5e84e956fb076bc8b0a03ca879": { "__closure": { - "8d27f5c7fac36d985d89bb6d6d8acddc": 100 + "8d27f5c7fac36d985d89bb6d6d8acddc": 1 }, "applicationId": null, "area": 0.0, @@ -171,8 +165,8 @@ }, "c3858f47dd3e7a308a1b465375f1645f": { "__closure": { - "045cbee36837d589b17f9d8483c90763": 100, - "1afc694774efa5913d0077302cd37888": 100 + "045cbee36837d589b17f9d8483c90763": 1, + "1afc694774efa5913d0077302cd37888": 2 }, "applicationId": "3", "id": "c3858f47dd3e7a308a1b465375f1645f", diff --git a/tests/Speckle.Sdk.Serialization.Tests/DetachedTests.CanSerialize_New_Detached_With_DataChunks2.verified.json b/tests/Speckle.Sdk.Serialization.Tests/DetachedTests.CanSerialize_New_Detached_With_DataChunks2.verified.json index ec12e5c4..6ef38720 100644 --- a/tests/Speckle.Sdk.Serialization.Tests/DetachedTests.CanSerialize_New_Detached_With_DataChunks2.verified.json +++ b/tests/Speckle.Sdk.Serialization.Tests/DetachedTests.CanSerialize_New_Detached_With_DataChunks2.verified.json @@ -18,8 +18,8 @@ }, "525b1e9eef4d07165abb4ffc518395fc": { "__closure": { - "0e61e61edee00404ec6e0f9f594bce24": 100, - "f70738e3e3e593ac11099a6ed6b71154": 100 + "0e61e61edee00404ec6e0f9f594bce24": 1, + "f70738e3e3e593ac11099a6ed6b71154": 1 }, "applicationId": "1", "arr": [ diff --git a/tests/Speckle.Sdk.Serialization.Tests/DetachedTests.cs b/tests/Speckle.Sdk.Serialization.Tests/DetachedTests.cs index 5bd3ef94..8b278d59 100644 --- a/tests/Speckle.Sdk.Serialization.Tests/DetachedTests.cs +++ b/tests/Speckle.Sdk.Serialization.Tests/DetachedTests.cs @@ -123,7 +123,56 @@ public class DetachedTests objects, null, default, - new SerializeProcessOptions(false, false, true, true) + new SerializeProcessOptions(false, false, true, true) { MaxParallelism = 1, MaxHttpSendSize = 1 } + ); + var results = await serializeProcess.Serialize(@base); + + await VerifyJsonDictionary(objects); + } + + [Fact] + public async Task CanSerialize_Attached() + { + var @base = new SampleObjectBase2(); + @base["dynamicProp"] = 123; + @base.applicationId = "1"; + @base.attachedProp = new SamplePropBase2() + { + name = "attachedProp", + applicationId = "4", + line = new Polyline() { units = "test", value = [3.0, 4.0] }, + }; + + var objects = new ConcurrentDictionary(); + + await using var serializeProcess = _factory.CreateSerializeProcess( + new ConcurrentDictionary(), + objects, + null, + default, + new SerializeProcessOptions(false, false, true, true) { MaxParallelism = 1, MaxHttpSendSize = 1 } + ); + var results = await serializeProcess.Serialize(@base); + + await VerifyJsonDictionary(objects); + } + + [Fact] + public async Task CanSerialize_Attached_2() + { + var @base = new SampleObjectBase2(); + @base["dynamicProp"] = 123; + @base.applicationId = "1"; + @base.attachedProp = new SamplePropBase2() { name = "attachedProp", applicationId = "4" }; + + var objects = new ConcurrentDictionary(); + + await using var serializeProcess = _factory.CreateSerializeProcess( + new ConcurrentDictionary(), + objects, + null, + default, + new SerializeProcessOptions(false, false, true, true) { MaxParallelism = 1, MaxHttpSendSize = 1 } ); var results = await serializeProcess.Serialize(@base); @@ -155,8 +204,8 @@ public class DetachedTests "dynamicProp" : 123, "id" : "efeadaca70a85ae6d3acfc93a8b380db", "__closure" : { - "0e61e61edee00404ec6e0f9f594bce24" : 100, - "f70738e3e3e593ac11099a6ed6b71154" : 100 + "0e61e61edee00404ec6e0f9f594bce24" : 1, + "f70738e3e3e593ac11099a6ed6b71154" : 1 } } """; diff --git a/tests/Speckle.Sdk.Serialization.Tests/ExternalIdTests.ExternalIdTest_Detached.verified.json b/tests/Speckle.Sdk.Serialization.Tests/ExternalIdTests.ExternalIdTest_Detached.verified.json index 04349a0b..741b22b6 100644 --- a/tests/Speckle.Sdk.Serialization.Tests/ExternalIdTests.ExternalIdTest_Detached.verified.json +++ b/tests/Speckle.Sdk.Serialization.Tests/ExternalIdTests.ExternalIdTest_Detached.verified.json @@ -10,7 +10,7 @@ }, "cfaf7ae0dfc5a7cf3343bb6db46ed238": { "__closure": { - "8d27f5c7fac36d985d89bb6d6d8acddc": 100 + "8d27f5c7fac36d985d89bb6d6d8acddc": 1 }, "applicationId": null, "area": 0.0, diff --git a/tests/Speckle.Sdk.Serialization.Tests/ExternalIdTests.ExternalIdTest_Detached_Nested.verified.json b/tests/Speckle.Sdk.Serialization.Tests/ExternalIdTests.ExternalIdTest_Detached_Nested.verified.json index f7db9127..9ff878bb 100644 --- a/tests/Speckle.Sdk.Serialization.Tests/ExternalIdTests.ExternalIdTest_Detached_Nested.verified.json +++ b/tests/Speckle.Sdk.Serialization.Tests/ExternalIdTests.ExternalIdTest_Detached_Nested.verified.json @@ -10,8 +10,8 @@ }, "93737bb0800970f29d68748ee206399d": { "__closure": { - "8d27f5c7fac36d985d89bb6d6d8acddc": 100, - "cfaf7ae0dfc5a7cf3343bb6db46ed238": 100 + "8d27f5c7fac36d985d89bb6d6d8acddc": 2, + "cfaf7ae0dfc5a7cf3343bb6db46ed238": 1 }, "applicationId": null, "area": 0.0, @@ -42,7 +42,7 @@ }, "cfaf7ae0dfc5a7cf3343bb6db46ed238": { "__closure": { - "8d27f5c7fac36d985d89bb6d6d8acddc": 100 + "8d27f5c7fac36d985d89bb6d6d8acddc": 1 }, "applicationId": null, "area": 0.0, diff --git a/tests/Speckle.Sdk.Serialization.Tests/ExternalIdTests.ExternalIdTest_Detached_Nested_More.verified.json b/tests/Speckle.Sdk.Serialization.Tests/ExternalIdTests.ExternalIdTest_Detached_Nested_More.verified.json index 96d651cd..260d1579 100644 --- a/tests/Speckle.Sdk.Serialization.Tests/ExternalIdTests.ExternalIdTest_Detached_Nested_More.verified.json +++ b/tests/Speckle.Sdk.Serialization.Tests/ExternalIdTests.ExternalIdTest_Detached_Nested_More.verified.json @@ -1,8 +1,8 @@ { "050585d98934e7a4d199e07bd92598a5": { "__closure": { - "8d27f5c7fac36d985d89bb6d6d8acddc": 100, - "cfaf7ae0dfc5a7cf3343bb6db46ed238": 100 + "8d27f5c7fac36d985d89bb6d6d8acddc": 2, + "cfaf7ae0dfc5a7cf3343bb6db46ed238": 1 }, "applicationId": null, "area": 0.0, @@ -61,7 +61,7 @@ }, "cfaf7ae0dfc5a7cf3343bb6db46ed238": { "__closure": { - "8d27f5c7fac36d985d89bb6d6d8acddc": 100 + "8d27f5c7fac36d985d89bb6d6d8acddc": 1 }, "applicationId": null, "area": 0.0, diff --git a/tests/Speckle.Sdk.Serialization.Tests/ExternalIdTests.ExternalIdTest_Detached_Nested_More_Too.verified.json b/tests/Speckle.Sdk.Serialization.Tests/ExternalIdTests.ExternalIdTest_Detached_Nested_More_Too.verified.json index 2b821b93..46cfd065 100644 --- a/tests/Speckle.Sdk.Serialization.Tests/ExternalIdTests.ExternalIdTest_Detached_Nested_More_Too.verified.json +++ b/tests/Speckle.Sdk.Serialization.Tests/ExternalIdTests.ExternalIdTest_Detached_Nested_More_Too.verified.json @@ -1,8 +1,8 @@ { "050585d98934e7a4d199e07bd92598a5": { "__closure": { - "8d27f5c7fac36d985d89bb6d6d8acddc": 100, - "cfaf7ae0dfc5a7cf3343bb6db46ed238": 100 + "8d27f5c7fac36d985d89bb6d6d8acddc": 2, + "cfaf7ae0dfc5a7cf3343bb6db46ed238": 1 }, "applicationId": null, "area": 0.0, @@ -52,9 +52,9 @@ }, "4314c177603b00228261d6f0f4686395": { "__closure": { - "050585d98934e7a4d199e07bd92598a5": 100, - "8d27f5c7fac36d985d89bb6d6d8acddc": 100, - "cfaf7ae0dfc5a7cf3343bb6db46ed238": 100 + "050585d98934e7a4d199e07bd92598a5": 1, + "8d27f5c7fac36d985d89bb6d6d8acddc": 3, + "cfaf7ae0dfc5a7cf3343bb6db46ed238": 2 }, "@profile": { "__closure": null, @@ -76,7 +76,7 @@ }, "cfaf7ae0dfc5a7cf3343bb6db46ed238": { "__closure": { - "8d27f5c7fac36d985d89bb6d6d8acddc": 100 + "8d27f5c7fac36d985d89bb6d6d8acddc": 1 }, "applicationId": null, "area": 0.0, diff --git a/tests/Speckle.Sdk.Tests.Unit/Common/NotNullTests.cs b/tests/Speckle.Sdk.Tests.Unit/Common/NotNullTests.cs index 3dc8f2ce..5a8dbb8d 100644 --- a/tests/Speckle.Sdk.Tests.Unit/Common/NotNullTests.cs +++ b/tests/Speckle.Sdk.Tests.Unit/Common/NotNullTests.cs @@ -90,4 +90,44 @@ public class NotNullTests .Invoking(async () => await ValueTask.FromResult((int?)null).NotNull()) .Should() .ThrowAsync(); + + [Theory] + [InlineData("hello")] + [InlineData(" world ")] + public void NotNullOrWhiteSpace_Valid(string input) + { + var result = input.NotNullOrWhiteSpace(); + result.Should().Be(input); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public void NotNullOrWhiteSpace_Invalid(string? input) + { + Action act = () => input.NotNullOrWhiteSpace(); + if (input is null) + { + act.Should().Throw(); + } + else + { + act.Should().Throw(); + } + } + + [Theory] + [InlineData("foo")] + [InlineData("bar baz")] + public void ValidateNullOrWhiteSpace_Valid(string input) => input.NotNullOrWhiteSpace(); // Should not throw + + [Theory] + [InlineData("")] + [InlineData(" ")] + public void ValidateNullOrWhiteSpace_Invalid(string input) + { + Action act = () => input.NotNullOrWhiteSpace(); + act.Should().Throw(); + } } diff --git a/tests/Speckle.Sdk.Tests.Unit/SQLite/SQLiteJsonCacheManagerTests.cs b/tests/Speckle.Sdk.Tests.Unit/SQLite/SQLiteJsonCacheManagerTests.cs index 6496451e..7dfb71f2 100644 --- a/tests/Speckle.Sdk.Tests.Unit/SQLite/SQLiteJsonCacheManagerTests.cs +++ b/tests/Speckle.Sdk.Tests.Unit/SQLite/SQLiteJsonCacheManagerTests.cs @@ -1,4 +1,4 @@ -using FluentAssertions; +using FluentAssertions; using Speckle.Sdk.SQLite; namespace Speckle.Sdk.Tests.Unit.SQLite; @@ -79,4 +79,72 @@ public class SQLiteJsonCacheManagerTests : IDisposable json2.Should().BeNull(); manager.HasObject(id2).Should().BeFalse(); } + + [Fact] + public void TestLargeJsonPayload() + { + var largeJson = new string('a', 100_000); + using var manager = new SqLiteJsonCacheManager(_basePath, 2); + manager.SaveObject("large", largeJson); + var result = manager.GetObject("large"); + result.Should().Be(largeJson); + } + + [Fact] + public void TestSpecialCharactersInIdAndJson() + { + var id = "spécial_字符_!@#$%^&*()"; + var json = /*lang=json,strict*/ + "{\"value\": \"特殊字符!@#$%^&*()\"}"; + using var manager = new SqLiteJsonCacheManager(_basePath, 2); + manager.SaveObject(id, json); + var result = manager.GetObject(id); + result.Should().Be(json); + manager.HasObject(id).Should().BeTrue(); + manager.DeleteObject(id); + manager.HasObject(id).Should().BeFalse(); + } + + [Fact] + public void TestBulkInsertEmptyCollection() + { + using var manager = new SqLiteJsonCacheManager(_basePath, 2); + manager.SaveObjects(new List<(string, string)>()); + manager.GetAllObjects().Count.Should().Be(0); + } + + [Fact] + public void TestRepeatedUpdateAndDelete() + { + using var manager = new SqLiteJsonCacheManager(_basePath, 2); + manager.SaveObject("id", "1"); + manager.UpdateObject("id", "2"); + manager.UpdateObject("id", "3"); + manager.GetObject("id").Should().Be("3"); + manager.DeleteObject("id"); + manager.DeleteObject("id"); // Should not throw + manager.GetObject("id").Should().BeNull(); + } + + [Fact] + public void TestGetAndDeleteNonExistentId() + { + using var manager = new SqLiteJsonCacheManager(_basePath, 2); + manager.GetObject("doesnotexist").Should().BeNull(); + manager.HasObject("doesnotexist").Should().BeFalse(); + manager.DeleteObject("doesnotexist"); // Should not throw + } + + [Fact] + public void TestNullOrEmptyInput() + { + using var manager = new SqLiteJsonCacheManager(_basePath, 2); + // Empty id + Assert.Throws(() => manager.SaveObject("", "emptyid")); + // Empty json + Assert.Throws(() => manager.SaveObject("eid", "")); + // Null id/json (should throw) + Assert.Throws(() => manager.SaveObject(null!, "json")); + Assert.Throws(() => manager.SaveObject("nid", null!)); + } }