This is the basic test for `/{Area}/{path}` requests, which will go through `AreaController`, in this particular setup—though it will support any controllers derived from `TopicController` and associated with areas.

This is the basic test for `/Area/{controller}/{action?}/{id?}` requests, which will go through `ControllerController`, in this particular setup—though it will support any controllers associated with areas.

This is the basic test for `/Area/{action?}/{id?}` or `/Area/{path}/` requests, which will go through `AreaController`, in this particular setup—though it will support any controllers which have the same name as their corresponding area (i.e., in this case, `[Area("Area")]` corresponds to `AreaController`). In this case, the `Controller` needn't be included in the route, as the `MapImplicitAreaControllerRoute()` will implicitly assume `AreaController` based on the `area` route parameter.

This satisfies CA1001.

As these are based on ASP.NET Core, which isn't CLS Compliant, these assemblies themselves are not CLS compliant. That's fine, but we should let consumers know. This resolves CA1014.

Typically, method names in public interfaces should not use underscores. Ignia follows this convention. In testing projects, however, underscores are frequently used to break up concepts in descriptive test names. As such, CA1707 is suppressed for testing projects. This resolves CA1707.

In production applications, this is used to determine if the application is running in developer mode. That's irrelevant in a unit test project, and so that dependency isn't necessary.

The `CreateFakeData()` method creates a number of `Topics` instances which are stored in memory, but not subsequently accessed. These should be explicitly set to the discard variable (`_`). This resolves CA1806—and makes the alignment more consistent!

Since it doesn't rely on any stateful data, and is a `private` function, there's no reason for it not to be marked `static`. This resolves CA1822.

In the original `StubTopicRepository`, this was used to track what `Id` should be assigned to each topic. That's now done using the local `currentAttributeId` variable. This resolves CA1823.

Typically, `Contract.Requires()` go at the beginning—and this one could be as well. For backward compatibility, we're keeping it where it is. `Contract.Requires()` effectively does the same thing as this method—and resolves CA2208, which expects the exact parameter name to be passed to an `ArgumentNullException`. (We could also use `Contract.Assume()` here, though `Contract.Require()` uses the appropriate `ArgumentNullException` by default.)

The empty constructor was a bit of a hack in order to ensure that the `TopicRelationshipMultiMap` constructor could relay an existing instance of a `TopicMultiMap()` to the base `ReadOnlyTopicMultiMap`. The `ReadOnlyTopicMultiMap` _always_ expects its `Source` property to be initialized, so this relied on the derived class being sure to set this.

A better approach is just to pass a `new TopicRelationshipMultiMap` to the `ReadOnlyTopicMultiMap()` constructor, and then set the local `_storage` backing variable to the `base.Source` property. This effectively does the same thing, but prevents the need for the `protected` empty constructor.

Technically, this is a breaking change. The constructor _should_ have been marked as `internal` since it was only intended for internal use. Fortunately, as this is new with OnTopic 5.0.0, we have high confidence that this hasn't been utilized, and this overload isn't documented in the `README` files, at least, so we feel comfortable removing it.

Normally, required `init` only properties should be initialized by the constructor. That's a good rule. For view and binding models, however, we expect these to be initialized not by the constructor, but instead by the e.g. `TopicMappingService` or the `ReverseTopicMappingService`. As such, these should be suppressed. Unfortunately, these can only be suppressed in the source, and not globally via e.g. `[SuppressMessage()]`. That means these need to be applied to each view or binding model with required properties.

This resolves IDE0017—and cleans up the code in the process.

This cleans up the code and makes it more consistent. This resolves IDE0090.

In a previous update, I modified the guard clause for `GetHierarchicalRoot()` to use `Contract.Requires()` (87e4296). But `Contract.Requires(object)` only checks that the object is null, not empty, and thus didn't maintain parity with the previous guard clause which used `IsNullOrEmpty()`.

