Unrendered sections does not throw when redefined and rendered in nested

layout.

Fixes #2252

src/Microsoft.AspNet.Mvc.Razor/IRazorPage.cs

@@ -72,14 +72,11 @@ namespace Microsoft.AspNet.Mvc.Razor
         Task ExecuteAsync();
 
         /// <summary>
-        /// Verifies that RenderBody is called for the page that is
+        /// Verifies that all sections defined in <see cref="PreviousSectionWriters"/> were rendered, or
-        /// part of view execution hierarchy.
+        /// the body was rendered if no sections were defined.
         /// </summary>
-        void EnsureBodyWasRendered();
+        /// <exception cref="InvalidOperationException">if one or more sections were not rendered or if no sections were
-
+        /// defined and the body was not rendered.</exception>
-        /// <summary>
+        void EnsureRenderedBodyOrSections();
-        /// Gets the sections that are rendered in the page.
-        /// </summary>
-        IEnumerable<string> RenderedSections { get; }
     }
 }

src/Microsoft.AspNet.Mvc.Razor/Properties/Resources.Designer.cs

@@ -219,7 +219,7 @@ namespace Microsoft.AspNet.Mvc.Razor
         }
 
         /// <summary>
-        /// {0} must be called from a layout page.
+        /// {0} has not been called for the page '{1}'.
         /// </summary>
         internal static string RenderBodyNotCalled
         {
@@ -227,11 +227,11 @@ namespace Microsoft.AspNet.Mvc.Razor
         }
 
         /// <summary>
-        /// {0} must be called from a layout page.
+        /// {0} has not been called for the page '{1}'.
         /// </summary>
-        internal static string FormatRenderBodyNotCalled(object p0)
+        internal static string FormatRenderBodyNotCalled(object p0, object p1)
         {
-            return string.Format(CultureInfo.CurrentCulture, GetString("RenderBodyNotCalled"), p0);
+            return string.Format(CultureInfo.CurrentCulture, GetString("RenderBodyNotCalled"), p0, p1);
         }
 
         /// <summary>
@@ -283,7 +283,7 @@ namespace Microsoft.AspNet.Mvc.Razor
         }
 
         /// <summary>
-        /// The following sections have been defined but have not been rendered: '{0}'.
+        /// The following sections have been defined but have not been rendered for the page '{0}': '{1}'.
         /// </summary>
         internal static string SectionsNotRendered
         {
@@ -291,11 +291,11 @@ namespace Microsoft.AspNet.Mvc.Razor
         }
 
         /// <summary>
-        /// The following sections have been defined but have not been rendered: '{0}'.
+        /// The following sections have been defined but have not been rendered for the page '{0}': '{1}'.
         /// </summary>
-        internal static string FormatSectionsNotRendered(object p0)
+        internal static string FormatSectionsNotRendered(object p0, object p1)
         {
-            return string.Format(CultureInfo.CurrentCulture, GetString("SectionsNotRendered"), p0);
+            return string.Format(CultureInfo.CurrentCulture, GetString("SectionsNotRendered"), p0, p1);
         }
 
         /// <summary>

src/Microsoft.AspNet.Mvc.Razor/RazorPage.cs

@@ -5,6 +5,7 @@ using System;
 using System.Collections.Generic;
 using System.Globalization;
 using System.IO;
+using System.Linq;
 using System.Security.Claims;
 using System.Text;
 using System.Threading.Tasks;
@@ -50,15 +51,6 @@ namespace Microsoft.AspNet.Mvc.Razor
             }
         }
 
-        /// <inheritdoc />
-        public IEnumerable<string> RenderedSections
-        {
-            get
-            {
-                return _renderedSections;
-            }
-        }
-
         /// <inheritdoc />
         public string Path { get; set; }
 
@@ -724,13 +716,27 @@ namespace Microsoft.AspNet.Mvc.Razor
         }
 
         /// <inheritdoc />
