Merge branch 'bugfix/RelationshipTopicCollection.RemoveTopic-IncomingRelationships' into develop

When adding a relationship via `SetTopic()`, a reciprocal relationship is created on the target (related) topic via its `IncomingRelationships` collection. This wasn't happening when `RemoveTopic()` is called. This could result in stale content and even bugs if a relationship was removed (e.g., via the editor) and the view related to a target (related) topic displayed content from the `IncomingRelationships` collection. To fix this, `RemoveTopic()` now removes the reciprocal relationship in `IncomingRelationships` as well.

While I was at it, I also centralized the code between the two `RemoveTopic()` overloads, added additional commenting for readability, and added a couple of unit tests to help validate both the new and existing functionality.

Author: JeremyCaney

OnTopic.Tests/RelatedTopicCollectionTest.cs

@@ -56,7 +56,47 @@ public void SetTopic_IsDirty() {
     }
 
     /*==========================================================================================================================
-    | TEST: SET TOPIC: CREATES INCOMING RELATIONSHIP
+    | TEST: REMOVE TOPIC: REMOVES RELATIONSHIP
+    \-------------------------------------------------------------------------------------------------------------------------*/
+    /// <summary>
+    ///   Sets a relationship and then removes it by key, and confirms that it is removed.
+    /// </summary>
+    [TestMethod]
+    public void RemoveTopic_RemovesRelationship() {
+
+      var parent                = TopicFactory.Create("Parent", "Page");
+      var related               = TopicFactory.Create("Related", "Page");
+
+      parent.Relationships.SetTopic("Friends", related);
+      parent.Relationships.RemoveTopic("Friends", related.Key);
+
+      Assert.IsNull(parent.Relationships.GetTopics("Friends").FirstOrDefault());
+
+    }
+
+    /*==========================================================================================================================
+    | TEST: REMOVE TOPIC: REMOVES INCOMING RELATIONSHIP
+    \-------------------------------------------------------------------------------------------------------------------------*/
+    /// <summary>
+    ///   Sets a relationship and then removes it by key, and confirms that it is removed from the incoming relationships
+    ///   property.
+    /// </summary>
+    [TestMethod]
+    public void RemoveTopic_RemovesIncomingRelationship() {
+
+      var parent                = TopicFactory.Create("Parent", "Page");
+      var related               = TopicFactory.Create("Related", "Page");
+      var relationships         = new RelatedTopicCollection(parent);
+
+      relationships.SetTopic("Friends", related);
+      relationships.RemoveTopic("Friends", related.Key);
+
+      Assert.IsNull(related.IncomingRelationships.GetTopics("Friends").FirstOrDefault());
+
+    }
+
+    /*==========================================================================================================================
+    | TEST: REMOVE TOPIC: REMOVES INCOMING RELATIONSHIP
     \-------------------------------------------------------------------------------------------------------------------------*/
     /// <summary>
     ///   Sets a relationship and confirms that it is accessible on incoming relationships property.

OnTopic/Collections/RelatedTopicCollection.cs

@@ -131,37 +131,91 @@ public void ClearTopics(string relationshipKey) {
     /// </summary>
     /// <param name="relationshipKey">The key of the relationship.</param>
     /// <param name="topicKey">The key of the topic to be removed.</param>
+    /// <param name="isIncoming">
+    ///   Notes that this is setting an internal relationship, and thus shouldn't set the reciprocal relationship.
+    /// </param>
     /// <returns>
     ///   Returns true if the <see cref="Topic"/> is removed; returns false if either the relationship key or the
     ///   <see cref="Topic"/> cannot be found.
     /// </returns>
-    public bool RemoveTopic(string relationshipKey, string topicKey) {
+    public bool RemoveTopic(string relationshipKey, string topicKey, bool isIncoming = false) {
+
+      /*------------------------------------------------------------------------------------------------------------------------
+      | Validate contracts
+      \-----------------------------------------------------------------------------------------------------------------------*/
       Contract.Requires<ArgumentNullException>(!String.IsNullOrWhiteSpace(relationshipKey));
       Contract.Requires<ArgumentNullException>(!String.IsNullOrWhiteSpace(topicKey));
-      if (Contains(relationshipKey)) {
-        var topics = this[relationshipKey];
-        return topics.Remove(topicKey);
+
+      /*------------------------------------------------------------------------------------------------------------------------
+      | Validate topic key
+      \-----------------------------------------------------------------------------------------------------------------------*/
+      var topics                = Contains(relationshipKey)? this[relationshipKey] : null;
+      var topic                 = topics?.Contains(topicKey)?? false? topics[topicKey] : null;
+
+      if (topics == null || topic == null) {
+        return false;
       }
-      return false;
+
+      /*------------------------------------------------------------------------------------------------------------------------
+      | Call overload
+      \-----------------------------------------------------------------------------------------------------------------------*/
+      return RemoveTopic(relationshipKey, topic, isIncoming);
+
     }
 
     /// <summary>
     ///   Removes a specific <see cref="Topic"/> object associated with a specific relationship key.
     /// </summary>
     /// <param name="relationshipKey">The key of the relationship.</param>
     /// <param name="topic">The topic to be removed.</param>
+    /// <param name="isIncoming">
+    ///   Notes that this is setting an internal relationship, and thus shouldn't set the reciprocal relationship.
+    /// </param>
     /// <returns>
     ///   Returns true if the <see cref="Topic"/> is removed; returns false if either the relationship key or the
     ///   <see cref="Topic"/> cannot be found.
     /// </returns>
-    public bool RemoveTopic(string relationshipKey, Topic topic) {
+    public bool RemoveTopic(string relationshipKey, Topic topic, bool isIncoming = false) {
+
+      /*------------------------------------------------------------------------------------------------------------------------
+      | Validate contracts
+      \-----------------------------------------------------------------------------------------------------------------------*/
       Contract.Requires<ArgumentNullException>(!String.IsNullOrWhiteSpace(relationshipKey));
       Contract.Requires(topic);
-      if (Contains(relationshipKey)) {
-        var topics = this[relationshipKey];
-        return topics.Remove(topic);
+
+      /*------------------------------------------------------------------------------------------------------------------------
+      | Remove reciprocal relationship, if appropriate
+      \-----------------------------------------------------------------------------------------------------------------------*/
+      if (!isIncoming) {
+        if (_isIncoming) {
+          throw new ArgumentException(
+            "You are attempting to remove an incoming relationship on a RelatedTopicCollection that is not flagged as " +
+            "IsIncoming",
+            nameof(isIncoming)
+          );
+        }
+        topic.IncomingRelationships.RemoveTopic(relationshipKey, _parent, true);
+      }
+
+      /*------------------------------------------------------------------------------------------------------------------------
+      | Validate relationshipKey
+      \-----------------------------------------------------------------------------------------------------------------------*/
+      var topics                = Contains(relationshipKey)? this[relationshipKey] : null;
+
+      if (topics == null || !topics.Contains(topic)) {
+        return false;
       }
-      return false;
+
+      /*------------------------------------------------------------------------------------------------------------------------
+      | Remove relationship
+      \-----------------------------------------------------------------------------------------------------------------------*/
+      topics.Remove(topic);
+
+      /*------------------------------------------------------------------------------------------------------------------------
+      | Remove true
+      \-----------------------------------------------------------------------------------------------------------------------*/
+      return true;
+
     }
 
     /*==========================================================================================================================