Merge branch 'feature/TopicRepositoryBase-Improvements' into develop

Improved handling of the GetContentTypes() cache in `TopicRepositoryBase`

In a previous update, I made significant updates to how the `GetContentTypeDescriptors()` are cached, in order to help ensure the current configuration is always being assessed when editing or saving topics via e.g. the OnTopic Editor (fb3366e). Unfortunately, while these updates were sufficient for handling most `Import()` scenarios needed for bootstrapping a new database (695664d), they were not enough to allow for live reconfiguration in most real-world scenarios. That is, after a change is made to the Oroborus Configuration (e.g., via `Root:Configuration:ContentTypes`), the application must be restarted before that change will take effect; it doesn't happen immediately.

This was largely due to the fact that the `CachedTopicRepository`, which most sites use for performance reasons, operates off of a decorator pattern. This makes it easy for it to operate cache _any_ `ITopicRepository` instance, not just e.g. `SqlTopicRepository`. But, in doing so, it also makes it difficult for it to interact with and share caching data with the underlying `TopicRepositoryBase` used by e.g. `SqlTopicRepository`. That's notable since `TopicRepositoryBase` maintains its own cache of `ContentTypes`. As such, when those are updated via e.g. the OnTopic Editor, they are updated in the `CacheTopicRepository` topic graph, but not necessarily the `SqlTopicRepository`'s underlying `TopicRepositoryBase._contentTypeDescriptors` cache.

As such, while the previous updates (fb3366e) added features for importing missing `AttributeDescriptor`s and `ContentTypeDescriptor`s from the topic graph, and even resetting aspects of the local cache, such as the `ContentTypeDescriptor.AttributeDescriptors` collection, when updating a `ContentTypeDescriptor` or `AttributeDescriptor`, this didn't do much good if the versions of the objects in the `TopicRepositoryBase._contentTypeDescriptors` cache were not necessarily the same as those in the `CachedtopicRepository` cache. In those cases, having e.g. the inheritance structure of `AttributeDescriptors` refreshed wouldn't necessarily pick up new results.

 To mitigate this, significant changes have been made to help ensure that the objects being cached in the `TopicRepositoryBase` and, thus, `SqlTopicRepository` are, in fact, the same ones being referenced by `CachedTopicRepository`. To support this, the following changes were made:
- Introduced a new `ContentTypeDescriptorCollection.Refresh()` method (3e3930f) and corresponding constructor overload (60d4d8b) which allows the cache to be updated based on a root `ContentTypeDescriptor` (b206f3e)
- Configured `GetContentTypeDescriptors()` (946ddb9) and `GetContentTypeDescriptors(ContentTypeDescriptor)` (afbfa47), and even `Delete()` (afe7b70) to use the new `Refresh()` method, thus simplifying and centralizing their logic
- Renamed `GetContentTypeDescriptors(ContentTypeDescriptor)` to `SetContentTypeDescriptors()` (f1ade10, 78a9d3f, c75bbe2) so it's clearer that this will update the cache based on the parameter
- Fixed a bug in `TopicRepositoryBase.Move()` which prevented `ResetAttributeDescriptors()` from being called when a topic was moved (5941385, d7a61cd, b2f27e8, 23f4cb1); this additionally depends on a bug fix in `OnTopic.Editor.AspNetCore`
- Initialize the `SetContentTypeDescriptors()` cache on the initial call to `TopicRepositoryBase.Load()`; in most scenarios, this will be the warming of the `CachedTopicRepository` cache, thus ensuring the cache is shared between both e.g. `SqlTopicRepository` and `CachedTopicRepository` (dcdbab2)
- Introduced a new `ContentTypeDescriptor.ResetPermittedContentTypes()` method (9a7ef01) so that the cached collection of permitted `ContentTypeDescriptor`s is updated whenever a `ContentTypeDescriptor`'s `Relationships` are updated

In addition, while I was at it, I performed a number of other cleanup tasks mostly related to the `TopicRepositoryBase` class:
- Updated the XML Docs to reflect previous extensions to `AttributeValueCollection.SetValue()` (8aea8e7, 10c7c09) and `RelatedTopicCollection.SetTopic()` (27391f5, 2f1e2bf)
- Removed redundant update to cache in `SqlTopicRepository.Save()` (df7e6d2)
- Expanded explanation for `ResetAttributeDescriptors()` method (3f952a6)
- Implemented preferred pattern-matching syntax (d175f22)
- Removed unnecessary `[NotNull]` attribute on base classes to avoid confusion with Roslyn's static flow analysis (2210b29)
- Fixed unit test issues which covered up previous bugs (f980ef8, 6c081d8)

Author: JeremyCaney

OnTopic.Data.Sql/SqlTopicRepository.cs

@@ -173,6 +173,16 @@ public override Topic Load(int topicId, bool isRecursive = true) {
         }
       }
 