-        public void EnsureBodyWasRendered()
+        public void EnsureRenderedBodyOrSections()
         {
-            // If BodyContent is set, ensure it was rendered.
+            // a) all sections defined for this page are rendered.
-            if (RenderBodyDelegate != null && !_renderedBody)
+            // b) if no sections are defined, then the body is rendered if it's available.
+            if (PreviousSectionWriters != null && PreviousSectionWriters.Count > 0)
             {
+                var sectionsNotRendered = PreviousSectionWriters.Keys.Except(
+                    _renderedSections,
+                    StringComparer.OrdinalIgnoreCase);
+
+                if (sectionsNotRendered.Any())
+                {
+                    var sectionNames = string.Join(", ", sectionsNotRendered);
+                    throw new InvalidOperationException(Resources.FormatSectionsNotRendered(Path, sectionNames));
+                }
+            }
+            else if (RenderBodyDelegate != null && !_renderedBody)
+            {
+                // There are no sections defined, but RenderBody was NOT called.
                 // If a body was defined, then RenderBody should have been called.
-                var message = Resources.FormatRenderBodyNotCalled(nameof(RenderBody));
+                var message = Resources.FormatRenderBodyNotCalled(nameof(RenderBody), Path);
                 throw new InvalidOperationException(message);
             }
         }

src/Microsoft.AspNet.Mvc.Razor/RazorView.cs

@@ -4,7 +4,6 @@
 using System;
 using System.Collections.Generic;
 using System.IO;
-using System.Linq;
 using System.Threading.Tasks;
 using Microsoft.AspNet.Mvc.Rendering;
 using Microsoft.AspNet.PageExecutionInstrumentation;
@@ -170,8 +169,7 @@ namespace Microsoft.AspNet.Mvc.Razor
             // A layout page can specify another layout page. We'll need to continue
             // looking for layout pages until they're no longer specified.
             var previousPage = RazorPage;
-            var unrenderedSections = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
+            var renderedLayouts = new List<IRazorPage>();
-
             while (!string.IsNullOrEmpty(previousPage.Layout))
             {
                 if (!bodyWriter.IsBuffering)
@@ -194,20 +192,14 @@ namespace Microsoft.AspNet.Mvc.Razor
                 layoutPage.RenderBodyDelegate = bodyWriter.CopyTo;
                 bodyWriter = await RenderPageAsync(layoutPage, context, executeViewStart: false);
 
-                // Verify that RenderBody is called
+                renderedLayouts.Add(layoutPage);
-                layoutPage.EnsureBodyWasRendered();
-
-                unrenderedSections.UnionWith(layoutPage.PreviousSectionWriters.Keys);
-                unrenderedSections.ExceptWith(layoutPage.RenderedSections);
-
                 previousPage = layoutPage;
             }
 
-            // If not all sections are rendered, throw.
+            // Ensure all defined sections were rendered or RenderBody was invoked for page without defined sections.
-            if (unrenderedSections.Any())
+            foreach (var layoutPage in renderedLayouts)
             {
-                var sectionNames = string.Join(", ", unrenderedSections);
+                layoutPage.EnsureRenderedBodyOrSections();
-                throw new InvalidOperationException(Resources.FormatSectionsNotRendered(sectionNames));
             }
 
             if (bodyWriter.IsBuffering)

src/Microsoft.AspNet.Mvc.Razor/Resources.resx

@@ -157,7 +157,7 @@
     <value>{0} can only be called from a layout page.</value>
   </data>
   <data name="RenderBodyNotCalled" xml:space="preserve">
-    <value>{0} must be called from a layout page.</value>
+    <value>{0} has not been called for the page at '{1}'.</value>
   </data>
   <data name="SectionAlreadyDefined" xml:space="preserve">
     <value>Section '{0}' is already defined.</value>
@@ -169,7 +169,7 @@
     <value>Section '{0}' is not defined.</value>
   </data>
   <data name="SectionsNotRendered" xml:space="preserve">
-    <value>The following sections have been defined but have not been rendered: '{0}'.</value>
+    <value>The following sections have been defined but have not been rendered by the page at '{0}': '{1}'.</value>
   </data>
   <data name="ViewCannotBeActivated" xml:space="preserve">
     <value>View of type '{0}' cannot be activated by '{1}'.</value>

test/Microsoft.AspNet.Mvc.Razor.Test/RazorPageTest.cs

@@ -403,21 +403,70 @@ namespace Microsoft.AspNet.Mvc.Razor
         }
 
         [Fact]
