YAPCSSF: Yet Another ProjectConfigurationStateSynchronizer fix

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

@@ -3,6 +3,7 @@
 
 using System.Collections.Generic;
 using Microsoft.CodeAnalysis.Razor;
+using Microsoft.CodeAnalysis.Razor.ProjectSystem;
 
 namespace Microsoft.AspNetCore.Razor.LanguageServer;
 
@@ -17,40 +18,13 @@ 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))
+            => (x, y) switch
             {
-                return false;
-            }
+                (Work(var keyX), Work(var keyY)) => keyX == keyY,
+                (null, null) => true,
 
-            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,
+                _ => false
             };
-        }
 
         public int GetHashCode(Work obj)
             => obj.GetHashCode();

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

@@ -23,38 +23,38 @@ namespace Microsoft.AspNetCore.Razor.LanguageServer;
 
 internal partial class ProjectConfigurationStateSynchronizer : IProjectConfigurationFileChangeListener, IDisposable
 {
-    private abstract record Work(string ConfigurationFilePath);
-    private sealed record AddProject(string ConfigurationFilePath, RazorProjectInfo ProjectInfo) : Work(ConfigurationFilePath);
-    private sealed record ResetProject(string ConfigurationFilePath, ProjectKey ProjectKey) : Work(ConfigurationFilePath);
-    private sealed record UpdateProject(string ConfigurationFilePath, ProjectKey ProjectKey, RazorProjectInfo ProjectInfo) : Work(ConfigurationFilePath);
+    private abstract record Work(ProjectKey ProjectKey);
+    private sealed record ResetProject(ProjectKey ProjectKey) : Work(ProjectKey);
+    private sealed record UpdateProject(ProjectKey ProjectKey, RazorProjectInfo ProjectInfo) : Work(ProjectKey);
 
     private static readonly TimeSpan s_delay = TimeSpan.FromMilliseconds(250);
 
     private readonly IRazorProjectService _projectService;
+    private readonly IProjectSnapshotManager _projectManager;
     private readonly LanguageServerFeatureOptions _options;
     private readonly ILogger _logger;
 
     private readonly CancellationTokenSource _disposeTokenSource;
     private readonly AsyncBatchingWorkQueue<Work> _workQueue;
 
-    private ImmutableDictionary<string, ProjectKey> _filePathToProjectKeyMap =
-        ImmutableDictionary<string, ProjectKey>.Empty.WithComparers(keyComparer: FilePathComparer.Instance);
-
     public ProjectConfigurationStateSynchronizer(
         IRazorProjectService projectService,
+        IProjectSnapshotManager projectManager,
         ILoggerFactory loggerFactory,
         LanguageServerFeatureOptions options)
-        : this(projectService, loggerFactory, options, s_delay)
+        : this(projectService, projectManager, loggerFactory, options, s_delay)
     {
     }
 
     protected ProjectConfigurationStateSynchronizer(
         IRazorProjectService projectService,
+        IProjectSnapshotManager projectManager,
         ILoggerFactory loggerFactory,
         LanguageServerFeatureOptions options,
         TimeSpan delay)
     {
         _projectService = projectService;
+        _projectManager = projectManager;
         _options = options;
         _logger = loggerFactory.GetOrCreateLogger<ProjectConfigurationStateSynchronizer>();
 
@@ -67,44 +67,21 @@ public void Dispose()
         _disposeTokenSource.Cancel();
         _disposeTokenSource.Dispose();
     }
+
     private async ValueTask ProcessBatchAsync(ImmutableArray<Work> items, CancellationToken token)
     {
         foreach (var item in items.GetMostRecentUniqueItems(Comparer.Instance))
         {
             var itemTask = item switch
             {
-                AddProject(var configurationFilePath, var projectInfo) => AddProjectAsync(configurationFilePath, projectInfo, token),
-                ResetProject(_, var projectKey) => ResetProjectAsync(projectKey, token),
-                UpdateProject(_, var projectKey, var projectInfo) => UpdateProjectAsync(projectKey, projectInfo, token),
+                ResetProject(var projectKey) => ResetProjectAsync(projectKey, token),
+                UpdateProject(var projectKey, var projectInfo) => UpdateProjectAsync(projectKey, projectInfo, token),
                 _ => Assumed.Unreachable<Task>()
             };
 
             await itemTask.ConfigureAwait(false);
         }
 
-        async Task AddProjectAsync(string configurationFilePath, RazorProjectInfo projectInfo, CancellationToken token)
-        {
-            var projectFilePath = FilePathNormalizer.Normalize(projectInfo.FilePath);
-            var intermediateOutputPath = FilePathNormalizer.GetNormalizedDirectoryName(configurationFilePath);
-
-            var projectKey = await _projectService
-                .AddProjectAsync(
-                    projectFilePath,
-                    intermediateOutputPath,
-                    projectInfo.Configuration,
-                    projectInfo.RootNamespace,
-                    projectInfo.DisplayName,
-                    token)
-                .ConfigureAwait(false);
-
-            _logger.LogInformation($"Added {projectKey.Id}.");
-
-            ImmutableInterlocked.AddOrUpdate(ref _filePathToProjectKeyMap, configurationFilePath, projectKey, static (k, v) => v);
-            _logger.LogInformation($"Project configuration file added for project '{projectFilePath}': '{configurationFilePath}'");
-
-            await UpdateProjectAsync(projectKey, projectInfo, token).ConfigureAwait(false);
-        }
-
         Task ResetProjectAsync(ProjectKey projectKey, CancellationToken token)
         {
             _logger.LogInformation($"Resetting {projectKey.Id}.");
@@ -138,53 +115,54 @@ Task UpdateProjectAsync(ProjectKey projectKey, RazorProjectInfo projectInfo, Can
 
     public void ProjectConfigurationFileChanged(ProjectConfigurationFileChangeEventArgs args)
     {
-        var configurationFilePath = FilePathNormalizer.Normalize(args.ConfigurationFilePath);
+        // The 
+        var intermediateOutputPath = FilePathNormalizer.GetNormalizedDirectoryName(args.ConfigurationFilePath);
+        var projectKey = ProjectKey.From(intermediateOutputPath);
 
         switch (args.Kind)
         {
             case RazorFileChangeKind.Changed:
                 {
                     if (args.TryDeserialize(_options, out var projectInfo))
                     {
-                        if (_filePathToProjectKeyMap.TryGetValue(configurationFilePath, out var projectKey))
+                        if (_projectManager.TryGetLoadedProject(projectKey, out _))
                         {
                             _logger.LogInformation($"""
                                 Configuration file changed for project '{projectKey.Id}'.
-                                Configuration file path: '{configurationFilePath}'
+                                Configuration file path: '{args.ConfigurationFilePath}'
                                 """);
 
-                            _workQueue.AddWork(new UpdateProject(configurationFilePath, projectKey, projectInfo));
+                            _workQueue.AddWork(new UpdateProject(projectKey, projectInfo));
                         }
                         else
                         {
                             _logger.LogWarning($"""
                                 Adding project for previously unseen configuration file.
-                                Configuration file path: '{configurationFilePath}'
+                                Configuration file path: '{args.ConfigurationFilePath}'
                                 """);
 
-                            _workQueue.AddWork(new AddProject(configurationFilePath, projectInfo));
+                            AddProject(projectKey, projectInfo, _disposeTokenSource.Token);
                         }
                     }
                     else
                     {
-                        if (_filePathToProjectKeyMap.TryGetValue(configurationFilePath, out var projectKey))
+                        if (_projectManager.TryGetLoadedProject(projectKey, out _))
                         {
                             _logger.LogWarning($"""
                                 Failed to deserialize after change to configuration file for project '{projectKey.Id}'.
-                                Configuration file path: '{configurationFilePath}'
+                                Configuration file path: '{args.ConfigurationFilePath}'
                                 """);
 
-                            // We found the last associated project file for the configuration file. Reset the project since we can't
-                            // accurately determine its configurations.
+                            // We a project for configuration file. Reset is since we couldn't deserialize the info.
 
-                            _workQueue.AddWork(new ResetProject(configurationFilePath, projectKey));
+                            _workQueue.AddWork(new ResetProject(projectKey));
                         }
                         else
                         {
                             // Could not resolve an associated project file.
                             _logger.LogWarning($"""
                                 Failed to deserialize after change to previously unseen configuration file.
-                                Configuration file path: '{configurationFilePath}'
+                                Configuration file path: '{args.ConfigurationFilePath}'
                                 """);
                         }
                     }
@@ -196,15 +174,15 @@ Failed to deserialize after change to previously unseen configuration file.
                 {
                     if (args.TryDeserialize(_options, out var projectInfo))
                     {
-                        _workQueue.AddWork(new AddProject(configurationFilePath, projectInfo));
+                        AddProject(projectKey, projectInfo, _disposeTokenSource.Token);
                     }
                     else
                     {
                         // This is the first time we've seen this configuration file, but we can't deserialize it.
                         // The only thing we can really do is issue a warning.
                         _logger.LogWarning($"""
                             Failed to deserialize previously unseen configuration file.
-                            Configuration file path: '{configurationFilePath}'
+                            Configuration file path: '{args.ConfigurationFilePath}'
                             """);
                     }
                 }
@@ -213,25 +191,45 @@ Failed to deserialize previously unseen configuration file.
 
             case RazorFileChangeKind.Removed:
                 {
-                    if (ImmutableInterlocked.TryRemove(ref _filePathToProjectKeyMap, configurationFilePath, out var projectKey))
+                    if (_projectManager.TryGetLoadedProject(projectKey, out _))
                     {
                         _logger.LogInformation($"""
                             Configuration file removed for project '{projectKey}'.
-                            Configuration file path: '{configurationFilePath}'
+                            Configuration file path: '{args.ConfigurationFilePath}'
                             """);
 
-                        _workQueue.AddWork(new ResetProject(configurationFilePath, projectKey));
+                        _workQueue.AddWork(new ResetProject(projectKey));
                     }
                     else
                     {
                         _logger.LogWarning($"""
                             Failed to resolve associated project on configuration removed event.
-                            Configuration file path: '{configurationFilePath}'
+                            Configuration file path: '{args.ConfigurationFilePath}'
                             """);
                     }
                 }
 
                 break;
         }
     }
+
+    private void AddProject(ProjectKey projectKey, RazorProjectInfo projectInfo, CancellationToken token)
+    {
+        // We fire-and-forget the call to AddProjectAsync below, which is OK because the operation will be
+        // enqueued on the ProjectSnapshotManager's dispatcher. So, it will occur before the work added
+        // below updates the project, since that also happens on the dispatcher.
+        _projectService
+            .AddProjectAsync(
+                filePath: FilePathNormalizer.Normalize(projectInfo.FilePath),
+                intermediateOutputPath: projectKey.Id,
+                projectInfo.Configuration,
+                projectInfo.RootNamespace,
+                projectInfo.DisplayName,
+                token)
+            .Forget();
+
+        _logger.LogInformation($"Added {projectKey.Id}.");
+
+        _workQueue.AddWork(new UpdateProject(projectKey, projectInfo));
+    }
 }

src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/ProjectSystem/MonitorProjectConfigurationFilePathEndpoint.cs

@@ -180,7 +180,7 @@ private Task RemoveMonitorAsync(string projectKeyId, bool removeProject, Cancell
                 {
                     updater.ProjectRemoved(projectKey);
                 },
-                state: ProjectKey.FromString(projectKeyId),
+                state: ProjectKey.From(projectKeyId),
                 cancellationToken);
         }
 

src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/ProjectSystem/ProjectInfoEndpoint.cs

@@ -33,7 +33,7 @@ public Task HandleNotificationAsync(ProjectInfoParams request, RazorRequestConte
         var razorProjectInfo =
             RazorProjectInfoDeserializer.Instance.DeserializeFromString(request.ProjectInfo);
 
-        var projectKey = ProjectKey.FromString(request.ProjectKeyId);
+        var projectKey = ProjectKey.From(request.ProjectKeyId);
 
         return _projectConfigurationStateManager.ProjectInfoUpdatedAsync(projectKey, razorProjectInfo, cancellationToken);
     }

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Extensions/VSProjectContextExtensions.cs

@@ -10,6 +10,6 @@ internal static class VSProjectContextExtensions
 {
     internal static ProjectKey ToProjectKey(this VSProjectContext projectContext)
     {
-        return ProjectKey.FromString(projectContext.Id);
+        return ProjectKey.From(projectContext.Id);
     }
 }

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/ProjectKey.cs

@@ -24,7 +24,10 @@ public static ProjectKey From(Project project)
         return new(intermediateOutputPath);
     }
 
-    internal static ProjectKey FromString(string projectKeyId) => new(projectKeyId);
+    /// <summary>
+    /// Creates a <see cref="ProjectKey"/> from a <see cref="string"/> representing a project's intermediate output path.
+    /// </summary>
+    public static ProjectKey From(string id) => new(id);
 
     public string Id { get; }
 

src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/LanguageClient/Endpoints/RazorCustomMessageTarget_UpdateCSharpBuffer.cs

@@ -77,7 +77,7 @@ request.ProjectKeyId is not null &&
 
             foreach (var virtualDocument in virtualDocuments)
             {
-                if (virtualDocument.ProjectKey.Equals(ProjectKey.FromString(request.ProjectKeyId)))
+                if (virtualDocument.ProjectKey.Equals(ProjectKey.From(request.ProjectKeyId)))
                 {
                     _logger.LogDebug($"UpdateCSharpBuffer virtual doc for {request.HostDocumentVersion} of {virtualDocument.Uri}");
 

src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/ProjectSystem/DefaultWindowsRazorProjectHost.cs

@@ -70,7 +70,7 @@ protected override async Task HandleProjectChangeAsync(string sliceDimensions, I
                 await UpdateAsync(
                     updater =>
                     {
-                        var beforeProjectKey = ProjectKey.FromString(beforeIntermediateOutputPath);
+                        var beforeProjectKey = ProjectKey.From(beforeIntermediateOutputPath);
                         updater.ProjectRemoved(beforeProjectKey);
                     },
                     CancellationToken.None)

src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/ProjectSystem/FallbackWindowsRazorProjectHost.cs

@@ -120,7 +120,7 @@ await UpdateAsync(
             await UpdateAsync(
                 updater =>
                 {
-                    var beforeProjectKey = ProjectKey.FromString(beforeIntermediateOutputPath);
+                    var beforeProjectKey = ProjectKey.From(beforeIntermediateOutputPath);
                     RemoveProject(updater, beforeProjectKey);
                 },
                 CancellationToken.None)