To remedy this, I've updated the clause to explicitly check `IsNullOrEmpty()`. Since this changes the overload to `Contract.Requires(isValid)`, however, that would result in it throwing an `InvalidOperationException` instead. (This makes logical sense, but is a unintuitive for users of the library.) As such, I'm explicitly setting `ArgumentNullException`.

Finally, since the whole point of using `Contract.Requires()` instead of `Contract.Assumes()` was to take advantage of its implicit use of `ArgumentNullException`, and that advantage doesn't exist with the `Contract.Requires(isValid)` overload, I'm updating this to use `Contract.Assume()` instead, which is more semantically appropriate given the placement.

Eep!

When displaying friendly 404 pages, the current URL may not resolve to a valid topic. In that case, components such as the page-level navigation should not throw an exception, but also shouldn't return any results. This test validates the expected output in this case.

If a page group is requested, `[ValidateTopic]` will redirect to the first `IsVisible()` child. If that child doesn't exist, a 403 should be returned.

This currently doesn't pass. This is the subject of Issue #62. This will be resolved in a subsequent commit.

A `PageGroup` cannot be displayed; it is just a container of child topics. So when a `PageGroup` is requested, the `[ValidateTopic]` attribute redirects to the first `IsVisible()` child. If no such child exists, a 403 should be returned. But, instead, an exception was being thrown due to the attempt to redirect to a null path. This resolves #62.

The `MapTopicAreaRoute()` and `MapImplicitAreaControllerRoute()` are simple convenience functions that define standard routes. As they are impractical to test alongside their far more complicated general overloads, the focus of the integration tests is instead of those overloads.

In previous versions of OnTopic, the `contenttype` route parameter was typically set by upstream logic, and thus there was more defensive code to ensure that it was reset in the case that the `TopicController` changed the target `Topic` (as might happen with e.g. a 404). The `contenttype` route parameter is no longer set upstream, however, and so there isn't a need to be as defensive in this case. If it is set, we should be able to trust the value—but, more likely, it won't be.

In a real-world scenario, the HTTP Accept header will contain multiple values, not just one value. As such, the test should reflect this reality. It turns out, this test actually fails—meaning this functionality would _not_ work in most real world scenarios! This issue is captured in #63, and will be fixed in a subsequent commit.

While this is a less expected scenario, the HTTP Accept headers can, potentially, include additional metadata with each value such as `level` and `q`. If these are present, they should be stripped out and not evaluated as part of the header. This isn't currently support. This is also being tracked as part of #63, and will be addressed in a subsequent commit.

The `TopicViewResultExecutor` will attempt to identify views based on the HTTP Accept headers. This is intended to allow e.g. a `JSON` view to be offered, and requested via the `Accept` headers. The previous code, however, only evaluated the first item, and then failed to account for potential metadata. This update rewrites (and simplifies) the code, thus allowing it to correctly account for these considerations. This resolves #63.

If the view cannot be identified at all, an internal server error should occur. To model this, a new `MissingView` topic was added to the `StubTopicRepository`, and a new `MissingViewModel` was added to ensure that an exception wasn't thrown upstream by the `ITopicMappingService`.

Established integration tests for the ASP.NET Core project. This includes a special host and `StubTopicRepository` exclusively for the integration tests so that it can operate against light-weight, in-memory data instead of a SQL database, thus allowing it to run in the Azure DevOps CI/CD pipeline. This covers the `ServiceCollectionExtensions`, `TopicViewLocationExpander`, and `TopicViewResultExecutor`. Other elements of the ASP.NET Core adapter, such as the controllers and view components, may be indirectly covered by these tests, but are also covered by the existing unit tests. The integration tests, instead, focus on the components that are difficult or impractical to unit test.

…vent

The `TopicLoadEventArgs` event has an optional `Version` property that should be populated when loading a specific version. This test currently fails due to a bug in the `StubTopicRepository` test double, which will be fixed in a subsequent commit.

When the `StubTopicRepository.Load(topicId, version, referenceTopic)` is called, it should raise the `TopicLoaded` event with the `version`. This wasn't happening, thus preventing the unit test from passing (9f63d37). This addresses a bug, #58.