-        public async Task EnsureBodyWasRendered_ThrowsIfRenderBodyIsNotCalledFromPage()
+        public async Task EnsureRenderedBodyOrSections_ThrowsIfRenderBodyIsNotCalledFromPage_AndNoSectionsAreDefined()
         {
             // Arrange
-            var expected = new HelperResult(action: null);
+            var path = "page-path";
             var page = CreatePage(v =>
             {
             });
+            page.Path = path;
             page.RenderBodyDelegate = CreateBodyAction("some content");
 
             // Act
             await page.ExecuteAsync();
-            var ex = Assert.Throws<InvalidOperationException>(() => page.EnsureBodyWasRendered());
+            var ex = Assert.Throws<InvalidOperationException>(() => page.EnsureRenderedBodyOrSections());
 
             // Assert
-            Assert.Equal("RenderBody must be called from a layout page.", ex.Message);
+            Assert.Equal($"RenderBody has not been called for the page at '{path}'.", ex.Message);
+        }
+
+        [Fact]
+        public async Task EnsureRenderedBodyOrSections_ThrowsIfDefinedSectionsAreNotRendered()
+        {
+            // Arrange
+            var path = "page-path";
+            var sectionName = "sectionA";
+            var page = CreatePage(v =>
+            {
+            });
+            page.Path = path;
+            page.RenderBodyDelegate = CreateBodyAction("some content");
+            page.PreviousSectionWriters = new Dictionary<string, RenderAsyncDelegate>
+            {
+                { sectionName, _nullRenderAsyncDelegate }
+            };
+
+            // Act
+            await page.ExecuteAsync();
+            var ex = Assert.Throws<InvalidOperationException>(() => page.EnsureRenderedBodyOrSections());
+
+            // Assert
+            Assert.Equal("The following sections have been defined but have not been rendered by the page at " +
+                $"'{path}': '{sectionName}'.", ex.Message);
+        }
+
+        [Fact]
+        public async Task EnsureRenderedBodyOrSections_SucceedsIfRenderBodyIsNotCalled_ButAllDefinedSectionsAreRendered()
+        {
+            // Arrange
+            var sectionA = "sectionA";
+            var sectionB = "sectionB";
+            var page = CreatePage(v =>
+            {
+                v.RenderSection(sectionA);
+                v.RenderSection(sectionB);
+            });
+            page.RenderBodyDelegate = CreateBodyAction("some content");
+            page.PreviousSectionWriters = new Dictionary<string, RenderAsyncDelegate>
+            {
+                { sectionA, _nullRenderAsyncDelegate },
+                { sectionB, _nullRenderAsyncDelegate },
+            };
+
+            // Act and Assert
+            await page.ExecuteAsync();
+            page.EnsureRenderedBodyOrSections();
         }
 
         [Fact]
@@ -801,7 +850,8 @@ namespace Microsoft.AspNet.Mvc.Razor
                 selfClosing: false,
                 items: new Dictionary<object, object>(),
                 uniqueId: string.Empty,
-                executeChildContentAsync: () => {
+                executeChildContentAsync: () =>
+                {
                     defaultTagHelperContent.SetContent(input);
                     return Task.FromResult(result: true);
                 },

test/Microsoft.AspNet.Mvc.Razor.Test/RazorViewTest.cs

@@ -438,7 +438,10 @@ namespace Microsoft.AspNet.Mvc.Razor
             var layout = new TestableRazorPage(v =>
             {
                 v.RenderBodyPublic();
-            });
+            })
+            {
+                Path = LayoutPath
+            };
             var viewEngine = new Mock<IRazorViewEngine>();
             viewEngine.Setup(p => p.FindPage(It.IsAny<ActionContext>(), LayoutPath))
                        .Returns(new RazorPageResult(LayoutPath, layout));
@@ -452,7 +455,128 @@ namespace Microsoft.AspNet.Mvc.Razor
 
             // Act and Assert
             var ex = await Assert.ThrowsAsync<InvalidOperationException>(() => view.RenderAsync(viewContext));
