YAPCSSF: Automatically add projects if they don't exist when updating (#10335)

Fixes #10306

This change takes a different approach than
#10308 and
#10330 and
#10332 because life was meant to be
interesting.

The original problem we were trying to solve is that when a Reset then
Add came in, we would migrate all of the documents out of the project,
then migrate them all back in again. The various solutions in the other
PRs you can read about at your leisure, this one takes a rather simple
approach:

1. Ignore everything but the most recent update for a specific project
2. If we need to update a project, but it doesn't exist, add it

In the long run we should remove `IRazorProjectService.AddProjectAsync`
entirely, and rename `IRazorProjectService.UpdateProjectAsync` to
`ResetProjectAsync`, but doing that now would just conflict with other
open PRs.

Author: davidwengier

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

@@ -6,6 +6,7 @@
 using System.Collections.Immutable;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.CodeAnalysis.Razor.Logging;
 using Microsoft.CodeAnalysis.Razor.ProjectSystem;
 using Microsoft.CodeAnalysis.Razor.Utilities;
 using Microsoft.CodeAnalysis.Razor.Workspaces;
@@ -29,14 +30,16 @@ internal partial class OpenDocumentGenerator : IRazorStartupService, IDisposable
     private readonly ImmutableArray<IDocumentProcessedListener> _listeners;
     private readonly IProjectSnapshotManager _projectManager;
     private readonly LanguageServerFeatureOptions _options;
+    private readonly ILogger _logger;
 
     private readonly AsyncBatchingWorkQueue<IDocumentSnapshot> _workQueue;
     private readonly CancellationTokenSource _disposeTokenSource;
 
     public OpenDocumentGenerator(
         IEnumerable<IDocumentProcessedListener> listeners,
         IProjectSnapshotManager projectManager,
-        LanguageServerFeatureOptions options)
+        LanguageServerFeatureOptions options,
+        ILoggerFactory loggerFactory)
     {
         _listeners = listeners.ToImmutableArray();
         _projectManager = projectManager;
@@ -49,6 +52,7 @@ public OpenDocumentGenerator(
             _disposeTokenSource.Token);
 
         _projectManager.Changed += ProjectManager_Changed;
+        _logger = loggerFactory.GetOrCreateLogger<OpenDocumentGenerator>();
     }
 
     public void Dispose()
@@ -88,6 +92,8 @@ private void ProjectManager_Changed(object? sender, ProjectChangeEventArgs args)
             return;
         }
 
+        _logger.LogDebug($"Got a project change of type {args.Kind} for {args.ProjectKey.Id}");
+
         switch (args.Kind)
         {
             case ProjectChangeKind.ProjectChanged:
@@ -106,25 +112,11 @@ private void ProjectManager_Changed(object? sender, ProjectChangeEventArgs args)
                 }
 
             case ProjectChangeKind.DocumentAdded:
-                {
-                    var newProject = args.Newer.AssumeNotNull();
-                    var documentFilePath = args.DocumentFilePath.AssumeNotNull();
-
-                    if (newProject.TryGetDocument(documentFilePath, out var document))
-                    {
-                        // We don't enqueue the current document because added documents are initially closed.
-
-                        foreach (var relatedDocument in newProject.GetRelatedDocuments(document))
-                        {
-                            EnqueueIfNecessary(relatedDocument);
-                        }
-                    }
-
-                    break;
-                }
-
             case ProjectChangeKind.DocumentChanged:
                 {
+                    // Most of the time Add will be called on closed files, but when migrating files to/from the misc files
+                    // project they could already be open, but with a different generated C# path
+
                     var newProject = args.Newer.AssumeNotNull();
                     var documentFilePath = args.DocumentFilePath.AssumeNotNull();
 
@@ -178,6 +170,8 @@ void EnqueueIfNecessary(IDocumentSnapshot document)
                 return;
             }
 
+            _logger.LogDebug($"Enqueuing generation of {document.FilePath} in {document.Project.Key.Id}");
+
             _workQueue.AddWork(document);
         }
     }

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

@@ -8,6 +8,7 @@
 using Microsoft.AspNetCore.Razor.ProjectSystem;
 using Microsoft.AspNetCore.Razor.Utilities;
 using Microsoft.CodeAnalysis.Razor;
