Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Adding IView.Path and ViewContext.ExecutingPagePath #1996

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions src/Microsoft.AspNet.Mvc.Core/Rendering/ViewEngine/IView.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,21 @@

namespace Microsoft.AspNet.Mvc.Rendering
{
/// <summary>
/// Specifies the contract for a view.
/// </summary>
public interface IView
{
/// <summary>
/// Gets the path of the view as resolved by the <see cref="ViewResult"/>.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't this come from the view engine? there's nothing stopping you from using this outside of ViewResult

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

/// </summary>
string Path { get; }

/// <summary>
/// Asynchronously renders the view using the specified <paramref name="context"/>.
/// </summary>
/// <param name="context">The <see cref="ViewContext"/>.</param>
/// <returns>A <see cref="Task"/> that on completion renders the view.</returns>
Task RenderAsync([NotNull] ViewContext context);
}
}
45 changes: 45 additions & 0 deletions src/Microsoft.AspNet.Mvc.Core/ViewContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@

namespace Microsoft.AspNet.Mvc
{
/// <summary>
/// Context for view execution.
/// </summary>
public class ViewContext : ActionContext
{
// We need a default FormContext if the user uses html <form> instead of an MvcForm
Expand All @@ -14,6 +17,13 @@ public class ViewContext : ActionContext
private FormContext _formContext;
private DynamicViewData _viewBag;

/// <summary>
/// Initializes a new instance of <see cref="ViewContext"/>.
/// </summary>
/// <param name="actionContext">The <see cref="ActionContext"/>.</param>
/// <param name="view">The <see cref="IView"/> being rendered.</param>
/// <param name="viewData">The <see cref="ViewDataDictionary"/>.</param>
/// <param name="writer">The <see cref="TextWriter"/> to render output to.</param>
public ViewContext(
[NotNull] ActionContext actionContext,
[NotNull] IView view,
Expand All @@ -31,6 +41,13 @@ public ViewContext(
ValidationMessageElement = "span";
}

/// <summary>
/// Initializes a new instance of <see cref="ViewContext"/>.
/// </summary>
/// <param name="viewContext">The <see cref="ViewContext"/> to copy values from.</param>
/// <param name="view">The <see cref="IView"/> being rendered.</param>
/// <param name="viewData">The <see cref="ViewDataDictionary"/>.</param>
/// <param name="writer">The <see cref="TextWriter"/> to render output to.</param>
public ViewContext(
[NotNull] ViewContext viewContext,
[NotNull] IView view,
Expand All @@ -49,6 +66,10 @@ public ViewContext(
Writer = writer;
}

/// <summary>
/// Gets or sets the <see cref="Mvc.FormContext"/> for the form element being rendered.
/// A default context is returned if no form is currently being rendered.
/// </summary>
public virtual FormContext FormContext
{
get
Expand All @@ -62,6 +83,9 @@ public virtual FormContext FormContext
}
}

/// <summary>
/// Gets or sets a value that indicates whether client-side validation is enabled.
/// </summary>
public bool ClientValidationEnabled { get; set; }

/// <summary>
Expand All @@ -84,6 +108,9 @@ public virtual FormContext FormContext
/// </summary>
public string ValidationMessageElement { get; set; }

/// <summary>
/// Gets the dynamic view bag.
/// </summary>
public dynamic ViewBag
{
get
Expand All @@ -97,12 +124,30 @@ public dynamic ViewBag
}
}

/// <summary>
/// Gets or sets the <see cref="IView"/> currently being rendered, if any.
/// </summary>
public IView View { get; set; }

/// <summary>
/// Gets or sets the <see cref="ViewDataDictionary"/>.
/// </summary>
public ViewDataDictionary ViewData { get; set; }

/// <summary>
/// Gets or sets the <see cref="TextWriter"/> used to write the output.
/// </summary>
public TextWriter Writer { get; set; }

/// <summary>
/// Gets or sets the path of the view file currently being rendered.
/// </summary>
/// <remarks>
/// The rendering of a view may involve one or more files (e.g. _ViewStart, Layouts etc).
/// This property contains the path of the file currently being rendered.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: remove first "being rendered" and (perhaps) add "specific" before "file currently"

/// </remarks>
public string ExecutingFilePath { get; set; }

public FormContext GetFormContextForClientValidation()
{
return (ClientValidationEnabled) ? FormContext : null;
Expand Down
25 changes: 20 additions & 5 deletions src/Microsoft.AspNet.Mvc.Razor/RazorView.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ public RazorView(IRazorViewEngine viewEngine,
IsPartial = isPartial;
}

/// <inheritdoc />
public string Path => RazorPage.Path;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: ick; can't we limit ourselves to properties that look like properties?

in this case, RazorPage.Path won't change during the lifetime of the RazorView. so the lambda is an unnecessary indirection; just use a get-only auto-property.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: ick; can't we limit ourselves to properties that look like properties?

Sadly not. Plus having the auto property just creates an unnecessary field

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you're trading off an unnecessary delegate (or two, depending on the IL generation) and its backing field against a single backing field?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The generated code looks like:

public string Path
{
   get { return RazorPage.Path; }
}

There are no delegates \ backing field in the generated code .

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

faster w/ the testing than I 😄
🆗 much simpler than I thought it would be. and, since RazorPage.Path is a simple property, not expensive to evaluate it multiple times.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pranavkm not sure the team's bought into using the new syntax for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We got an ok from the engineering team to use C# 6.0 features that aren't experimental. That said, seeing this is so contentious I'll just switch back to using a regular property declaration.


/// <summary>
/// Gets <see cref="IRazorPage"/> instance that the views executes on.
/// </summary>
Expand Down Expand Up @@ -93,7 +96,9 @@ private async Task<IBufferedTextWriter> RenderPageAsync(IRazorPage page,
// The writer for the body is passed through the ViewContext, allowing things like HtmlHelpers
// and ViewComponents to reference it.
var oldWriter = context.Writer;
var oldFilePath = context.ExecutingFilePath;
context.Writer = writer;
context.ExecutingFilePath = page.Path;

try
{
Expand All @@ -109,6 +114,7 @@ private async Task<IBufferedTextWriter> RenderPageAsync(IRazorPage page,
finally
{
context.Writer = oldWriter;
context.ExecutingFilePath = oldFilePath;
writer.Dispose();
}
}
Expand All @@ -131,12 +137,21 @@ private async Task RenderViewStartAsync(ViewContext context)
var viewStarts = _viewStartProvider.GetViewStartPages(RazorPage.Path);

string layout = null;
foreach (var viewStart in viewStarts)
var oldFilePath = context.ExecutingFilePath;
try
{
foreach (var viewStart in viewStarts)
{
context.ExecutingFilePath = viewStart.Path;
// Copy the layout value from the previous view start (if any) to the current.
viewStart.Layout = layout;
await RenderPageCoreAsync(viewStart, context);
layout = viewStart.Layout;
}
}
finally
{
// Copy the layout value from the previous view start (if any) to the current.
viewStart.Layout = layout;
await RenderPageCoreAsync(viewStart, context);
layout = viewStart.Layout;
context.ExecutingFilePath = oldFilePath;
}

// Copy over interesting properties from the ViewStart page to the entry page.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<Layout>
/Views/ViewWithPaths/_Layout.cshtml
/Views/ViewWithPaths/Index.cshtml
</Layout>

<ViewStart>
Views\ViewWithPaths\_ViewStart.cshtml
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this case using the Windows-specific path separator? should be consistent whatever we're rendering

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interestingly, the VIewStart path generation is the only one that goes through Path::GetDirectoryName which normalizes the directory separator char. The other ones are produced by the view engine. Not super sure if there's a nice way to fix this or is it's particularly interesting to address this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree a separate issue might come of this but I wouldn't file it unless we get complaints. it's an existing problem that's just made a bit more visible here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interestingly, the VIewStart path generation is the only one that goes through Path::GetDirectoryName which normalizes the directory separator char. The other ones are produced by the view engine. Not super sure if there's a nice way to fix this or is it's particularly interesting to address this.

IMO worth considering in a follow-up. Will be wierd if half of the system normalizes paths and the other doesn't. Also recall our discussion last week about the hidden cost of Path.GetDirectoryName

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/Views/ViewWithPaths/Index.cshtml
</ViewStart>
<Index>
/Views/ViewWithPaths/Index.cshtml
/Views/ViewWithPaths/Index.cshtml
<component>
/Views/Shared/Components/ComponentForViewWithPaths/Default.cshtml
/Views/Shared/Components/ComponentForViewWithPaths/Default.cshtml
</component>
<Partial>
/Views/ViewWithPaths/_Partial.cshtml
/Views/ViewWithPaths/_Partial.cshtml
</Partial>
</Index>
17 changes: 17 additions & 0 deletions test/Microsoft.AspNet.Mvc.FunctionalTests/ViewEngineTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Reflection;
using System.Threading.Tasks;
using Microsoft.AspNet.Builder;
using Microsoft.AspNet.TestHost;
Expand Down Expand Up @@ -395,5 +396,21 @@ Partial that does not specify Layout
// Assert
Assert.Equal(expected, body.Trim());
}

[Fact]
public async Task RazorView_SetsViewPathAndExecutingPagePath()
{
// Arrange
var expected = await GetType().GetTypeInfo().Assembly
.ReadResourceAsStringAsync("compiler/resources/ViewEngineController.ViewWithPaths.txt");
var server = TestServer.Create(_provider, _app);
var client = server.CreateClient();

// Act
var body = await client.GetStringAsync("http://localhost/ViewWithPaths");

// Assert
Assert.Equal(expected, body.Trim());
}
}
}
56 changes: 56 additions & 0 deletions test/Microsoft.AspNet.Mvc.Razor.Test/RazorViewTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Threading.Tasks;
Expand Down Expand Up @@ -83,6 +84,61 @@ public async Task RenderAsync_AsPartial_ActivatesViews_WithThePassedInViewContex
Assert.Same(expectedWriter, viewContext.Writer);
}

[Fact]
public async Task ViewContext_ExecutingPagePath_ReturnsPathOfRazorPageBeingExecuted()
{
// Arrange
var pagePath = "/my/view";
var paths = new List<string>();
var page = new TestableRazorPage(v =>
{
paths.Add(v.ViewContext.ExecutingFilePath);
Assert.Equal(pagePath, v.ViewContext.View.Path);
})
{
Path = pagePath
};

var viewStart = new TestableRazorPage(v =>
{
v.Layout = LayoutPath;
paths.Add(v.ViewContext.ExecutingFilePath);
Assert.Equal(pagePath, v.ViewContext.View.Path);
})
{
Path = "_ViewStart"
};

var layout = new TestableRazorPage(v =>
{
v.RenderBodyPublic();
paths.Add(v.ViewContext.ExecutingFilePath);
Assert.Equal(pagePath, v.ViewContext.View.Path);
})
{
Path = LayoutPath
};

var activator = Mock.Of<IRazorPageActivator>();
var viewEngine = new Mock<IRazorViewEngine>();
viewEngine.Setup(v => v.FindPage(It.IsAny<ActionContext>(), LayoutPath))
.Returns(new RazorPageResult(LayoutPath, layout));
var view = new RazorView(viewEngine.Object,
activator,
CreateViewStartProvider(viewStart),
page,
isPartial: false);

var viewContext = CreateViewContext(view);
var expectedWriter = viewContext.Writer;

// Act
await view.RenderAsync(viewContext);

// Assert
Assert.Equal(new[] { "_ViewStart", pagePath, LayoutPath }, paths);
}

[Fact]
public async Task RenderAsync_AsPartial_ActivatesViews()
{
Expand Down
2 changes: 2 additions & 0 deletions test/WebSites/CompositeViewEngineWebSite/TestPartialView.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ namespace CompositeViewEngineWebSite
{
public class TestPartialView : IView
{
public string Path { get; set; }

public async Task RenderAsync(ViewContext context)
{
await context.Writer.WriteLineAsync("world");
Expand Down
2 changes: 2 additions & 0 deletions test/WebSites/CompositeViewEngineWebSite/TestView.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ namespace CompositeViewEngineWebSite
{
public class TestView : IView
{
public string Path { get; set; }

public async Task RenderAsync(ViewContext context)
{
await context.Writer.WriteLineAsync("Content from test view");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ public Person Person(Person boundPerson)

private sealed class TestView : IView
{
public string Path { get; set; }

public Task RenderAsync(ViewContext context)
{
throw new NotImplementedException();
Expand Down
16 changes: 16 additions & 0 deletions test/WebSites/RazorWebSite/Components/ComponentForViewWithPaths.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using Microsoft.AspNet.Mvc;

namespace MvcSample.Web.Components
{
[ViewComponent(Name = "ComponentForViewWithPaths")]
public class ComponentForViewWithPaths : ViewComponent
{
public IViewComponentResult Invoke()
{
return View();
}
}
}
16 changes: 16 additions & 0 deletions test/WebSites/RazorWebSite/Controllers/ViewWithPathsController.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using Microsoft.AspNet.Mvc;

namespace RazorWebSite.Controllers
{
public class ViewWithPathsController : Controller
{
[HttpGet("/ViewWithPaths")]
public IActionResult Index()
{
return View();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<component>
@ViewContext.ExecutingFilePath
@ViewContext.View.Path
</component>
6 changes: 6 additions & 0 deletions test/WebSites/RazorWebSite/Views/ViewWithPaths/Index.cshtml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<Index>
@ViewContext.ExecutingFilePath
@ViewContext.View.Path
@Component.Invoke("ComponentForViewWithPaths")
@Html.Partial("_Partial")
</Index>
5 changes: 5 additions & 0 deletions test/WebSites/RazorWebSite/Views/ViewWithPaths/_Layout.cshtml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<Layout>
@ViewContext.ExecutingFilePath
@ViewContext.View.Path
</Layout>
@RenderBody()
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<Partial>
@ViewContext.ExecutingFilePath
@ViewContext.View.Path
</Partial>
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
@{
Layout = "_Layout";
}
<ViewStart>
@ViewContext.ExecutingFilePath
@ViewContext.View.Path
</ViewStart>