-            Assert.Equal("The following sections have been defined but have not been rendered: 'head, foot'.", ex.Message);
+            Assert.Equal("The following sections have been defined but have not been rendered by the page "
+                + $"at '{LayoutPath}': 'head, foot'.", ex.Message);
+        }
+
+        [Fact]
+        public async Task RenderAsync_SucceedsIfNestedSectionsAreRendered()
+        {
+            // Arrange
+            var expected = string.Join(
+                Environment.NewLine,
+                "layout-section-content",
+                "page-section-content");
+
+            var htmlEncoder = new HtmlEncoder();
+            var page = new TestableRazorPage(v =>
+            {
+                v.HtmlEncoder = htmlEncoder;
+                v.Layout = "~/Shared/Layout1.cshtml";
+                v.DefineSection("foo", async writer =>
+                {
+                    await writer.WriteAsync("page-section-content");
+                });
+            });
+            var nestedLayout = new TestableRazorPage(v =>
+            {
+                v.HtmlEncoder = htmlEncoder;
+                v.Layout = "~/Shared/Layout2.cshtml";
+                v.RenderBodyPublic();
+                v.DefineSection("foo", async writer =>
+                {
+                    await writer.WriteLineAsync("layout-section-content");
+                    await v.RenderSectionAsync("foo");
+                });
+            })
+            {
+                Path = "/Shared/Layout1.cshtml"
+            };
+            var baseLayout = new TestableRazorPage(v =>
+            {
+                v.HtmlEncoder = htmlEncoder;
+                v.RenderBodyPublic();
+                v.RenderSection("foo");
+            })
+            {
+                Path = "/Shared/Layout2.cshtml"
+            };
+
+            var viewEngine = new Mock<IRazorViewEngine>();
+            viewEngine.Setup(p => p.FindPage(It.IsAny<ActionContext>(), "~/Shared/Layout1.cshtml"))
+                       .Returns(new RazorPageResult("~/Shared/Layout1.cshtml", nestedLayout));
+            viewEngine.Setup(p => p.FindPage(It.IsAny<ActionContext>(), "~/Shared/Layout2.cshtml"))
+                       .Returns(new RazorPageResult("~/Shared/Layout2.cshtml", baseLayout));
+
+            var view = new RazorView(viewEngine.Object,
+                                     Mock.Of<IRazorPageActivator>(),
+                                     CreateViewStartProvider(),
+                                     page,
+                                     isPartial: false);
+            var viewContext = CreateViewContext(view);
+
+            // Act
+            await view.RenderAsync(viewContext);
+
+            // Assert
+            Assert.Equal(expected, viewContext.Writer.ToString());
+        }
+
+        [Fact]
+        public async Task RenderAsync_SucceedsIfRenderBodyIsNotInvoked_ButAllSectionsAreRendered()
+        {
+            // Arrange
+            var expected = string.Join(
+                Environment.NewLine,
+                "layout-section-content",
+                "page-section-content");
+
+            var htmlEncoder = new HtmlEncoder();
+            var page = new TestableRazorPage(v =>
+            {
+                v.HtmlEncoder = htmlEncoder;
+                v.Layout = "NestedLayout";
+                v.WriteLiteral("Page body content that will not be written");
+                v.DefineSection("sectionA", async writer =>
+                {
+                    await writer.WriteAsync("page-section-content");
+                });
+            });
+            var nestedLayout = new TestableRazorPage(v =>
+            {
+                v.HtmlEncoder = htmlEncoder;
+                v.Layout = "Layout";
+                v.WriteLiteral("Nested layout content that will not be written");
+                v.DefineSection("sectionB", async writer =>
+                {
+                    await writer.WriteLineAsync("layout-section-content");
+                    await v.RenderSectionAsync("sectionA");
+                });
+            });
+            var baseLayout = new TestableRazorPage(v =>
+            {
+                v.HtmlEncoder = htmlEncoder;
+                v.RenderSection("sectionB");
+            });
+
+            var viewEngine = new Mock<IRazorViewEngine>();
+            viewEngine.Setup(p => p.FindPage(It.IsAny<ActionContext>(), "NestedLayout"))
+                       .Returns(new RazorPageResult("NestedLayout", nestedLayout));
+            viewEngine.Setup(p => p.FindPage(It.IsAny<ActionContext>(), "Layout"))
+                       .Returns(new RazorPageResult("Layout", baseLayout));
+
+            var view = new RazorView(viewEngine.Object,
+                                     Mock.Of<IRazorPageActivator>(),
+                                     CreateViewStartProvider(),
+                                     page,
+                                     isPartial: false);
+            var viewContext = CreateViewContext(view);
+
+            // Act
+            await view.RenderAsync(viewContext);
+
+            // Assert
+            Assert.Equal(expected, viewContext.Writer.ToString());
         }
 
         [Fact]
@@ -476,17 +600,23 @@ namespace Microsoft.AspNet.Mvc.Razor
                 v.Layout = "~/Shared/Layout2.cshtml";
                 v.Write("NestedLayout" + Environment.NewLine);
                 v.RenderBodyPublic();
-                v.DefineSection("foo", async writer =>
+                v.DefineSection("foo", async _ =>
                 {
-                    await writer.WriteLineAsync(htmlEncoder.HtmlEncode(v.RenderSection("foo").ToString()));
+                    await v.RenderSectionAsync("foo");
                 });
-            });
+            })
+            {
+                Path = "/Shared/Layout1.cshtml"
+            };
             var baseLayout = new TestableRazorPage(v =>
             {
                 v.HtmlEncoder = htmlEncoder;
                 v.Write("BaseLayout" + Environment.NewLine);
                 v.RenderBodyPublic();
-            });
+            })
+            {
+                Path = "/Shared/Layout2.cshtml"
+            };
 
             var viewEngine = new Mock<IRazorViewEngine>();
             viewEngine.Setup(p => p.FindPage(It.IsAny<ActionContext>(), "~/Shared/Layout1.cshtml"))
