From f78dbf506dd6b17bc6245e2906400d2e9ddb80fb Mon Sep 17 00:00:00 2001
From: Connor Ivy <connor@speckle.systems>
Date: Tue, 2 Jul 2024 10:46:15 -0500
Subject: [PATCH] proper transaction disposal

---
 .../Operations/Receive/ITransactionManager.cs |  3 --
 .../Receive/RevitHostObjectBuilder.cs         | 33 ++++++++-------
 .../Operations/Receive/TransactionManager.cs  | 40 -------------------
 3 files changed, 18 insertions(+), 58 deletions(-)

diff --git a/DUI3-DX/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/ITransactionManager.cs b/DUI3-DX/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/ITransactionManager.cs
index 7d80125b43..d698dfac0f 100644
--- a/DUI3-DX/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/ITransactionManager.cs
+++ b/DUI3-DX/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/DUI3-DX/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/RevitHostObjectBuilder.cs b/DUI3-DX/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/RevitHostObjectBuilder.cs
index 3d65414372..245284543d 100644
--- a/DUI3-DX/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/RevitHostObjectBuilder.cs
+++ b/DUI3-DX/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/RevitHostObjectBuilder.cs
@@ -5,6 +5,7 @@
 using Speckle.Core.Models.GraphTraversal;
 using Speckle.Core.Models;
 using Speckle.Converters.RevitShared.Helpers;
+using Autodesk.Revit.DB;
 
 namespace Speckle.Connectors.Revit.Operations.Receive;
 
@@ -12,7 +13,7 @@ namespace Speckle.Connectors.Revit.Operations.Receive;
 /// 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/DUI3-DX/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/TransactionManager.cs b/DUI3-DX/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/TransactionManager.cs
index 26c02217fd..aa2ec3153a 100644
--- a/DUI3-DX/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/TransactionManager.cs
+++ b/DUI3-DX/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();
   }
 }