SiteMap provider - adding code comments and refactoring

Composite/AspNet/CmsPageSiteMapProvider.cs

@@ -77,24 +77,16 @@ public override SiteMapNodeCollection GetChildNodes(SiteMapNode node)
         {
             Verify.ArgumentNotNull(node, nameof(node));
 
-            var childNodes = new List<SiteMapNode>();
+            var childNodes = _plugins.SelectMany(plugin => plugin.GetChildNodes(node)
+                                                           ?? Enumerable.Empty<SiteMapNode>()).ToList();
 
-            foreach (var plugin in _plugins)
-            {
-                var pluginChildNodes = plugin.GetChildNodes(node);
-                if (pluginChildNodes != null)
-                {
-                    childNodes.AddRange(pluginChildNodes);
-                }
-            }
+            childNodes = SecurityTrimList(childNodes);
 
             if (!childNodes.Any())
             {
                 return EmptyCollection;
             }
 
-            childNodes = SecurityTrimList(childNodes);
-
             return new SiteMapNodeCollection(childNodes.ToArray());
         }
 
@@ -139,7 +131,7 @@ protected override SiteMapNode GetRootNodeCore()
                         {
                             var pageNode = data.SitemapNavigator.GetPageNodeByHostname(context.Request.Url.Host);
 
-                            homePageId = pageNode.Id;
+                            homePageId = pageNode?.Id ?? Guid.Empty;
                         }
                     }
                 }
@@ -203,16 +195,7 @@ public override bool IsAccessibleToUser(HttpContext ctx, SiteMapNode node)
         {
             var ctxBase = new HttpContextWrapper(ctx);
 
-            foreach (var plugin in _plugins)
-            {
-                var isAccessibleToUser = plugin.IsAccessibleToUser(ctxBase, node);
-                if (!isAccessibleToUser)
-                {
-                    return false;
-                }
-            }
-
-            return true;
+            return _plugins.All(plugin => plugin.IsAccessibleToUser(ctxBase, node));
         }
 
         private T SecurityTrimNode<T>(T node) where T : SiteMapNode
@@ -241,7 +224,7 @@ private List<T> SecurityTrimList<T>(List<T> list) where T : SiteMapNode
                 return null;
             }
 
