Add PeriodicThreadedEvent to EA and use it (#530)

* Add PeriodicThreadedEvent to EA and use it * All timers gone * add tests * bug: build issues * bug: form not being disposed properly --------- Co-authored-by: Björn <[email protected]>
specklesystems · Jan 29, 2025 · 9219cdf · 9219cdf
1 parent 03f0b1f
commit 9219cdf
Show file tree

Hide file tree

Showing 12 changed files with 191 additions and 76 deletions.
diff --git a/Connectors/CSi/Speckle.Connectors.CSiShared/Bindings/CsiSharedSelectionBinding.cs b/Connectors/CSi/Speckle.Connectors.CSiShared/Bindings/CsiSharedSelectionBinding.cs
@@ -1,16 +1,15 @@
-using Speckle.Connectors.CSiShared.HostApp;
+using Speckle.Connectors.CSiShared.Events;
+using Speckle.Connectors.CSiShared.HostApp;
 using Speckle.Connectors.CSiShared.Utils;
 using Speckle.Connectors.DUI.Bindings;
 using Speckle.Connectors.DUI.Bridge;
+using Speckle.Connectors.DUI.Eventing;
 using Speckle.Converters.CSiShared.Utils;
-using Timer = System.Timers.Timer;
 
 namespace Speckle.Connectors.CSiShared.Bindings;
 
-public class CsiSharedSelectionBinding : ISelectionBinding, IDisposable
+public sealed class CsiSharedSelectionBinding : ISelectionBinding
 {
-  private bool _disposed;
-  private readonly Timer _selectionTimer;
   private readonly ICsiApplicationService _csiApplicationService;
   private HashSet<string> _lastSelection = new();
 
@@ -20,18 +19,16 @@ public class CsiSharedSelectionBinding : ISelectionBinding, IDisposable
   public CsiSharedSelectionBinding(
     IBrowserBridge parent,
     ICsiApplicationService csiApplicationService,
-    ITopLevelExceptionHandler topLevelExceptionHandler
+    IEventAggregator eventAggregator
   )
   {
     Parent = parent;
     _csiApplicationService = csiApplicationService;
 
-    _selectionTimer = new Timer(1000);
-    _selectionTimer.Elapsed += (_, _) => topLevelExceptionHandler.CatchUnhandled(CheckSelectionChanged);
-    _selectionTimer.Start();
+    eventAggregator.GetEvent<SelectionBindingEvent>().SubscribePeriodic(TimeSpan.FromSeconds(1), CheckSelectionChanged);
   }
 
-  private void CheckSelectionChanged()
+  private void CheckSelectionChanged(object _)
   {
     var currentSelection = GetSelection();
     var currentIds = new HashSet<string>(currentSelection.SelectedObjectIds);
@@ -43,24 +40,6 @@ private void CheckSelectionChanged()
     }
   }
 
-  protected virtual void Dispose(bool disposing)
-  {
-    if (!_disposed)
-    {
-      if (disposing)
-      {
-        _selectionTimer?.Dispose();
-      }
-      _disposed = true;
-    }
-  }
-
-  public void Dispose()
-  {
-    Dispose(true);
-    GC.SuppressFinalize(this);
-  }
-
   /// <summary>
   /// Gets the selection and creates an encoded ID (objectType and objectName).
   /// </summary>

diff --git a/Connectors/CSi/Speckle.Connectors.CSiShared/Events/ModelChangedEvent.cs b/Connectors/CSi/Speckle.Connectors.CSiShared/Events/ModelChangedEvent.cs
@@ -0,0 +1,8 @@
+using Speckle.Connectors.Common.Threading;
+using Speckle.Connectors.DUI.Bridge;
+using Speckle.Connectors.DUI.Eventing;
+
+namespace Speckle.Connectors.CSiShared.Events;
+
+public class ModelChangedEvent(IThreadContext threadContext, ITopLevelExceptionHandler exceptionHandler)
+  : PeriodicThreadedEvent(threadContext, exceptionHandler);
diff --git a/Connectors/CSi/Speckle.Connectors.CSiShared/Events/SelectionBindingEvent.cs b/Connectors/CSi/Speckle.Connectors.CSiShared/Events/SelectionBindingEvent.cs
@@ -0,0 +1,8 @@
+using Speckle.Connectors.Common.Threading;
+using Speckle.Connectors.DUI.Bridge;
+using Speckle.Connectors.DUI.Eventing;
+
+namespace Speckle.Connectors.CSiShared.Events;
+
+public class SelectionBindingEvent(IThreadContext threadContext, ITopLevelExceptionHandler exceptionHandler)
+  : PeriodicThreadedEvent(threadContext, exceptionHandler);
diff --git a/Connectors/CSi/Speckle.Connectors.CSiShared/HostApp/CsiDocumentModelStore.cs b/Connectors/CSi/Speckle.Connectors.CSiShared/HostApp/CsiDocumentModelStore.cs
@@ -1,25 +1,22 @@
 using System.IO;
-using System.Timers;
 using Microsoft.Extensions.Logging;
+using Speckle.Connectors.CSiShared.Events;
 using Speckle.Connectors.DUI.Eventing;
 using Speckle.Connectors.DUI.Models;
 using Speckle.Connectors.DUI.Utils;
 using Speckle.Sdk;
 using Speckle.Sdk.Helpers;
 using Speckle.Sdk.Logging;
-using Timer = System.Timers.Timer;
 
 namespace Speckle.Connectors.CSiShared.HostApp;
 
-public class CsiDocumentModelStore : DocumentModelStore, IDisposable
+public class CsiDocumentModelStore : DocumentModelStore
 {
   private readonly ISpeckleApplication _speckleApplication;
   private readonly ILogger<CsiDocumentModelStore> _logger;
   private readonly ICsiApplicationService _csiApplicationService;
-  private readonly Timer _modelCheckTimer;
   private readonly IEventAggregator _eventAggregator;
   private string _lastModelFilename = string.Empty;
-  private bool _disposed;
   private string HostAppUserDataPath { get; set; }
   private string DocumentStateFile { get; set; }
   private string ModelPathHash { get; set; }
@@ -37,14 +34,10 @@ IEventAggregator eventAggregator
     _logger = logger;
     _csiApplicationService = csiApplicationService;
     _eventAggregator = eventAggregator;
-
-    // initialize timer to check for model changes
-    _modelCheckTimer = new Timer(1000);
-    _modelCheckTimer.Elapsed += CheckModelChanges;
-    _modelCheckTimer.Start();
+    eventAggregator.GetEvent<ModelChangedEvent>().SubscribePeriodic(TimeSpan.FromSeconds(1), CheckModelChanges);
   }
 
-  private async void CheckModelChanges(object? source, ElapsedEventArgs e)
+  private async Task CheckModelChanges(object _)
   {
     string currentFilename = _csiApplicationService.SapModel.GetModelFilename();
 
@@ -127,25 +120,4 @@ protected override void LoadState()
       ClearAndSave();
     }
   }
