Remove ProjectSnapshotManagerDispatcher from ProjectConfigurationStateSynchronizer (#10283)

This change removes `ProjectSnapshotManagerDispatcher` usage from
`ProjectConfigurationStateSynchronizer` and
`ProjectConfigurationFileChangeDetector`. Ultimately, a lot of this code
will go away once #10173 is the default behavior.

As part of this change, I updated
`ProjectConfigurationStateSynchronizer` to use an
`AsyncBatchingWorkQueue`. In my opinion, this is a huge improvement over
the previous implementation.

Author: DustinCampbell

SpellingExclusions.dic

@@ -1,6 +1,7 @@
 csharp
 cshtml
 csproj
+deserializer
 hresult
 microsoft
 vsls

src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/ProjectConfigurationFileChangeDetector.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Collections.Immutable;
 using System.IO;
 using System.Threading;
 using System.Threading.Tasks;
@@ -14,66 +15,42 @@
 
 namespace Microsoft.AspNetCore.Razor.LanguageServer;
 
-internal class ProjectConfigurationFileChangeDetector : IFileChangeDetector
+internal class ProjectConfigurationFileChangeDetector(
+    IEnumerable<IProjectConfigurationFileChangeListener> listeners,
+    LanguageServerFeatureOptions options,
+    ILoggerFactory loggerFactory) : IFileChangeDetector
 {
-    private readonly ProjectSnapshotManagerDispatcher _dispatcher;
-    private readonly IEnumerable<IProjectConfigurationFileChangeListener> _listeners;
-    private readonly LanguageServerFeatureOptions _options;
-    private readonly ILogger _logger;
-    private FileSystemWatcher? _watcher;
-
-    private static readonly IReadOnlyCollection<string> s_ignoredDirectories = new string[]
-    {
+    private static readonly ImmutableArray<string> s_ignoredDirectories =
+    [
         "node_modules",
         "bin",
         ".vs",
-    };
+    ];
 
-    public ProjectConfigurationFileChangeDetector(
-        ProjectSnapshotManagerDispatcher dispatcher,
-        IEnumerable<IProjectConfigurationFileChangeListener> listeners,
-        LanguageServerFeatureOptions options,
-        ILoggerFactory loggerFactory)
-    {
-        if (loggerFactory is null)
-        {
-            throw new ArgumentNullException(nameof(loggerFactory));
-        }
+    private readonly ImmutableArray<IProjectConfigurationFileChangeListener> _listeners = listeners.ToImmutableArray();
+    private readonly LanguageServerFeatureOptions _options = options;
+    private readonly ILogger _logger = loggerFactory.GetOrCreateLogger<ProjectConfigurationFileChangeDetector>();
 
-        _dispatcher = dispatcher ?? throw new ArgumentNullException(nameof(dispatcher));
-        _listeners = listeners ?? throw new ArgumentNullException(nameof(listeners));
-        _options = options ?? throw new ArgumentNullException(nameof(options));
-        _logger = loggerFactory.GetOrCreateLogger<ProjectConfigurationFileChangeDetector>();
-    }
+    private FileSystemWatcher? _watcher;
 
-    public async Task StartAsync(string workspaceDirectory, CancellationToken cancellationToken)
+    public Task StartAsync(string workspaceDirectory, CancellationToken cancellationToken)
     {
-        if (workspaceDirectory is null)
-        {
-            throw new ArgumentNullException(nameof(workspaceDirectory));
-        }
-
         // Dive through existing project configuration files and fabricate "added" events so listeners can accurately listen to state changes for them.
 
         workspaceDirectory = FilePathNormalizer.Normalize(workspaceDirectory);
         var existingConfigurationFiles = GetExistingConfigurationFiles(workspaceDirectory);
 
         _logger.LogDebug($"Triggering events for existing project configuration files");
-        await _dispatcher.RunAsync(() =>
+
+        foreach (var configurationFilePath in existingConfigurationFiles)
         {
-            foreach (var configurationFilePath in existingConfigurationFiles)
-            {
-                FileSystemWatcher_ProjectConfigurationFileEvent(configurationFilePath, RazorFileChangeKind.Added);
-            }
-        }, cancellationToken).ConfigureAwait(false);
+            NotifyListeners(new(configurationFilePath, RazorFileChangeKind.Added));
+        }
 
         // This is an entry point for testing
-        OnInitializationFinished();
-
-        if (cancellationToken.IsCancellationRequested)
+        if (!InitializeFileWatchers)
         {
-            // Client cancelled connection, no need to setup any file watchers. Server is about to tear down.
-            return;
+            return Task.CompletedTask;
         }
 
         try
@@ -85,23 +62,19 @@ await _dispatcher.RunAsync(() =>
                 Directory.CreateDirectory(workspaceDirectory);
             }
         }
-        catch (OperationCanceledException)
-        {
-            return;
-        }
         catch (Exception ex)
         {
             // Directory.Exists will throw on things like long paths
             _logger.LogError(ex, $"Failed validating that file watcher would be successful for '{workspaceDirectory}'");
 
             // No point continuing because the FileSystemWatcher constructor would just throw too.
-            return;
+            return Task.FromException(ex);
         }
 
         if (cancellationToken.IsCancellationRequested)
         {
             // Client cancelled connection, no need to setup any file watchers. Server is about to tear down.
-            return;
+            return Task.FromCanceled(cancellationToken);
         }
 
         _logger.LogInformation($"Starting configuration file change detector for '{workspaceDirectory}'");
@@ -111,26 +84,28 @@ await _dispatcher.RunAsync(() =>
             IncludeSubdirectories = true,
         };
 
-        _watcher.Created += (sender, args) => FileSystemWatcher_ProjectConfigurationFileEvent_Background(args.FullPath, RazorFileChangeKind.Added);
-        _watcher.Deleted += (sender, args) => FileSystemWatcher_ProjectConfigurationFileEvent_Background(args.FullPath, RazorFileChangeKind.Removed);
-        _watcher.Changed += (sender, args) => FileSystemWatcher_ProjectConfigurationFileEvent_Background(args.FullPath, RazorFileChangeKind.Changed);
+        _watcher.Created += (sender, args) => NotifyListeners(args.FullPath, RazorFileChangeKind.Added);
+        _watcher.Deleted += (sender, args) => NotifyListeners(args.FullPath, RazorFileChangeKind.Removed);
+        _watcher.Changed += (sender, args) => NotifyListeners(args.FullPath, RazorFileChangeKind.Changed);
         _watcher.Renamed += (sender, args) =>
         {
             // Translate file renames into remove / add
 
             if (args.OldFullPath.EndsWith(_options.ProjectConfigurationFileName, FilePathComparison.Instance))
             {
                 // Renaming from project configuration file to something else. Just remove the configuration file.
-                FileSystemWatcher_ProjectConfigurationFileEvent_Background(args.OldFullPath, RazorFileChangeKind.Removed);
+                NotifyListeners(args.OldFullPath, RazorFileChangeKind.Removed);
             }
             else if (args.FullPath.EndsWith(_options.ProjectConfigurationFileName, FilePathComparison.Instance))
             {
                 // Renaming from a non-project configuration file file to a real one. Just add the configuration file.
-                FileSystemWatcher_ProjectConfigurationFileEvent_Background(args.FullPath, RazorFileChangeKind.Added);
+                NotifyListeners(args.FullPath, RazorFileChangeKind.Added);
             }
         };
 
         _watcher.EnableRaisingEvents = true;
+
+        return Task.CompletedTask;
     }
 
     public void Stop()
