feat(revit)!: Enable config for disabling API listening events (#1255)

* Expose options to disable revit listening events

* fix mistake

* Disable change tracking for revit when document listener is disabled

* Check first

Connectors/Navisworks/Speckle.Connectors.NavisworksShared/Operations/Send/NavisworksRootObjectBuilder.cs

@@ -30,15 +30,13 @@ public class NavisworksRootObjectBuilder(
   NavisworksColorUnpacker colorUnpacker,
   Speckle.Converter.Navisworks.Constants.Registers.IInstanceFragmentRegistry instanceRegistry,
   IElementSelectionService elementSelectionService,
-  IUiUnitsCache uiUnitsCache,
-  bool disableGroupingForInstanceTesting,
-  bool skipNodeMerging
+  IUiUnitsCache uiUnitsCache
 ) : IRootObjectBuilder<NAV.ModelItem>
 {
 #pragma warning disable CA1823
 #pragma warning restore CA1823
-  private bool SkipNodeMerging { get; } = skipNodeMerging;
-  private bool DisableGroupingForInstanceTesting { get; } = disableGroupingForInstanceTesting;
+  private bool SkipNodeMerging { get; set; }
+  private bool DisableGroupingForInstanceTesting { get; set; }
 
   public async Task<RootObjectBuilderResult> Build(
     IReadOnlyList<NAV.ModelItem> navisworksModelItems,

Connectors/Revit/Speckle.Connectors.RevitShared/Bindings/RevitSendBinding.cs

@@ -40,6 +40,7 @@ internal sealed class RevitSendBinding : RevitBaseBinding, ISendBinding
   private readonly ISendOperationManagerFactory _sendOperationManagerFactory;
   private bool _isDocChangedSubscribed;
   private EventHandler<Autodesk.Revit.DB.Events.DocumentChangedEventArgs>? _documentChangedHandler;
+  private readonly ConnectorConfig _config;
 
   /// <summary>
   /// Used internally to aggregate the changed objects' id. Note we're using a concurrent dictionary here as the expiry check method is not thread safe, and this was causing problems. See:
@@ -64,7 +65,8 @@ internal sealed class RevitSendBinding : RevitBaseBinding, ISendBinding
     LinkedModelHandler linkedModelHandler,
     IThreadContext threadContext,
     IRevitTask revitTask,
-    ISendOperationManagerFactory sendOperationManagerFactory
+    ISendOperationManagerFactory sendOperationManagerFactory,
+    IConfigStore configStore
   )
     : base("sendBinding", bridge)
   {
@@ -81,6 +83,7 @@ internal sealed class RevitSendBinding : RevitBaseBinding, ISendBinding
     _linkedModelHandler = linkedModelHandler;
     _threadContext = threadContext;
     _sendOperationManagerFactory = sendOperationManagerFactory;
+    _config = configStore.GetConnectorConfig();
 
     Commands = new SendBindingUICommands(bridge);
     // TODO expiry events
@@ -98,7 +101,11 @@ internal sealed class RevitSendBinding : RevitBaseBinding, ISendBinding
 
   private void OnModelCardsChanged(ModelCardsChangedEventArgs e)
   {
-    if (e.ModelCards.Count > 0 && e.ModelCards.Any(m => m.TypeDiscriminator == nameof(SenderModelCard)))
+    if (
+      !_config.DocumentChangeListeningDisabled
+      && e.ModelCards.Count > 0
+      && e.ModelCards.Any(m => m.TypeDiscriminator == nameof(SenderModelCard))
+    )
     {
       SubscribeDocChanged();
     }

Connectors/Revit/Speckle.Connectors.RevitShared/Bindings/SelectionBinding.cs

@@ -1,5 +1,6 @@
 using Speckle.Connectors.DUI.Bindings;
 using Speckle.Connectors.DUI.Bridge;
+using Speckle.Connectors.DUI.Settings;
 using Speckle.Connectors.Revit.Plugin;
 using Speckle.Converters.RevitShared.Helpers;
 using Speckle.Sdk.Common;
@@ -18,26 +19,31 @@ internal sealed class SelectionBinding : RevitBaseBinding, ISelectionBinding, ID
     RevitContext revitContext,
     IBrowserBridge parent,
     RevitIdleManager idleManager,
+#if REVIT2022
     ITopLevelExceptionHandler topLevelExceptionHandler,
-    IRevitTask revitTask
+#endif
+    IRevitTask revitTask,
+    IConfigStore configStore
   )
     : base("selectionBinding", parent)
   {
     _revitContext = revitContext;
 
+    if (!configStore.GetConnectorConfig().SelectionChangeListeningDisabled)
+    {
 #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.
-    _selectionTimer = new System.Timers.Timer(1000);
-    _selectionTimer.Elapsed += (_, _) => topLevelExceptionHandler.CatchUnhandled(OnSelectionChanged);
-    _selectionTimer.Start();
+      // 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.
+      _selectionTimer = new System.Timers.Timer(1000);
+      _selectionTimer.Elapsed += (_, _) => topLevelExceptionHandler.CatchUnhandled(OnSelectionChanged);
+      _selectionTimer.Start();
 #else
-
-    revitTask.Run(
-      () =>
-        _revitContext.UIApplication.NotNull().SelectionChanged += (_, _) =>
-          idleManager.SubscribeToIdle(nameof(OnSelectionChanged), OnSelectionChanged)
-    );
+      revitTask.Run(
+        () =>
+          _revitContext.UIApplication.NotNull().SelectionChanged += (_, _) =>
+            idleManager.SubscribeToIdle(nameof(OnSelectionChanged), OnSelectionChanged)
+      );
 #endif
+    }
   }
 
   private void OnSelectionChanged()

Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/RevitHostObjectBuilder.cs

@@ -21,6 +21,7 @@ using Speckle.Sdk.Common;
 using Speckle.Sdk.Common.Exceptions;
 using Speckle.Sdk.Logging;
 using Speckle.Sdk.Models;
+using Speckle.Sdk.Models.Instances;
 using Transform = Speckle.Objects.Other.Transform;
 
 namespace Speckle.Connectors.Revit.Operations.Receive;
@@ -38,12 +39,15 @@ public sealed class RevitHostObjectBuilder(
   IThreadContext threadContext,
   RevitToHostCacheSingleton revitToHostCacheSingleton,
   ITypedConverter<
-    (Base atomicObject, IReadOnlyCollection<Matrix4x4> matrix),
+    (Base atomicObject, IReadOnlyCollection<Matrix4x4> matrix, DataObject? parentDataObject),
     DirectShape
   > localToGlobalDirectShapeConverter,
   IReceiveConversionHandler conversionHandler
 ) : IHostObjectBuilder, IDisposable
 {
+  // Maps atomic object applicationId -> parent DataObject
+  private readonly Dictionary<string, DataObject> _atomicObjectToParentDataObject = new();
+
   public Task<HostObjectBuilderResult> Build(
     Base rootObject,
     string projectName,
@@ -102,6 +106,9 @@ public sealed class RevitHostObjectBuilder(
       unpackedRoot.ObjectsToConvert.ToList()
     );
 
+    // Register DataObjects with InstanceProxy displayValues
+    RegisterDataObjectsWithInstanceProxies(unpackedRoot);
+
     // NOTE: below is 💩... https://github.com/specklesystems/speckle-sharp-connectors/pull/813 broke sketchup to revit workflow
     // ids were modified to fix receiving instances [CNX-1707](https://linear.app/speckle/issue/CNX-1707/revit-curves-and-meshes-in-blocks-come-as-duplicated)
     // but we then broke sketchup to revit because applicationIds in proxies didn't match modified application ids which cam from #813 hack
@@ -176,6 +183,9 @@ public sealed class RevitHostObjectBuilder(
       }
     }
 
+    // Update DataObject lookup IDs
+    UpdateAtomicObjectLookupWithModifiedIds(originalToModifiedIds);
+
     // 2 - Bake materials (now with the updated IDs)
     if (unpackedRoot.RenderMaterialProxies != null)
     {
@@ -234,6 +244,87 @@ public sealed class RevitHostObjectBuilder(
     return conversionResults.builderResult;
   }
 
+  /// <summary>
+  /// Registers DataObjects that have InstanceProxy displayValues and builds the lookup.
+  /// </summary>
+  private void RegisterDataObjectsWithInstanceProxies(RootObjectUnpackerResult unpackedRoot)
+  {
+    var definitionToDataObject = new Dictionary<string, DataObject>();
+
+    foreach (var tc in unpackedRoot.ObjectsToConvert)
+    {
+      if (tc.Current is DataObject dataObject)
+      {
+        var instanceProxies = dataObject.displayValue.OfType<InstanceProxy>().ToList();
+        if (instanceProxies.Count > 0)
+        {
+          foreach (var ip in instanceProxies)
+          {
+            definitionToDataObject[ip.definitionId] = dataObject;
+          }
+        }
+      }
+    }
+
+    // Build lookup: definition object applicationId -> parent DataObject
+    _atomicObjectToParentDataObject.Clear();
+    if (unpackedRoot.DefinitionProxies is not null)
+    {
+      foreach (var defProxy in unpackedRoot.DefinitionProxies)
+      {
+        if (
+          defProxy.applicationId is not null
+          && definitionToDataObject.TryGetValue(defProxy.applicationId, out var parentDataObject)
+        )
+        {
+          foreach (var objectId in defProxy.objects)
+          {
+            _atomicObjectToParentDataObject[objectId] = parentDataObject;
+          }
+        }
+        else
+        {
+          logger.LogError(
+            "Could not find parent DataObject for DefinitionProxy {ApplicationId}",
+            defProxy.applicationId
+          );
+        }
+      }
+    }
+  }
+
+  /// <summary>
+  /// Updates the atomic object lookup with modified IDs
+  /// </summary>
+  private void UpdateAtomicObjectLookupWithModifiedIds(Dictionary<string, List<string>> originalToModifiedIds)
+  {
+    // Build updated entries first to avoid modifying collection during iteration
+    var entriesToAdd = new List<KeyValuePair<string, DataObject>>();
+    var keysToRemove = new List<string>();
+
+    foreach (var kvp in _atomicObjectToParentDataObject)
+    {
+      if (originalToModifiedIds.TryGetValue(kvp.Key, out var modifiedIds))
+      {
+        keysToRemove.Add(kvp.Key);
+        foreach (var modifiedId in modifiedIds)
+        {
+          entriesToAdd.Add(new(modifiedId, kvp.Value));
+        }
+      }
+    }
+
+    foreach (var key in keysToRemove)
+    {
+      _atomicObjectToParentDataObject.Remove(key);
+    }
+
+    foreach (var entry in entriesToAdd)
+    {
+      _atomicObjectToParentDataObject[entry.Key] = entry.Value;
+    }
+  }
+
   private Autodesk.Revit.DB.Transform? CalculateNewTransform(
     Autodesk.Revit.DB.Transform? receiveTransform,
     Autodesk.Revit.DB.Transform? rootTransform
@@ -278,9 +369,17 @@ public sealed class RevitHostObjectBuilder(
         onOperationProgressed.Report(new("Converting", (double)++count / localToGlobalMaps.Count));
         if (result is DirectShapeDefinitionWrapper)
         {
+          // Look up parent DataObject for this atomic object (handles InstanceProxy displayValue)
+          var atomicId = localToGlobalMap.AtomicObject.applicationId;
+          DataObject? parentDataObject = null;
+          if (atomicId is not null)
+          {
+            _atomicObjectToParentDataObject.TryGetValue(atomicId, out parentDataObject);
+          }
+
           // direct shape creation happens here
           DirectShape directShapes = localToGlobalDirectShapeConverter.Convert(
-            (localToGlobalMap.AtomicObject, localToGlobalMap.Matrix)
+            (localToGlobalMap.AtomicObject, localToGlobalMap.Matrix, parentDataObject)
           );
 
           bakedObjectIds.Add(directShapes.UniqueId);
@@ -351,6 +450,7 @@ public sealed class RevitHostObjectBuilder(
     DirectShapeLibrary.GetDirectShapeLibrary(converterSettings.Current.Document).Reset(); // Note: this needs to be cleared, as it is being used in the converter
 
     revitToHostCacheSingleton.MaterialsByObjectId.Clear(); // Massive hack!
+    _atomicObjectToParentDataObject.Clear();
     groupManager.PurgeGroups(baseGroupName);
     materialBaker.PurgeMaterials(baseGroupName);
   }

Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Send/RevitRootObjectBuilder.cs

@@ -1,3 +1,4 @@
+using System.Diagnostics.CodeAnalysis;
 using Autodesk.Revit.DB;
 using Microsoft.Extensions.Logging;
 using Speckle.Connectors.Common.Builders;
@@ -7,6 +8,7 @@ using Speckle.Connectors.Common.Extensions;
 using Speckle.Connectors.Common.Operations;
 using Speckle.Connectors.Common.Threading;
 using Speckle.Connectors.DUI.Exceptions;
+using Speckle.Connectors.DUI.Settings;
 using Speckle.Connectors.Revit.HostApp;
 using Speckle.Converters.Common;
 using Speckle.Converters.RevitShared.Helpers;
@@ -29,7 +31,8 @@ public class RevitRootObjectBuilder(
   SendCollectionManager sendCollectionManager,
   ILogger<RevitRootObjectBuilder> logger,
   RevitToSpeckleCacheSingleton revitToSpeckleCacheSingleton,
-  LinkedModelHandler linkedModelHandler
+  LinkedModelHandler linkedModelHandler,
+  IConfigStore configStore
 ) : IRootObjectBuilder<DocumentToConvert>
 {
   public Task<RootObjectBuilderResult> Build(
@@ -42,6 +45,7 @@ public class RevitRootObjectBuilder(
       () => Task.FromResult(BuildSync(documentElementContexts, projectId, onOperationProgressed, ct))
     );
 
+  [SuppressMessage("Maintainability", "CA1506:Avoid excessive class coupling")]
   private RootObjectBuilderResult BuildSync(
     IReadOnlyList<DocumentToConvert> documentElementContexts,
     string projectId,
@@ -134,6 +138,8 @@ public class RevitRootObjectBuilder(
     var cacheHitCount = 0;
     var skippedObjectCount = 0;
 
+    var config = configStore.GetConnectorConfig();
+
     foreach (var atomicObjectByDocumentAndTransform in atomicObjectsByDocumentAndTransform)
     {
       string? modelDisplayName = null;
@@ -185,7 +191,11 @@ public class RevitRootObjectBuilder(
             // TODO: Potential here to transform cached objects and NOT reconvert,
             // TODO: we wont do !hasTransform here, and re-set application id before this
 
-            if (!hasTransform && sendConversionCache.TryGetValue(projectId, applicationId, out ObjectReference? value))
+            if (
+              !hasTransform
+              && !config.DocumentChangeListeningDisabled //This is experimental
+              && sendConversionCache.TryGetValue(projectId, applicationId, out ObjectReference? value)
+            )
             {
               converted = value;
               cacheHitCount++;

Connectors/Revit/Speckle.Connectors.RevitShared/Plugin/RevitIdleManager.cs

@@ -1,75 +1,112 @@
-using System.Collections.Concurrent;
-using System.Diagnostics;
 using Autodesk.Revit.UI;
 using Autodesk.Revit.UI.Events;
+using Microsoft.Extensions.Logging;
+using Speckle.Connectors.DUI.Bridge;
 using Speckle.Converters.RevitShared.Helpers;
 using Speckle.Sdk.Common;
 
 namespace Speckle.Connectors.Revit.Plugin;
 
-/// <summary>
-/// OK.
-/// Please do not try to generalize this class with other IdleManagers for whatever reason.
-/// This class is simple, targeted to host app and singleton.
-/// </summary>
-public class RevitIdleManager(RevitContext revitContext)
+/// <remarks>
+/// Please do NOT try and refactor this class.
+/// Whether it's to try and generalize with the <see cref="IdleCallManager"/> class
+/// or to unnecessary try and make this class thread safe.
+/// This class is a simple singleton, targeted to a Revit's host app requirements
+/// where everything happens on the main thread, and we can avoid overly complex threading/thread-safty.
+///
+/// Previous good refactors with good intention have lead to poor debugging experiences, over-engineered threading,
+/// and low confidence in the reliability.
+/// </remarks>
+/// should be registered as singleton
+public class RevitIdleManager(
+  RevitContext revitContext,
+  ITopLevelExceptionHandler topLevelExceptionHandler,
+  ILogger<RevitIdleManager> logger
+)
 {
   private readonly UIApplication _uiApplication = revitContext.UIApplication.NotNull();
 
-  private readonly ConcurrentDictionary<string, Func<Task>> _calls = new();
-  private volatile bool _hasSubscribed;
+  private readonly Dictionary<string, Func<Task>> _calls = new();
+  private bool _hasSubscribed;
+
+  private bool _isExecutingIdle;
 
   /// <summary>
-  /// Subscribe deferred action to Idling event to run it whenever Revit becomes idle.
+  /// Defers the invocation of an <paramref name="action"/> until next Revit idle tick (deduped by name).
+  /// The <paramref name="action"/> will be called only once.
   /// </summary>
-  /// <param name="action"> Action to call whenever Revit becomes Idle.</param>
-  /// some events in host app are trigerred many times, we might get 10x per object
+  /// <param name="name">A key that prevents enqueuing duplicate events</param>
+  /// <param name="action">The action to be invoked</param>
+  /// <example>
+  /// Some events in host app are triggered many times, we might get 10x per object
   /// Making this more like a deferred action, so we don't update the UI many times
+  /// </example>
+  /// <remarks>
+  /// This function must be called on the main thread
+  /// </remarks>
   public void SubscribeToIdle(string name, Action action)
   {
-    // I want to be called back ONCE when the host app has become idle once more
-    _calls[name] = () =>
-    {
-      action();
-      return Task.CompletedTask;
-    };
-
-    if (_hasSubscribed)
-    {
-      return;
-    }
-
-    _hasSubscribed = true;
-    _uiApplication.Idling += RevitAppOnIdle;
+    SubscribeToIdle(
+      name,
+      () =>
+      {
+        action.Invoke();
+        return Task.CompletedTask;
+      }
+    );
   }
 
-  /// <summary>
-  /// Run once on the next Revit idle tick (deduped by name).
-  /// </summary>
+  /// <inheritdoc cref="SubscribeToIdle(string, Action)"/>
   public void SubscribeToIdle(string name, Func<Task> action)
   {
+    if (_isExecutingIdle)
+    {
+      logger.LogWarning("SubscribeToIdle called while already executing idle events");
+    }
+
     _calls[name] = action;
+
     if (_hasSubscribed)
     {
       return;
     }
-
     _hasSubscribed = true;
+
     _uiApplication.Idling += RevitAppOnIdle;
   }
 
   private void RevitAppOnIdle(object? sender, IdlingEventArgs e)
   {
-    foreach (KeyValuePair<string, Func<Task>> kvp in _calls)
+    topLevelExceptionHandler.CatchUnhandled(() =>
     {
-      Debug.WriteLine($"{kvp.Key}");
-      kvp.Value();
-    }
+      if (_isExecutingIdle)
+      {
+        logger.LogWarning("SubscribeToIdle called while already executing idle events");
+      }
 
-    _calls.Clear();
-    _uiApplication.Idling -= RevitAppOnIdle;
+      _isExecutingIdle = true;
+      try
+      {
+        try
+        {
+          foreach (KeyValuePair<string, Func<Task>> kvp in _calls)
+          {
+            topLevelExceptionHandler.FireAndForget(kvp.Value.Invoke);
+          }
+        }
+        finally
+        {
+          _calls.Clear();
+        }
+      }
+      finally
+      {
+        _uiApplication.Idling -= RevitAppOnIdle;
 
-    // setting last will delay entering re-subscritption
-    _hasSubscribed = false;
+        _isExecutingIdle = false;
+        // setting last will delay entering re-subscription
+        _hasSubscribed = false;
+      }
+    });
   }
 }

Converters/Revit/Speckle.Converters.RevitShared/ToHost/Raw/LocalToGlobalToDirectShapeConverter.cs

@@ -2,6 +2,7 @@ using Speckle.Converters.Common;
 using Speckle.Converters.Common.Objects;
 using Speckle.Converters.RevitShared.Settings;
 using Speckle.DoubleNumerics;
+using Speckle.Objects.Data;
 using Speckle.Sdk.Common;
 using Speckle.Sdk.Models;
 using Speckle.Sdk.Models.Extensions;
@@ -10,11 +11,14 @@ namespace Speckle.Converters.RevitShared.ToSpeckle;
 
 /// <summary>
 /// Converts local to global maps to direct shapes.
-/// Spirit of the LocalToGlobalMap, we can't pass that object directly here bc it lives in Connectors.Common which I (ogu) don't want to bother with it.
-/// All this is  poc that should be burned, once we enable proper block support to revit.
+/// When atomicObject comes from an InstanceProxy displayValue, parentDataObject
+/// provides the original DataObject's metadata (category, name) for semantic preservation.
 /// </summary>
 public class LocalToGlobalToDirectShapeConverter
-  : ITypedConverter<(Base atomicObject, IReadOnlyCollection<Matrix4x4> matrix), DB.DirectShape>
+  : ITypedConverter<
+    (Base atomicObject, IReadOnlyCollection<Matrix4x4> matrix, DataObject? parentDataObject),
+    DB.DirectShape
+  >
 {
   private readonly IConverterSettingsStore<RevitConversionSettings> _converterSettings;
   private readonly ITypedConverter<(Matrix4x4 matrix, string units), DB.Transform> _transformConverter;
@@ -28,22 +32,13 @@ public class LocalToGlobalToDirectShapeConverter
     _transformConverter = transformConverter;
   }
 
-  public DB.DirectShape Convert((Base atomicObject, IReadOnlyCollection<Matrix4x4> matrix) target)
+  public DB.DirectShape Convert(
+    (Base atomicObject, IReadOnlyCollection<Matrix4x4> matrix, DataObject? parentDataObject) target
+  )
   {
     // 1- set ds category
-    // NOTE: previously, builtInCategory was on the atomicObject level. this was subsequently moved to properties
-    string? category = null;
-
-    // NOTE: no longer limited to DataObject since the introduction of mapper
-    // The change from `if (target.atomicObject is DataObject dataObject)` is very hacky, but nothing else to do for now
-    // TODO: better define prop interfaces for different applications
-    if (
-      target.atomicObject["properties"] is Dictionary<string, object?> properties
-      && properties.TryGetValue("builtInCategory", out var builtInCategory)
-    )
-    {
-      category = builtInCategory?.ToString();
-    }
+    var category = ExtractBuiltInCategory(target.parentDataObject, target.atomicObject);
+    var name = target.parentDataObject?.name ?? target.atomicObject.TryGetName();
 
     var dsCategory = DB.BuiltInCategory.OST_GenericModel;
     if (category is not null)
@@ -62,10 +57,6 @@ public class LocalToGlobalToDirectShapeConverter
     // 2 - init DirectShape
     var result = DB.DirectShape.CreateElement(_converterSettings.Current.Document, new DB.ElementId(dsCategory));
 
-    // NOTE: this should technically be in a property extraction class / helper method
-    // This change is localised to [CNX-1825](https://linear.app/speckle/issue/CNX-1825/set-directshape-name)
-    // TODO: Property extraction is a greater conversation which needs to be had: [CNX-1830](https://linear.app/speckle/issue/CNX-1830/data-exchange-investigations)
-    var name = target.atomicObject.TryGetName();
     if (name is not null)
     {
       result.SetName(name);
@@ -121,4 +112,24 @@ public class LocalToGlobalToDirectShapeConverter
     result.SetShape(transformedGeometries);
     return result;
   }
+
+  private static string? ExtractBuiltInCategory(DataObject? parentDataObject, Base atomicObject)
+  {
+    // Try parent DataObject first (for InstanceProxy displayValue case)
+    if (parentDataObject?.properties.TryGetValue("builtInCategory", out var cat) == true)
+    {
+      return cat?.ToString();
+    }
+
+    // Fallback to atomicObject properties
+    if (
+      atomicObject["properties"] is Dictionary<string, object?> props
+      && props.TryGetValue("builtInCategory", out var fallbackCat)
+    )
+    {
+      return fallbackCat?.ToString();
+    }
+
+    return null;
+  }
 }

DUI3/Speckle.Connectors.DUI/Bindings/ConfigBinding.cs

@@ -1,10 +1,7 @@
 using System.Diagnostics.CodeAnalysis;
-using System.Runtime.Serialization;
 using Speckle.Connectors.DUI.Bridge;
-using Speckle.Connectors.DUI.Utils;
+using Speckle.Connectors.DUI.Settings;
 using Speckle.Connectors.Logging;
-using Speckle.Sdk;
-using Speckle.Sdk.SQLite;
 
 namespace Speckle.Connectors.DUI.Bindings;
 
@@ -15,26 +12,10 @@ namespace Speckle.Connectors.DUI.Bindings;
 /// ['Rhino',   serialised config]
 /// ['Revit',   serialised config]
 /// </summary>
-public class ConfigBinding : IBinding
+public class ConfigBinding(IConfigStore configStore, IBrowserBridge bridge) : IBinding
 {
   public string Name => "configBinding";
-  public IBrowserBridge Parent { get; }
-  private readonly ISqLiteJsonCacheManager _jsonCacheManager;
-  private readonly ISpeckleApplication _speckleApplication;
-  private readonly IJsonSerializer _serializer;
-
-  public ConfigBinding(
-    IJsonSerializer serializer,
-    ISpeckleApplication speckleApplication,
-    IBrowserBridge bridge,
-    ISqLiteJsonCacheManagerFactory sqLiteJsonCacheManagerFactory
-  )
-  {
-    Parent = bridge;
-    _jsonCacheManager = sqLiteJsonCacheManagerFactory.CreateForUser("DUI3Config"); // POC: maybe inject? (if we ever want to use a different storage for configs later down the line)
-    _speckleApplication = speckleApplication;
-    _serializer = serializer;
-  }
+  public IBrowserBridge Parent { get; } = bridge;
 
 #pragma warning disable CA1024
   public bool GetIsDevMode()
@@ -47,176 +28,32 @@ public class ConfigBinding : IBinding
 #endif
   }
 
-  public ConnectorConfig GetConfig()
-  {
-    var rawConfig = _jsonCacheManager.GetObject(_speckleApplication.HostApplication);
-    if (rawConfig is null)
-    {
-      return SeedConfig();
-    }
+  public ConnectorConfig GetConfig() => configStore.GetConnectorConfig();
 
-    try
-    {
-      var config = _serializer.Deserialize<ConnectorConfig>(rawConfig);
-      if (config is null)
-      {
-        throw new SerializationException("Failed to deserialize config");
-      }
-
-      return config;
-    }
-    catch (SerializationException)
-    {
-      return SeedConfig();
-    }
-  }
-
-  private ConnectorConfig SeedConfig()
-  {
-    var cfg = new ConnectorConfig();
-    UpdateConfig(cfg);
-    return cfg;
-  }
-
-  public void UpdateConfig(ConnectorConfig config)
-  {
-    var str = _serializer.Serialize(config);
-    _jsonCacheManager.UpdateObject(_speckleApplication.HostApplication, str);
-  }
+  public void UpdateConfig(ConnectorConfig config) => configStore.UpdateConnectorConfig(config);
 
   public void SetUserSelectedAccountId(string userSelectedAccountId)
   {
-    var str = _serializer.Serialize(new AccountsConfig() { UserSelectedAccountId = userSelectedAccountId });
-    _jsonCacheManager.UpdateObject("accounts", str);
+    var config = new AccountsConfig() { UserSelectedAccountId = userSelectedAccountId };
+    configStore.UpdateAccountConfig(config);
   }
 
   // TODO: need to be replaced with `GetAccountsConfig` function after some amount of time to not confuse ourselves.
-  public AccountsConfig? GetUserSelectedAccountId()
-  {
-    var rawConfig = _jsonCacheManager.GetObject("accounts");
-    if (rawConfig is null)
-    {
-      return null;
-    }
+  public AccountsConfig? GetUserSelectedAccountId() => GetAccountsConfig();
 
-    try
-    {
-      var config = _serializer.Deserialize<AccountsConfig>(rawConfig);
-      if (config is null)
-      {
-        throw new SerializationException("Failed to deserialize accounts config");
-      }
+  public GlobalConfig? GetGlobalConfig() => configStore.GetGlobalConfig();
 
-      return config;
-    }
-    catch (SerializationException)
-    {
-      return null;
-    }
-  }
-
-  public GlobalConfig? GetGlobalConfig()
-  {
-    var rawConfig = _jsonCacheManager.GetObject("global");
-    if (rawConfig is null)
-    {
-      return null;
-    }
-
-    try
-    {
-      var config = _serializer.Deserialize<GlobalConfig>(rawConfig);
-      if (config is null)
-      {
-        throw new SerializationException("Failed to deserialize global config");
-      }
-
-      return config;
-    }
-    catch (SerializationException)
-    {
-      return null;
-    }
-  }
-
-  public AccountsConfig? GetAccountsConfig()
-  {
-    var rawConfig = _jsonCacheManager.GetObject("accounts");
-    if (rawConfig is null)
-    {
-      return null;
-    }
-
-    try
-    {
-      var config = _serializer.Deserialize<AccountsConfig>(rawConfig);
-      if (config is null)
-      {
-        throw new SerializationException("Failed to deserialize accounts config");
-      }
-
-      return config;
-    }
-    catch (SerializationException)
-    {
-      return null;
-    }
-  }
+  public AccountsConfig? GetAccountsConfig() => configStore.GetAccountsConfig();
 
   public void SetUserSelectedWorkspaceId(string workspaceId)
   {
-    var str = _serializer.Serialize(new WorkspacesConfig() { UserSelectedWorkspaceId = workspaceId });
-    _jsonCacheManager.UpdateObject("workspaces", str);
+    var config = new WorkspacesConfig() { UserSelectedWorkspaceId = workspaceId };
+    configStore.UpdateWorkspacesConfig(config);
   }
 
-  public WorkspacesConfig? GetWorkspacesConfig()
-  {
-    var rawConfig = _jsonCacheManager.GetObject("workspaces");
-    if (rawConfig is null)
-    {
-      return null;
-    }
-
-    try
-    {
-      var config = _serializer.Deserialize<WorkspacesConfig>(rawConfig);
-      if (config is null)
-      {
-        throw new SerializationException("Failed to deserialize workspaces config");
-      }
-
-      return config;
-    }
-    catch (SerializationException)
-    {
-      return null;
-    }
-  }
+  public WorkspacesConfig? GetWorkspacesConfig() => configStore.GetWorkspacesConfig();
 
   [SuppressMessage("Design", "CA1024:Use properties where appropriate", Justification = "Expose to UI")]
   [SuppressMessage("Performance", "CA1822:Mark members as static", Justification = "Expose to UI")]
   public string GetSessionId() => Consts.StaticSessionId;
 }
-
-/// <summary>
-/// POC: A simple POCO for keeping track of settings. I see this as extensible in the future by each host application if and when we will need global per-app connector settings.
-/// </summary>
-public class ConnectorConfig
-{
-  public bool DarkTheme { get; set; } = true;
-}
-
-public class GlobalConfig
-{
-  public bool IsUpdateNotificationDisabled { get; set; }
-}
-
-public class AccountsConfig
-{
-  public string? UserSelectedAccountId { get; set; }
-}
-
-public class WorkspacesConfig
-{
-  public string? UserSelectedWorkspaceId { get; set; }
-}

DUI3/Speckle.Connectors.DUI/Bridge/IdleCallManager.cs

@@ -3,6 +3,12 @@ using Speckle.InterfaceGenerator;
 
 namespace Speckle.Connectors.DUI.Bridge;
 
+/// <remarks>
+/// This class was initially designed as an evolution
+/// of hostapp specific idle managers, since they followed a similar logic.
+/// However, has ended up a little over-engineered, so since then, for Revit connector
+/// we've started to prefer a simpler solution that fits only the needs of said host app.
+/// </remarks>
 //should be registered as singleton
 [GenerateAutoInterface]
 public sealed class IdleCallManager(ITopLevelExceptionHandler topLevelExceptionHandler) : IIdleCallManager

DUI3/Speckle.Connectors.DUI/Settings/ConfigStore.cs

@@ -0,0 +1,193 @@
+using System.Runtime.Serialization;
+using Speckle.Connectors.DUI.Bindings;
+using Speckle.Connectors.DUI.Utils;
+using Speckle.InterfaceGenerator;
+using Speckle.Sdk;
+using Speckle.Sdk.SQLite;
+
+namespace Speckle.Connectors.DUI.Settings;
+
+/// <summary>
+/// See <see cref="ConfigBinding"/>, as it was driving Dim nuts he couldn't swap to a dark theme.
+/// How does it store configs? In a sqlite db called 'DUI3Config', we create a row for each host application:
+/// [ hash,     contents         ]
+/// ['Rhino',   serialised config]
+/// ['Revit',   serialised config]
+/// </summary>
+/// <remarks>
+/// We separated the business logic that's in this class from the <see cref="ConfigBinding"/> so that
+/// <see cref="ConfigStore"/> can be injected into other bindings (you can't inject one binding into another)
+/// </remarks>
+[GenerateAutoInterface]
+public sealed class ConfigStore : IConfigStore
+{
+  private readonly ISqLiteJsonCacheManager _jsonCacheManager;
+  private readonly ISpeckleApplication _speckleApplication;
+  private readonly IJsonSerializer _serializer;
+
+  public ConfigStore(
+    IJsonSerializer serializer,
+    ISpeckleApplication speckleApplication,
+    ISqLiteJsonCacheManagerFactory sqLiteJsonCacheManagerFactory
+  )
+  {
+    _jsonCacheManager = sqLiteJsonCacheManagerFactory.CreateForUser("DUI3Config"); // POC: maybe inject? (if we ever want to use a different storage for configs later down the line)
+    _speckleApplication = speckleApplication;
+    _serializer = serializer;
+  }
+
+  public ConnectorConfig GetConnectorConfig()
+  {
+    var rawConfig = _jsonCacheManager.GetObject(_speckleApplication.HostApplication);
+    if (rawConfig is null)
+    {
+      return SeedConnectorConfig();
+    }
+
+    try
+    {
+      var config = _serializer.Deserialize<ConnectorConfig>(rawConfig);
+      if (config is null)
+      {
+        throw new SerializationException("Failed to deserialize config");
+      }
+
+      return config;
+    }
+    catch (SerializationException)
+    {
+      return SeedConnectorConfig();
+    }
+  }
+
+  private ConnectorConfig SeedConnectorConfig()
+  {
+    var cfg = new ConnectorConfig();
+    UpdateConnectorConfig(cfg);
+    return cfg;
+  }
+
+  public void UpdateConnectorConfig(ConnectorConfig config)
+  {
+    var str = _serializer.Serialize(config);
+    _jsonCacheManager.UpdateObject(_speckleApplication.HostApplication, str);
+  }
+
+  public void UpdateAccountConfig(AccountsConfig accountsConfig)
+  {
+    var str = _serializer.Serialize(accountsConfig);
+    _jsonCacheManager.UpdateObject("accounts", str);
+  }
+
+  public GlobalConfig? GetGlobalConfig()
+  {
+    var rawConfig = _jsonCacheManager.GetObject("global");
+    if (rawConfig is null)
+    {
+      return null;
+    }
+
+    try
+    {
+      var config = _serializer.Deserialize<GlobalConfig>(rawConfig);
+      if (config is null)
+      {
+        throw new SerializationException("Failed to deserialize global config");
+      }
+
+      return config;
+    }
+    catch (SerializationException)
+    {
+      return null;
+    }
+  }
+
+  public AccountsConfig? GetAccountsConfig()
+  {
+    var rawConfig = _jsonCacheManager.GetObject("accounts");
+    if (rawConfig is null)
+    {
+      return null;
+    }
+
+    try
+    {
+      var config = _serializer.Deserialize<AccountsConfig>(rawConfig);
+      if (config is null)
+      {
+        throw new SerializationException("Failed to deserialize accounts config");
+      }
+
+      return config;
+    }
+    catch (SerializationException)
+    {
+      return null;
+    }
+  }
+
+  public WorkspacesConfig? GetWorkspacesConfig()
+  {
+    var rawConfig = _jsonCacheManager.GetObject("workspaces");
+    if (rawConfig is null)
+    {
+      return null;
+    }
+
+    try
+    {
+      var config = _serializer.Deserialize<WorkspacesConfig>(rawConfig);
+      if (config is null)
+      {
+        throw new SerializationException("Failed to deserialize workspaces config");
+      }
+
+      return config;
+    }
+    catch (SerializationException)
+    {
+      return null;
+    }
+  }
+
+  public void UpdateWorkspacesConfig(WorkspacesConfig workspacesConfig)
+  {
+    var str = _serializer.Serialize(workspacesConfig);
+    _jsonCacheManager.UpdateObject("workspaces", str);
+  }
+}
+
+/// <summary>
+/// POC: A simple POCO for keeping track of settings. I see this as extensible in the future by each host application if and when we will need global per-app connector settings.
+/// </summary>
+public sealed class ConnectorConfig
+{
+  public bool DarkTheme { get; init; } = true;
+
+  /// <remarks>
+  /// Only used by Revit Connector !!
+  /// We're exposing some settings to disable event listening inorder to debug app crash issues caused by Revit event handlers
+  /// Normal users are expected to have both enabled.
+  /// We can consider removing this in future once issues are resolved.
+  /// </remarks>
+  public bool SelectionChangeListeningDisabled { get; init; }
+
+  /// <inheritdoc cref="SelectionChangeListeningDisabled" />
+  public bool DocumentChangeListeningDisabled { get; init; }
+}
+
+public sealed class GlobalConfig
+{
+  public bool IsUpdateNotificationDisabled { get; init; }
+}
+
+public sealed class AccountsConfig
+{
+  public string? UserSelectedAccountId { get; init; }
+}
+
+public sealed class WorkspacesConfig
+{
+  public string? UserSelectedWorkspaceId { get; init; }
+}