Ensured that the `StubTopicRepository`'s `Load(topicId, version, referenceTopic)` overload correctly raises the `TopicLoaded` event with the `Version` property set so that it can be tested (1b64c06). Added a unit test to evaluate this specific scenario to confirm the functionality works as expected (9f63d37). This resolves Issue #58.

Previously, calling `SetValue(key, null)` would set the `TrackedRecord<T>.Value` to null, but wouldn't delete the record. This was confusing, if not buggy, and the behavior was fixed (3d1ad6b, 0bccf1f). In doing so, however, unit tests which validated the previous assumption broke, and needed to be updated.

This isn't expected to harm any actual implementations, as they should already be written to account for missing records, since this is the default when loading from the database. The null value scenario would only occur when updating previously saved values.

Introduced a test to ensure that calling `Clear()` enforces business logic by adding a property that will throw an exception if its set to null. This validates the behavior introduced in abfe213. And since `Clear()` now calls `Remove()`, it also effectively validates the behavior introduced in 0bccf1f. This validates the new functionality introduced as part of #79.

…to develop

Previously, calling `Remove()` and `Clear()` on `TrackedRecordCollection<>` and its derivatives `AttributeCollection` and `TopicReferenceCollection` would bypass any business logic on associated properties. It was also inconsistent with `SetValue()` which would accept null values, and correctly enforce business logic, but would keep the `TrackedRecord<>` in place, thus causing potential confusion and bugs (since e.g. `Contains()` would report that the `Key` existed, even though it had a null `Value`).

To mitigate this, the `SetValue()` logic was updated to call `Remove()` if the `Value` was null (3d1ad6b), `Remove()` was updated to enforce business logic (0bccf1f), and `Clear()` was updated to call `Remove()` (abfe213)—thus enforcing business logic on each individual item. In addition to updating existing tests which assumed that `SetValue()` wouldn't delete a record (7aae9bc), I also introduced a _new_ unit test to validate that `Clear()` (and, thus, `Remove()`) will, in fact, enforce business logic (fa08bfb).

This satisfies the requirements for #79.

The `Reflexive` value instructs callers of an `AttributeDescriptor` to gets its `ModelType` from the `AttributeDescriptor.ModelType` of the `AttributeDescriptor` derivative that the `AttributeDescriptor` is placed on.

This is a special scenario which allows e.g. the OnTopic Editor to display an attribute on an `AttributeDescriptor` which uses the current topic's values, and stores the result in the same location as the current topic.

