proper transaction disposal

specklesystems · Jul 2, 2024 · f78dbf5 · f78dbf5
1 parent f750ee3
commit f78dbf5
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 58 deletions.
diff --git a/...Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/ITransactionManager.cs b/...Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/ITransactionManager.cs
@@ -6,11 +6,8 @@ public interface ITransactionManager : IDisposable
 {
   TransactionStatus CommitSubtransaction();
   TransactionStatus CommitTransaction();
-  void CommitTransactionGroup();
   void RollbackSubTransaction();
   void RollbackTransaction();
-  void RollbackTransactionGroup();
   void StartSubtransaction();
   void StartTransaction();
-  void StartTransactionGroup(string transactionName);
 }
diff --git a/...nectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/RevitHostObjectBuilder.cs b/...nectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/RevitHostObjectBuilder.cs
@@ -5,14 +5,15 @@
 using Speckle.Core.Models.GraphTraversal;
 using Speckle.Core.Models;
 using Speckle.Converters.RevitShared.Helpers;
+using Autodesk.Revit.DB;
 
 namespace Speckle.Connectors.Revit.Operations.Receive;
 
 /// <summary>
 /// Potentially consolidate all application specific IHostObjectBuilders
 /// https://spockle.atlassian.net/browse/DUI3-465
 /// </summary>
-internal class RevitHostObjectBuilder : IHostObjectBuilder
+internal class RevitHostObjectBuilder : IHostObjectBuilder, IDisposable
 {
   private readonly IRootToHostConverter _converter;
   private readonly IRevitConversionContextStack _contextStack;
@@ -40,23 +41,20 @@ public HostObjectBuilderResult Build(
     CancellationToken cancellationToken
   )
   {
-    try
-    {
-      var objectsToConvert = _traverseFunction
-        .TraverseWithProgress(rootObject, onOperationProgressed, cancellationToken)
-        .Where(obj => obj.Current is not Collection);
+    var objectsToConvert = _traverseFunction
+      .TraverseWithProgress(rootObject, onOperationProgressed, cancellationToken)
+      .Where(obj => obj.Current is not Collection);
 
-      _transactionManager.StartTransactionGroup($"Received data from {projectName}");
+    using TransactionGroup transactionGroup = new(_contextStack.Current.Document, $"Received data from {projectName}");
+    transactionGroup.Start();
+    _transactionManager.StartTransaction();
 
-      var conversionResults = BakeObjects(objectsToConvert);
+    var conversionResults = BakeObjects(objectsToConvert);
 
-      _transactionManager.CommitTransactionGroup();
-      return conversionResults;
-    }
-    finally
-    {
-      _transactionManager.Dispose();
-    }
+    _transactionManager.CommitTransaction();
+    transactionGroup.Assimilate();
+
+    return conversionResults;
   }
 
   // POC: Potentially refactor out into an IObjectBaker.
@@ -79,4 +77,9 @@ private HostObjectBuilderResult BakeObjects(IEnumerable<TraversalContext> object
 
     return new(bakedObjectIds, conversionResults);
   }
+
+  public void Dispose()
+  {
+    _transactionManager?.Dispose();
+  }
 }
diff --git a/.../Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/TransactionManager.cs b/.../Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/TransactionManager.cs
@@ -19,49 +19,10 @@ public TransactionManager(IRevitConversionContextStack contextStack)
 
   // poc : these are being disposed. I'm not sure why I need to supress this warning
 #pragma warning disable CA2213 // Disposable fields should be disposed
-  private TransactionGroup? _transactionGroup;
   private Transaction? _transaction;
   private SubTransaction? _subTransaction;
 #pragma warning restore CA2213 // Disposable fields should be disposed
 
-  public void StartTransactionGroup(string transactionName)
-  {
-    if (_transactionGroup == null)
-    {
-      _transactionGroup = new TransactionGroup(Document, transactionName);
-      _transactionGroup.Start();
-    }
-    StartTransaction();
-  }
-
-  public void CommitTransactionGroup()
-  {
-    try
-    {
-      CommitTransaction();
-    }
-    finally
-    {
-      if (_transactionGroup?.GetStatus() == TransactionStatus.Started)
-      {
-        _transactionGroup.Assimilate();
-      }
-    }
-  }
-
-  public void RollbackTransactionGroup()
-  {
-    RollbackTransaction();
-    if (
-      _transactionGroup != null
-      && _transactionGroup.IsValidObject
-      && _transactionGroup.GetStatus() == TransactionStatus.Started
-    )
-    {
-      _transactionGroup.Assimilate();
-    }
-  }
-
   public void StartTransaction()
   {
     if (_transaction == null || !_transaction.IsValidObject || _transaction.GetStatus() != TransactionStatus.Started)
@@ -159,6 +120,5 @@ public void Dispose()
   {
     _subTransaction?.Dispose();
     _transaction?.Dispose();
-    _transactionGroup?.Dispose();
   }
 }