Merge branch 'feature/empty-repository-support' into develop

Previously, while new topics could be imported into a repository using the new **OnTopic Data Transfer** library, which is exposed via the **OnTopic Editor**, there was no way to access these features when working with an empty repository. That is because there were a number of validation points that would throw an exception if e.g. the repository didn't return any topics, couldn't retrieve the configuration, couldn't find a content type, &c. As a result, there was no way to import a reference configuration to bootstrap the database. This made it difficult to stand up a new OnTopic database.

This commit resolves that by removing a lot of those safeguards. Exceptions are still thrown if a specific topic cannot be loaded, but there's a lot more tolerance when loading the entire topic graph, the configuration, or trying to resolve individual content types upon save. When combined with the recent introduction of support for dynamically loading content type and attribute descriptors from the in-memory topic graph (fb3366e), this allows an application, such as **OnTopic Editor**, to load an empty database, then import a reference JSON file with standard configuration topics, thus bootstrapping an OnTopic repository.

As part of this, there's also improved support for how topic references are handled. For example, neither `Topic.DerivedTopic` nor `Topic.Relationships` will persist if the target `Topic.Id` until it's been `Save()`d, thus preventing `-1` being written for references to unsaved topics. But when `Save()` is called, it also checks e.g. `DerivedTopic.Id` and, if it's set, updates the `TopicID` reference. This addresses a bug where meaningless `TopicID` references were persisted as `-1` to the repository—then not necessarily updated once those references were saved.

This also introduces a new `TopicNotFoundException` which derives from the existing `TopicRepositoryException` so that when an `ITopicRepository` implementation _does_ encounter a missing topic (graph) that merits an exception, it throws a more specific, meaningful exception. As this still derives from the traditional `TopicRepositoryException`, this shouldn't require any updates to downstream implementations—though this also fixed some scenarios where off-target, non-specific exceptions could be thrown by `ITopicRepository` implementations (e.g., a `NullReferenceException` when a topic couldn't be loaded). Technically, those are breaking changes, but as they're fixing exceptions that should never have been thrown, and which weren't documented, we're treating it as a bug fix.

Author: JeremyCaney

GitVersion.yml

@@ -1,5 +1,7 @@
 assembly-versioning-scheme: MajorMinorPatch
-mode: ContinuousDelivery
-branches: {}
+mode: ContinuousDeployment
+branches:
+  master:
+    mode: ContinuousDelivery
 ignore:
-  sha: []
+  sha: []

OnTopic.AspNetCore.Mvc.Tests/TopicRepositoryExtensionsTest.cs

@@ -65,5 +65,27 @@ public void Load_ByRoute_ReturnsTopic() {
 
     }
 
+    /*==========================================================================================================================
+    | TEST: LOAD: BY ROUTE: RETURNS ROOT TOPIC
+    \-------------------------------------------------------------------------------------------------------------------------*/
+    /// <summary>
+    ///   Establishes route data and ensures that the root topic is correctly identified based on that route.
+    /// </summary>
+    [TestMethod]
+    public void Load_ByRoute_ReturnsRootTopic() {
+
+      var routes                = new RouteData();
+      var topic                 = _topicRepository.Load("Root");
+
+      routes.Values.Add("path", "Root/");
+
+      var currentTopic          = _topicRepository.Load(routes);
+
+      Assert.IsNotNull(currentTopic);
+      Assert.ReferenceEquals(topic, currentTopic);
+      Assert.AreEqual<string>("Root", currentTopic.Key);
+
+    }
+
   } //Class
 } //Namespace

OnTopic.Data.Sql.Database/Functions/GetTopicID.sql

@@ -19,6 +19,7 @@ BEGIN
   -- DECLARE AND DEFINE VARIABLES
   ------------------------------------------------------------------------------------------------------------------------------
   DECLARE	@TopicID		INT
+  SET	@TopicID		= -1
 
   ------------------------------------------------------------------------------------------------------------------------------
   -- GET TOPIC ID BASED ON TOPIC KEY

OnTopic.Data.Sql/SqlCommandExtensions.cs