This is useful for e.g. `AttributeDescriptor.DefaultValue`, which should be set to use the same attribute type and storage location as the `AttributeDescriptor` it is set for (OnTopicCMS/OnTopic-Editor-AspNetCore#44).

Introduced a special `ModelType.Reflexive` enum value in order to support the proposed `ReflexiveAttribute` plugin for the OnTopic Editor (OnTopicCMS/OnTopic-Editor-AspNetCore#44), which allows an attribute placed on an `AttributeDescriptor` to "reflect" back that `AttributeDescriptor` as an attribute, thus allowing e.g. `AttributeDescriptor.DefaultValue` to use the same values as the `AttributeDescriptor` it describes, including the same storage location, as described by its `AttributeDescriptor.ModelType`. It does this by falling back to the `ModelType` of the current topic when the `ModelType` of an `AttributeDescriptor` on its `ContentTypeDescriptor` is set to `Reflexive`.

It makes sense that only classes that are collections should end with `Collection`. Unfortunately, xUnit.net uses the name "Collection" for items that don't implement `IEnumerable`. As such, we're suppressing this warning here.

These are following conventions for consistency.

A few unaddressed warnings from Code Analysis had crept in during the 5.0.1 updates, which have been addressed as part of this update.

This is a build time dependency, and doesn't affect consumers of the NuGet packages.

This corresponds with today's release of .NET 5.0.5

It seems we were a few patches behind. This catches us up to the latest version. This is a build time dependency, and doesn't affect consumers of the NuGet packages.

Updated build time NuGet dependencies, such as GitVersion as the Microsoft Test SDK. This doesn't affect subscribers of the NuGet packages.

The Roslyn code analyzers will throw a `CS8616` if a non-nullable property will not be initialized after the constructor has executed. This is a problem for models that are expected to be initialized by e.g. data binding, mapping, or deserialization.

We used to work around this using the `[NotNull, DisallowNull]` attributes on a nullable property, which I liked as it was explicit about what it was doing. Unfortunately, as of .NET 5.0.4 (SDK 5.0.201), that now throws a `CS8616` as well. Given that, we need to instead use the less obvious null-forgiving operator instead with these cases.

Given this, I've replaced the cases where we were using `[NotNull, DisallowNull]` with `= default!;` on a non-nullable type instead.

The Roslyn code analyzers will throw a `CS8616` if a non-nullable property will not be initialized after the constructor has executed. This is a problem for models that are expected to be initialized by e.g. data binding, mapping, or deserialization.

We used to work around this using the `[NotNull, DisallowNull]` attributes on a nullable property, which I liked as it was explicit about what it was doing. Unfortunately, as of .NET 5.0.4 (SDK 5.0.201), that now throws a `CS8616` as well. Given that, we need to instead use the less obvious null-forgiving operator instead with these cases.

Given this, I've replaced the cases where we were using `[NotNull, DisallowNull]` with `= default!;` on a non-nullable type instead.

With the `[NotNull, DisallowNull]` attributes removed (d9ff464), this is no longer necessary.

Removed some unnecessary `using` statement left behind by previous cleanup.

If calling `TrackedRecordCollection<>.SetValue()` without any updates—i.e., an existing record is present, but the `Value` hasn't changed, and neither `markDirty` nor `version` are specified—then return without further processing. In that case, there's no need to create a new instance of the record, to potentially call the property on the corresponding `Topic`, or to store the new record in the collection. This is a minor performance enhancement.

For reasons that aren't entirely clear, the `==` check isn't using the value comparison when both `value` and `TrackedRecord<String>.Value` are `string`. This is unexpected. As a result, the values are _always_ being seen as having been changed, even when they haven't been. I'd like to investigate this further, as it could suggest behavior that will introduce bugs elsewhere in the library. In the meanwhile, I'm explicitly calling `Equals()` which remedies the problem, and resolves #86.

Fixed a bug where a calling `SetValue()` with the same value that an existing record was set to would mark the record as `IsDirty`. Only changes in `Value` should result in `IsDirty`. This seems to stem from an issue with the `==` operator on `T` not using the `==` from `String`, even when `T` is of type `String`. This is an unexpected scenario, and will require further investigation. In the meanwhile, using `Equals()` solves #86.

Previously, the `DeleteOrphanedLastModifiedAttributes` only accounted for `LastModified`, not `LastModifiedBy`—and, as such, would not get tripped if both the `LastModified` _and_ `LastModifiedBy` attributes were updated at the same time, even if no other attributes were updated. This resolves that issue, as a first step toward #88.

Previously, the `DeleteOrphanedLastModifiedAttributes` only accounted for `Attributes` and `ExtendedAttributes`, not `Relationships` and `TopicReferences`—and, as such, would not tripped if the only updates beyond `LastModified` or `LastModifiedBy` were in the association tables. This resolves that issue, as well as #88.

…se/5.1.0

Previously, the `DeleteOrphanedLastModifiedAttributes` only accounted for `LastModified`, not `LastModifiedBy`—and, as such, would not get tripped if both the `LastModified` _and_ `LastModifiedBy` attributes were updated at the same time, even if no other attributes were updated. Further, it didn't account for `TopicReferences` or `Relationships`—which, as of OnTopic 5.0.0, are also versioned. As a result, this would yield a false positive if the `LastModified` and `LastModifiedBy` attributes accompanied a change that only otherwise affected topic associations. The resolves #88.