Skip to content

Commit 8d921a7

Browse files
JeremyCaneyJeremyCaney
committed
Merge branch 'release/4.6.0' into master
This update includes a few minor features, most notably improvements over routing and reporting for the ASP.NET Core `TopicController`. In addition, it upgrades the underlying code to C# 9.0 in order to take advantage of some new features such as improved pattern matching and implicitly typed initializers. Features - Updated `[ValidateTopic]` attribute to redirect to canonical URL to ensure consistency in analytics, indexes (0edab34) - Updated `Topic.Load(RouteData)` extension to return null if an invalid `Key` is submitted (b303f13); this results in the `TopicController` treating incorrect topic paths (e.g., `/Web/!@~/`) as 404s instead of exceptions - Added support for enabling (`IsIndexed`) and configuring (`IndexLabel`) an optional table of contents to the `ContentListTopicViewModel` (e9ccb06) Bug Fixes - Fixed issue with where the delegate is validated in the `HierarchicalTopicMappingService.GetViewModelAsnc()` method (69dff49) Improvements - Updated code to use C# 9.0 pattern matching and implicitly typed initializers (d63f2b5) - Updated to latest versions of internal packages, and implemented (4838da8) - Updated to latest version of Code Analysis and fixed most issues introduced with C# 9.0 (089c96a) Note: There are some Code Analysis false positives that remain with C# 9.0. Those are presumably fixed with the new `Microsoft.CodeAnalysis.NetAnalyzers` 5.0.0. There appears to be a bug in getting that to work with .NET 3.x projects, however, so it's not yet implemented.
2 parents 371f279 + 089c96a commit 8d921a7

File tree

86 files changed

+348
-312
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

86 files changed

+348
-312
lines changed

‎OnTopic.AspNetCore.Mvc.Host/OnTopic.AspNetCore.Mvc.Host.csproj

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
<Project Sdk="Microsoft.NET.Sdk.Web">
1+
<Project Sdk="Microsoft.NET.Sdk.Web">
22

33
<PropertyGroup>
4-
<TargetFramework>netcoreapp3.0</TargetFramework>
4+
<TargetFramework>netcoreapp3.1</TargetFramework>
55
<UserSecretsId>62eb85bf-f802-4afd-8bec-3d344e1cfc79</UserSecretsId>
66
<IsPackable>false</IsPackable>
77
</PropertyGroup>

‎OnTopic.AspNetCore.Mvc.Host/Views/Layout/_Layout.cshtml

+1-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
<!-- /Page Header Area -->
2727

2828
<article itemscope itemtype="http://schema.org/WebPageElement" itemprop="mainContentOfPage" class="grid-container">
29-
<h1>@(Model.ContentType.Equals("PageGroup")? "Overview" : Model.Title)</h1>
29+
<h1>@(Model.ContentType == "PageGroup"? "Overview" : Model.Title)</h1>
3030
<p class="subtitle">@Model.Subtitle</p>
3131
<section class="body">
3232
<partial name="_TopicAttributes" />

‎OnTopic.AspNetCore.Mvc.Tests/OnTopic.AspNetCore.Mvc.Tests.csproj

+3-2
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
<Project Sdk="Microsoft.NET.Sdk">
22

33
<PropertyGroup>
4-
<TargetFramework>netcoreapp3.0</TargetFramework>
4+
<TargetFramework>netcoreapp3.1</TargetFramework>
55
<IsPackable>false</IsPackable>
6+
<LangVersion>9.0</LangVersion>
67
</PropertyGroup>
78

89
<ItemGroup>
9-
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.7.1" />
10+
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.8.0" />
1011
<PackageReference Include="MSTest.TestAdapter" Version="2.1.2" />
1112
<PackageReference Include="MSTest.TestFramework" Version="2.1.2" />
1213
</ItemGroup>

‎OnTopic.AspNetCore.Mvc.Tests/TopicControllerTest.cs

+3-3
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public TopicControllerTest() {
7373
RouteData = routes,
7474
ActionDescriptor = new ControllerActionDescriptor()
7575
};
76-
_context = new ControllerContext(actionContext);
76+
_context = new(actionContext);
7777

7878
}
7979

