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

Sections within partial views shouldn't fail silently #997

Closed
wants to merge 1 commit into from
Closed

Sections within partial views shouldn't fail silently #997

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 16 additions & 0 deletions src/Microsoft.AspNet.Mvc.Razor/Properties/Resources.Designer.cs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Microsoft.AspNet.Mvc.Razor/RazorView.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@ public async Task RenderAsync([NotNull] ViewContext context)
else
{
await RenderPageCoreAsync(_page, context);
// Ensure that the partial did not declare any sections.
if (_page.SectionWriters.Count > 0)
{
throw new InvalidOperationException(Resources.PartialViewsCannotDefineSections);
}
Copy link
Member

@rynowak rynowak Aug 13, 2014

Choose a reason for hiding this comment

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

Possible to bail out before rendering or is that required to populate _page.SectionWriters?

Copy link
Contributor Author

@pranavkm pranavkm Aug 14, 2014

Choose a reason for hiding this comment

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

The population of the section writers happens as part of the execution of the page. We could pass in the flag into the IRazorPage and have it throw when you call DefineSection, but it seems like mixing view behavior (full or partial) with page behavior which felt dirty.

Copy link
Member

@rynowak rynowak Aug 14, 2014

Choose a reason for hiding this comment

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

That seems fine then. Is there a way for a RazorPage to know whether or not it's a partial?

For our scenarios this seems rather cut and dry. Wondering about 3rd party view engines

Copy link
Contributor Author

@pranavkm pranavkm Aug 14, 2014

Choose a reason for hiding this comment

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

There isn't as yet. We could always pass something in - maybe as part of the ViewContext or make it a first class property of IView. The latter might not be a bad idea given that the notion partial and full views exists in the IViewEngine

Copy link
Contributor

@yishaigalatzer yishaigalatzer Aug 14, 2014

Choose a reason for hiding this comment

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

Probably not a bad idea to know the rendering mode, lets file an issue (low pri IMHO, but we will get to it eventually)

Copy link
Contributor

@yishaigalatzer yishaigalatzer Aug 14, 2014

Choose a reason for hiding this comment

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

is there a way to optionally define section?

Copy link
Contributor Author

@pranavkm pranavkm Aug 14, 2014

Choose a reason for hiding this comment

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

Nope, but there's a way to optionally render it if it's defined RenderSection("foo", required: false)

Copy link
Contributor Author

@pranavkm pranavkm Aug 14, 2014

Choose a reason for hiding this comment

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

Tracking bug for this #1002

}
}

Expand Down
3 changes: 3 additions & 0 deletions src/Microsoft.AspNet.Mvc.Razor/Resources.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,9 @@
<data name="MvcRazorCodeParser_OnlyOneModelStatementIsAllowed" xml:space="preserve">
<value>Only one '{0}' statement is allowed in a file.</value>
</data>
<data name="PartialViewsCannotDefineSections" xml:space="preserve">
<value>Sections cannot be defined in a partial view.</value>
</data>
<data name="RenderBodyCannotBeCalled" xml:space="preserve">
<value>{0} can only be called from a layout page.</value>
</data>
Expand Down
21 changes: 21 additions & 0 deletions test/Microsoft.AspNet.Mvc.Razor.Test/RazorViewTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,27 @@ public async Task RenderAsync_WithoutHierarchy_DoesNotExecuteLayoutOrViewStartPa
Mock.Get(viewStartProvider).Verify(v => v.GetViewStartPages(It.IsAny<string>()), Times.Never());
}

[Fact]
public async Task RenderAsync_WithoutHierarchy_ThrowsIfItDeclaresSections()
{
var page = new TestableRazorPage(v =>
{
v.DefineSection("test", new HelperResult(action => { }));
});
var pageFactory = new Mock<IRazorPageFactory>();
var viewStartProvider = CreateViewStartProvider();
var view = new RazorView(pageFactory.Object,
Mock.Of<IRazorPageActivator>(),
viewStartProvider,
page);
var viewContext = CreateViewContext(view);

// Act and Assert
var ex = await Assert.ThrowsAsync<InvalidOperationException>(
async () => await view.RenderAsync(viewContext));
Assert.Equal("Sections cannot be defined in a partial view.", ex.Message);
}

[Fact]
public async Task RenderAsync_WithHierarchy_CreatesOutputBuffer()
{
Expand Down