@@ -27,13 +27,17 @@ internal static class SqlCommandExtensions {
     ///   Retrieves the return code from a stored procedure.
     /// </summary>
     /// <param name="command">The SQL command object.</param>
+    /// <param name="sqlParameter">The name of the SQL parameter to retrieve as the return code.</param>
     internal static int GetReturnCode(this SqlCommand command, string sqlParameter = "ReturnCode") {
       Contract.Assume<InvalidOperationException>(
         command.Parameters.Contains($"@{sqlParameter}"),
         $"The call to the {command.CommandText} stored procedure did not return the expected 'ReturnCode' parameter."
       );
       var returnCode = command.Parameters[$"@{sqlParameter}"].Value.ToString();
-      return Int32.Parse(returnCode, CultureInfo.InvariantCulture);
+      if (Int32.TryParse(returnCode, NumberStyles.Integer, CultureInfo.InvariantCulture, out var returnValue)) {
+        return returnValue;
+      }
+      return -1;
     }
 
     /*==========================================================================================================================
@@ -43,9 +47,7 @@ internal static int GetReturnCode(this SqlCommand command, string sqlParameter =
     ///   Adds a SQL parameter to a command object, additionally setting the specified parameter direction.
     /// </summary>
     /// <param name="command">The SQL command object.</param>
-    /// <param name="sqlParameter">The SQL parameter.</param>
-    /// <param name="paramDirection">The SQL parameter's directional setting (input-only, output-only, etc.).</param>
-    /// <param name="sqlDbType">The SQL field data type.</param>
+    /// <param name="sqlParameter">The name of the SQL output parameter.</param>
     internal static void AddOutputParameter(this SqlCommand command, string sqlParameter = "ReturnCode") =>
       AddParameter(command, sqlParameter, null, SqlDbType.Int, ParameterDirection.ReturnValue);
 

OnTopic.Data.Sql/SqlTopicRepository.cs

@@ -80,7 +80,7 @@ public override Topic Load(string? topicKey = null, bool isRecursive = true) {
       \-----------------------------------------------------------------------------------------------------------------------*/
       var connection            = new SqlConnection(_connectionString);
       var command               = new SqlCommand("GetTopicID", connection);
-      int topicId;
+      var topicId               = -1;
 
       command.CommandType       = CommandType.StoredProcedure;
 
@@ -117,6 +117,13 @@ public override Topic Load(string? topicKey = null, bool isRecursive = true) {
         connection.Close();
       }
 
+      /*------------------------------------------------------------------------------------------------------------------------
+      | Validate results
+      \-----------------------------------------------------------------------------------------------------------------------*/
+      if (topicId < 0) {
+        throw new TopicNotFoundException(topicKey);
+      }
+
       /*------------------------------------------------------------------------------------------------------------------------
       | Return topic
       \-----------------------------------------------------------------------------------------------------------------------*/
@@ -173,7 +180,12 @@ public override Topic Load(int topicId, bool isRecursive = true) {
       | Validate results
       \-----------------------------------------------------------------------------------------------------------------------*/
       if (topic == null) {
-        throw new NullReferenceException($"Load() was unable to successfully load the topic with the TopicID '{topicId}'");
+        if (topicId == -1) {
+          topic = TopicFactory.Create("Root", "Container");
+        }
+        else {
+          throw new TopicNotFoundException(topicId);
+        }
       }
 
       /*------------------------------------------------------------------------------------------------------------------------
@@ -242,7 +254,7 @@ public override Topic Load(int topicId, DateTime version) {
       /*------------------------------------------------------------------------------------------------------------------------
       | Return objects
       \-----------------------------------------------------------------------------------------------------------------------*/
-      return topic?? throw new NullReferenceException("The specified Topic version could not be loaded");
+      return topic?? throw new TopicNotFoundException(topicId);
 
     }
 
@@ -257,18 +269,6 @@ public override int Save([NotNull]Topic topic, bool isRecursive = false, bool is
       \-----------------------------------------------------------------------------------------------------------------------*/
       base.Save(topic, isRecursive, isDraft);
 
-      /*------------------------------------------------------------------------------------------------------------------------
-      | Validate content type
-      \-----------------------------------------------------------------------------------------------------------------------*/
-      var contentTypes          = GetContentTypeDescriptors();
-      var contentType           = topic.Attributes.GetValue("ContentType", "Page")?? "Page";
-      var contentTypeDescriptor = contentTypes.GetTopic(contentType) as ContentTypeDescriptor;
-
-      Contract.Assume(
-        contentTypeDescriptor,
-        $"The Topics repository or database does not contain a ContentTypeDescriptor for the {contentType} content type."
-      );
-
       /*------------------------------------------------------------------------------------------------------------------------
       | Establish attribute containers with schema
       \-----------------------------------------------------------------------------------------------------------------------*/
@@ -596,7 +596,7 @@ private static string PersistRelations(Topic topic, SqlConnection connection, bo
             CommandType         = CommandType.StoredProcedure
           };
 
-          foreach (var targetTopicId in scope.Select<Topic, int>(m => m.Id)) {
+          foreach (var targetTopicId in scope.Where(t => t.Id > 0).Select<Topic, int>(m => m.Id)) {
             var record          = targetIds.NewRow();
             record["TopicID"]   = targetTopicId;
             targetIds.Rows.Add(record);

OnTopic.TestDoubles/StubTopicRepository.cs

@@ -97,7 +97,7 @@ public StubTopicRepository() : base() {
       /*------------------------------------------------------------------------------------------------------------------------
       | Return objects
       \-----------------------------------------------------------------------------------------------------------------------*/
-      return topic?? throw new NullReferenceException("The specified Topic version could not be loaded");
+      return topic?? throw new TopicNotFoundException(topicId);
 
     }
 

OnTopic.Tests/TopicQueryingTest.cs

@@ -64,6 +64,25 @@ public void GetRootTopic_ReturnsRootTopic() {
 
     }
 
+    /*==========================================================================================================================
+    | TEST: GET BY UNIQUE KEY: ROOT KEY: RETURNS ROOT TOPIC
+    \-------------------------------------------------------------------------------------------------------------------------*/
+    /// <summary>
+    ///   Given a root <see cref="Topic"/>, returns the root <see cref="Topic"/>.
+    /// </summary>
+    [TestMethod]
+    public void GetByUniqueKey_RootKey_ReturnsRootTopic() {
+
+      var parentTopic           = TopicFactory.Create("ParentTopic", "Page", 1);
+      _                         = TopicFactory.Create("ChildTopic", "Page", 2, parentTopic);
+
+      var foundTopic = parentTopic.GetByUniqueKey("ParentTopic");
+
+      Assert.IsNotNull(foundTopic);
+      Assert.ReferenceEquals(parentTopic, foundTopic);
+
+    }
+
     /*==========================================================================================================================
     | TEST: GET BY UNIQUE KEY: VALID KEY: RETURNS TOPIC
     \-------------------------------------------------------------------------------------------------------------------------*/

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/GlobalSuppressions.cs

@@ -3,5 +3,8 @@
 // Project-level suppressions either have no target or are given
 // a specific target and scoped to a namespace, type, member, etc.
 
-[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Globalization", "CA1307:Specify StringComparison", Justification = "Invalid overload; known bug in code analysis", Scope = "member", Target = "~P:OnTopic.Metadata.AttributeDescriptor.Configuration")]
-[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Globalization", "CA1307:Specify StringComparison", Justification = "Invalid overload; known bug in code analysis", Scope = "member", Target = "~M:OnTopic.Topic.GetWebPath~System.String")]
+using System.Diagnostics.CodeAnalysis;
+
+[assembly: SuppressMessage("Globalization", "CA1307:Specify StringComparison", Justification = "Invalid overload; known bug in code analysis", Scope = "member", Target = "~P:OnTopic.Metadata.AttributeDescriptor.Configuration")]
+[assembly: SuppressMessage("Globalization", "CA1307:Specify StringComparison", Justification = "Invalid overload; known bug in code analysis", Scope = "member", Target = "~M:OnTopic.Topic.GetWebPath~System.String")]
+[assembly: SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "False positive; apparent bug in code analysis", Scope = "member", Target = "~M:OnTopic.Repositories.TopicRepositoryBase.GetContentTypeDescriptors~OnTopic.Metadata.ContentTypeDescriptorCollection")]

OnTopic/Querying/TopicExtensions.cs

@@ -193,6 +193,13 @@ public static Topic GetRootTopic(this Topic topic) {
       \-----------------------------------------------------------------------------------------------------------------------*/
       var currentTopic          = (Topic?)topic.GetRootTopic();
 
+      /*------------------------------------------------------------------------------------------------------------------------
+      | Handle request for root
+      \-----------------------------------------------------------------------------------------------------------------------*/
+      if (currentTopic!.Key.Equals(uniqueKey, StringComparison.InvariantCultureIgnoreCase)) {
+        return currentTopic;
+      }
+
       /*------------------------------------------------------------------------------------------------------------------------
       | Process keys
       \-----------------------------------------------------------------------------------------------------------------------*/