-
-  protected virtual void Dispose(bool disposing)
-  {
-    if (_disposed)
-    {
-      return;
-    }
-
-    if (disposing)
-    {
-      _modelCheckTimer.Dispose();
-    }
-
-    _disposed = true;
-  }
-
-  public void Dispose()
-  {
-    Dispose(true);
-    GC.SuppressFinalize(this);
-  }
 }
diff --git a/Connectors/CSi/Speckle.Connectors.CSiShared/Plugin/SpeckleFormBase.cs b/Connectors/CSi/Speckle.Connectors.CSiShared/Plugin/SpeckleFormBase.cs
@@ -16,6 +16,7 @@ public abstract class SpeckleFormBase : Form, ICsiApplicationService
 {
   private ElementHost Host { get; set; }
   private cPluginCallback _pluginCallback;
+  private bool _disposed;
 #pragma warning disable CA2213
   private ServiceProvider _container;
 #pragma warning restore CA2213
@@ -82,4 +83,18 @@ public void Initialize(ref cSapModel sapModel, ref cPluginCallback pluginCallbac
   }
 
   private void Form1Closing(object? sender, FormClosingEventArgs e) => _pluginCallback.Finish(0);
+
+  protected override void Dispose(bool disposing)
+  {
+    if (!_disposed)
+    {
+      if (disposing)
+      {
+        _container.Dispose();
+        Host.Dispose();
+        base.Dispose(disposing);
+      }
+      _disposed = true;
+    }
+  }
 }