@@ -142,33 +117,28 @@ public void Stop()
     }
 
     // Protected virtual for testing
-    protected virtual void OnInitializationFinished()
-    {
-    }
+    protected virtual bool InitializeFileWatchers => true;
 
     // Protected virtual for testing
-    protected virtual IEnumerable<string> GetExistingConfigurationFiles(string workspaceDirectory)
+    protected virtual ImmutableArray<string> GetExistingConfigurationFiles(string workspaceDirectory)
     {
         return DirectoryHelper.GetFilteredFiles(
             workspaceDirectory,
             _options.ProjectConfigurationFileName,
             s_ignoredDirectories,
-            logger: _logger);
+            logger: _logger).ToImmutableArray();
     }
 
-    private void FileSystemWatcher_ProjectConfigurationFileEvent_Background(string physicalFilePath, RazorFileChangeKind kind)
+    private void NotifyListeners(string physicalFilePath, RazorFileChangeKind kind)
     {
-        _ = _dispatcher.RunAsync(
-            () => FileSystemWatcher_ProjectConfigurationFileEvent(physicalFilePath, kind),
-            CancellationToken.None);
+        NotifyListeners(new(physicalFilePath, kind));
     }
 
-    private void FileSystemWatcher_ProjectConfigurationFileEvent(string physicalFilePath, RazorFileChangeKind kind)
+    private void NotifyListeners(ProjectConfigurationFileChangeEventArgs args)
     {
-        var args = new ProjectConfigurationFileChangeEventArgs(physicalFilePath, kind);
         foreach (var listener in _listeners)
         {
-            _logger.LogDebug($"Notifying listener '{listener}' of config file path '{physicalFilePath}' change with kind '{kind}'");
+            _logger.LogDebug($"Notifying listener '{listener}' of config file path '{args.ConfigurationFilePath}' change with kind '{args.Kind}'");
             listener.ProjectConfigurationFileChanged(args);
         }
     }

src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/ProjectConfigurationFileChangeEventArgs.cs

@@ -12,28 +12,20 @@
 
 namespace Microsoft.AspNetCore.Razor.LanguageServer;
 
