Merge pull request #388 from specklesystems/dev

Main to dev (no squash)

.github/workflows/pr.yml

@@ -11,7 +11,7 @@ jobs:
       uses: actions/checkout@v5
 
     - name: Setup .NET
-      uses: actions/setup-dotnet@v4
+      uses: actions/setup-dotnet@v5
       with:
         dotnet-version: 8.x.x
         

.github/workflows/release.yml

@@ -12,7 +12,7 @@ jobs:
         uses: actions/checkout@v5
         
       - name: Setup .NET
-        uses: actions/setup-dotnet@v4
+        uses: actions/setup-dotnet@v5
         with:
           dotnet-version: 8.x.x
 

src/Speckle.Sdk/Api/GraphQL/Models/ServerInfo.cs

@@ -19,6 +19,9 @@ public sealed class ServerInfo
   [Obsolete("Don't use")]
   public bool frontend2 { get; set; } = true;
 
+  /// <summary>
+  /// The URL that should be used to talk with the server
+  /// </summary>
   /// <remarks>
   /// This field is not returned from the GQL API,
   /// it should be populated after construction.

src/Speckle.Sdk/Credentials/Account.cs

@@ -59,15 +59,20 @@ public class Account : IEquatable<Account>
 
   #region public methods
 
+
+  /// <remarks>The logic is aligned with <c>distinct_id</c> mixpanel property</remarks>
+  /// <exception cref="ArgumentNullException">Thrown if <see name="userInfo.email"/> was <see langword="null"/></exception>
   public string GetHashedEmail()
   {
-    string email = userInfo?.email ?? "unknown";
+    string email = userInfo.email.NotNull();
     return "@" + Md5.GetString(email).ToUpperInvariant();
   }
 
+  /// <remarks>The logic is aligned with <c>server</c> mixpanel property</remarks>
+  /// <exception cref="ArgumentNullException">Thrown if <see name="serverInfo.url"/> was <see langword="null"/></exception>
   public string GetHashedServer()
   {
-    string url = serverInfo?.url ?? AccountManager.DEFAULT_SERVER_URL;
+    string url = serverInfo.url.NotNull();
     return Md5.GetString(CleanURL(url)).ToUpperInvariant();
   }
 
@@ -97,6 +102,8 @@ public class Account : IEquatable<Account>
 
   #endregion
 
+  internal const string LOCAL_IDENTIFIER_DEPRECATION_MESSAGE = "Local identifiers no longer nesseary";
+
   /// <summary>
   /// Retrieves the local identifier for the current user.
   /// </summary>
@@ -121,5 +128,6 @@ public class Account : IEquatable<Account>
   ///     https://speckle.xyz?id=123
   ///   </code>
   /// </example>
+  [Obsolete(LOCAL_IDENTIFIER_DEPRECATION_MESSAGE)]
   internal Uri GetLocalIdentifier() => new($"{serverInfo.url}?id={userInfo.id}");
 }

src/Speckle.Sdk/Credentials/AccountManager.cs