diff --git a/Connectors/CSi/Speckle.Connectors.CSiShared/Speckle.Connectors.CSiShared.projitems b/Connectors/CSi/Speckle.Connectors.CSiShared/Speckle.Connectors.CSiShared.projitems
@@ -17,6 +17,8 @@
     <Compile Include="$(MSBuildThisFileDirectory)Bindings\CsiSharedBasicConnectorBinding.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bindings\CsiSharedSelectionBinding.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bindings\CsiSharedSendBinding.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Events\ModelChangedEvent.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Events\SelectionBindingEvent.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Filters\CsiSharedSelectionFilter.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)HostApp\MaterialUnpacker.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)HostApp\CsiSendCollectionManager.cs" />

diff --git a/Connectors/Revit/Speckle.Connectors.RevitShared/Plugin/RevitEvents.cs b/Connectors/Revit/Speckle.Connectors.RevitShared/Plugin/RevitEvents.cs
@@ -25,19 +25,18 @@ public class SelectionChangedEvent(IThreadContext threadContext, ITopLevelExcept
 public class DocumentChangedEvent(IThreadContext threadContext, ITopLevelExceptionHandler exceptionHandler)
   : ThreadedEvent<Autodesk.Revit.DB.Events.DocumentChangedEventArgs>(threadContext, exceptionHandler);
 
+#if REVIT2022
+public class PeriodicSelectionEvent(IThreadContext threadContext, ITopLevelExceptionHandler exceptionHandler)
+  : PeriodicThreadedEvent(threadContext, exceptionHandler);
+#endif
+
 public static class RevitEvents
 {
-#if REVIT2022
-  private static readonly System.Timers.Timer s_selectionTimer = new(1000);
-#else
   private static IEventAggregator? s_eventAggregator;
-#endif
 
   public static void Register(IEventAggregator eventAggregator, UIControlledApplication application)
   {
-#if !REVIT2022
     s_eventAggregator = eventAggregator;
-#endif
     application.Idling += async (_, _) => await eventAggregator.GetEvent<IdleEvent>().PublishAsync(new object());
     application.ControlledApplication.ApplicationInitialized += async (sender, _) =>
       await eventAggregator
@@ -54,17 +53,14 @@ await eventAggregator
 
 #if REVIT2022
     // NOTE: getting the selection data should be a fast function all, even for '000s of elements - and having a timer hitting it every 1s is ok.
-    s_selectionTimer.Elapsed += async (_, _) =>
-      await eventAggregator.GetEvent<SelectionChangedEvent>().PublishAsync(new object());
-    s_selectionTimer.Start();
+    eventAggregator.GetEvent<PeriodicSelectionEvent>().SubscribePeriodic(TimeSpan.FromSeconds(1), OnSelectionChanged);
 #else
 
     application.SelectionChanged += (_, _) =>
       eventAggregator.GetEvent<IdleEvent>().OneTimeSubscribe("Selection", OnSelectionChanged);
 #endif
   }
 
-#if !REVIT2022
   private static async Task OnSelectionChanged(object _)
   {
     if (s_eventAggregator is null)
@@ -73,5 +69,4 @@ private static async Task OnSelectionChanged(object _)
     }
     await s_eventAggregator.GetEvent<SelectionChangedEvent>().PublishAsync(new object());
   }
-#endif
 }
diff --git a/DUI3/Speckle.Connectors.DUI.Tests/Eventing/EventAggregatorTests.cs b/DUI3/Speckle.Connectors.DUI.Tests/Eventing/EventAggregatorTests.cs
@@ -15,6 +15,9 @@ public class TestEvent(IThreadContext threadContext, ITopLevelExceptionHandler e
 public class TestOneTimeEvent(IThreadContext threadContext, ITopLevelExceptionHandler exceptionHandler)
   : OneTimeThreadedEvent<object>(threadContext, exceptionHandler);
 
+public class TestPeriodicThreadedEvent(IThreadContext threadContext, ITopLevelExceptionHandler exceptionHandler)
+  : PeriodicThreadedEvent(threadContext, exceptionHandler);
+
 public class EventAggregatorTests : MoqTest
 {
   [Test]
@@ -348,4 +351,90 @@ public void Test1(object _)
       action();
     }
   }
