Skip to content

Conversation

@JeremyCaney
Copy link
Member

OnTopic Editor 5.2.0 is a very minor update, which doesn't introduce any new public-facing features, but does make a number of performance improvements, such as reducing the amount of mapping that needs to be performed for each topic by relying on the new lightweight PermittedContentType view model (#50) and supporting the new AttributeDictionary constructor for view models (#49). In addition, it also includes a number of internal code improvements, such as adoption of new C# 10 capabilities (#52), including global using directives (#53).

Improvements

Code Changes

This will allow access to some new features, particularly with regard to pattern matching and possibly generic attributes.
This will allow us to take advantage of the latest analyzers.
Prefer range operator, when practical
The `AttributeBindingModelBinder` is instantiated by the framework, so this warning is a false positive.
Implicit usings are a feature of the .NET 6 SDK (`dotnet`, and underlying `msbuild`) which take advantage of C# 10's new global using directives to _implicitly_ add global directives based on the current SDK being targeted (e.g., `Microsoft.NET.Sdk.Web`).

Despite being enabled by the .NET 6 SDK, this doesn't necessitate that the _project_ target .NET 6, and thus we can take advantage of this, even while targeting e.g. .NET Standard 2.1, thus maintaining backward compatibility with earlier versions of .NET Core and even .NET Framework 4.8.
With implicit usings enabled (0148bcd), we can now remove seven of the most common `using` statements in .NET:
- `System`
- `System.Collections.Generic`
- `System.IO`
- `System.Linq`
- `System.Net.Http`
- `System.Threading`
- `System.Threading.Tasks`
With implicit usings enabled (0148bcd), we can now remove nine of the most common `using` statements in ASP.NET Core:

- `System.Net.Http.Json`
- `Microsoft.AspNetCore.Builder`
- `Microsoft.AspNetCore.Hosting`
- `Microsoft.AspNetCore.Http`
- `Microsoft.AspNetCore.Routing`
- `Microsoft.Extensions.Configuration`
- `Microsoft.Extensions.DependencyInjection`
- `Microsoft.Extensions.Hosting`
- `Microsoft.Extensions.Logging`

***Note:*** This can't be applied to the `OnTopic.Editor.AspNetCore` project itself because it's actually using the `Microsoft.NET.Sdk`, not `Microsoft.NET.Sdk.Razor`. As such, this only actually affects the `Host` project.
The most most commonly referenced namespaces in `OnTopic.Editor.AspNetCore` are:
- `System.Collections.ObjectModel` (x9)
- `OnTopic.Editor.AspNetCore.Models` (x5)
- `OnTopic.Editor.AspNetCore.Models.Metadata` (x8)
- `OnTopic.Internal.Diagnostics` (x10)
- `OnTopic.Metadata` (x7)

These have been added as `global using` directives (in the `AssemblyInfo`) and removed from the individual files.
The most most commonly referenced namespaces in `OnTopic.Editor.AspNetCore.Attributes` are:
- `Microsoft.AspNetCore.Mvc` (x21)
- `OnTopic.Editor.AspNetCore.Models` (x33)
- `OnTopic.Editor.AspNetCore.Models.Metadata` (x19)
- `OnTopic.Internal.Diagnostics` (x18)
- `OnTopic.Metadata` (x20)

These have been added as `global using` directives (in the `AssemblyInfo`) and removed from the individual files.
Implemented C# 10 across all projects. As part of this, implemented both implicit usings capability, as well as global usings for the most commonly used namespaces, thus reducing the number of explicit `using` directives in each class. In addition, upgraded the .NET Code Analyzers to the .NET 6 version, and fixed a handful of warnings exposed by the improved code analysis.

Note that while the project continues to use ASP.NET Core 3.1, the use of C# 10 and, in particular, implicit usings, requires the .NET 6 SDK for compiling the project. This is independent of what version of .NET Core implements the Razor Class Library, which continues to support ASP.NET Core 3.1+.
While the OnTopic Editor won't be upgraded to require .NET 6, the `Host` project is intended for internal testing and client reference, and should reflect the latest version that we expect clients to be using. This won't affect the library itself.
This is only used in the `Host` project and doesn't affect the distributable library or its dependencies.
In ASP.NET Core 6, the new Hosting Model allows for the `Program` and `Startup` files to be merged into one. While the final merged file will live at `Program.cs`, almost the entirety of the logic will be from the `Startup` class and, thus, we want to start from that foundation. In preparation for this, we're deleting the current `Program` class.
In ASP.NET Core 6, the new Hosting Model allows for the `Program` and `Startup` files to be merged into one. Since almost the entirety of the logic will be from the `Startup` class, we want to start from that foundation. In preparation for this, we're moving `Startup` to `Program`, since that's where the app will ultimately be loaded from.
With the ASP.NET 6 Hosting Model, an explicit `Startup` or `Program` class aren't required; instead, all statements can be moved to C# 9.0 top-level statements. This reduces some of the ceremony and boilerplate around the application configuration, while also reducing the indentation.

