From f9f786a8b5b391408df68c2f3d3b8f5428b5c49f Mon Sep 17 00:00:00 2001 From: Marthijn van den Heuvel Date: Mon, 22 Jul 2024 16:21:42 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20Added=20helper=20functions=20to=20h?= =?UTF-8?q?andle=20null=20reference=20warnings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- README.md | 13 +++- .../Controllers/SitemapController.cs | 28 ++++++--- .../SitemapExtensionsTests.cs | 63 +++++++++++++++++++ .../SitemapIndexExtensionsTests.cs | 35 +++++++++++ .../Services/ControllerSitemapService.cs | 25 ++++++-- .../Sidio.Sitemap.AspNetCore.csproj | 2 +- .../SitemapExtensions.cs | 40 ++++++++++++ .../SitemapIndexExtensions.cs | 25 ++++++++ 8 files changed, 214 insertions(+), 17 deletions(-) create mode 100644 src/Sidio.Sitemap.AspNetCore.Tests/SitemapExtensionsTests.cs create mode 100644 src/Sidio.Sitemap.AspNetCore.Tests/SitemapIndexExtensionsTests.cs create mode 100644 src/Sidio.Sitemap.AspNetCore/SitemapExtensions.cs create mode 100644 src/Sidio.Sitemap.AspNetCore/SitemapIndexExtensions.cs diff --git a/README.md b/README.md index a84126f..e7a72f9 100644 --- a/README.md +++ b/README.md @@ -24,7 +24,7 @@ services.AddDefaultSitemapServices(); [HttpGet] public IActionResult Sitemap() { - var nodes = new List { new ("page.html"), new (Url.Action("Index")) }; + var nodes = new List { new ("page.html"), SitemapNode.Create(Url.Action("Index")) }; var sitemap = new Sitemap(nodes); return new SitemapResult(sitemap); } @@ -36,8 +36,17 @@ public IActionResult Sitemap() public IActionResult SitemapIndex() { var sitemapIndex = new SitemapIndex(); + + // basic usage: sitemapIndex.Add(new SitemapIndexNode(Url.Action("Sitemap1"))); - sitemapIndex.Add(new SitemapIndexNode(Url.Action("Sitemap2"))); + + // or: this extension function fixes the null reference warning + // on the line above: + var addResult = sitemapIndex.TryAdd(Url.Action("Sitemap2")); + + // or: use the Create function + sitemapIndex.Add(SitemapIndexNode.Create(Url.Action("Sitemap1"))); + return new SitemapResult(sitemapIndex); } diff --git a/src/Sidio.Sitemap.AspNetCore.Examples.MvcWebApplication/Controllers/SitemapController.cs b/src/Sidio.Sitemap.AspNetCore.Examples.MvcWebApplication/Controllers/SitemapController.cs index 20c9c52..0b66dc2 100644 --- a/src/Sidio.Sitemap.AspNetCore.Examples.MvcWebApplication/Controllers/SitemapController.cs +++ b/src/Sidio.Sitemap.AspNetCore.Examples.MvcWebApplication/Controllers/SitemapController.cs @@ -10,10 +10,10 @@ public sealed class SitemapController : Controller public IActionResult SitemapIndex() { var sitemapIndex = new SitemapIndex(); - sitemapIndex.Add(new SitemapIndexNode(Url.Action("SitemapHome"))); - sitemapIndex.Add(new SitemapIndexNode(Url.Action("SitemapNews"))); - sitemapIndex.Add(new SitemapIndexNode(Url.Action("SitemapImages"))); - sitemapIndex.Add(new SitemapIndexNode(Url.Action("SitemapVideos"))); + _ = sitemapIndex.TryAdd(Url.Action("SitemapHome")); + _ = sitemapIndex.TryAdd(Url.Action("SitemapNews")); + _ = sitemapIndex.TryAdd(Url.Action("SitemapImages")); + _ = sitemapIndex.TryAdd(Url.Action("SitemapVideos")); return new SitemapResult(sitemapIndex); } @@ -22,8 +22,10 @@ public IActionResult SitemapIndex() public IActionResult SitemapHome() { var sitemap = new Core.Sitemap(); - sitemap.Add(new SitemapNode(Url.Action("Index", "Home"))); - sitemap.Add(new SitemapNode(Url.Action("Privacy", "Home"))); + + // handle null warnings by using the TryAdd function + _ = sitemap.TryAdd(Url.Action("Index", "Home")); + _ = sitemap.TryAdd(Url.Action("Privacy", "Home")); return new SitemapResult(sitemap); } @@ -32,7 +34,13 @@ public IActionResult SitemapHome() public IActionResult SitemapNews() { var sitemap = new Core.Sitemap(); - sitemap.Add(new SitemapNewsNode(Url.Action("Article1", "News"), "Article1", "John Doe", "EN", DateTime.UtcNow)); + + // handle null warnings by checking the URL for null + var url = Url.Action("Article1", "News"); + if (url != null) + { + sitemap.Add(new SitemapNewsNode(url, "Article1", "John Doe", "EN", DateTime.UtcNow)); + } return new SitemapResult(sitemap); } @@ -43,7 +51,9 @@ public IActionResult SitemapImages() var imageLocation = new ImageLocation("non-existing-image.jpg"); var sitemap = new Core.Sitemap(); - sitemap.Add(new SitemapImageNode(Url.Action("Index", "Home"), imageLocation)); + + // handle null warnings by using the Create function + sitemap.Add(SitemapImageNode.Create(Url.Action("Index", "Home"), imageLocation)); return new SitemapResult(sitemap); } @@ -54,7 +64,7 @@ public IActionResult SitemapVideos() var video = new VideoContent("non-existing-video-thumbnail.jpg", "Video1", "Video1 description", "non-existing-video.mp4", null); var sitemap = new Core.Sitemap(); - sitemap.Add(new SitemapVideoNode(Url.Action("Index", "Home"), video)); + sitemap.Add(SitemapVideoNode.Create(Url.Action("Index", "Home"), video)); return new SitemapResult(sitemap); } diff --git a/src/Sidio.Sitemap.AspNetCore.Tests/SitemapExtensionsTests.cs b/src/Sidio.Sitemap.AspNetCore.Tests/SitemapExtensionsTests.cs new file mode 100644 index 0000000..e7e80bb --- /dev/null +++ b/src/Sidio.Sitemap.AspNetCore.Tests/SitemapExtensionsTests.cs @@ -0,0 +1,63 @@ +namespace Sidio.Sitemap.AspNetCore.Tests; + +public sealed class SitemapExtensionsTests +{ + [Fact] + public void TryAdd_WhenUrlIsValid_SitemapNodeAdded() + { + // arrange + var sitemap = new Core.Sitemap(); + const string Url = "/index.html"; + + // act + var result = sitemap.TryAdd(Url); + + // assert + result.Should().BeTrue(); + sitemap.Nodes.Count.Should().Be(1); + } + + [Fact] + public void TryAdd_WhenUrlIsEmpty_SitemapNodeNotAdded() + { + // arrange + var sitemap = new Core.Sitemap(); + + // act + var result = sitemap.TryAdd(string.Empty); + + // assert + result.Should().BeFalse(); + sitemap.Nodes.Should().BeEmpty(); + } + + [Fact] + public void TryAdd_WhenUrlsAreValid_SitemapNodesAdded() + { + // arrange + var sitemap = new Core.Sitemap(); + const string Url = "/index.html"; + + // act + var result = sitemap.TryAdd(Url, Url, Url); + + // assert + result.Should().Be(3); + sitemap.Nodes.Count.Should().Be(3); + } + + [Fact] + public void TryAdd_WhenSomeUrlsAreEmpty_SitemapNodeAdded() + { + // arrange + var sitemap = new Core.Sitemap(); + const string Url = "/index.html"; + + // act + var result = sitemap.TryAdd(Url, string.Empty, Url, " "); + + // assert + result.Should().Be(2); + sitemap.Nodes.Count.Should().Be(2); + } +} \ No newline at end of file diff --git a/src/Sidio.Sitemap.AspNetCore.Tests/SitemapIndexExtensionsTests.cs b/src/Sidio.Sitemap.AspNetCore.Tests/SitemapIndexExtensionsTests.cs new file mode 100644 index 0000000..cbaded3 --- /dev/null +++ b/src/Sidio.Sitemap.AspNetCore.Tests/SitemapIndexExtensionsTests.cs @@ -0,0 +1,35 @@ +using Sidio.Sitemap.Core; + +namespace Sidio.Sitemap.AspNetCore.Tests; + +public sealed class SitemapIndexExtensionsTests +{ + [Fact] + public void TryAdd_WhenUrlIsValid_SitemapIndexNodeAdded() + { + // arrange + var sitemapIndex = new SitemapIndex(); + const string Url = "/index.html"; + + // act + var result = sitemapIndex.TryAdd(Url); + + // assert + result.Should().BeTrue(); + sitemapIndex.Nodes.Count.Should().Be(1); + } + + [Fact] + public void TryAdd_WhenUrlIsEmpty_SitemapIndexNodeNotAdded() + { + // arrange + var sitemapIndex = new SitemapIndex(); + + // act + var result = sitemapIndex.TryAdd(string.Empty); + + // assert + result.Should().BeFalse(); + sitemapIndex.Nodes.Should().BeEmpty(); + } +} \ No newline at end of file diff --git a/src/Sidio.Sitemap.AspNetCore/Services/ControllerSitemapService.cs b/src/Sidio.Sitemap.AspNetCore/Services/ControllerSitemapService.cs index 9c66b6b..9bbf7f1 100644 --- a/src/Sidio.Sitemap.AspNetCore/Services/ControllerSitemapService.cs +++ b/src/Sidio.Sitemap.AspNetCore/Services/ControllerSitemapService.cs @@ -88,19 +88,28 @@ public IReadOnlySet CreateSitemap(Type controllerType) private HttpContext HttpContext => _httpContextAccessor.HttpContext ?? throw new InvalidOperationException("HttpContext is null"); - private SitemapNode CreateNode(ControllerActionDescriptor action) + private SitemapNode? CreateNode(ControllerActionDescriptor action) { var url = _linkGenerator.GetUriByAction(HttpContext, action.ActionName, action.ControllerName); - if (_logger.IsEnabled(LogLevel.Trace)) + if (_logger.IsEnabled(LogLevel.Warning) && string.IsNullOrWhiteSpace(url)) + { + _logger.LogWarning( + "Unable to create sitemap URL for action `{Action}` in controller `{Controller}`", + action.ActionName, + action.ControllerName); + } + + if (_logger.IsEnabled(LogLevel.Trace) && !string.IsNullOrWhiteSpace(url)) { _logger.LogTrace( "Created sitemap URL for action `{Action}` in controller `{Controller}`", action.ActionName, action.ControllerName); + } - return new SitemapNode(url); + return !string.IsNullOrWhiteSpace(url) ? new SitemapNode(url) : null; } private HashSet GetControllerMethodsOptIn(Type controllerType, IEnumerable actions) @@ -114,7 +123,10 @@ private HashSet GetControllerMethodsOptIn(Type controllerType, IEnu if (includeFunction) { var node = CreateNode(action); - nodes.Add(node); + if (node != null) + { + nodes.Add(node); + } } else { @@ -153,7 +165,10 @@ private HashSet GetControllerMethodsOptOut(Type controllerType, IEn if (!excludeFunction) { var node = CreateNode(action); - nodes.Add(node); + if (node != null) + { + nodes.Add(node); + } } else { diff --git a/src/Sidio.Sitemap.AspNetCore/Sidio.Sitemap.AspNetCore.csproj b/src/Sidio.Sitemap.AspNetCore/Sidio.Sitemap.AspNetCore.csproj index 3f84782..020ee56 100644 --- a/src/Sidio.Sitemap.AspNetCore/Sidio.Sitemap.AspNetCore.csproj +++ b/src/Sidio.Sitemap.AspNetCore/Sidio.Sitemap.AspNetCore.csproj @@ -40,7 +40,7 @@ - + diff --git a/src/Sidio.Sitemap.AspNetCore/SitemapExtensions.cs b/src/Sidio.Sitemap.AspNetCore/SitemapExtensions.cs new file mode 100644 index 0000000..0ea5546 --- /dev/null +++ b/src/Sidio.Sitemap.AspNetCore/SitemapExtensions.cs @@ -0,0 +1,40 @@ +using System.Diagnostics; +using Sidio.Sitemap.Core; + +namespace Sidio.Sitemap.AspNetCore; + +/// +/// The Sitemap extensions. +/// +public static class SitemapExtensions +{ + /// + /// Adds a nullable URL to the sitemap. + /// + /// The sitemap. + /// The URL. + /// Returns true when the url was added. + public static bool TryAdd(this Core.Sitemap sitemap, string? url) + { + if (string.IsNullOrWhiteSpace(url)) + { + return false; + } + + return sitemap.Add(new SitemapNode(url)) == 1; + } + + /// + /// Adds a range of nullable URLs to the sitemap. + /// + /// The sitemap. + /// The urls. + /// Returns actual the number of nodes that were added. + public static int TryAdd(this Core.Sitemap sitemap, params string?[] urls) + { + var nonNullableUrls = urls.Where(x => !string.IsNullOrWhiteSpace(x)) + .Select(x => new SitemapNode(x ?? throw new UnreachableException())).Cast().ToArray(); + + return nonNullableUrls.Length > 0 ? sitemap.Add(nonNullableUrls) : 0; + } +} \ No newline at end of file diff --git a/src/Sidio.Sitemap.AspNetCore/SitemapIndexExtensions.cs b/src/Sidio.Sitemap.AspNetCore/SitemapIndexExtensions.cs new file mode 100644 index 0000000..2d40c6e --- /dev/null +++ b/src/Sidio.Sitemap.AspNetCore/SitemapIndexExtensions.cs @@ -0,0 +1,25 @@ +using Sidio.Sitemap.Core; + +namespace Sidio.Sitemap.AspNetCore; + +/// +/// The Sitemap-index extensions. +/// +public static class SitemapIndexExtensions +{ + /// + /// Adds a nullable URL to the sitemap index. + /// + /// The sitemap index. + /// The URL. + /// Returns true when the url was added. + public static bool TryAdd(this SitemapIndex sitemapIndex, string? url) + { + if (string.IsNullOrWhiteSpace(url)) + { + return false; + } + + return sitemapIndex.Add(new SitemapIndexNode(url)) == 1; + } +} \ No newline at end of file