@@ -419,6 +419,7 @@ public sealed class AccountManager(
   /// <remarks>
   /// <inheritdoc cref="Account.GetLocalIdentifier"/>
   /// </remarks>
+  [Obsolete(Account.LOCAL_IDENTIFIER_DEPRECATION_MESSAGE)]
   public Uri? GetLocalIdentifierForAccount(Account account)
   {
     var identifier = account.GetLocalIdentifier();
@@ -440,6 +441,7 @@ public sealed class AccountManager(
   /// </summary>
   /// <param name="localIdentifier">The local identifier of the account.</param>
   /// <returns>The account that matches the local identifier, or null if no match is found.</returns>
+  [Obsolete(Account.LOCAL_IDENTIFIER_DEPRECATION_MESSAGE)]
   public Account? GetAccountForLocalIdentifier(Uri localIdentifier)
   {
     var searchResult = GetAccounts()

src/Speckle.Sdk/Models/GraphTraversal/GraphTraversal.cs

@@ -109,9 +109,9 @@ public abstract class GraphTraversal<T>
         break;
       case IList list:
       {
-        foreach (object? obj in list)
+        for (int i = list.Count - 1; i >= 0; i--)
         {
-          TraverseMemberToStack(stack, obj, memberName, parent);
+          TraverseMemberToStack(stack, list[i], memberName, parent);
         }
 
         break;

src/Speckle.Sdk/Serialisation/V2/Send/BaseSerializer.cs

@@ -21,12 +21,7 @@ public class BaseSerializer(
   public IReadOnlyDictionary<Id, ObjectReference> ObjectReferences => _objectReferences;
 
   //leave this sync
-  public IEnumerable<BaseItem> Serialise(
-    Base obj,
-    IReadOnlyDictionary<Id, NodeInfo> childInfo,
-    bool skipCacheRead,
-    CancellationToken cancellationToken
-  )
+  public IEnumerable<BaseItem> Serialise(Base obj, bool skipCacheRead, CancellationToken cancellationToken)
   {
     if (!skipCacheRead && obj.id != null)
     {
@@ -38,7 +33,7 @@ public class BaseSerializer(
       }
     }
 
-    using var serializer2 = objectSerializerFactory.Create(childInfo, cancellationToken);
+    using var serializer2 = objectSerializerFactory.Create(cancellationToken);
     var items = _pool.Get();
     try
     {

src/Speckle.Sdk/Serialisation/V2/Send/ObjectSaver.cs

@@ -20,10 +20,10 @@ public sealed class ObjectSaver(
   ISqLiteJsonCacheManager sqLiteJsonCacheManager,
   IServerObjectManager serverObjectManager,
   ILogger<ObjectSaver> logger,
-  CancellationToken cancellationToken,
+  SerializeProcessOptions options,
+  CancellationToken cancellationToken
 #pragma warning disable CS9107
 #pragma warning disable CA2254
-  SerializeProcessOptions? options = null
 ) : ChannelSaver<BaseItem>, IObjectSaver
 #pragma warning restore CA2254
 #pragma warning restore CS9107

src/Speckle.Sdk/Serialisation/V2/Send/ObjectSerializer.cs

@@ -26,8 +26,6 @@ public sealed class ObjectSerializer : IObjectSerializer
 {
   private HashSet<object> _parentObjects = new();
 
-  private readonly IReadOnlyDictionary<Id, NodeInfo> _childCache;
-
   private readonly IBasePropertyGatherer _propertyGatherer;
   private readonly CancellationToken _cancellationToken;
 
@@ -52,7 +50,6 @@ public sealed class ObjectSerializer : IObjectSerializer
   /// <param name="cancellationToken"></param>
   public ObjectSerializer(
     IBasePropertyGatherer propertyGatherer,
-    IReadOnlyDictionary<Id, NodeInfo> childCache,
     Pool<List<(Id, Json, Closures)>> chunksPool,
     Pool<List<DataChunk>> chunks2Pool,
     Pool<List<object?>> chunks3Pool,
@@ -60,7 +57,6 @@ public sealed class ObjectSerializer : IObjectSerializer
   )
   {
     _propertyGatherer = propertyGatherer;
-    _childCache = childCache;
     _chunksPool = chunksPool;
     _chunks2Pool = chunks2Pool;
     _chunks3Pool = chunks3Pool;
@@ -299,28 +295,14 @@ public sealed class ObjectSerializer : IObjectSerializer
 
   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
-    {
-      childClosures = [];
-      var sb = Pools.StringBuilders.Get();
-      using var writer = new StringWriter(sb);
-      using var jsonWriter = SpeckleObjectSerializerPool.Instance.GetJsonTextWriter(writer);
-      id = SerializeBaseWithClosures(baseObj, jsonWriter, childClosures, true);
-      closures.IncrementClosures(childClosures);
-      json = new Json(writer.ToString());
-      Pools.StringBuilders.Return(sb);
-    }
+    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, true);
+    var json = new Json(writer.ToString());
+    Pools.StringBuilders.Return(sb);
+    closures.IncrementClosures(childClosures);
     var json2 = ReferenceGenerator.CreateReference(id);
     closures.MergeClosure(id);
     // add to obj refs to return

src/Speckle.Sdk/Serialisation/V2/Send/ObjectSerializerFactory.cs

@@ -12,6 +12,6 @@ public class ObjectSerializerFactory(IBasePropertyGatherer propertyGatherer) : I
   private readonly Pool<List<DataChunk>> _chunk2Pool = Pools.CreateListPool<DataChunk>();
   private readonly Pool<List<object?>> _chunk3Pool = Pools.CreateListPool<object?>();
 
-  public IObjectSerializer Create(IReadOnlyDictionary<Id, NodeInfo> baseCache, CancellationToken cancellationToken) =>
-    new ObjectSerializer(propertyGatherer, baseCache, _chunkPool, _chunk2Pool, _chunk3Pool, cancellationToken);
+  public IObjectSerializer Create(CancellationToken cancellationToken) =>
+    new ObjectSerializer(propertyGatherer, _chunkPool, _chunk2Pool, _chunk3Pool, cancellationToken);
 }

src/Speckle.Sdk/Serialisation/V2/Send/SerializeProcess.cs

@@ -1,4 +1,3 @@
-using System.Collections.Concurrent;
 using System.Diagnostics.CodeAnalysis;
 using Microsoft.Extensions.Logging;
 using Speckle.InterfaceGenerator;
@@ -37,8 +36,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<Id, NodeInfo> EMPTY_CLOSURES = new();
@@ -64,13 +63,8 @@ public sealed class SerializeProcess(
     ThreadPriority.BelowNormal,
     Environment.ProcessorCount * 2
   );
-  private readonly SerializeProcessOptions _options = options ?? new();
 
   private readonly Pool<Dictionary<Id, NodeInfo>> _currentClosurePool = Pools.CreateDictionaryPool<Id, NodeInfo>();
-  private readonly Pool<ConcurrentDictionary<Id, NodeInfo>> _childClosurePool = Pools.CreateConcurrentDictionaryPool<
-    Id,
-    NodeInfo
-  >();
 
   private readonly Pool<List<Task<Dictionary<Id, NodeInfo>>>> _taskResultPool = Pools.CreateListPool<
     Task<Dictionary<Id, NodeInfo>>
@@ -113,13 +107,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(
@@ -225,7 +219,6 @@ public sealed class SerializeProcess(
         return EMPTY_CLOSURES;
       }
 
-      var childClosures = _childClosurePool.Get();
       foreach (var childClosure in taskClosures)
       {
         if (IsCancelled())
@@ -234,7 +227,6 @@ public sealed class SerializeProcess(
         }
         foreach (var kvp in childClosure)
         {
-          childClosures[kvp.Key] = kvp.Value;
           if (IsCancelled())
           {
             return EMPTY_CLOSURES;
@@ -249,7 +241,7 @@ public sealed class SerializeProcess(
         return EMPTY_CLOSURES;
       }
 
-      var items = baseSerializer.Serialise(obj, childClosures, _options.SkipCacheRead, _processSource.Token);
+      var items = baseSerializer.Serialise(obj, options.SkipCacheRead, _processSource.Token);
 
       if (IsCancelled())
       {
@@ -257,32 +249,26 @@ public sealed class SerializeProcess(
       }
 
       var currentClosures = _currentClosurePool.Get();
-      try
+
+      Interlocked.Increment(ref _objectCount);
+      progress?.Report(new(ProgressEvent.FromCacheOrSerialized, _objectCount, Math.Max(_objectCount, _objectsFound)));
+      foreach (var item in items)
       {
-        Interlocked.Increment(ref _objectCount);
-        progress?.Report(new(ProgressEvent.FromCacheOrSerialized, _objectCount, Math.Max(_objectCount, _objectsFound)));
-        foreach (var item in items)
+        if (IsCancelled())
         {
-          if (IsCancelled())
-          {
-            return EMPTY_CLOSURES;
-          }
-
-          if (item.NeedsStorage)
-          {
-            Interlocked.Increment(ref _objectsSerialized);
-            await objectSaver.SaveAsync(item).ConfigureAwait(false);
-          }
-
-          if (!currentClosures.ContainsKey(item.Id))
-          {
-            currentClosures.Add(item.Id, new NodeInfo(item.Json, item.Closures));
-          }
+          return EMPTY_CLOSURES;
+        }
+
+        if (item.NeedsStorage)
+        {
+          Interlocked.Increment(ref _objectsSerialized);
+          await objectSaver.SaveAsync(item).ConfigureAwait(false);
+        }
+
+        if (!currentClosures.ContainsKey(item.Id))
+        {
+          currentClosures.Add(item.Id, new NodeInfo(item.Json, item.Closures));
         }
-      }
-      finally
-      {
-        _childClosurePool.Return(childClosures);
       }
 
       return currentClosures;

src/Speckle.Sdk/Serialisation/V2/SerializeProcessFactory.cs

@@ -44,13 +44,14 @@ public class SerializeProcessFactory(
         sqLiteJsonCacheManager,
         serverObjectManager,
         loggerFactory.CreateLogger<ObjectSaver>(),
+        options ?? new SerializeProcessOptions(),
         cancellationToken
       ),
       baseChildFinder,
       new BaseSerializer(sqLiteJsonCacheManager, objectSerializerFactory),
       loggerFactory,
-      cancellationToken,
-      options
+      options ?? new SerializeProcessOptions(),
+      cancellationToken
     );
 
   public ISerializeProcess CreateSerializeProcess(

tests/Speckle.Objects.Tests.Unit/Serialization/ObjectsSerializationTest.cs

@@ -18,10 +18,7 @@ public sealed class ObjectsSerializationTest
 
   private static IReadOnlyList<(Id, Json, Dictionary<Id, int>)> Serialize(Base data)
   {
-    using var serializer = new ObjectSerializerFactory(new BasePropertyGatherer()).Create(
-      new Dictionary<Id, NodeInfo>(),
-      default
-    );
+    using var serializer = new ObjectSerializerFactory(new BasePropertyGatherer()).Create(default);
     return serializer.Serialize(data).ToList();
   }
 

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);

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);

tests/Speckle.Sdk.Serialization.Tests/ExceptionTests.cs

@@ -203,7 +203,7 @@ public class ExceptionTests
   public void Test_SpeckleSerializerException()
   {
     var factory = new ObjectSerializerFactory(new BasePropertyGatherer());
-    var serializer = factory.Create(new Dictionary<Id, NodeInfo>(), default);
+    var serializer = factory.Create(default);
     Assert.Throws<SpeckleSerializeException>(() =>
     {
       var _ = serializer.Serialize(new BadBase()).ToList();

tests/Speckle.Sdk.Serialization.Tests/ExternalIdTests.cs

@@ -3,7 +3,6 @@ using Speckle.Objects.Primitive;
 using Speckle.Sdk.Host;
 using Speckle.Sdk.Models;
 using Speckle.Sdk.Models.Extensions;
-using Speckle.Sdk.Serialisation;
 using Speckle.Sdk.Serialisation.V2.Send;
 
 namespace Speckle.Sdk.Serialization.Tests;
@@ -20,10 +19,7 @@ public class ExternalIdTests
   public async Task ExternalIdTest_Detached()
   {
     var p = new Polyline() { units = "cm", value = [1, 2] };
-    using var serializer = new ObjectSerializerFactory(new BasePropertyGatherer()).Create(
-      new Dictionary<Id, NodeInfo>(),
-      default
-    );
+    using var serializer = new ObjectSerializerFactory(new BasePropertyGatherer()).Create(default);
     var objects = serializer.Serialize(p).ToDictionary(x => x.Item1, x => x.Item2);
 
     await VerifyJsonDictionary(objects);
@@ -45,10 +41,7 @@ public class ExternalIdTests
       knots = [],
       weights = [],
     };
-    using var serializer = new ObjectSerializerFactory(new BasePropertyGatherer()).Create(
-      new Dictionary<Id, NodeInfo>(),
-      default
-    );
+    using var serializer = new ObjectSerializerFactory(new BasePropertyGatherer()).Create(default);
     var objects = serializer.Serialize(curve).ToDictionary(x => x.Item1, x => x.Item2);
 
     await VerifyJsonDictionary(objects);
@@ -71,10 +64,7 @@ public class ExternalIdTests
       weights = [],
     };
     var polycurve = new Polycurve() { segments = [curve], units = "cm" };
-    using var serializer = new ObjectSerializerFactory(new BasePropertyGatherer()).Create(
-      new Dictionary<Id, NodeInfo>(),
-      default
-    );
+    using var serializer = new ObjectSerializerFactory(new BasePropertyGatherer()).Create(default);
     var objects = serializer.Serialize(polycurve).ToDictionary(x => x.Item1, x => x.Item2);
 
     await VerifyJsonDictionary(objects);
@@ -99,10 +89,7 @@ public class ExternalIdTests
     var polycurve = new Polycurve() { segments = [curve], units = "cm" };
     var @base = new Base();
     @base.SetDetachedProp("profile", polycurve);
-    using var serializer = new ObjectSerializerFactory(new BasePropertyGatherer()).Create(
-      new Dictionary<Id, NodeInfo>(),
-      default
-    );
+    using var serializer = new ObjectSerializerFactory(new BasePropertyGatherer()).Create(default);
     var objects = serializer.Serialize(@base).ToDictionary(x => x.Item1, x => x.Item2);
     await VerifyJsonDictionary(objects);
   }

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());