+using Microsoft.CodeAnalysis.Razor.ProjectSystem;
 using Microsoft.CodeAnalysis.Razor.Workspaces;
 
 namespace Microsoft.AspNetCore.Razor.LanguageServer;
@@ -77,4 +78,10 @@ public bool TryDeserialize(LanguageServerFeatureOptions options, [NotNullWhen(tr
 
         return projectInfo is not null;
     }
+
+    internal ProjectKey GetProjectKey()
+    {
+        var intermediateOutputPath = FilePathNormalizer.GetNormalizedDirectoryName(ConfigurationFilePath);
+        return ProjectKey.FromString(intermediateOutputPath);
+    }
 }

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

@@ -3,6 +3,7 @@
 
 using System.Collections.Generic;
 using Microsoft.CodeAnalysis.Razor;
+using Microsoft.Extensions.Internal;
 
 namespace Microsoft.AspNetCore.Razor.LanguageServer;
 
@@ -17,42 +18,15 @@ 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,
+             => (x, y) switch
+             {
+                 (Work(var keyX), Work(var keyY)) => keyX == keyY,
+                 (null, null) => 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();
+            => obj.ProjectKey.GetHashCode();
     }
 }

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

@@ -4,29 +4,26 @@
 using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
-using System.IO;
+using System.Diagnostics;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Razor.LanguageServer.Common;
 using Microsoft.AspNetCore.Razor.LanguageServer.ProjectSystem;
+using Microsoft.AspNetCore.Razor.PooledObjects;
 using Microsoft.AspNetCore.Razor.ProjectSystem;
 using Microsoft.AspNetCore.Razor.Utilities;
-using Microsoft.CodeAnalysis;
-using Microsoft.CodeAnalysis.Razor;
 using Microsoft.CodeAnalysis.Razor.Logging;
 using Microsoft.CodeAnalysis.Razor.ProjectSystem;
 using Microsoft.CodeAnalysis.Razor.Utilities;
 using Microsoft.CodeAnalysis.Razor.Workspaces;
-using Microsoft.VisualStudio.Threading;
 
 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);
 
@@ -37,8 +34,7 @@ private sealed record UpdateProject(string ConfigurationFilePath, ProjectKey Pro
     private readonly CancellationTokenSource _disposeTokenSource;
     private readonly AsyncBatchingWorkQueue<Work> _workQueue;
 
-    private ImmutableDictionary<string, ProjectKey> _filePathToProjectKeyMap =
-        ImmutableDictionary<string, ProjectKey>.Empty.WithComparers(keyComparer: FilePathComparer.Instance);
+    private readonly Dictionary<ProjectKey, ResetProject> _resetProjectMap = new();
 
     public ProjectConfigurationStateSynchronizer(
         IRazorProjectService projectService,
@@ -67,44 +63,26 @@ public void Dispose()
         _disposeTokenSource.Cancel();
         _disposeTokenSource.Dispose();
     }
+
     private async ValueTask ProcessBatchAsync(ImmutableArray<Work> items, CancellationToken token)
     {
         foreach (var item in items.GetMostRecentUniqueItems(Comparer.Instance))
         {
+            if (token.IsCancellationRequested)
+            {
+                return;
+            }
+
             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}.");
@@ -125,8 +103,9 @@ Task UpdateProjectAsync(ProjectKey projectKey, RazorProjectInfo projectInfo, Can
             _logger.LogInformation($"Updating {projectKey.Id}.");
 
             return _projectService
-                .UpdateProjectAsync(
+                .AddOrUpdateProjectAsync(
                     projectKey,
+                    projectInfo.FilePath,
                     projectInfo.Configuration,
                     projectInfo.RootNamespace,
                     projectInfo.DisplayName,
@@ -138,55 +117,25 @@ Task UpdateProjectAsync(ProjectKey projectKey, RazorProjectInfo projectInfo, Can
 
     public void ProjectConfigurationFileChanged(ProjectConfigurationFileChangeEventArgs args)
     {
-        var configurationFilePath = FilePathNormalizer.Normalize(args.ConfigurationFilePath);
-
         switch (args.Kind)
         {
             case RazorFileChangeKind.Changed:
                 {
                     if (args.TryDeserialize(_options, out var projectInfo))
                     {
-                        if (_filePathToProjectKeyMap.TryGetValue(configurationFilePath, out var projectKey))
-                        {
-                            _logger.LogInformation($"""
-                                Configuration file changed for project '{projectKey.Id}'.
-                                Configuration file path: '{configurationFilePath}'
-                                """);
-
-                            _workQueue.AddWork(new UpdateProject(configurationFilePath, projectKey, projectInfo));
-                        }
-                        else
-                        {
-                            _logger.LogWarning($"""
-                                Adding project for previously unseen configuration file.
-                                Configuration file path: '{configurationFilePath}'
-                                """);
-
-                            _workQueue.AddWork(new AddProject(configurationFilePath, projectInfo));
-                        }
+                        var projectKey = ProjectKey.From(projectInfo);
+                        _logger.LogInformation($"Configuration file changed for project '{projectKey.Id}'.");
+
+                        _workQueue.AddWork(new UpdateProject(projectKey, projectInfo));
                     }
                     else
                     {
-                        if (_filePathToProjectKeyMap.TryGetValue(configurationFilePath, out var projectKey))
-                        {
-                            _logger.LogWarning($"""
-                                Failed to deserialize after change to configuration file for project '{projectKey.Id}'.
-                                Configuration file path: '{configurationFilePath}'
-                                """);
-
-                            // We found the last associated project file for the configuration file. Reset the project since we can't
-                            // accurately determine its configurations.
-
-                            _workQueue.AddWork(new ResetProject(configurationFilePath, 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}'
-                                """);
-                        }
+                        var projectKey = args.GetProjectKey();
+                        _logger.LogWarning($"Failed to deserialize after change to configuration file for project '{projectKey.Id}'.");
+
+                        // We found the last associated project file for the configuration file. Reset the project since we can't
+                        // accurately determine its configurations.
+                        _workQueue.AddWork(new ResetProject(projectKey));
                     }
                 }
 
@@ -196,39 +145,28 @@ Failed to deserialize after change to previously unseen configuration file.
                 {
                     if (args.TryDeserialize(_options, out var projectInfo))
                     {
-                        _workQueue.AddWork(new AddProject(configurationFilePath, projectInfo));
+                        var projectKey = ProjectKey.From(projectInfo);
+                        _logger.LogInformation($"Configuration file added for project '{projectKey.Id}'.");
+
+                        // Update will add the project if it doesn't exist
+                        _workQueue.AddWork(new UpdateProject(projectKey, projectInfo));
                     }
                     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}'
-                            """);
+                        _logger.LogWarning($"Failed to deserialize previously unseen configuration file '{args.ConfigurationFilePath}'");
                     }
                 }
 
                 break;
 
             case RazorFileChangeKind.Removed:
                 {
-                    if (ImmutableInterlocked.TryRemove(ref _filePathToProjectKeyMap, configurationFilePath, out var projectKey))
-                    {
-                        _logger.LogInformation($"""
-                            Configuration file removed for project '{projectKey}'.
-                            Configuration file path: '{configurationFilePath}'
-                            """);
+                    var projectKey = args.GetProjectKey();
+                    _logger.LogInformation($"Configuration file removed for project '{projectKey}'.");
 
-                        _workQueue.AddWork(new ResetProject(configurationFilePath, projectKey));
-                    }
-                    else
-                    {
-                        _logger.LogWarning($"""
-                            Failed to resolve associated project on configuration removed event.
-                            Configuration file path: '{configurationFilePath}'
-                            """);
-                    }
+                    _workQueue.AddWork(new ResetProject(projectKey));
                 }
 
                 break;

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

@@ -36,4 +36,14 @@ Task UpdateProjectAsync(
         ProjectWorkspaceState projectWorkspaceState,
         ImmutableArray<DocumentSnapshotHandle> documents,
         CancellationToken cancellationToken);
+
+    Task AddOrUpdateProjectAsync(
+        ProjectKey projectKey,
+        string filePath,
+        RazorConfiguration? configuration,
+        string? rootNamespace,
+        string displayName,
+        ProjectWorkspaceState projectWorkspaceState,
+        ImmutableArray<DocumentSnapshotHandle> documents,
+        CancellationToken cancellationToken);
 }