+
+  [Test]
+  public async Task Periodic_Async()
+  {
+    s_val = false;
+    var services = new ServiceCollection();
+    var exceptionHandler = new TopLevelExceptionHandler(
+      Create<ILogger<TopLevelExceptionHandler>>().Object,
+      Create<IEventAggregator>().Object
+    );
+    services.AddSingleton(Create<IThreadContext>().Object);
+    services.AddSingleton<ITopLevelExceptionHandler>(exceptionHandler);
+    services.AddTransient<TestPeriodicThreadedEvent>();
+
+    services.AddSingleton<IEventAggregator, EventAggregator>();
+    var serviceProvider = services.BuildServiceProvider();
+
+    var subscriptionToken = Test_Periodic_Sub_Async(serviceProvider);
+    GC.Collect();
+    GC.WaitForPendingFinalizers();
+    subscriptionToken.IsActive.Should().BeTrue();
+
+    await Task.Delay(2000);
+    s_val.Should().BeTrue();
+    subscriptionToken.Unsubscribe();
+
+    GC.Collect();
+    GC.WaitForPendingFinalizers();
+    subscriptionToken.IsActive.Should().BeFalse();
+    subscriptionToken.Dispose();
+    GC.Collect();
+    GC.WaitForPendingFinalizers();
+    subscriptionToken.IsActive.Should().BeFalse();
+  }
+
+  private SubscriptionToken Test_Periodic_Sub_Async(IServiceProvider serviceProvider)
+  {
+    var eventAggregator = serviceProvider.GetRequiredService<IEventAggregator>();
+    var subscriptionToken = eventAggregator
+      .GetEvent<TestPeriodicThreadedEvent>()
+      .SubscribePeriodic(TimeSpan.FromSeconds(1), OnTestAsyncSubscribe);
+    return subscriptionToken;
+  }
+
+  [Test]
+  public async Task Periodic_Sync()
+  {
+    s_val = false;
+    var services = new ServiceCollection();
+    var exceptionHandler = new TopLevelExceptionHandler(
+      Create<ILogger<TopLevelExceptionHandler>>().Object,
+      Create<IEventAggregator>().Object
+    );
+    services.AddSingleton(Create<IThreadContext>().Object);
+    services.AddSingleton<ITopLevelExceptionHandler>(exceptionHandler);
+    services.AddTransient<TestPeriodicThreadedEvent>();
+
+    services.AddSingleton<IEventAggregator, EventAggregator>();
+    var serviceProvider = services.BuildServiceProvider();
+
+    var subscriptionToken = Test_Periodic_Sub_Sync(serviceProvider);
+    GC.Collect();
+    GC.WaitForPendingFinalizers();
+    subscriptionToken.IsActive.Should().BeTrue();
+
+    await Task.Delay(2000);
+    s_val.Should().BeTrue();
+    subscriptionToken.Unsubscribe();
+
+    GC.Collect();
+    GC.WaitForPendingFinalizers();
+    subscriptionToken.IsActive.Should().BeFalse();
+    subscriptionToken.Dispose();
+    GC.Collect();
+    GC.WaitForPendingFinalizers();
+    subscriptionToken.IsActive.Should().BeFalse();
+  }
+
+  private SubscriptionToken Test_Periodic_Sub_Sync(IServiceProvider serviceProvider)
+  {
+    var eventAggregator = serviceProvider.GetRequiredService<IEventAggregator>();
+    var subscriptionToken = eventAggregator
+      .GetEvent<TestPeriodicThreadedEvent>()
+      .SubscribePeriodic(TimeSpan.FromSeconds(1), OnTestSyncSubscribe);
+    return subscriptionToken;
+  }
 }
diff --git a/DUI3/Speckle.Connectors.DUI/Eventing/OneTimeThreadedEvent.cs b/DUI3/Speckle.Connectors.DUI/Eventing/OneTimeThreadedEvent.cs
@@ -53,7 +53,7 @@ public SubscriptionToken OneTimeSubscribe(
     }
   }
 
-  public override async Task PublishAsync(T payload)
+  public async Task PublishAsync(T payload)
   {
     SubscriptionToken[] tokensToDestory = [];
     lock (_activeTokens)
@@ -64,7 +64,7 @@ public override async Task PublishAsync(T payload)
         _activeTokens.Clear();
       }
     }
-    await base.PublishAsync(payload);
+    await InternalPublish(payload);
     if (tokensToDestory.Length > 0)
     {
       foreach (var token in tokensToDestory)