+      /*------------------------------------------------------------------------------------------------------------------------
+      | Establish content type cache
+      >-------------------------------------------------------------------------------------------------------------------------
+      | If this load represents the entire topic graph, then relay the content type configuration to the TopicRepositoryBase in
+      | order to either update or establish the content type cache. Not only does this prevent the need for a separate redundant
+      | call later but, even more importantly, it helps ensure the same object references are maintained so that any updates to
+      | subsequently cached content types are available.
+      \-----------------------------------------------------------------------------------------------------------------------*/
+      base.SetContentTypeDescriptors(topic);
+
       /*------------------------------------------------------------------------------------------------------------------------
       | Return objects
       \-----------------------------------------------------------------------------------------------------------------------*/

OnTopic.TestDoubles/StubTopicRepository.cs

@@ -180,8 +180,16 @@ public IEnumerable<AttributeValue> GetAttributesProxy(
     | METHOD: GET CONTENT TYPE DESCRIPTORS (PROXY)
     \-------------------------------------------------------------------------------------------------------------------------*/
     /// <inheritdoc cref="TopicRepositoryBase.GetContentTypeDescriptors(ContentTypeDescriptor)" />
+    [Obsolete("Deprecated. Instead, use the new SetContentTypeDescriptorsProxy(), which provides the same function.", false)]
     public ContentTypeDescriptorCollection GetContentTypeDescriptorsProxy(ContentTypeDescriptor topicGraph) =>
-      base.GetContentTypeDescriptors(topicGraph);
+      base.SetContentTypeDescriptors(topicGraph);
+
+    /*==========================================================================================================================
+    | METHOD: SET CONTENT TYPE DESCRIPTORS (PROXY)
+    \-------------------------------------------------------------------------------------------------------------------------*/
+    /// <inheritdoc cref="TopicRepositoryBase.SetContentTypeDescriptors(ContentTypeDescriptor)" />
+    public ContentTypeDescriptorCollection SetContentTypeDescriptorsProxy(ContentTypeDescriptor topicGraph) =>
+      base.SetContentTypeDescriptors(topicGraph);
 
     /*==========================================================================================================================
     | METHOD: GET CONTENT TYPE DESCRIPTOR (PROXY)

OnTopic.Tests/TopicRepositoryBaseTest.cs

@@ -321,7 +321,7 @@ public void GetContentTypeDescriptors_WithTopicGraph_ReturnsMergedContentTypes()
       var newContentType        = TopicFactory.Create("NewContentType", "ContentTypeDescriptor", rootContentType);
       var contentTypeCount      = contentTypes.Count;
 
-      _topicRepository.GetContentTypeDescriptorsProxy((ContentTypeDescriptor)newContentType);
+      _topicRepository.SetContentTypeDescriptorsProxy(rootContentType);
 
       Assert.AreNotEqual<int>(contentTypeCount, contentTypes.Count);
       Assert.IsNotNull(contentTypes.Contains(newContentType));
@@ -380,6 +380,30 @@ public void Save_ContentTypeDescriptor_UpdatesContentTypeCache() {
 
     }
 
+    /*==========================================================================================================================
+    | TEST: SAVE: CONTENT TYPE DESCRIPTOR: UPDATES PERMITTED CONTENT TYPES
+    \-------------------------------------------------------------------------------------------------------------------------*/
+    /// <summary>
+    ///   Loads the <see cref="TopicRepositoryBase.GetContentTypeDescriptors()"/>, then saves an existing <see cref=
+    ///   "ContentTypeDescriptor"/> via <see cref="TopicRepositoryBase.Save(Topic, Boolean, Boolean)"/>, and ensures that
+    ///   it the <see cref="ContentTypeDescriptor.PermittedContentTypes"/> cache is updated.
+    /// </summary>
+    [TestMethod]
+    public void Save_ContentTypeDescriptor_UpdatesPermittedContentTypes() {
+
+      var contentTypes          = _topicRepository.GetContentTypeDescriptors();
+      var pageContentType       = contentTypes.GetTopic("Page");
+      var lookupContentType     = contentTypes.GetTopic("Lookup");
+      var initialCount          = pageContentType.PermittedContentTypes.Count;
+
+      pageContentType.Relationships.SetTopic("ContentTypes", lookupContentType);
+
+      _topicRepository.Save(pageContentType);
+
+      Assert.AreNotEqual<int>(initialCount, pageContentType.PermittedContentTypes.Count);
+
+    }
+
     /*==========================================================================================================================
     | TEST: DELETE: CONTENT TYPE DESCRIPTOR: UPDATES CONTENT TYPE CACHE
     \-------------------------------------------------------------------------------------------------------------------------*/
@@ -418,7 +442,7 @@ public void Move_ContentTypeDescriptor_UpdatesContentTypeCache() {
 
       _topicRepository.Move(contactContentType, pageContentType);
 
-      Assert.IsFalse(contactContentType?.AttributeDescriptors.Count > contactAttributeCount);
+      Assert.AreNotEqual<int?>(contactContentType?.AttributeDescriptors.Count, contactAttributeCount);
 
     }
 

OnTopic/Attributes/AttributeSetterAttribute.cs

@@ -13,15 +13,16 @@ namespace OnTopic.Attributes {
   \---------------------------------------------------------------------------------------------------------------------------*/
   /// <summary>
   ///   Flags that a property should be used when setting an attribute via
-  ///   <see cref="AttributeValueCollection.SetValue(String, String, Boolean?, DateTime?)"/>.
+  ///   <see cref="AttributeValueCollection.SetValue(String, String?, Boolean?, DateTime?, Boolean?)"/>.
   /// </summary>
   /// <remarks>
   ///   <para>
-  ///     When a call is made to <see cref="AttributeValueCollection.SetValue(String, String, Boolean?, DateTime?)"/>, the code
-  ///     will check to see if a property with the same name as the attribute key exists, and whether that property is decorated
-  ///     with the <see cref="AttributeSetterAttribute"/> (i.e., <code>[AttributeSetter]</code>). If it is, then the update will
-  ///     be routed through that property. This ensures that business logic is enforced by local properties, instead of allowing
-  ///     business logic to be potentially bypassed by writing directly to the <see cref="Topic.Attributes"/> collection.
+  ///     When a call is made to <see cref="AttributeValueCollection.SetValue(String, String, Boolean?, DateTime?, Boolean?)"/>,
+  ///     the code will check to see if a property with the same name as the attribute key exists, and whether that property is
+  ///     decorated with the <see cref="AttributeSetterAttribute"/> (i.e., <code>[AttributeSetter]</code>). If it is, then the
+  ///     update will be routed through that property. This ensures that business logic is enforced by local properties, instead
+  ///     of allowing business logic to be potentially bypassed by writing directly to the <see cref="Topic.Attributes"/>
+  ///     collection.
   ///   </para>
   ///   <para>
   ///     As an example, the <see cref="Topic.Key"/> property is adorned with the <see cref="AttributeSetterAttribute"/>. As a
@@ -33,11 +34,12 @@ namespace OnTopic.Attributes {
   ///   </para>
   ///   <para>
   ///     To ensure this logic, it is critical that implementers of <see cref="AttributeSetterAttribute"/> ensure that the
-  ///     property setters call <see cref="AttributeValueCollection.SetValue(String, String, Boolean?, Boolean, DateTime?)"/>
-  ///     overload with the final parameter set to false to disable the enforcement of business logic. Otherwise, an infinite
-  ///     loop will occur. Calling that overload tells <see cref="AttributeValueCollection"/> that the business logic has
-  ///     already been enforced by the caller. As this is an internal overload, implementers should use the local proxy at
-  ///     <see cref="Topic.SetAttributeValue(String, String, Boolean?)"/>, which ensures that final parameter is set to false.
+  ///     property setters call <see cref="AttributeValueCollection.SetValue(String, String?, Boolean?, Boolean, DateTime?,
+  ///     Boolean?)"/> overload with the final parameter set to false to disable the enforcement of business logic. Otherwise,
+  ///     an infinite loop will occur. Calling that overload tells <see cref="AttributeValueCollection"/> that the business
+  ///     logic has already been enforced by the caller. As this is an internal overload, implementers should use the local
+  ///     proxy at <see cref="Topic.SetAttributeValue(String, String, Boolean?)"/>, which ensures that final parameter is set to
+  ///     false.
   ///   </para>
   /// </remarks>
   [AttributeUsage(AttributeTargets.Property)]

OnTopic/Metadata/ContentTypeDescriptor.cs

@@ -127,7 +127,7 @@ public bool DisableChildTopics {
     ///   </para>
     ///   <para>
     ///     To add content types to the <see cref="PermittedContentTypes"/> collection, use <see
-    ///     cref="RelatedTopicCollection.SetTopic(String, Topic, Boolean)"/>.
+    ///     cref="RelatedTopicCollection.SetTopic(String, Topic, Boolean, Boolean?)"/>.
     ///   </para>
     /// </remarks>
     public ReadOnlyTopicCollection<ContentTypeDescriptor> PermittedContentTypes {
@@ -238,6 +238,21 @@ internal void ResetAttributeDescriptors() {
       }
     }
 
+    /*==========================================================================================================================
+    | METHOD: RESET PERMITTED CONTENT TYPES
+    \-------------------------------------------------------------------------------------------------------------------------*/
+    /// <summary>
+    ///   Resets the list of <see cref="ContentTypeDescriptor"/>s stored in the <see cref="PermittedContentTypes"/> collection
+    ///   on this <see cref="ContentTypeDescriptor"/>.
+    /// </summary>
+    /// <remarks>
+    ///   Each <see cref="ContentTypeDescriptor"/> has a <see cref="PermittedContentTypes"/> collection which includes the <see
+    ///   cref="ContentTypeDescriptor"/>s associated with that <see cref="ContentTypeDescriptor"/>. As this is cached, however,
+    ///   the value should be reset whenever a <see cref="ContentTypeDescriptor"/> has been modified, in case those values have
+    ///   changed.
+    /// </remarks>
+    internal void ResetPermittedContentTypes() => _permittedContentTypes = null;
+
     /*==========================================================================================================================
     | METHOD: IS TYPE OF?
     \-------------------------------------------------------------------------------------------------------------------------*/

OnTopic/Metadata/ContentTypeDescriptorCollection.cs

@@ -3,7 +3,12 @@
 | Client        Ignia, LLC
 | Project       Topics Library
 \=============================================================================================================================*/
+using System;
+using System.Diagnostics.Contracts;
+using System.Linq;
 using OnTopic.Collections;
+using OnTopic.Querying;
+using OnTopic.Repositories;
 
 namespace OnTopic.Metadata {
 
@@ -13,6 +18,13 @@ namespace OnTopic.Metadata {
   /// <summary>
   ///   Represents a collection of <see cref="ContentTypeDescriptor"/> objects.
   /// </summary>
+  /// <remarks>
+  ///   While the <see cref="ContentTypeDescriptorCollection"/> can be used to store any collection of <see cref=
+  ///   "ContentTypeDescriptor"/> objects, it has additional tooling in the form of the <see cref="Refresh(
+  ///   ContentTypeDescriptor?)"/> as well as the <see cref="ContentTypeDescriptorCollection(ContentTypeDescriptor?)"/>
+  ///   constructor for handling a flattened list of <see cref="ContentTypeDescriptor"/>s used for the <see cref=
+  ///   "ITopicRepository.GetContentTypeDescriptors()"/> method.
+  /// </remarks>
   public class ContentTypeDescriptorCollection : TopicCollection<ContentTypeDescriptor> {
 
     /*==========================================================================================================================
@@ -24,5 +36,54 @@ public class ContentTypeDescriptorCollection : TopicCollection<ContentTypeDescri
     public ContentTypeDescriptorCollection() : base(null) {
     }
 
+    /// <summary>
+    ///   Initializes a new instance of the <see cref="ContentTypeDescriptorCollection"/> class based on a root <see cref=
+    ///   "ContentTypeDescriptor"/>.
+    /// </summary>
+    /// <param name="rootContentType">The <see cref="ContentTypeDescriptor"/> from which to initialize the collection.</param>
+    public ContentTypeDescriptorCollection(ContentTypeDescriptor? rootContentType) : base(null) {
+      Refresh(rootContentType);
+    }
+
+    /*==========================================================================================================================
+    | REFRESH
+    \-------------------------------------------------------------------------------------------------------------------------*/
+    /// <summary>
+    ///   Updates the current instance of the <see cref="ContentTypeDescriptorCollection"/> based on a new root <see cref=
+    ///   "ContentTypeDescriptor"/>.
+    /// </summary>
+    /// <param name="rootContentType">The <see cref="ContentTypeDescriptor"/> from which to initialize the collection.</param>
+    public void Refresh(ContentTypeDescriptor? rootContentType) {
+
+      /*------------------------------------------------------------------------------------------------------------------------
+      | Validate parameters
+      \-----------------------------------------------------------------------------------------------------------------------*/
+      if (rootContentType == null || rootContentType.Children.Count == 0) {
+        return;
+      }
+
+      Contract.Requires(
+        rootContentType.Key.Equals("ContentTypes", StringComparison.OrdinalIgnoreCase),
+        $"The {nameof(rootContentType)} is expected to represent the root of the content type topic graph, with a " +
+        $"key of 'ContentTypes'. Instead, the supplied ContentTypeDescriptor has a key of '{rootContentType.Key}'."
+      );
+
+      /*------------------------------------------------------------------------------------------------------------------------
+      | Clear any existing values
+      \-----------------------------------------------------------------------------------------------------------------------*/
+      Clear();
+
+      /*------------------------------------------------------------------------------------------------------------------------
+      | Add all ContentTypeDescriptors to collection
+      \-----------------------------------------------------------------------------------------------------------------------*/
+      var contentTypeDescriptors = rootContentType
+        .FindAll(t => typeof(ContentTypeDescriptor).IsAssignableFrom(t.GetType()))
+        .Cast<ContentTypeDescriptor>();
+      foreach (var contentType in contentTypeDescriptors) {
+        Add((ContentTypeDescriptor)contentType);
+      }
+
+    }
+
   } //Class
 } //Namespace