-            if (SecurityTrimmingEnabled)
+            if (SecurityTrimmingEnabled && list.Count > 0)
             {
                 var context = HttpContext.Current;
 

Composite/AspNet/ICmsSiteMapNode.cs

@@ -1,14 +1,15 @@
 using System.Globalization;
+using System.Web;
 
 namespace Composite.AspNet
 {
     /// <summary>
-    /// 
+    /// Used as an extension to <see cref="SiteMapNode"/> when building a ASP.NET sitemap based on cms pages.
     /// </summary>
     public interface ICmsSiteMapNode
     {
         /// <summary>
-        /// Gets or sets the culture.
+        /// Gets the culture to which the site map node belongs.
         /// </summary>
         /// <value>
         /// The culture.

Composite/AspNet/ICmsSiteMapProvider.cs

@@ -2,10 +2,14 @@
 
 namespace Composite.AspNet
 {
-    /// <exclude />
+    /// <summary>
+    /// An inteface for getting site map data required for rendering /Sitemap.xml file.
+    /// </summary>
     public interface ICmsSiteMapProvider
     {
-        /// <exclude />
+        /// <summary>
+        /// Gets the root nodes.
+        /// </summary>
         ICollection<CmsPageSiteMapNode> GetRootNodes();
     }
 }

Composite/AspNet/ISchemaOrgSiteMapNode.cs

@@ -1,30 +1,32 @@
 using System;
+using System.Web;
 
 namespace Composite.AspNet
 {
     /// <summary>
-    /// 
+    /// Provides infomation that is used as an addition to <see cref="SiteMapNode"/> when generating sitemap xml file.
+    /// See https://www.sitemaps.org for details
     /// </summary>
     public interface ISchemaOrgSiteMapNode
     {
         /// <summary>
-        /// Gets or sets the last modified.
+        /// Gets the last modification time.
         /// </summary>
         /// <value>
-        /// The last modified.
+        /// The last modification time.
         /// </value>
         DateTime LastModified { get; }
 
         /// <summary>
-        /// Gets or sets the change frequency.
+        /// Gets the change frequency.
         /// </summary>
         /// <value>
         /// The change frequency.
         /// </value>
         SiteMapNodeChangeFrequency? ChangeFrequency { get; }
 
         /// <summary>
-        /// Gets or sets the priority.
+        /// Gets the priority.
         /// </summary>
         /// <value>
         /// The priority.

Composite/AspNet/ISiteMapPlugin.cs

@@ -9,45 +9,45 @@ namespace Composite.AspNet
     public interface ISiteMapPlugin
     {
         /// <summary>
-        /// 
+        /// Retrieves the child nodes of a specific <see cref="SiteMapNode"/>.
         /// </summary>
         /// <param name="node"></param>
         /// <returns></returns>
         List<SiteMapNode> GetChildNodes(SiteMapNode node);
 
         /// <summary>
-        /// 
+        /// Retrieves the parent node of a specific <see cref="SiteMapNode"/> object.
         /// </summary>
         /// <param name="node"></param>
         /// <returns></returns>
         SiteMapNode GetParentNode(SiteMapNode node);
 
         /// <summary>
-        /// 
+        /// Retrieves a <see cref="SiteMapNode"/> object that represents a page.
         /// </summary>
         /// <param name="provider"></param>
         /// <param name="rawUrl"></param>
         /// <returns></returns>
         SiteMapNode FindSiteMapNode(SiteMapProvider provider, string rawUrl);
 
         /// <summary>
-        /// 
+        /// Retrieves a <see cref="SiteMapNode"/> object based on <see cref="HttpContextBase"/>.
         /// </summary>
         /// <param name="provider"></param>
         /// <param name="context"></param>
         /// <returns></returns>
         SiteMapNode FindSiteMapNode(SiteMapProvider provider, HttpContextBase context);
 
         /// <summary>
-        /// 
+        /// Retrieves a <see cref="SiteMapNode"/> object based on a specified key.
         /// </summary>
         /// <param name="provider"></param>
         /// <param name="key"></param>
         /// <returns></returns>
         SiteMapNode FindSiteMapNodeFromKey(SiteMapProvider provider, string key);
 
         /// <summary>
-        /// 
+        /// Retrieves a Boolean value indicating whether the specified <see cref="SiteMapNode"/> object can be viewed by the user in the specified context.
         /// </summary>
         /// <param name="context"></param>
         /// <param name="node"></param>

Composite/Core/WebClient/ApplicationLevelEventHandlers.cs

@@ -158,17 +158,15 @@ public static void Application_End(object sender, EventArgs e)
                     throw;
                 }
 
-                Log.LogVerbose("Global.asax", string.Format("--- Web Application End, {0} Id = {1}---",
-                              DateTime.Now.ToLongTimeString(),
-                              AppDomain.CurrentDomain.Id));
+                Log.LogVerbose("Global.asax", $"--- Web Application End, {DateTime.Now.ToLongTimeString()} Id = {AppDomain.CurrentDomain.Id}---");
             }
         }
 
 
         /// <exclude />
         public static void Application_BeginRequest(object sender, EventArgs e)
         {
-            var context = (sender as HttpApplication).Context;
+            var context = ((HttpApplication) sender).Context;
 
             ThreadDataManager.InitializeThroughHttpContext();
 
@@ -188,7 +186,7 @@ public static void Application_BeginRequest(object sender, EventArgs e)
         /// <exclude />
         public static void Application_EndRequest(object sender, EventArgs e)
         {
-            var context = (sender as HttpApplication).Context;
+            var context = ((HttpApplication) sender).Context;
 
             try
             {
@@ -198,7 +196,7 @@ public static void Application_EndRequest(object sender, EventArgs e)
                 {
                     int startTimer = (int)context.Items["Global.asax timer"];
                     string requestPath = context.Request.Path;
-                    Log.LogVerbose("End request", string.Format("{0} - took {1} ms", requestPath, (Environment.TickCount - startTimer)));
+                    Log.LogVerbose("End request", $"{requestPath} - took {Environment.TickCount - startTimer} ms");
                 }
             }
             finally
@@ -212,7 +210,7 @@ public static void Application_EndRequest(object sender, EventArgs e)
         /// <exclude />
         public static void Application_Error(object sender, EventArgs e)
         {
-            var httpApplication = (sender as HttpApplication);
+            var httpApplication = (HttpApplication) sender;
             Exception exception = httpApplication.Server.GetLastError();
 
             var eventType = TraceEventType.Error;
@@ -221,7 +219,8 @@ public static void Application_Error(object sender, EventArgs e)
 
             if (httpContext != null)
             {
-                bool is404 = (exception is HttpException && ((HttpException)exception).GetHttpCode() == 404);
+                bool is404 = exception is HttpException httpException
+                             && httpException.GetHttpCode() == 404;
 
                 if (is404)
                 {
@@ -235,7 +234,7 @@ public static void Application_Error(object sender, EventArgs e)
                         {
                             if (rawUrl == customPageNotFoundUrl)
                             {
-                                throw new HttpException(500, "'Page not found' url isn't handled. Url: '{0}'".FormatWith(rawUrl));
+                                throw new HttpException(500, $"'Page not found' url isn't handled. Url: '{rawUrl}'");
                             }
 
                             httpContext.Server.ClearError();
@@ -267,8 +266,7 @@ public static void Application_Error(object sender, EventArgs e)
                     if (request != null)
                     {
                         LoggingService.LogEntry("Application Error",
-                                            "Failed to process '{0}' request to url '{1}'"
-                                            .FormatWith(request.RequestType, request.RawUrl),
+                                            $"Failed to process '{request.RequestType}' request to url '{request.RawUrl}'",
                                             LoggingService.Category.General, eventType);
                     }
                 }
@@ -372,7 +370,7 @@ private static void CurrentDomain_DomainUnload(object sender, EventArgs e)
         {
             if (RuntimeInformation.IsDebugBuild)
             {
-                Log.LogInformation(_verboseLogEntryTitle, "AppDomain {0} unloaded at {1}", AppDomain.CurrentDomain.Id, DateTime.Now.ToString("HH:mm:ss:ff"));
+                Log.LogInformation(_verboseLogEntryTitle, $"AppDomain {AppDomain.CurrentDomain.Id} unloaded at {DateTime.Now:HH:mm:ss:ff}");
             }
         }
 
@@ -404,9 +402,8 @@ private static void LogShutDownReason()
                                                                        System.Reflection.BindingFlags.GetField,
                                                                        null, runtime, null);
 
-            Log.LogVerbose("RGB(250,50,50)ASP.NET Shut Down", String.Format("_shutDownMessage=\n{0}\n\n_shutDownStack=\n{1}",
-                                         shutDownMessage.Replace("\n", "   \n"),
-                                         shutDownStack));
+            Log.LogVerbose("RGB(250,50,50)ASP.NET Shut Down",
+                $"_shutDownMessage=\n{shutDownMessage.Replace("\n", "   \n")}\n\n_shutDownStack=\n{shutDownStack}");
 
         }
     }