Skip to content

Commit 852a33e

Browse files
committed
Fixed bug when querying root
When we introduced the `Topic.GetTopicByUniqueKey()` extension method (f2cb109), we significantly optimized the implementation relative to the previous, analogous implementation in `CachedTopicRepository.GetTopic()` (867b110). In doing so, however, we also introduced an inadvertent loophole: It fails to return the unique topic is the unique topic is the root topic. This is easily remedied by checking this condition at the top of the `GetTopicByUniqueKey()` method. In addition, to be safe, I introduced two unit tests for this scenario. One is for the extension itself (as part of `TopicQueryingTest`). The other is for another extension method, `TopicRepositoryExtensions.Load()` (via `TopicRepositoryExtensionsTest`) which currently relies on that extension method. (In practice, these two unit tests are evaluating the same underlying logic and are, thus, redundant. But in the past we've modified the underlying logic used by `TopicRepositoryExtensions.Load()`, and so it's useful to evaluate this at that level as well.)
1 parent aa63b4a commit 852a33e

3 files changed

Lines changed: 48 additions & 0 deletions

File tree

OnTopic.AspNetCore.Mvc.Tests/TopicRepositoryExtensionsTest.cs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,5 +65,27 @@ public void Load_ByRoute_ReturnsTopic() {
6565

6666
}
6767

68+
/*==========================================================================================================================
69+
| TEST: LOAD: BY ROUTE: RETURNS ROOT TOPIC
70+
\-------------------------------------------------------------------------------------------------------------------------*/
71+
/// <summary>
72+
/// Establishes route data and ensures that the root topic is correctly identified based on that route.
73+
/// </summary>
74+
[TestMethod]
75+
public void Load_ByRoute_ReturnsRootTopic() {
76+
77+
var routes = new RouteData();
78+
var topic = _topicRepository.Load("Root");
79+
80+
routes.Values.Add("path", "Root/");
81+
82+
var currentTopic = _topicRepository.Load(routes);
83+
84+
Assert.IsNotNull(currentTopic);
85+
Assert.ReferenceEquals(topic, currentTopic);
86+
Assert.AreEqual<string>("Root", currentTopic.Key);
87+
88+
}
89+
6890
} //Class
6991
} //Namespace

OnTopic.Tests/TopicQueryingTest.cs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,25 @@ public void GetRootTopic_ReturnsRootTopic() {
6464

6565
}
6666

67+
/*==========================================================================================================================
68+
| TEST: GET BY UNIQUE KEY: ROOT KEY: RETURNS ROOT TOPIC
69+
\-------------------------------------------------------------------------------------------------------------------------*/
70+
/// <summary>
71+
/// Given a root <see cref="Topic"/>, returns the root <see cref="Topic"/>.
72+
/// </summary>
73+
[TestMethod]
74+
public void GetByUniqueKey_RootKey_ReturnsRootTopic() {
75+
76+
var parentTopic = TopicFactory.Create("ParentTopic", "Page", 1);
77+
_ = TopicFactory.Create("ChildTopic", "Page", 2, parentTopic);
78+
79+
var foundTopic = parentTopic.GetByUniqueKey("ParentTopic");
80+
81+
Assert.IsNotNull(foundTopic);
82+
Assert.ReferenceEquals(parentTopic, foundTopic);
83+
84+
}
85+
6786
/*==========================================================================================================================
6887
| TEST: GET BY UNIQUE KEY: VALID KEY: RETURNS TOPIC
6988
\-------------------------------------------------------------------------------------------------------------------------*/

OnTopic/Querying/TopicExtensions.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,13 @@ public static Topic GetRootTopic(this Topic topic) {
193193
\-----------------------------------------------------------------------------------------------------------------------*/
194194
var currentTopic = (Topic?)topic.GetRootTopic();
195195

196+
/*------------------------------------------------------------------------------------------------------------------------
197+
| Handle request for root
198+
\-----------------------------------------------------------------------------------------------------------------------*/
199+
if (currentTopic!.Key.Equals(uniqueKey, StringComparison.InvariantCultureIgnoreCase)) {
200+
return currentTopic;
201+
}
202+
196203
/*------------------------------------------------------------------------------------------------------------------------
197204
| Process keys
198205
\-----------------------------------------------------------------------------------------------------------------------*/

0 commit comments

Comments
 (0)