From da935bfa95a8d6112cb892744d62dc8c5e5da88d Mon Sep 17 00:00:00 2001 From: "N. Taylor Mullen" Date: Thu, 6 Sep 2018 18:12:24 -0700 Subject: [PATCH] Generated document output persists result to generated code container. - Prior to this we had a `BackgroundDocumentGenerator` that would constantly be updating generated code containers. With this changes we've changed the details in how a `GeneratedCodeContainer` can be mutated. It can now be touched from any thread and is updated when an underlying `DocumentSnapshot` has available content. However, if a generated output comes through that's older then the last seen output we no-op. - Removed `BackgroundDocumentGenerator` SetOutput logic. - Updated tests to react to new behavior of `GetGeneratedOutputAsync`. - Added new test to verify getting generated output results in the setting of the host documents output. --- .../DocumentGeneratedOutputTracker.cs | 9 +- .../ProjectSystem/GeneratedCodeContainer.cs | 168 ++++++++++++++---- .../BackgroundDocumentGenerator.cs | 22 --- .../DefaultDocumentSnapshotTest.cs | 72 ++++++++ .../GeneratedCodeContainerTest.cs | 16 +- 5 files changed, 224 insertions(+), 63 deletions(-) create mode 100644 test/Microsoft.CodeAnalysis.Razor.Workspaces.Test/ProjectSystem/DefaultDocumentSnapshotTest.cs diff --git a/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/DocumentGeneratedOutputTracker.cs b/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/DocumentGeneratedOutputTracker.cs index a3e11da04f..e84cd5fd0b 100644 --- a/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/DocumentGeneratedOutputTracker.cs +++ b/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/DocumentGeneratedOutputTracker.cs @@ -111,7 +111,14 @@ namespace Microsoft.CodeAnalysis.Razor.ProjectSystem var projectEngine = project.GetProjectEngine(); - return projectEngine.ProcessDesignTime(documentSource, importSources, tagHelpers); + var codeDocument = projectEngine.ProcessDesignTime(documentSource, importSources, tagHelpers); + var csharpDocument = codeDocument.GetCSharpDocument(); + if (document is DefaultDocumentSnapshot defaultDocument) + { + defaultDocument.State.HostDocument.GeneratedCodeContainer.SetOutput(csharpDocument, defaultDocument); + } + + return codeDocument; } private async Task GetRazorSourceDocumentAsync(DocumentSnapshot document) diff --git a/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/GeneratedCodeContainer.cs b/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/GeneratedCodeContainer.cs index 342da76b09..548a6733a6 100644 --- a/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/GeneratedCodeContainer.cs +++ b/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/GeneratedCodeContainer.cs @@ -2,32 +2,89 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using Microsoft.CodeAnalysis.Text; -using Microsoft.CodeAnalysis.Experiment; -using Microsoft.AspNetCore.Razor.Language; -using System.Collections.Immutable; -using System.Threading.Tasks; -using System.Threading; using System.Collections.Generic; +using System.Collections.Immutable; +using System.Diagnostics; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Razor.Language; +using Microsoft.CodeAnalysis.Experiment; +using Microsoft.CodeAnalysis.Text; namespace Microsoft.CodeAnalysis.Razor.ProjectSystem { internal class GeneratedCodeContainer : IDocumentServiceFactory, ISpanMapper { + public event EventHandler GeneratedCodeChanged; + + private SourceText _source; + private VersionStamp _sourceVersion; + private RazorCSharpDocument _output; + private DocumentSnapshot _latestDocument; + + private readonly object _setOutputLock = new object(); private readonly TextContainer _textContainer; public GeneratedCodeContainer() { - _textContainer = new TextContainer(); + _textContainer = new TextContainer(_setOutputLock); + _textContainer.TextChanged += TextContainer_TextChanged; } - public SourceText Source { get; private set; } + public SourceText Source + { + get + { + lock (_setOutputLock) + { + return _source; + } + } + } - public VersionStamp SourceVersion { get; private set; } + public VersionStamp SourceVersion + { + get + { + lock (_setOutputLock) + { + return _sourceVersion; + } + } + } - public RazorCSharpDocument Output { get; private set; } + public RazorCSharpDocument Output + { + get + { + lock (_setOutputLock) + { + return _output; + } + } + } - public SourceTextContainer SourceTextContainer => _textContainer; + public DocumentSnapshot LatestDocument + { + get + { + lock (_setOutputLock) + { + return _latestDocument; + } + } + } + + public SourceTextContainer SourceTextContainer + { + get + { + lock (_setOutputLock) + { + return _textContainer; + } + } + } public TService GetService() { @@ -39,28 +96,59 @@ namespace Microsoft.CodeAnalysis.Razor.ProjectSystem return default(TService); } - public void SetOutput(SourceText source, RazorCodeDocument codeDocument) + public void SetOutput(RazorCSharpDocument csharpDocument, DefaultDocumentSnapshot document) { - Source = source; - Output = codeDocument.GetCSharpDocument(); + lock (_setOutputLock) + { + if (!document.TryGetTextVersion(out var version)) + { + Debug.Fail("The text version should have already been evaluated."); + return; + } - _textContainer.SetText(SourceText.From(Output.GeneratedCode)); + var newerVersion = SourceVersion.GetNewerVersion(version); + if (newerVersion == SourceVersion) + { + // Latest document is newer than the provided document. + return; + } + + if (!document.TryGetText(out var source)) + { + Debug.Fail("The text should have already been evaluated."); + return; + } + + _source = source; + _sourceVersion = version; + _output = csharpDocument; + _latestDocument = document; + _textContainer.SetText(SourceText.From(Output.GeneratedCode)); + } } public Task> MapSpansAsync( - Document document, - IEnumerable spans, - CancellationToken cancellationToken) + Document document, + IEnumerable spans, + CancellationToken cancellationToken) { - if (Output == null) + RazorCSharpDocument output; + SourceText source; + lock (_setOutputLock) { - return Task.FromResult(ImmutableArray.Empty); + if (Output == null) + { + return Task.FromResult(ImmutableArray.Empty); + } + + output = Output; + source = Source; } var results = ImmutableArray.CreateBuilder(); foreach (var span in spans) { - if (TryGetLinePositionSpan(span, out var linePositionSpan)) + if (TryGetLinePositionSpan(span, source, output, out var linePositionSpan)) { results.Add(new SpanMapResult(document, linePositionSpan)); } @@ -70,11 +158,11 @@ namespace Microsoft.CodeAnalysis.Razor.ProjectSystem } // Internal for testing. - internal bool TryGetLinePositionSpan(TextSpan span, out LinePositionSpan linePositionSpan) + internal static bool TryGetLinePositionSpan(TextSpan span, SourceText source, RazorCSharpDocument output, out LinePositionSpan linePositionSpan) { - for (var i = 0; i < Output.SourceMappings.Count; i++) + for (var i = 0; i < output.SourceMappings.Count; i++) { - var mapping = Output.SourceMappings[i]; + var mapping = output.SourceMappings[i]; if (span.Length > mapping.GeneratedSpan.Length) { // If the length of the generated span is smaller they can't match. A C# expression @@ -94,7 +182,7 @@ namespace Microsoft.CodeAnalysis.Razor.ProjectSystem { // This span mapping contains the span. var adjusted = new TextSpan(original.Start + leftOffset, (original.End + rightOffset) - (original.Start + leftOffset)); - linePositionSpan = Source.Lines.GetLinePositionSpan(adjusted); + linePositionSpan = source.Lines.GetLinePositionSpan(adjusted); return true; } } @@ -103,15 +191,22 @@ namespace Microsoft.CodeAnalysis.Razor.ProjectSystem return false; } + private void TextContainer_TextChanged(object sender, TextChangeEventArgs args) + { + GeneratedCodeChanged?.Invoke(this, args); + } + private class TextContainer : SourceTextContainer { public override event EventHandler TextChanged; + private readonly object _outerLock; private SourceText _currentText; - public TextContainer() + public TextContainer(object outerLock) : this(SourceText.From(string.Empty)) { + _outerLock = outerLock; } public TextContainer(SourceText sourceText) @@ -124,7 +219,16 @@ namespace Microsoft.CodeAnalysis.Razor.ProjectSystem _currentText = sourceText; } - public override SourceText CurrentText => _currentText; + public override SourceText CurrentText + { + get + { + lock (_outerLock) + { + return _currentText; + } + } + } public void SetText(SourceText sourceText) { @@ -133,10 +237,14 @@ namespace Microsoft.CodeAnalysis.Razor.ProjectSystem throw new ArgumentNullException(nameof(sourceText)); } - var e = new TextChangeEventArgs(_currentText, sourceText); - _currentText = sourceText; + lock (_outerLock) + { - TextChanged?.Invoke(this, e); + var e = new TextChangeEventArgs(_currentText, sourceText); + _currentText = sourceText; + + TextChanged?.Invoke(this, e); + } } } } diff --git a/src/Microsoft.VisualStudio.LanguageServices.Razor/BackgroundDocumentGenerator.cs b/src/Microsoft.VisualStudio.LanguageServices.Razor/BackgroundDocumentGenerator.cs index b0a99e8e24..e06a26fe6a 100644 --- a/src/Microsoft.VisualStudio.LanguageServices.Razor/BackgroundDocumentGenerator.cs +++ b/src/Microsoft.VisualStudio.LanguageServices.Razor/BackgroundDocumentGenerator.cs @@ -187,12 +187,6 @@ namespace Microsoft.CodeAnalysis.Razor OnCompletingBackgroundWork(); - await Task.Factory.StartNew( - () => ReportUpdates(work), - CancellationToken.None, - TaskCreationOptions.None, - _foregroundDispatcher.ForegroundScheduler); - lock (_work) { // Resetting the timer allows another batch of work to start. @@ -219,22 +213,6 @@ namespace Microsoft.CodeAnalysis.Razor } } - private void ReportUpdates(KeyValuePair[] work) - { - for (var i = 0; i < work.Length; i++) - { - var key = work[i].Key; - var document = work[i].Value; - - if (document.TryGetText(out var source) && - document.TryGetGeneratedOutput(out var output)) - { - var container = ((DefaultDocumentSnapshot)document).State.GeneratedCodeContainer; - container.SetOutput(source, output); - } - } - } - private void ReportError(DocumentSnapshot document, Exception ex) { GC.KeepAlive(Task.Factory.StartNew( diff --git a/test/Microsoft.CodeAnalysis.Razor.Workspaces.Test/ProjectSystem/DefaultDocumentSnapshotTest.cs b/test/Microsoft.CodeAnalysis.Razor.Workspaces.Test/ProjectSystem/DefaultDocumentSnapshotTest.cs new file mode 100644 index 0000000000..860334c66a --- /dev/null +++ b/test/Microsoft.CodeAnalysis.Razor.Workspaces.Test/ProjectSystem/DefaultDocumentSnapshotTest.cs @@ -0,0 +1,72 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Threading.Tasks; +using Microsoft.AspNetCore.Razor.Language; +using Microsoft.CodeAnalysis.Host; +using Microsoft.CodeAnalysis.Text; +using Xunit; + +namespace Microsoft.CodeAnalysis.Razor.ProjectSystem +{ + public class DefaultDocumentSnapshotTest + { + public DefaultDocumentSnapshotTest() + { + var services = TestServices.Create( + new[] { new TestProjectSnapshotProjectEngineFactory() }, + new[] { new TestTagHelperResolver() }); + Workspace = TestWorkspace.Create(services); + var hostProject = new HostProject("C:/some/path/project.csproj", RazorConfiguration.Default); + var projectState = ProjectState.Create(Workspace.Services, hostProject); + var project = new DefaultProjectSnapshot(projectState); + HostDocument = new HostDocument("C:/some/path/file.cshtml", "C:/some/path/file.cshtml"); + SourceText = Text.SourceText.From("

Hello World

"); + Version = VersionStamp.Default.GetNewerVersion(); + var textAndVersion = TextAndVersion.Create(SourceText, Version); + var documentState = DocumentState.Create(Workspace.Services, HostDocument, () => Task.FromResult(textAndVersion)); + Document = new DefaultDocumentSnapshot(project, documentState); + + } + + private Workspace Workspace { get; } + + private SourceText SourceText { get; } + + private VersionStamp Version { get; } + + private HostDocument HostDocument { get; } + + private DefaultDocumentSnapshot Document { get; } + + [Fact] + public async Task GetGeneratedOutputAsync_SetsHostDocumentOutput() + { + // Act + await Document.GetGeneratedOutputAsync(); + + // Assert + Assert.NotNull(HostDocument.GeneratedCodeContainer.Output); + Assert.Same(SourceText, HostDocument.GeneratedCodeContainer.Source); + } + + [Fact] + public async Task GetGeneratedOutputAsync_OnlySetsOutputIfDocumentNewer() + { + // Arrange + var newSourceText = SourceText.From("NEW!"); + var newDocumentState = Document.State.WithText(newSourceText, Version.GetNewerVersion()); + var newDocument = new DefaultDocumentSnapshot(Document.Project, newDocumentState); + + // Force the output to be the new output + await newDocument.GetGeneratedOutputAsync(); + + // Act + await Document.GetGeneratedOutputAsync(); + + // Assert + Assert.NotNull(HostDocument.GeneratedCodeContainer.Output); + Assert.Same(newSourceText, HostDocument.GeneratedCodeContainer.Source); + } + } +} diff --git a/test/Microsoft.CodeAnalysis.Razor.Workspaces.Test/ProjectSystem/GeneratedCodeContainerTest.cs b/test/Microsoft.CodeAnalysis.Razor.Workspaces.Test/ProjectSystem/GeneratedCodeContainerTest.cs index 0f2c03c02e..ede9b52d30 100644 --- a/test/Microsoft.CodeAnalysis.Razor.Workspaces.Test/ProjectSystem/GeneratedCodeContainerTest.cs +++ b/test/Microsoft.CodeAnalysis.Razor.Workspaces.Test/ProjectSystem/GeneratedCodeContainerTest.cs @@ -20,10 +20,8 @@ namespace Microsoft.CodeAnalysis.Razor.ProjectSystem "; var sourceText = SourceText.From(content); var codeDocument = GetCodeDocument(content); - var generatedCode = codeDocument.GetCSharpDocument().GeneratedCode; - - var container = new GeneratedCodeContainer(); - container.SetOutput(sourceText, codeDocument); + var csharpDocument = codeDocument.GetCSharpDocument(); + var generatedCode = csharpDocument.GeneratedCode; // TODO: Make writing these tests a little less manual. // Position of `SomeProperty` in the generated code. @@ -34,7 +32,7 @@ namespace Microsoft.CodeAnalysis.Razor.ProjectSystem var expectedLineSpan = new LinePositionSpan(new LinePosition(2, 22), new LinePosition(2, 34)); // Act - var result = container.TryGetLinePositionSpan(span, out var lineSpan); + var result = GeneratedCodeContainer.TryGetLinePositionSpan(span, sourceText, csharpDocument, out var lineSpan); // Assert Assert.True(result); @@ -52,17 +50,15 @@ namespace Microsoft.CodeAnalysis.Razor.ProjectSystem "; var sourceText = SourceText.From(content); var codeDocument = GetCodeDocument(content); - var generatedCode = codeDocument.GetCSharpDocument().GeneratedCode; - - var container = new GeneratedCodeContainer(); - container.SetOutput(sourceText, codeDocument); + var csharpDocument = codeDocument.GetCSharpDocument(); + var generatedCode = csharpDocument.GeneratedCode; // Position of `ExecuteAsync` in the generated code. var symbol = "ExecuteAsync"; var span = new TextSpan(generatedCode.IndexOf(symbol), symbol.Length); // Act - var result = container.TryGetLinePositionSpan(span, out var lineSpan); + var result = GeneratedCodeContainer.TryGetLinePositionSpan(span, sourceText, csharpDocument, out var lineSpan); // Assert Assert.False(result);