Merge branch 'bugfix/PermittedContentTypes-tests' into develop

To better reflect real-world conditions, the `StubTopicRepository` was set to treat all topics as "saved"—i.e., by assigning them an `Id`. Most of the topics already had an `Id` set, but not the `ContentTypeDescriptors`. This exposed two bugs.

The first bug was simply an issue with the unit test; the `TopicMappingService` cached view models based on their `Topic.Id`, and so `IsNew` topics aren't cached; when these were migrated to "saved" topics, a circular reference in the test case was exposed which failed to adequately test associations of associations (#82).

The second bug fix was due to an underlying problem in the `TopicRepository.Save()`. This test confirmed that the `PermittedContentTypes` were refreshed after a `ContentTypeDescriptor` with updated relationships was saved. The problem is that it evaluated if the relationships were dirty _after_ the topic had been saved—and marked as clean. As such, the relationships should _never_ be dirty. This issue was obscured by the `IsNew` content types because a relationship cannot be marked clean if it still references `IsNew` topics, since in a real-world scenario, only relationships to saved topics can be persisted. This is easily fixed by evaluating the `Topic.Relationships.IsDirty()` state prior to marking the collection as saved. This resolved #81.

Author: JeremyCaney

OnTopic.TestDoubles/StubTopicRepository.cs