As an initial step, the namespace, class, and methods are removed, but the indentation and variable names remain the same. This makes the diff more readable. In subsequent commits, I'll fix the indentation and standardize the variable names.
With the ASP.NET 6 Hosting Model, most of the services can be configured directly via the `builder` object, or the subsequent `app` object. For instance, instead of maintaining variables like `services`, we can just use `builder.Services`.
With the ASP.NET Core 6 Hosting Model now in place, we can now unify the indentation, taking full advantage of C# 9 top-level statements.
The `Program` file is automatically instantiated by .NET, and thus it is expected that it won't otherwise be instantiated _within_ the application.
With the new ASP.NET 6 Hosting Model, we must explicitly instruct the application to `Run()` at the end of the `Program` file.
But, really, the goal here is just to commit a change so that the next merge request will maintain the histories of `Program` and the new minimal hosting model.
While the OnTopic Editor will continue to use ASP.NET Core 3.1 for backward compatibility with .NET Core, we expect _most_ customers will be using ASP.NET Core 6. As such, I've upgraded the host project to run on ASP.NET Core 6 so it continues to act as a relevant reference for implementers.

As part of this, I also migrated the `OnTopic.Editor.AspNetCore.Host` project over to the new ASP.NET Core 6 minimal hosting model, which merges `Startup` and `Program` into a single top-level program. This is more consistent with the out-of-the-box ASP.NET Core 6 projects that new implementations will be working off of, and is also much easier to read.
This doesn't affect the downstream dependencies of implementations directly, but doesn't impact how the packages are produced, and better integrates with e.g. Visual Studio for debugging packages.
This is a build-only dependency, and doesn't impact downstream dependencies of implementers.
This is a dependency of Bootstraps related to tooltips.
OnTopic 5.2.0 implemented the Beta 3 version of Bootstrap. This gives us an opportunity to upgrade to the final release—and, in fact, to 5.1.3 now.
The `StyleSheets` and `Scripts` collections are populated by the constructors of derived types, and don't map to associations on the source `AttributeDescriptor`. The `IsValueRequired` is a calculated property based on other properties; while those dependency properties are mapped, the `IsValueRequired` property is not. As none of these had setters, they would have been passed over by the `TopicMappingService`, but that to happen much earlier in the process. This should have a small performance benefit.
The local `MaximumValue` property wasn't being set in the `AttributeDictionary` constructor. That effectively disabled this functionality. Whoops!
The `RelationshipAttributeDescriptorViewModel` was inadvertently missed when originally implementing the `AttributeDictionary` constructor overloads on the `AttributeDescriptorViewModel` implementations (7e15db9). This doesn't _harm_ anything, but it also prevents this particular attribute from taking advantage of the performance benefits of the `AttributeDictionary` constructor.
Picked up a few gaps from the original `AttributeDictionary` constructors (e3a1b7), such as a missing property assignment, and an attribute view model without a `AttributeDictionary` constructor. Additionally provided some minor cleanup, such as removing empty constructors where they aren't needed, removing some extraneous comment headers, and fixing some alignment issues in the code.
Updated the OnTopic Library from 5.2.0 Alpha 234 (919d6f3) to 5.2.0 Alpha 255. This will break references to `AttributeDictionary`, which will need to be fixed in a subsequent update.
The `AttributeDictionary` class (OnTopicCMS/OnTopic-Library#99) introduces a conflict with the `AttributeDictionary` in `Microsoft.AspNetCore.Mvc.ViewFeatures`, which is often used as a namespace in model libraries (OnTopicCMS/OnTopic-Library#107). To disambiguate this, it's been renamed to `AttributeValueDictionary`. The editor's view models have been updated to utilize this new naming convention.
The `AttributeDictionary` class (OnTopicCMS/OnTopic-Library#99) introduces a conflict with the `AttributeDictionary` in `Microsoft.AspNetCore.Mvc.ViewFeatures`, which is often used as a namespace in model libraries (OnTopicCMS/OnTopic-Library#107). To disambiguate this, it's been renamed to `AttributeValueDictionary` in OnTopic Library 5.2.0 Alpha 255 (b243853). The editor's view models have been updated to utilize this new naming convention (1ca4eb4).
Updated ASP.NET Runtime Validation from 6.0.1 to 6.0.6. This is a design-time dependency and won't impact consumers of this package.
Updated GitVersion from 5.8.1 to 5.10.3 This is a build dependency only and won't impact consumers of this package.
Update CSS Nano package from 5.0.0 to 5.1.12. This is a build dependency and won't impact consumers of this package directly.
Updated the Gulp Post CSS plugin from 9.0.0 to 9.0.1. This was previously updated for the main `OnTopic.Editor.AspNetCore` project, but was missed for `OnTopic.Editor.AspNetCore.Attributes`; this picks that up. This is a build dependency and won't impact consumers of this package directly.
Updated the JSHint plugin from 2.12.0 to 2.12.4. This was previously updated to 2.12.2 in the main `OnTopic.Editor.AspNetCore` project, but was missed for `OnTopic.Editor.AspNetCore.Attributes`; this picks that up, and updates both to the latest patch. This is a build dependency and won't impact consumers of this package directly.
Updated the Gulp SASS plugin from 4.1.0 to 5.1.0, and also referenced the currently used `node-sass` compiler. This was previously updated for the main `OnTopic.Editor.AspNetCore` project, but was missed for `OnTopic.Editor.AspNetCore.Attributes`; this picks that up. This is a build dependency and won't impact consumers of this package directly. As part of this, updated the `gulpFile` to specify the SASS compiler (`node-sass`), as required by the 5.x release. The compiler will be updated for both projects in a subsequent commit.
Updated the Node SASS compiler plugin from 6.0.1 to 7.0.1. This is a build dependency and won't impact consumers of this package directly. This may potentially have a notable impact on the compiled style sheets and necessitates careful testing.
Updated Popper.JS from 2.11.0 to 2.11.5. This is directly used by the interface and will potentially affect how the popups on the Editor are rendered.
Updated jQuery UI from 1.13.0 to 1.13.1. This is directly used by the interface and will potentially affect how some controls are rendered.
Updated jQuery Validation from 1.19.3 to 1.19.4. This is directly used by the interface and will potentially affect how the form validation works, including the rollup to the tabs.
In the previous commits, I updated top-level dependencies, and any dependencies those packages explicitly defined. In this commit, I update all downstream dependencies, beyond the minimum version requested.
Updated all NuGet and npm dependencies to their latest versions. All of the NuGet updates only affect design- or build-time dependencies, and won't affect downstream consumers. That's also true of some of the npm dependencies, but there are also updates to some libraries used directly in the front end, such as jQuery UI, jQuery Validation, and Popper.js. Those have been carefully tested to ensure they continue to work as expected.
Previously, `OnTopic.Attributes` couldn't be (easily) introduced as a global using directive (e37de82, 1e83841; #53, OnTopicCMS/OnTopic-Library#93) because of a naming conflict with `AttributeDictionary`, which existed in both `OnTopic.Attributes` and `Microsoft.AspNetCore.Mvc.ViewFeatures`. With `AttributeDictionary` renamed to `AttributeValueDictionary` (OnTopicCMS/OnTopic-Library#107), this can be reintroduced.
With `OnTopic.Attributes` added as a global using directive for both `OnTopic` and `OnTopic.Tests` (0ca571a), we can now remove all local using directives for it.
With `AttributeDictionary` renamed to `AttributeValueDictionary` (OnTopicCMS/OnTopic-Library#107) the potential naming conflict with the out-of-the-box `AttributeValueDictionary` is removed, and we can thus (re)introduce `OnTopic.Attributes` as a global using directive (0ca571a), and remove the local using directives across `OnTopic.Editor.AspNetCore.Attributes` (f87ad72).
Updated OnTopic Library to the #260 build of the `revert-AttributeValueDictionary` branch. This reverts `AttributeValueDictionary` back to `AttributeDictionary`, in preparation to undo OnTopicCMS/OnTopic-Library#107). The conflict that was trying to avoid wasn't necessary.
The latest patch renamed `AttributeValueDictionary` back to `AttributeDictionary`, thus reverting a large part of the previous patch (b243853). It seems this decision was premature, though, as we can mitigate the naming conflicts without renaming the class, since the out-of-the-box `ViewFeatures` namespace is almost exclusively used in views, and the `AttributeDictionary` is almost exclusively used in view models. This effectively undoes 1ca4eb4 and OnTopicCMS/OnTopic-Library#107.
Renamed `AttributeValueDictionary` back to `AttributeDictionary`, thus reverting cec9de4. This decision was made prematurely, and we can mitigate the naming conflicts without renaming the class. This is because the out-of-the-box `ViewFeatures` namespace is almost exclusively used in views, and the `AttributeDictionary` is almost exclusively used in view models. This effectively undoes OnTopicCMS/OnTopic-Library#107, while still allowing `OnTopic.Attributes` to be added as a global using directive, where appropriate.
With OnTopic 5.2.0 (finally!) released, we can update to the release version and finalize OnTopic Editor 5.2.0.
Updated OnTopic Library to 5.2.0 Release in preparation for the final deployment of OnTopic Editor 5.2.0.
@JeremyCaney JeremyCaney added this to the OnTopic Editor 5.2.0 milestone Jun 26, 2022
@JeremyCaney JeremyCaney self-assigned this Jun 26, 2022
@JeremyCaney JeremyCaney merged commit ef23c1f into master Jun 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants