Skip to content

Commit

Permalink
Port #9601 to 17.8: Don't monitor the whole workspace for config file…
Browse files Browse the repository at this point in the history
…s in Visual Studio (#9750)

Cherry picked the commits with no conflicts. Nice to get a straight
forward one.
  • Loading branch information
davidwengier authored Jan 2, 2024
2 parents 8c7fb27 + d650b82 commit d135dd8
Show file tree
Hide file tree
Showing 14 changed files with 95 additions and 72 deletions.
10 changes: 5 additions & 5 deletions azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,10 @@ stages:
pool:
${{ if eq(variables['System.TeamProject'], 'public') }}:
name: $(DncEngPublicBuildPool)
demands: ImageOverride -equals windows.vs2022preview.amd64.open
demands: ImageOverride -equals windows.vs2022.amd64.open
${{ if ne(variables['System.TeamProject'], 'public') }}:
name: $(DncEngInternalBuildPool)
demands: ImageOverride -equals windows.vs2022preview.amd64
demands: ImageOverride -equals windows.vs2022.amd64
steps:
- task: NodeTool@0
displayName: Install Node 10.x
Expand Down Expand Up @@ -131,10 +131,10 @@ stages:
pool:
${{ if eq(variables['System.TeamProject'], 'public') }}:
name: $(DncEngPublicBuildPool)
demands: ImageOverride -equals windows.vs2022preview.amd64.open
demands: ImageOverride -equals windows.vs2022.amd64.open
${{ if ne(variables['System.TeamProject'], 'public') }}:
name: $(DncEngInternalBuildPool)
demands: ImageOverride -equals windows.vs2022preview.amd64
demands: ImageOverride -equals windows.vs2022.amd64
strategy:
matrix:
${{ if eq(variables['System.TeamProject'], 'public') }}:
Expand Down Expand Up @@ -250,7 +250,7 @@ stages:
-integrationTest
name: Run_Integration_Tests
displayName: Run Integration Tests
condition: and(succeeded(), in(variables['Build.Reason'], 'PullRequest'))
condition: and(succeeded(), in(variables['Build.Reason'], 'DISABLE'))

- powershell: ./eng/scripts/FinishDumpCollectionForHangingBuilds.ps1 artifacts/log/$(_BuildConfig)
displayName: Finish background dump collection
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ internal class ConfigurableLanguageServerFeatureOptions : LanguageServerFeatureO
private readonly bool? _usePreciseSemanticTokenRanges;
private readonly bool? _updateBuffersForClosedDocuments;
private readonly bool? _includeProjectKeyInGeneratedFilePath;
private readonly bool? _monitorWorkspaceFolderForConfigurationFiles;

public override bool SupportsFileManipulation => _supportsFileManipulation ?? _defaults.SupportsFileManipulation;
public override string ProjectConfigurationFileName => _projectConfigurationFileName ?? _defaults.ProjectConfigurationFileName;
Expand All @@ -35,6 +36,7 @@ internal class ConfigurableLanguageServerFeatureOptions : LanguageServerFeatureO
public override bool UsePreciseSemanticTokenRanges => _usePreciseSemanticTokenRanges ?? _defaults.UsePreciseSemanticTokenRanges;
public override bool UpdateBuffersForClosedDocuments => _updateBuffersForClosedDocuments ?? _defaults.UpdateBuffersForClosedDocuments;
public override bool IncludeProjectKeyInGeneratedFilePath => _includeProjectKeyInGeneratedFilePath ?? _defaults.IncludeProjectKeyInGeneratedFilePath;
public override bool MonitorWorkspaceFolderForConfigurationFiles => _monitorWorkspaceFolderForConfigurationFiles ?? _defaults.MonitorWorkspaceFolderForConfigurationFiles;

public ConfigurableLanguageServerFeatureOptions(string[] args)
{
Expand All @@ -57,6 +59,7 @@ public ConfigurableLanguageServerFeatureOptions(string[] args)
TryProcessBoolOption(nameof(UsePreciseSemanticTokenRanges), ref _usePreciseSemanticTokenRanges, option, args, i);
TryProcessBoolOption(nameof(UpdateBuffersForClosedDocuments), ref _updateBuffersForClosedDocuments, option, args, i);
TryProcessBoolOption(nameof(IncludeProjectKeyInGeneratedFilePath), ref _includeProjectKeyInGeneratedFilePath, option, args, i);
TryProcessBoolOption(nameof(MonitorWorkspaceFolderForConfigurationFiles), ref _monitorWorkspaceFolderForConfigurationFiles, option, args, i);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,6 @@ public override bool ReturnCodeActionAndRenamePathsWithPrefixedSlash
public override bool IncludeProjectKeyInGeneratedFilePath => false;

public override bool UsePreciseSemanticTokenRanges => false;

public override bool MonitorWorkspaceFolderForConfigurationFiles => true;
}
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,14 @@ public static void AddDocumentManagementServices(this IServiceCollection service
services.AddSingleton<IProjectConfigurationFileChangeListener, ProjectConfigurationStateSynchronizer>();
services.AddSingleton<IRazorFileChangeListener, RazorFileSynchronizer>();

// File Change detectors
services.AddSingleton<IFileChangeDetector, ProjectConfigurationFileChangeDetector>();
// If we're not monitoring the whole workspace folder for configuration changes, then we don't actually need the the file change
// detector wired up via DI, as the razor/monitorProjectConfigurationFilePath endpoint will directly construct one. This means
// it can be a little simpler, and doesn't need to worry about which folders it's told to listen to.
if (featureOptions.MonitorWorkspaceFolderForConfigurationFiles)
{
services.AddSingleton<IFileChangeDetector, ProjectConfigurationFileChangeDetector>();
}

services.AddSingleton<IFileChangeDetector, RazorFileChangeDetector>();

// Document processed listeners
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,18 @@
using System.IO;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Razor.LanguageServer.Common;
using Microsoft.AspNetCore.Razor.LanguageServer.EndpointContracts;
using Microsoft.AspNetCore.Razor.Utilities;
using Microsoft.CodeAnalysis.Razor;
using Microsoft.CodeAnalysis.Razor.Workspaces;
using Microsoft.CommonLanguageServerProtocol.Framework;
using Microsoft.Extensions.Logging;

namespace Microsoft.AspNetCore.Razor.LanguageServer;
namespace Microsoft.AspNetCore.Razor.LanguageServer.ProjectSystem;

internal class MonitorProjectConfigurationFilePathEndpoint : IMonitorProjectConfigurationFilePathHandler, IDisposable
[LanguageServerEndpoint(LanguageServerConstants.RazorMonitorProjectConfigurationFilePathEndpoint)]
internal class MonitorProjectConfigurationFilePathEndpoint : IRazorNotificationHandler<MonitorProjectConfigurationFilePathParams>, IDisposable
{
private readonly ProjectSnapshotManagerDispatcher _dispatcher;
private readonly WorkspaceDirectoryPathResolver _workspaceDirectoryPathResolver;
Expand Down Expand Up @@ -81,28 +84,31 @@ public async Task HandleNotificationAsync(MonitorProjectConfigurationFilePathPar
}

var configurationDirectory = Path.GetDirectoryName(request.ConfigurationFilePath);
var normalizedConfigurationDirectory = FilePathNormalizer.NormalizeDirectory(configurationDirectory);
var workspaceDirectory = _workspaceDirectoryPathResolver.Resolve();
var normalizedWorkspaceDirectory = FilePathNormalizer.NormalizeDirectory(workspaceDirectory);

Assumes.NotNull(configurationDirectory);

var previousMonitorExists = _outputPathMonitors.TryGetValue(request.ProjectKeyId, out var entry);

if (normalizedConfigurationDirectory.StartsWith(normalizedWorkspaceDirectory, FilePathComparison.Instance))
if (_options.MonitorWorkspaceFolderForConfigurationFiles)
{
if (previousMonitorExists)
{
_logger.LogInformation("Configuration directory changed from external directory -> internal directory for project '{0}, terminating existing monitor'.", request.ProjectKeyId);
RemoveMonitor(request.ProjectKeyId);
}
else
var normalizedConfigurationDirectory = FilePathNormalizer.NormalizeDirectory(configurationDirectory);
var workspaceDirectory = _workspaceDirectoryPathResolver.Resolve();
var normalizedWorkspaceDirectory = FilePathNormalizer.NormalizeDirectory(workspaceDirectory);

if (normalizedConfigurationDirectory.StartsWith(normalizedWorkspaceDirectory, FilePathComparison.Instance))
{
_logger.LogInformation("No custom configuration directory required. The workspace directory is sufficient for '{0}'.", request.ProjectKeyId);
if (previousMonitorExists)
{
_logger.LogInformation("Configuration directory changed from external directory -> internal directory for project '{0}, terminating existing monitor'.", request.ProjectKeyId);
RemoveMonitor(request.ProjectKeyId);
}
else
{
_logger.LogInformation("No custom configuration directory required. The workspace directory is sufficient for '{0}'.", request.ProjectKeyId);
}

// Configuration directory is already in the workspace directory. We already monitor everything in the workspace directory.
return;
}

// Configuration directory is already in the workspace directory. We already monitor everything in the workspace directory.
return;
}

if (previousMonitorExists)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the MIT license. See License.txt in the project root for license information.

namespace Microsoft.AspNetCore.Razor.LanguageServer.EndpointContracts;
namespace Microsoft.AspNetCore.Razor.LanguageServer.ProjectSystem;

internal class MonitorProjectConfigurationFilePathParams
{
public required string ProjectKeyId { get; set; }

public required string ConfigurationFilePath { get; set; }
public required string? ConfigurationFilePath { get; set; }
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
using Microsoft.AspNetCore.Razor.LanguageServer.Implementation;
using Microsoft.AspNetCore.Razor.LanguageServer.LinkedEditingRange;
using Microsoft.AspNetCore.Razor.LanguageServer.ProjectContexts;
using Microsoft.AspNetCore.Razor.LanguageServer.ProjectSystem;
using Microsoft.AspNetCore.Razor.LanguageServer.Refactoring;
using Microsoft.AspNetCore.Razor.LanguageServer.SignatureHelp;
using Microsoft.AspNetCore.Razor.LanguageServer.WrapWithTag;
Expand Down Expand Up @@ -180,7 +181,7 @@ protected override ILspServices ConstructLspServices()

static void AddHandlers(IServiceCollection services)
{
// Not calling AddHandler because we want to register this endpoint as an IOnIntialized too
// Not calling AddHandler because we want to register this endpoint as an IOnInitialized too
services.AddSingleton<RazorConfigurationEndpoint>();
services.AddSingleton<IMethodHandler, RazorConfigurationEndpoint>(s => s.GetRequiredService<RazorConfigurationEndpoint>());
// Transient because it should only be used once and I'm hoping it doesn't stick around.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,13 @@ internal abstract class LanguageServerFeatureOptions
/// ensure a unique file path per project.
/// </summary>
public abstract bool IncludeProjectKeyInGeneratedFilePath { get; }

/// <summary>
/// Whether to monitor the entire workspace folder for any project.razor.bin files
/// </summary>
/// <remarks>
/// When this is off, the language server won't have any project knowledge unless the
/// razor/monitorProjectConfigurationFilePath notification is sent.
/// </remarks>
public abstract bool MonitorWorkspaceFolderForConfigurationFiles { get; }
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System.Threading.Tasks;
using Microsoft.AspNetCore.Razor.LanguageServer;
using Microsoft.AspNetCore.Razor.LanguageServer.Common;
using Microsoft.AspNetCore.Razor.LanguageServer.ProjectSystem;
using Microsoft.AspNetCore.Razor.Telemetry;
using Microsoft.CodeAnalysis.Host;
using Microsoft.CodeAnalysis.Razor;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,4 +80,6 @@ public VisualStudioWindowsLanguageServerFeatureOptions(LSPEditorFeatureDetector
public override bool IncludeProjectKeyInGeneratedFilePath => _includeProjectKeyInGeneratedFilePath.Value;

public override bool UsePreciseSemanticTokenRanges => _usePreciseSemanticTokenRanges.Value;

public override bool MonitorWorkspaceFolderForConfigurationFiles => false;
}
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the MIT license. See License.txt in the project root for license information.

#nullable disable

using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Razor.LanguageServer.EndpointContracts;
using Microsoft.AspNetCore.Razor.LanguageServer.ProjectSystem;
using Microsoft.AspNetCore.Razor.Test.Common;
using Microsoft.CodeAnalysis.Razor;
using Microsoft.CodeAnalysis.Razor.Workspaces;
using Microsoft.Extensions.Logging;
using Moq;
using Xunit;
Expand Down Expand Up @@ -135,6 +134,32 @@ public async Task Handle_InWorkspaceDirectory_Noops()
Assert.Equal(0, detector.StartCount);
}

[Fact]
public async Task Handle_InWorkspaceDirectory_MonitorsIfLanguageFeatureOptionSet()
{
// Arrange
var detector = new TestFileChangeDetector();
var configurationFileEndpoint = new TestMonitorProjectConfigurationFilePathEndpoint(
() => detector,
LegacyDispatcher,
_directoryPathResolver,
Enumerable.Empty<IProjectConfigurationFileChangeListener>(),
LoggerFactory,
options: new TestLanguageServerFeatureOptions(monitorWorkspaceFolderForConfigurationFiles: false));
var startRequest = new MonitorProjectConfigurationFilePathParams()
{
ProjectKeyId = TestProjectKey.Create("C:/dir/obj").Id,
ConfigurationFilePath = "C:/dir/obj/Debug/project.razor.bin",
};
var requestContext = CreateRazorRequestContext(documentContext: null);

// Act
await configurationFileEndpoint.HandleNotificationAsync(startRequest, requestContext, DisposalToken);

// Assert
Assert.Equal(1, detector.StartCount);
}

[Fact]
public async Task Handle_DuplicateMonitors_Noops()
{
Expand Down Expand Up @@ -323,31 +348,18 @@ private class TestMonitorProjectConfigurationFilePathEndpoint : MonitorProjectCo
{
private readonly Func<IFileChangeDetector> _fileChangeDetectorFactory;

public TestMonitorProjectConfigurationFilePathEndpoint(
ProjectSnapshotManagerDispatcher projectSnapshotManagerDispatcher,
WorkspaceDirectoryPathResolver workspaceDirectoryPathResolver,
IEnumerable<IProjectConfigurationFileChangeListener> listeners,
ILoggerFactory loggerFactory)
: this(
fileChangeDetectorFactory: null,
projectSnapshotManagerDispatcher,
workspaceDirectoryPathResolver,
listeners,
loggerFactory)
{
}

public TestMonitorProjectConfigurationFilePathEndpoint(
Func<IFileChangeDetector> fileChangeDetectorFactory,
ProjectSnapshotManagerDispatcher projectSnapshotManagerDispatcher,
WorkspaceDirectoryPathResolver workspaceDirectoryPathResolver,
IEnumerable<IProjectConfigurationFileChangeListener> listeners,
ILoggerFactory loggerFactory)
ILoggerFactory loggerFactory,
LanguageServerFeatureOptions? options = null)
: base(
projectSnapshotManagerDispatcher,
workspaceDirectoryPathResolver,
listeners,
TestLanguageServerFeatureOptions.Instance,
options ?? TestLanguageServerFeatureOptions.Instance,
loggerFactory)
{
_fileChangeDetectorFactory = fileChangeDetectorFactory ?? (() => Mock.Of<IFileChangeDetector>(MockBehavior.Strict));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,15 @@ internal class TestLanguageServerFeatureOptions : LanguageServerFeatureOptions
{
public static readonly LanguageServerFeatureOptions Instance = new TestLanguageServerFeatureOptions();

private bool _includeProjectKeyInGeneratedFilePath;
private readonly bool _includeProjectKeyInGeneratedFilePath;
private readonly bool _monitorWorkspaceFolderForConfigurationFiles;

public TestLanguageServerFeatureOptions(
bool includeProjectKeyInGeneratedFilePath = false)
bool includeProjectKeyInGeneratedFilePath = false,
bool monitorWorkspaceFolderForConfigurationFiles = true)
{
_includeProjectKeyInGeneratedFilePath = includeProjectKeyInGeneratedFilePath;
_monitorWorkspaceFolderForConfigurationFiles = monitorWorkspaceFolderForConfigurationFiles;
}

public override bool SupportsFileManipulation => false;
Expand All @@ -40,4 +43,6 @@ public TestLanguageServerFeatureOptions(
public override bool UpdateBuffersForClosedDocuments => false;

public override bool IncludeProjectKeyInGeneratedFilePath => _includeProjectKeyInGeneratedFilePath;

public override bool MonitorWorkspaceFolderForConfigurationFiles => _monitorWorkspaceFolderForConfigurationFiles;
}

0 comments on commit d135dd8

Please sign in to comment.