@@ -503,7 +633,72 @@ namespace Microsoft.AspNet.Mvc.Razor
 
             // Act and Assert
             var ex = await Assert.ThrowsAsync<InvalidOperationException>(() => view.RenderAsync(viewContext));
-            Assert.Equal("The following sections have been defined but have not been rendered: 'foo'.", ex.Message);
+            Assert.Equal("The following sections have been defined but have not been rendered by the page at "
+                + "'/Shared/Layout1.cshtml': 'foo'.", ex.Message);
+        }
+
+        [Fact]
+        public async Task RenderAsync_WithNestedSectionsOfTheSameName_ThrowsIfSectionsWereDefinedButNotRendered()
+        {
+            // Arrange
+            var htmlEncoder = new HtmlEncoder();
+            var page = new TestableRazorPage(v =>
+            {
+                v.HtmlEncoder = htmlEncoder;
+                v.Layout = "~/Shared/Layout1.cshtml";
+                v.WriteLiteral("BodyContent");
+                v.DefineSection("foo", async writer =>
+                {
+                    await writer.WriteLineAsync("foo-content");
+                });
+            })
+            {
+                Path = "Page"
+            };
+
+            var nestedLayout = new TestableRazorPage(v =>
+            {
+                v.HtmlEncoder = htmlEncoder;
+                v.Layout = "~/Shared/Layout2.cshtml";
+                v.Write("NestedLayout" + Environment.NewLine);
+                v.RenderBodyPublic();
+                v.DefineSection("foo", async writer =>
+                {
+                    await writer.WriteLineAsync("dont-render-inner-foo");
+                });
+            })
+            {
+                Path = "/Shared/Layout1.cshtml"
+            };
+
+            var baseLayout = new TestableRazorPage(v =>
+            {
+                v.HtmlEncoder = htmlEncoder;
+                v.Write("BaseLayout" + Environment.NewLine);
+                v.RenderBodyPublic();
+                v.RenderSection("foo");
+            })
+            {
+                Path = "/Shared/Layout2.cshtml"
+            };
+
+            var viewEngine = new Mock<IRazorViewEngine>();
+            viewEngine.Setup(p => p.FindPage(It.IsAny<ActionContext>(), "~/Shared/Layout1.cshtml"))
+                       .Returns(new RazorPageResult("~/Shared/Layout1.cshtml", nestedLayout));
+            viewEngine.Setup(p => p.FindPage(It.IsAny<ActionContext>(), "~/Shared/Layout2.cshtml"))
+                       .Returns(new RazorPageResult("~/Shared/Layout2.cshtml", baseLayout));
+
+            var view = new RazorView(viewEngine.Object,
+                                     Mock.Of<IRazorPageActivator>(),
+                                     CreateViewStartProvider(),
+                                     page,
+                                     isPartial: false);
+            var viewContext = CreateViewContext(view);
+
+            // Act and Assert
+            var ex = await Assert.ThrowsAsync<InvalidOperationException>(() => view.RenderAsync(viewContext));
+            Assert.Equal("The following sections have been defined but have not been rendered by the page at " +
+                "'/Shared/Layout1.cshtml': 'foo'.", ex.Message);
         }
 
         [Fact]
@@ -516,7 +711,10 @@ namespace Microsoft.AspNet.Mvc.Razor
             });
             var layout = new TestableRazorPage(v =>
             {
-            });
+            })
+            {
+                Path = LayoutPath
+            };
             var viewEngine = new Mock<IRazorViewEngine>();
             viewEngine.Setup(p => p.FindPage(It.IsAny<ActionContext>(), LayoutPath))
                        .Returns(new RazorPageResult(LayoutPath, layout));
@@ -530,7 +728,7 @@ namespace Microsoft.AspNet.Mvc.Razor
 
             // Act and Assert
             var ex = await Assert.ThrowsAsync<InvalidOperationException>(() => view.RenderAsync(viewContext));
-            Assert.Equal("RenderBody must be called from a layout page.", ex.Message);
+            Assert.Equal($"RenderBody has not been called for the page at '{LayoutPath}'.", ex.Message);
         }
 
         [Fact]