Don't persist unsaved Topic.DerivedTopic

Previously, when the `Topic.DerivedTopic` was set, the underlying `Topic.Attributes`'s `TopicID` attribute was also set—even if the `DerivedTopic.Id` was `-1` (i.e., meaning it was unsaved). This provides no value to the persistence store since that `Topic.Id` cannot be used to rehydrate the topic graph.

This update prevents this from happening by putting in a check that the `DerivedTopic.Id` is a positive integer (i.e., has been saved). Of course, the source topic will need to be saved once the `DerivedTopic` is saved to ensure that the relationship is persisted.

This brings this functionality in line with how relationships work.

Includes unit tests for validating the behavior.

Author: JeremyCaney

OnTopic.Tests/TopicTest.cs

@@ -9,6 +9,7 @@
 using OnTopic.Metadata;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 using OnTopic.Collections;
+using OnTopic.Attributes;
 
 namespace OnTopic.Tests {
 
@@ -278,21 +279,71 @@ public void LastModified_UpdateValue_ReturnsExpectedValue() {
       Assert.AreEqual<DateTime>(lastModified, topic.LastModified);
 
     }
+
     /*==========================================================================================================================
     | TEST: DERIVED TOPIC: UPDATE VALUE: RETURNS EXPECTED VALUE
     \-------------------------------------------------------------------------------------------------------------------------*/
     /// <summary>
-    ///   Sets a derived topic, and ensures it is referenced correctly.
+    ///   Sets a derived topic to a topic entity, then replaces the references with a new topic entity. Ensures that both the
+    ///   derived topic as well as the underlying <see cref="AttributeValue"/> correctly reference the new value.
     /// </summary>
     [TestMethod]
     public void DerivedTopic_UpdateValue_ReturnsExpectedValue() {
 
+      var topic                 = TopicFactory.Create("Topic", "Page");
+      var firstDerivedTopic     = TopicFactory.Create("DerivedTopic", "Page");
+      var secondDerivedTopic    = TopicFactory.Create("DerivedTopic", "Page", 1);
+      var finalDerivedTopic     = TopicFactory.Create("DerivedTopic", "Page", 2);
+
+      topic.DerivedTopic        = firstDerivedTopic;
+      topic.DerivedTopic        = secondDerivedTopic;
+      topic.DerivedTopic        = finalDerivedTopic;
+
+      Assert.ReferenceEquals(topic.DerivedTopic, finalDerivedTopic);
+      Assert.AreEqual<int>(2, topic.Attributes.GetInteger("TopicID", 0));
+
+    }
+
+    /*==========================================================================================================================
+    | TEST: DERIVED TOPIC: UNSAVED VALUE: RETURNS EXPECTED VALUE
+    \-------------------------------------------------------------------------------------------------------------------------*/
+    /// <summary>
+    ///   Sets a derived topic to an unsaved topic entity. Ensures that the derived topic is correctly set, but that the <see
+    ///   cref="Topic.Id"/> is not persisted as an underlying <see cref="AttributeValue"/>.
+    /// </summary>
+    [TestMethod]
+    public void DerivedTopic_UnsavedValue_ReturnsExpectedValue() {
+
       var topic                 = TopicFactory.Create("Topic", "Page");
       var derivedTopic          = TopicFactory.Create("DerivedTopic", "Page");
 
       topic.DerivedTopic        = derivedTopic;
 
       Assert.ReferenceEquals(topic.DerivedTopic, derivedTopic);
+      Assert.AreEqual<int>(-2, topic.Attributes.GetInteger("TopicID", -2));
+
+    }
+
+    /*==========================================================================================================================
+    | TEST: DERIVED TOPIC: RESAVED VALUE: RETURNS EXPECTED VALUE
+    \-------------------------------------------------------------------------------------------------------------------------*/
+    /// <summary>
+    ///   Sets a derived topic to an unsaved topic entity, then saves the entity and reestablishes the relationship. Ensures
+    ///   that the derived topic is correctly set, including the <see cref="Topic.Id"/> as an underlying <see
+    ///   cref="AttributeValue"/>.
+    /// </summary>
+    [TestMethod]
+    public void DerivedTopic_ResavedValue_ReturnsExpectedValue() {
+
+      var topic                 = TopicFactory.Create("Topic", "Page");
+      var derivedTopic          = TopicFactory.Create("DerivedTopic", "Page");
+
+      topic.DerivedTopic        = derivedTopic;
+      derivedTopic.Id           = 5;
+      topic.DerivedTopic        = derivedTopic;
+
+      Assert.ReferenceEquals(topic.DerivedTopic, derivedTopic);
+      Assert.AreEqual<int>(5, topic.Attributes.GetInteger("TopicID", -2));
 
     }
 

OnTopic/Topic.cs

@@ -537,6 +537,16 @@ public string GetWebPath() {
     ///     Be aware that while multiple levels of derived topics can be configured, the <see
     ///     cref="AttributeValueCollection.GetValue(String, Boolean)" /> method defaults to a maximum level of five "hops".
     ///   </para>
+    ///   <para>
+    ///     The underlying value of the <see cref="DerivedTopic"/> is stored as the <c>TopicID</c> <see cref="AttributeValue"/>.
+    ///     If the <see cref="Topic"/> hasn't been saved, then the relationship will be established, but the <c>TopicID</c>
+    ///     won't be persisted to the underlying repository upon <see cref="Repositories.ITopicRepository.Save"/>. That said,
+    ///     when <see cref="Repositories.TopicRepositoryBase.Save"/> is called, the <see cref="DerivedTopic"/> will be
+    ///     reevaluated and, if it has subsequently been saved, then the <c>TopicID</c> will be updated accordingly. This allows
+    ///     in-memory topic graphs to be constructed, while preventing invalid <see cref="Topic.Id"/>s from being persisted to
+    ///     the underlying data storage. As a result, however, a <see cref="Topic"/> referencing a <see cref="DerivedTopic"/>
+    ///     that is unsaved will need to be saved again once the <see cref="DerivedTopic"/> has been saved.
+    ///   </para>
     /// </remarks>
     /// <value>The <see cref="Topic"/> that values should be derived from, if not otherwise available.</value>
     /// <requires description="A topic key must not derive from itself." exception="T:System.ArgumentException">
@@ -549,11 +559,8 @@ public Topic? DerivedTopic {
           value != this,
           "A topic may not derive from itself."
         );
-        if (!_derivedTopic?.Equals(value)?? false) {
-          return;
-        }
         _derivedTopic = value;
-        if (value != null) {
+        if (value != null && value.Id > 0) {
           SetAttributeValue("TopicID", value.Id.ToString(CultureInfo.InvariantCulture));
         }
         else {