@@ -131,11 +131,11 @@ public void SitemapController_Index_ReturnsSitemapXml() {
131131

132132
var actionContext = new ActionContext {
133133
HttpContext = new DefaultHttpContext(),
134-
RouteData = new RouteData(),
134+
RouteData = new(),
135135
ActionDescriptor = new ControllerActionDescriptor()
136136
};
137137
var controller = new SitemapController(_topicRepository) {
138-
ControllerContext = new ControllerContext(actionContext)
138+
ControllerContext = new(actionContext)
139139
};
140140
var result = controller.Index() as ContentResult;
141141
var model = result.Content as string;

‎OnTopic.AspNetCore.Mvc.Tests/ValidateTopicAttributeTest.cs

+5-5
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public static ActionExecutingContext GetActionExecutingContext(Controller contro
4040

4141
var actionContext = new ActionContext(
4242
new DefaultHttpContext(),
43-
new RouteData(),
43+
new(),
4444
new ControllerActionDescriptor(),
4545
modelState
4646
);
@@ -63,10 +63,10 @@ public static ActionExecutingContext GetActionExecutingContext(Controller contro
6363
/// Generates a barebones <see cref="ControllerContext"/> for testing a controller.
6464
/// </summary>
6565
public static ControllerContext GetControllerContext() =>
66-
new ControllerContext(
67-
new ActionContext() {
66+
new(
67+
new() {
6868
HttpContext = new DefaultHttpContext(),
69-
RouteData = new RouteData(),
69+
RouteData = new(),
7070
ActionDescriptor = new ControllerActionDescriptor()
7171
}
7272
);
@@ -78,7 +78,7 @@ public static ControllerContext GetControllerContext() =>
7878
/// Generates a barebones <see cref="ControllerContext"/> for testing a controller.
7979
/// </summary>
8080
public static TopicController GetTopicController(Topic topic) =>
81-
new TopicController(
81+
new(
8282
new DummyTopicRepository(),
8383
new DummyTopicMappingService()
8484
) {

‎OnTopic.AspNetCore.Mvc/Components/MenuViewComponentBase{T}.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ IHierarchicalTopicMappingService<T> hierarchicalTopicMappingService
102102
await HierarchicalTopicMappingService.GetRootViewModelAsync(
103103
navigationRootTopic!,
104104
3,
105-
t => t.ContentType != "PageGroup"
105+
t => t is not { ContentType: "List" } and not { Parent: { ContentType: "PageGroup" } }
106106
).ConfigureAwait(false);
107107

108108
/*==========================================================================================================================

‎OnTopic.AspNetCore.Mvc/Components/NavigationTopicViewComponentBase{T}.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ IHierarchicalTopicMappingService<T> hierarchicalTopicMappingService
7777
/// <returns>The Topic associated with the current request.</returns>
7878
protected Topic? CurrentTopic {
7979
get {
80-
if (_currentTopic == null) {
80+
if (_currentTopic is null) {
8181
_currentTopic = TopicRepository.Load(RouteData);
8282
}
8383
return _currentTopic;

‎OnTopic.AspNetCore.Mvc/Components/PageLevelNavigationViewComponentBase{T}.cs

+3-4
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,9 @@ IHierarchicalTopicMappingService<T> hierarchicalTopicMappingService
7474
/*------------------------------------------------------------------------------------------------------------------------
7575
| Identify navigation root
7676
\-----------------------------------------------------------------------------------------------------------------------*/
77-
if (navigationRootTopic != null) {
77+
if (navigationRootTopic is not null) {
7878
while (
79-
navigationRootTopic.Parent != null &&
80-
!navigationRootTopic.ContentType.Equals("PageGroup", StringComparison.InvariantCulture)
79+
navigationRootTopic is not null and not ({ Parent: null } or { ContentType: "PageGroup" })
8180
) {
8281
navigationRootTopic = navigationRootTopic.Parent;
8382
}
@@ -86,7 +85,7 @@ IHierarchicalTopicMappingService<T> hierarchicalTopicMappingService
8685
/*------------------------------------------------------------------------------------------------------------------------
8786
| Return root
8887
\-----------------------------------------------------------------------------------------------------------------------*/
89-
return navigationRootTopic?.Parent == null? null : navigationRootTopic;
88+
return navigationRootTopic?.Parent is null? null : navigationRootTopic;
9089

9190
}
9291

‎OnTopic.AspNetCore.Mvc/Controllers/RedirectController.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public virtual ActionResult Redirect(int topicId) {
5959
/*------------------------------------------------------------------------------------------------------------------------
6060
| Provide error handling
6161
\-----------------------------------------------------------------------------------------------------------------------*/
62-
if (topic == null) {
62+
if (topic is null) {
6363
return NotFound("Invalid TopicID.");
6464
}
6565

‎OnTopic.AspNetCore.Mvc/Controllers/SitemapController.cs

+4-4
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ public virtual ActionResult Index(bool indent = false, bool includeMetadata = fa
146146
/// <returns>A Sitemap.org sitemap.</returns>
147147
[Obsolete("The GenerateSitemap() method should not be public. It will be marked private in OnTopic Library 5.0.")]
148148
public virtual XDocument GenerateSitemap(Topic rootTopic, bool includeMetadata = false) =>
149-
new XDocument(
149+
new(
150150
new XElement(_sitemapNamespace + "urlset",
151151
from topic in rootTopic?.Children
152152
select AddTopic(topic, includeMetadata)
@@ -172,9 +172,9 @@ public IEnumerable<XElement> AddTopic(Topic topic, bool includeMetadata = false)
172172
/*------------------------------------------------------------------------------------------------------------------------
173173
| Validate topic
174174
\-----------------------------------------------------------------------------------------------------------------------*/
175-
if (topic == null) return topics;
176-
if (topic.Attributes.GetValue("NoIndex") == "1") return topics;
177-
if (topic.Attributes.GetValue("IsDisabled") == "1") return topics;
175+
if (topic is null) return topics;
176+
if (topic.Attributes.GetValue("NoIndex") is "1") return topics;
177+
if (topic.Attributes.GetValue("IsDisabled") is "1") return topics;
178178
if (ExcludeContentTypes.Any(c => topic.ContentType.Equals(c, StringComparison.InvariantCultureIgnoreCase))) return topics;
179179

180180
/*------------------------------------------------------------------------------------------------------------------------

‎OnTopic.AspNetCore.Mvc/Controllers/TopicController.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ ITopicMappingService topicMappingService
7171
/// <returns>The Topic associated with the current request.</returns>
7272
public Topic? CurrentTopic {
7373
get {
74-
if (_currentTopic == null) {
74+
if (_currentTopic is null) {
7575
_currentTopic = TopicRepository.Load(RouteData);
7676
}
7777
return _currentTopic;
@@ -122,7 +122,7 @@ public async virtual Task<IActionResult> IndexAsync(string path) {
122122
/// <returns>The created <see cref="TopicViewResult"/> object for the response.</returns>
123123
[NonAction]
124124
public virtual TopicViewResult TopicView(object model, string? viewName = null) =>
125-
new TopicViewResult(ViewData, TempData, model, CurrentTopic?.ContentType, viewName);
125+
new(ViewData, TempData, model, CurrentTopic?.ContentType, viewName);
126126

127127
} //Class
128128
} //Namespace

‎OnTopic.AspNetCore.Mvc/OnTopic.AspNetCore.Mvc.csproj

+4-4
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@
33
<PropertyGroup>
44
<ProjectGuid>{B7F136A1-C86D-4A74-AC4F-3693CD1358A4}</ProjectGuid>
55
<RootNamespace>OnTopic.AspNetCore.Mvc</RootNamespace>
6-
<TargetFramework>netcoreapp3.0</TargetFramework>
6+
<TargetFramework>netcoreapp3.1</TargetFramework>
77
<ShouldCreateLogs>True</ShouldCreateLogs>
88
<AdvancedSettingsExpanded>False</AdvancedSettingsExpanded>
9-
<LangVersion>8.0</LangVersion>
9+
<LangVersion>9.0</LangVersion>
1010
<Nullable>enable</Nullable>
1111
</PropertyGroup>
1212

@@ -39,11 +39,11 @@
3939

4040
<ItemGroup>
4141
<FrameworkReference Include="Microsoft.AspNetCore.App" />
42-
<PackageReference Include="GitVersionTask" Version="5.3.7">
42+
<PackageReference Include="GitVersionTask" Version="5.5.1">
4343
<PrivateAssets>all</PrivateAssets>
4444
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
4545
</PackageReference>
46-
<PackageReference Include="Microsoft.CodeAnalysis.FxCopAnalyzers" Version="3.3.0">
46+
<PackageReference Include="Microsoft.CodeAnalysis.FxCopAnalyzers" Version="3.3.1">
4747
<PrivateAssets>all</PrivateAssets>
4848
<IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
4949
</PackageReference>

‎OnTopic.AspNetCore.Mvc/TopicRepositoryExtensions.cs

+9-2
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,16 @@ RouteData routeData
7777
var topic = (Topic?)null;
7878

7979
foreach (var searchPath in paths) {
80-
if (topic != null) break;
80+
if (topic is not null) break;
8181
if (String.IsNullOrEmpty(searchPath)) continue;
82-
topic = topicRepository.Load(searchPath);
82+
try {
83+
topic = topicRepository.Load(searchPath);
84+
}
85+
catch (InvalidKeyException) {
86+
//As route data comes from user-submitted requests, it's expected that some may contain invalid keys. From this
87+
//method's perspective, this is no different than having an invalid URL. As such, we should return a null result, not
88+
//bubble up an exception.
89+
}
8390
}
8491

8592
return topic;

‎OnTopic.AspNetCore.Mvc/TopicRouteValueTransformer.cs

+3-3
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public override async ValueTask<RouteValueDictionary> TransformAsync(HttpContext
4545
\-----------------------------------------------------------------------------------------------------------------------*/
4646
var controller = (string)values["controller"];
4747
var area = (string)values["area"];
48-
if (area != null && controller == null) {
48+
if (area is not null && controller is null) {
4949
values["controller"] = area;
5050
}
5151

@@ -55,7 +55,7 @@ public override async ValueTask<RouteValueDictionary> TransformAsync(HttpContext
5555
| If the action isn't defined in the route, assume Index—which is the default action for the TopicController.
5656
\-----------------------------------------------------------------------------------------------------------------------*/
5757
var action = (string)values["action"];
58-
if (action == null) {
58+
if (action is null) {
5959
action = "Index";
6060
values["action"] = action;
6161
}
@@ -68,7 +68,7 @@ public override async ValueTask<RouteValueDictionary> TransformAsync(HttpContext
6868
| based on the path. It is not needed when routing by controller/action pairs.
6969
\-----------------------------------------------------------------------------------------------------------------------*/
7070
var path = (string)values["path"];
71-
if (path != null || action.Equals("Index", StringComparison.OrdinalIgnoreCase)) {
71+
if (path is not null || action.Equals("Index", StringComparison.OrdinalIgnoreCase)) {
7272
values["rootTopic"] = area;
7373
}
7474

‎OnTopic.AspNetCore.Mvc/TopicViewResultExecutor.cs

+5-5
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ public ViewEngineResult FindView(ActionContext actionContext, TopicViewResult vi
103103
\-----------------------------------------------------------------------------------------------------------------------*/
104104
if (!(view?.Success ?? false) && requestContext.Query.ContainsKey("View")) {
105105
var queryStringValue = requestContext.Query["View"].First<string>();
106-
if (queryStringValue != null) {
106+
if (queryStringValue is not null) {
107107
view = viewEngine.FindView(actionContext, queryStringValue, isMainPage: true);
108108
searchedPaths = searchedPaths.Union(view.SearchedLocations ?? Array.Empty<string>()).ToList();
109109
}
@@ -122,14 +122,14 @@ public ViewEngineResult FindView(ActionContext actionContext, TopicViewResult vi
122122
// Get content-type after the slash and replace '+' characters in the content-type to '-' for view file encoding
123123
// purposes
124124
var acceptHeader = splitHeaders[i]
125-
.Substring(splitHeaders[i].IndexOf("/", StringComparison.InvariantCulture) + 1)
125+
[(splitHeaders[i].IndexOf("/", StringComparison.InvariantCulture) + 1)..]
126126
.Replace("+", "-", StringComparison.InvariantCulture);
127127
// Validate against available views; if content-type represents a valid view, stop validation
128-
if (acceptHeader != null) {
128+
if (acceptHeader is not null) {
129129
view = viewEngine.FindView(actionContext, acceptHeader, isMainPage: true);
130130
searchedPaths = searchedPaths.Union(view.SearchedLocations ?? Array.Empty<string>()).ToList();
131131
}
132-
if (view != null) {
132+
if (view is not null) {
133133
break;
134134
}
135135
}
@@ -174,7 +174,7 @@ public ViewEngineResult FindView(ActionContext actionContext, TopicViewResult vi
174174
/*------------------------------------------------------------------------------------------------------------------------
175175
| Return view, if found
176176
\-----------------------------------------------------------------------------------------------------------------------*/
177-
if (view != null && view.Success) {
177+
if (view is not null and { Success: true }) {
178178
return view;
179179
}
180180
return ViewEngineResult.NotFound(contentType, searchedPaths);

‎OnTopic.AspNetCore.Mvc/ValidateTopicAttribute.cs

+17-5
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public override void OnActionExecuting(ActionExecutingContext filterContext) {
7171
/*------------------------------------------------------------------------------------------------------------------------
7272
| Validate context
7373
\-----------------------------------------------------------------------------------------------------------------------*/
74-
if (controller == null) {
74+
if (controller is null) {
7575
throw new InvalidOperationException(
7676
$"The {nameof(ValidateTopicAttribute)} can only be applied to a controller deriving from {nameof(TopicController)}."
7777
);
@@ -80,7 +80,7 @@ public override void OnActionExecuting(ActionExecutingContext filterContext) {
8080
/*------------------------------------------------------------------------------------------------------------------------
8181
| Handle exceptions
8282
\-----------------------------------------------------------------------------------------------------------------------*/
83-
if (currentTopic == null) {
83+
if (currentTopic is null) {
8484
if (!AllowNull) {
8585
filterContext.Result = controller.NotFound("There is no topic associated with this path.");
8686
}
@@ -111,7 +111,7 @@ public override void OnActionExecuting(ActionExecutingContext filterContext) {
111111
| Nested topics are not expected to be viewed directly; if a user requests a nested topic, return a 403 to indicate that
112112
| the request is valid, but forbidden.
113113
\-----------------------------------------------------------------------------------------------------------------------*/
114-
if (currentTopic.ContentType == "List" || currentTopic.Parent?.ContentType == "List") {
114+
if (currentTopic is { ContentType: "List"} or { Parent: {ContentType: "List" } }) {
115115
filterContext.Result = new StatusCodeResult(403);
116116
return;
117117
}
@@ -122,7 +122,7 @@ public override void OnActionExecuting(ActionExecutingContext filterContext) {
122122
| Like nested topics, containers are not expected to be viewed directly; if a user requests a container, return a 403 to
123123
| indicate that the request is valid, but forbidden. Unlike nested topics, children of containers are potentially valid.
124124
\-----------------------------------------------------------------------------------------------------------------------*/
125-
if (currentTopic.ContentType == "Container") {
125+
if (currentTopic.ContentType is "Container") {
126126
filterContext.Result = new StatusCodeResult(403);
127127
return;
128128
}
@@ -133,13 +133,25 @@ public override void OnActionExecuting(ActionExecutingContext filterContext) {
133133
| PageGroups are a special content type for packaging multiple pages together. When a PageGroup is identified, the user is
134134
| redirected to the first (non-hidden, non-disabled) page in the page group.
135135
\-----------------------------------------------------------------------------------------------------------------------*/
136-
if (currentTopic.ContentType == "PageGroup") {
136+
if (currentTopic.ContentType is "PageGroup") {
137137
filterContext.Result = controller.Redirect(
138138
currentTopic.Children.Where(t => t.IsVisible()).FirstOrDefault().GetWebPath()
139139
);
140140
return;
141141
}
142142

143+
/*------------------------------------------------------------------------------------------------------------------------
144+
| Handle canonical URL
145+
>-------------------------------------------------------------------------------------------------------------------------
146+
| Most search engines are case sensitive, even though many web servers are configured case insensitive. To help avoid
147+
| mismatches between the requested URL and the canonical URL, and to help ensure that references to topics maintain the
148+
| same case as assigned in the topic graph, URLs that vary only by case will be redirected to the expected case.
149+
\-----------------------------------------------------------------------------------------------------------------------*/
150+
if (!currentTopic.GetWebPath().Equals(filterContext.HttpContext.Request.Path, StringComparison.Ordinal)) {
151+
filterContext.Result = controller.RedirectPermanent(currentTopic.GetWebPath());
152+
return;
153+
}
154+
143155
/*------------------------------------------------------------------------------------------------------------------------
144156
| Base processing
145157
\-----------------------------------------------------------------------------------------------------------------------*/

‎OnTopic.Data.Caching/CachedTopicRepository.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ public CachedTopicRepository(ITopicRepository dataProvider) : base() {
101101
/*------------------------------------------------------------------------------------------------------------------------
102102
| Lookup by TopicKey
103103
\-----------------------------------------------------------------------------------------------------------------------*/
104-
if (topicKey != null && !topicKey.Length.Equals(0)) {
104+
if (topicKey is not null && topicKey.Length is not 0) {
105105
return _cache.GetByUniqueKey(topicKey);
106106
}
107107

0 commit comments

Comments
 (0)