@@ -234,21 +234,21 @@ private Topic CreateFakeData() {
       /*------------------------------------------------------------------------------------------------------------------------
       | Establish root
       \-----------------------------------------------------------------------------------------------------------------------*/
-      var rootTopic = new Topic("Root", "Container", null, 900);
       var currentAttributeId = 800;
+      var rootTopic = new Topic("Root", "Container", null, currentAttributeId++);
 
       /*------------------------------------------------------------------------------------------------------------------------
       | Establish configuration
       \-----------------------------------------------------------------------------------------------------------------------*/
-      var configuration = new Topic("Configuration", "Container", rootTopic);
-      var contentTypes = new ContentTypeDescriptor("ContentTypes", "ContentTypeDescriptor", configuration);
+      var configuration = new Topic("Configuration", "Container", rootTopic, currentAttributeId++);
+      var contentTypes = new ContentTypeDescriptor("ContentTypes", "ContentTypeDescriptor", configuration, currentAttributeId++);
 
       addAttribute(contentTypes, "Key", "TextAttributeDescriptor", false, true);
       addAttribute(contentTypes, "ContentType", "TextAttributeDescriptor", false, true);
       addAttribute(contentTypes, "Title", "TextAttributeDescriptor", true, true);
       addAttribute(contentTypes, "BaseTopic", "TopicReferenceAttributeDescriptor", false);
 
-      var contentTypeDescriptor = new ContentTypeDescriptor("ContentTypeDescriptor", "ContentTypeDescriptor", contentTypes);
+      var contentTypeDescriptor = new ContentTypeDescriptor("ContentTypeDescriptor", "ContentTypeDescriptor", contentTypes, currentAttributeId++);
 
       addAttribute(contentTypeDescriptor, "ContentTypes", "RelationshipAttributeDescriptor");
       addAttribute(contentTypeDescriptor, "Attributes", "NestedTopicListAttributeDescriptor");
@@ -258,7 +258,7 @@ private Topic CreateFakeData() {
       TopicFactory.Create("LookupListItem", "ContentTypeDescriptor", contentTypes);
       TopicFactory.Create("List", "ContentTypeDescriptor", contentTypes);
 
-      var attributeDescriptor = new ContentTypeDescriptor("AttributeDescriptor", "ContentTypeDescriptor", contentTypes);
+      var attributeDescriptor = new ContentTypeDescriptor("AttributeDescriptor", "ContentTypeDescriptor", contentTypes, currentAttributeId++);
 
       addAttribute(attributeDescriptor, "DefaultValue", "TextAttributeDescriptor", false, true);
       addAttribute(attributeDescriptor, "IsRequired", "TextAttributeDescriptor", false, true);
@@ -270,22 +270,23 @@ private Topic CreateFakeData() {
       TopicFactory.Create("TextAttributeDescriptor", "ContentTypeDescriptor", attributeDescriptor);
       TopicFactory.Create("TopicReferenceAttributeDescriptor", "ContentTypeDescriptor", attributeDescriptor);
 
-      var pageContentType = new ContentTypeDescriptor("Page", "ContentTypeDescriptor", contentTypes);
+      var pageContentType = new ContentTypeDescriptor("Page", "ContentTypeDescriptor", contentTypes, currentAttributeId++);
 
       addAttribute(pageContentType, "MetaTitle");
       addAttribute(pageContentType, "MetaDescription");
       addAttribute(pageContentType, "IsHidden", "TextAttributeDescriptor", false);
       addAttribute(pageContentType, "TopicReference", "TopicReferenceAttributeDescriptor", false);
 
-      pageContentType.Relationships.SetValue("ContentTypes", pageContentType);
-      pageContentType.Relationships.SetValue("ContentTypes", contentTypeDescriptor);
-
-      var contactContentType = new ContentTypeDescriptor("Contact", "ContentTypeDescriptor", contentTypes);
+      var contactContentType = new ContentTypeDescriptor("Contact", "ContentTypeDescriptor", contentTypes, currentAttributeId++);
 
       addAttribute(contactContentType, "Name", isExtended: false);
       addAttribute(contactContentType, "AlternateEmail", isExtended: false);
       addAttribute(contactContentType, "BillingContactEmail", isExtended: false);
 
+      pageContentType.Relationships.SetValue("ContentTypes", pageContentType);
+      pageContentType.Relationships.SetValue("ContentTypes", contactContentType);
+      contactContentType.Relationships.SetValue("ContentTypes", pageContentType);
+
       /*------------------------------------------------------------------------------------------------------------------------
       | Local addAttribute() helper function
       \-----------------------------------------------------------------------------------------------------------------------*/
@@ -310,9 +311,9 @@ AttributeDescriptor addAttribute(
       /*------------------------------------------------------------------------------------------------------------------------
       | Establish metadata
       \-----------------------------------------------------------------------------------------------------------------------*/
-      var metadata = new Topic("Metadata", "Container", configuration);
-      var categories = new Topic("Categories", "Lookup", metadata);
-      var lookup = new Topic("LookupList", "List", categories);
+      var metadata              = new Topic("Metadata", "Container", configuration, currentAttributeId++);
+      var categories            = new Topic("Categories", "Lookup", metadata, currentAttributeId++);
+      var lookup                = new Topic("LookupList", "List", categories, currentAttributeId++);
 
       for (var i=1; i<=5; i++) {
         _ = new Topic("Category" + i, "LookupListItem", lookup);
@@ -325,10 +326,10 @@ AttributeDescriptor addAttribute(
 
       CreateFakeData(web, 2, 3);
 
-      var pageGroup             = new Topic("Web_3", "PageGroup", web);
-      _                         = new Topic("Web_3_0", "Page", pageGroup);
-      var childPage             = new Topic("Web_3_1", "Page", pageGroup);
-      var leafPage              = new Topic("Web_3_1_0", "Page", childPage);
+      var pageGroup             = new Topic("Web_3", "PageGroup", web, currentAttributeId++);
+      _                         = new Topic("Web_3_0", "Page", pageGroup, currentAttributeId++);
+      var childPage             = new Topic("Web_3_1", "Page", pageGroup, currentAttributeId++);
+      var leafPage              = new Topic("Web_3_1_0", "Page", childPage, currentAttributeId++);
 
       leafPage.Attributes.SetValue("NavigationRoot", "Configuration");
 

OnTopic.Tests/TopicMappingServiceTest.cs

@@ -680,14 +680,16 @@ public async Task Map_AlternateRelationship_ReturnsCorrectRelationship() {
     [Fact]
     public async Task Map_CustomCollection_ReturnsCollection() {
 
-      var topic                 = _topicRepository.Load("Root:Configuration:ContentTypes:Page");
+      var topic                 = (ContentTypeDescriptor)_topicRepository.Load("Root:Configuration:ContentTypes:Page");
       var target                = await _mappingService.MapAsync<ContentTypeDescriptorTopicViewModel>(topic).ConfigureAwait(false);
 
       Assert.Equal<int?>(8, target?.AttributeDescriptors.Count);
       Assert.Equal<int?>(2, target?.PermittedContentTypes.Count);
 
       //Ensure custom collections are not recursively followed without instruction
-      Assert.Empty(target?.PermittedContentTypes.FirstOrDefault()?.PermittedContentTypes);
+      Assert.NotEqual<Topic?>(topic, topic?.PermittedContentTypes.LastOrDefault());
+      Assert.NotEmpty(topic?.PermittedContentTypes.LastOrDefault()?.PermittedContentTypes);
+      Assert.Empty(target?.PermittedContentTypes.LastOrDefault()?.PermittedContentTypes);
 
     }
 

OnTopic/Repositories/TopicRepository.cs

@@ -327,6 +327,7 @@ private void Save([NotNull]Topic topic, bool isRecursive, TopicCollection unreso
       | Establish variables
       \-----------------------------------------------------------------------------------------------------------------------*/
       var isNew                 = topic.IsNew;
+      var areRelationshipsDirty = topic.Relationships.IsDirty();
 
       /*------------------------------------------------------------------------------------------------------------------------
       | Validate content type
@@ -405,7 +406,7 @@ _contentTypeDescriptors is not null &&
       /*------------------------------------------------------------------------------------------------------------------------
       | If content type, and relationships have been updated, refresh permitted content types
       \-----------------------------------------------------------------------------------------------------------------------*/
-      if (asContentType is not null && asContentType.Relationships.IsDirty()) {
+      if (asContentType is not null && areRelationshipsDirty) {
         asContentType.ResetPermittedContentTypes();
       }