-internal sealed class ProjectConfigurationFileChangeEventArgs : EventArgs
+internal sealed class ProjectConfigurationFileChangeEventArgs(
+    string configurationFilePath,
+    RazorFileChangeKind kind,
+    IRazorProjectInfoDeserializer? deserializer = null) : EventArgs
 {
-    public string ConfigurationFilePath { get; }
-    public RazorFileChangeKind Kind { get; }
+    public string ConfigurationFilePath { get; } = configurationFilePath;
+    public RazorFileChangeKind Kind { get; } = kind;
 
-    private readonly IRazorProjectInfoDeserializer _deserializer;
+    private readonly IRazorProjectInfoDeserializer _deserializer = deserializer ?? RazorProjectInfoDeserializer.Instance;
     private RazorProjectInfo? _projectInfo;
-    private readonly object _gate;
+    private readonly object _gate = new();
     private bool _deserialized;
 
-    public ProjectConfigurationFileChangeEventArgs(
-        string configurationFilePath,
-        RazorFileChangeKind kind,
-        IRazorProjectInfoDeserializer? projectInfoDeserializer = null)
-    {
-        ConfigurationFilePath = configurationFilePath ?? throw new ArgumentNullException(nameof(configurationFilePath));
-        Kind = kind;
-        _deserializer = projectInfoDeserializer ?? RazorProjectInfoDeserializer.Instance;
-        _gate = new object();
-    }
-
-    public bool TryDeserialize(LanguageServerFeatureOptions languageServerFeatureOptions, [NotNullWhen(true)] out RazorProjectInfo? projectInfo)
+    public bool TryDeserialize(LanguageServerFeatureOptions options, [NotNullWhen(true)] out RazorProjectInfo? projectInfo)
     {
         if (Kind == RazorFileChangeKind.Removed)
         {
@@ -65,7 +57,7 @@ public bool TryDeserialize(LanguageServerFeatureOptions languageServerFeatureOpt
                     {
                         Configuration = deserializedProjectInfo.Configuration with
                         {
-                            LanguageServerFlags = languageServerFeatureOptions.ToLanguageServerFlags()
+                            LanguageServerFlags = options.ToLanguageServerFlags()
                         }
                     };
 
@@ -79,15 +71,10 @@ public bool TryDeserialize(LanguageServerFeatureOptions languageServerFeatureOpt
                     return false;
                 }
             }
-        }
 
-        projectInfo = _projectInfo;
-        if (projectInfo is null)
-        {
-            // Deserialization failed
-            return false;
+            projectInfo = _projectInfo;
         }
 
-        return true;
+        return projectInfo is not null;
     }
 }

src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/ProjectConfigurationStateSynchronizer.Comparer.cs

@@ -0,0 +1,58 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the MIT license. See License.txt in the project root for license information.
+
+using System.Collections.Generic;
+using Microsoft.CodeAnalysis.Razor;
+
+namespace Microsoft.AspNetCore.Razor.LanguageServer;
+
+internal partial class ProjectConfigurationStateSynchronizer
+{
+    private sealed class Comparer : IEqualityComparer<Work>
+    {
+        public static readonly Comparer Instance = new();
+
+        private Comparer()
+        {
+        }
+
+        public bool Equals(Work? x, Work? y)
+        {
+            if (x is null)
+            {
+                return y is null;
+            }
+            else if (y is null)
+            {
+                return x is null;
+            }
+
+            // For purposes of removing duplicates from batches, two Work instances
+            // are equal only if their identifying properties are equal. So, only
+            // configuration file paths and project keys.
+
+            if (!FilePathComparer.Instance.Equals(x.ConfigurationFilePath, y.ConfigurationFilePath))
+            {
+                return false;
+            }
+
+            return (x, y) switch
+            {
+                (AddProject, AddProject) => true,
+
+                (ResetProject { ProjectKey: var keyX },
+                 ResetProject { ProjectKey: var keyY })
+                    => keyX == keyY,
+
+                (UpdateProject { ProjectKey: var keyX },
+                 UpdateProject { ProjectKey: var keyY })
+                    => keyX == keyY,
+
+                _ => false,
+            };
+        }
+
+        public int GetHashCode(Work obj)
+            => obj.GetHashCode();
+    }
+}

src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/ProjectConfigurationStateSynchronizer.TestAccessor.cs

@@ -0,0 +1,17 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the MIT license. See License.txt in the project root for license information.
+
+using System.Threading.Tasks;
+
+namespace Microsoft.AspNetCore.Razor.LanguageServer;
+
+internal partial class ProjectConfigurationStateSynchronizer
+{
+    internal TestAccessor GetTestAccessor() => new(this);
+
+    internal sealed class TestAccessor(ProjectConfigurationStateSynchronizer instance)
+    {
+        public Task WaitUntilCurrentBatchCompletesAsync()
+            => instance._workQueue.WaitUntilCurrentBatchCompletesAsync();
+    }
+}