Merge pull request #587 from specklesystems/adam/cnx-1238-rhino-8-restarting-during-an-operation-throws-with-key-not

fix: (cancellation manager should return cancelled token instead of throwing exception)

Sdk/Speckle.Connectors.Common.Tests/CancellationManagerTests.cs

@@ -24,9 +24,9 @@ public class CancellationManagerTests
     item.Dispose();
 
     manager.IsExist(id).Should().BeFalse();
-    Assert.Throws<KeyNotFoundException>(() => item.Token.IsCancellationRequested.Should().BeTrue());
+    item.Token.IsCancellationRequested.Should().BeTrue();
 
-    Assert.Throws<KeyNotFoundException>(() => manager.IsCancellationRequested(id).Should().BeTrue());
+    manager.IsCancellationRequested(id).Should().BeTrue();
   }
 
   [Test]
@@ -49,9 +49,9 @@ public class CancellationManagerTests
     item1.Dispose();
 
     manager.IsExist(id1).Should().BeFalse();
-    Assert.Throws<KeyNotFoundException>(() => item1.Token.IsCancellationRequested.Should().BeTrue());
+    item1.Token.IsCancellationRequested.Should().BeTrue();
 
-    Assert.Throws<KeyNotFoundException>(() => manager.IsCancellationRequested(id1).Should().BeTrue());
+    manager.IsCancellationRequested(id1).Should().BeTrue();
 
     manager.NumberOfOperations.Should().Be(1);
     manager.IsExist(id2).Should().BeTrue();
@@ -62,9 +62,9 @@ public class CancellationManagerTests
     item2.Dispose();
 
     manager.IsExist(id2).Should().BeFalse();
-    Assert.Throws<KeyNotFoundException>(() => item2.Token.IsCancellationRequested.Should().BeTrue());
+    item2.Token.IsCancellationRequested.Should().BeTrue();
 
-    Assert.Throws<KeyNotFoundException>(() => manager.IsCancellationRequested(id2).Should().BeTrue());
+    manager.IsCancellationRequested(id2).Should().BeTrue();
   }
 
   [Test]
@@ -92,5 +92,18 @@ public class CancellationManagerTests
 
     manager.IsExist(id1).Should().BeFalse();
     manager.IsExist(id2).Should().BeFalse();
+
+    item1.Token.IsCancellationRequested.Should().BeTrue();
+    item2.Token.IsCancellationRequested.Should().BeTrue();
+  }
+
+  [Test]
+  public void Cancel_No_Existing()
+  {
+    var manager = new CancellationManager();
+    manager.NumberOfOperations.Should().Be(0);
+    var x = Guid.NewGuid().ToString();
+    manager.IsCancellationRequested(x).Should().BeTrue();
+    manager.IsExist(x).Should().BeFalse();
   }
 }

Sdk/Speckle.Connectors.Common/Cancellation/CancellationManager.cs

@@ -27,18 +27,10 @@ public class CancellationManager : ICancellationManager
 
   public int NumberOfOperations => _operationsInProgress.Count;
 
-  /// <summary>
-  /// Get token with registered id.
-  /// </summary>
-  /// <param name="id"> Id of the operation.</param>
-  /// <returns> CancellationToken that belongs to operation.</returns>
-  public CancellationToken GetToken(string id) => _operationsInProgress[id].Token;
+  //if we can't find it then it must be cancelled
+  private CancellationToken GetToken(string id) =>
+    _operationsInProgress.TryGetValue(id, out var source) ? source.Token : new CancellationToken(true);
 
-  /// <summary>
-  /// Whether given id registered or not.
-  /// </summary>
-  /// <param name="id"> Id to check registration.</param>
-  /// <returns> Whether given id registered or not.</returns>
   public bool IsExist(string id) => _operationsInProgress.ContainsKey(id);
 
   public void CancelAllOperations()
@@ -91,5 +83,5 @@ public class CancellationManager : ICancellationManager
   /// </summary>
   /// <param name="id"> Id to check cancellation requested already or not.</param>
   /// <returns></returns>
-  public bool IsCancellationRequested(string id) => _operationsInProgress[id].IsCancellationRequested;
+  public bool IsCancellationRequested(string id) => GetToken(id).IsCancellationRequested;
 }

Sdk/Speckle.Connectors.Common/Operations/ReceiveOperation.cs

@@ -26,6 +26,7 @@ public sealed class ReceiveOperation(
   )
   {
     using var execute = activityFactory.Start("Receive Operation");
+    cancellationToken.ThrowIfCancellationRequested();
     execute?.SetTag("receiveInfo", receiveInfo);
     // 2 - Check account exist
     Account account = accountService.GetAccountWithServerUrlFallback(receiveInfo.AccountId, receiveInfo.ServerUrl);
@@ -34,6 +35,7 @@ public sealed class ReceiveOperation(
 
     var version = await apiClient.Version.Get(receiveInfo.SelectedVersionId, receiveInfo.ProjectId, cancellationToken);
 
+    cancellationToken.ThrowIfCancellationRequested();
     var commitObject = await threadContext.RunOnWorkerAsync(
       () => ReceiveData(account, version, receiveInfo, onOperationProgressed, cancellationToken)
     );
@@ -47,6 +49,7 @@ public sealed class ReceiveOperation(
       )
       .ConfigureAwait(false);
 
+    cancellationToken.ThrowIfCancellationRequested();
     await apiClient.Version.Received(
       new(version.id, receiveInfo.ProjectId, receiveInfo.SourceApplication),
       cancellationToken

Sdk/Speckle.Connectors.Common/Operations/SendOperation.cs

@@ -31,8 +31,10 @@ public sealed class SendOperation<T>(
     CancellationToken ct = default
   )
   {
+    ct.ThrowIfCancellationRequested();
     var buildResult = await rootObjectBuilder.Build(objects, sendInfo, onOperationProgressed, ct);
 
+    ct.ThrowIfCancellationRequested();
     // POC: Jonathon asks on behalf of willow twin - let's explore how this can work
     // buildResult.RootObject["@report"] = new Report { ConversionResults = buildResult.ConversionResults };
 
@@ -42,6 +44,7 @@ public sealed class SendOperation<T>(
     var (rootObjId, convertedReferences) = await threadContext.RunOnWorkerAsync(
       () => Send(buildResult.RootObject, sendInfo, onOperationProgressed, ct)
     );
+    ct.ThrowIfCancellationRequested();
 
     return new(rootObjId, convertedReferences, buildResult.ConversionResults);
   }