Skip to content

Commit 2486ab2

Browse files
JeremyCaneyJeremyCaney
committed
Merge branch 'feature/Topic.BaseTopic' into develop
One of the key properties off of `Topic` is `DerivedTopic`, which points to another topic entity from which the current topic should inherit attribute values, if they're not defined locally. So if I request `topic.Attributes.GetValue("Title")`, it will first search `topic.Attributes`, but then search `topic.DerivedTopic.Attributes`, and so on. The name of this property—and its underlying `AttributeDescriptor` and `ReferenceKey`—is confusing. It's not pointing to a _derived_ topic, but an _inherited_ topic. Unfortunately, the semantics of _inherited_, while more defensible, remain ambiguous, with many sources using the term e.g. "derived class" to refer to the derived class, while others use it to refer to the base class. To mitigate this, I've adopted the term `BaseTopic`. This is similar to Microsoft's popular convention of exposing a property named `Base` to refer to _object_ inheritance. But by adding `Topic` to it, I hope to better distinguish between the local `base` keyword, which refers to _class_ inheritance. I'm not sure that the name alone does that, but it at least makes it more obvious when reading derived classes that `base` and `BaseTopic` are different identifiers, whereas `base` and `Base` could easily be confused. This is an invasive change. In addition to the widespread references to `Topic.DerivedTopic`, the attribute descriptor named `DerivedTopic`, and the `TopicReferences` records with a `ReferenceKey` set to `DerivedTopic`, there are also parameters and variables related to the concept—such as `inheritFromDerived`—plus a lot of comments that allude to the relationship—while other cases of "derived" refer instead to class inheritance, and should be persisted. I've made a careful effort to distinguish and disambiguate these, and hopefully have caught all references.
2 parents ab4253d + 29cce45 commit 2486ab2

21 files changed

+157
-139
lines changed

‎OnTopic.Data.Sql.Database/Scripts/Upgrade from OnTopic 3 to OnTopic 4.sql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@ COLUMN AttributeID
2222
-- INHERIT TYPES
2323
--------------------------------------------------------------------------------------------------------------------------------
2424
-- Attribute Descriptors will be converted to more specific content types based on their legacy attribute type. Attribute types
25-
-- may be inherited from derived topics, however. This script identifies any cases where the attribute type is inherited, and
26-
-- updates the target topic with the derived value. This may need to be run multiple times if there are multiple layers of
25+
-- may be inherited from base topics, however. This script identifies any cases where the attribute type is inherited, and
26+
-- updates the target topic with the inherited value. This may need to be run multiple times if there are multiple layers of
2727
-- inheritance (e.g., an attribute derives from an attribute which derives from another attribute).
2828
--------------------------------------------------------------------------------------------------------------------------------
2929

‎OnTopic.Data.Sql.Database/Scripts/Upgrade from OnTopic 4 to OnTopic 5.sql

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ IN ( 'Key',
9393
--------------------------------------------------------------------------------------------------------------------------------
9494
-- MIGRATE TOPIC REFERENCES
9595
--------------------------------------------------------------------------------------------------------------------------------
96-
-- In OnTopic 5, references to other topics—such as `DerivedTopic`—have been moved from the Attributes table to a new
96+
-- In OnTopic 5, references to other topics—such as `BaseTopic`—have been moved from the Attributes table to a new
9797
-- TopicReferences table, where they act more like relationships. This allows referential integrity to be enforced through
9898
-- foreign key constraints, and formalizes the relationship so we don't need to rely on hacks in e.g. the Topic Data Transer
9999
-- service to infer which attributes represent relationships in order to translate their values from `TopicID` to `UniqueKey`.
@@ -118,10 +118,24 @@ WHERE AttributeKey LIKE '%ID'
118118
AND ISNUMERIC(AttributeValue) = 1
119119
AND Topics.TopicID IS NOT NULL
120120

121+
--------------------------------------------------------------------------------------------------------------------------------
122+
-- MIGRATE DERIVED TOPICS
123+
--------------------------------------------------------------------------------------------------------------------------------
124+
-- The above migration to topic references includes the DerivedTopic. To better clarify the purpose and intent of that
125+
-- relationship, we're renaming the attribute from 'DerivedTopic' to 'BaseTopic', and the actual storage field from 'Topic(ID)'
126+
-- to 'BaseTopic'. This is not only a more accurate identifier, but also unifies the label between the attribute descriptor
127+
-- and how its 'ReferenceKey'.
128+
--------------------------------------------------------------------------------------------------------------------------------
129+
121130
UPDATE TopicReferences
122-
SET ReferenceKey = 'DerivedTopic'
131+
SET ReferenceKey = 'BaseTopic'
123132
WHERE ReferenceKey = 'Topic'
124133

134+
UPDATE Topics
135+
SET TopicKey = 'BaseTopic'
136+
WHERE TopicKey = 'DerivedTopic'
137+
AND ContentType = 'TopicReferenceAttributeDescriptor'
138+
125139
--------------------------------------------------------------------------------------------------------------------------------
126140
-- MIGRATE ATTRIBUTE KEYS
127141
--------------------------------------------------------------------------------------------------------------------------------

‎OnTopic.Data.Sql/SqlDataReaderExtensions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ internal static class SqlDataReaderExtensions {
5454
/// </param>
5555
/// <param name="includeExternalReferences">
5656
/// Optionally disables populating external references such as <see cref="Topic.Relationships"/> and <see
57-
/// cref="Topic.DerivedTopic"/>. This is useful for cases where it's known that a shallow copy is being retrieved, and
57+
/// cref="Topic.BaseTopic"/>. This is useful for cases where it's known that a shallow copy is being retrieved, and
5858
/// thus external references aren't likely to be available.
5959
/// </param>
6060
internal static Topic? LoadTopicGraph(

‎OnTopic.TestDoubles/StubTopicRepository.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ private Topic CreateFakeData() {
197197
addAttribute(contentTypes, "Key", "TextAttributeDescriptor", false, true);
198198
addAttribute(contentTypes, "ContentType", "TextAttributeDescriptor", false, true);
199199
addAttribute(contentTypes, "Title", "TextAttributeDescriptor", true, true);
200-
addAttribute(contentTypes, "DerivedTopic", "TopicReferenceAttributeDescriptor", false);
200+
addAttribute(contentTypes, "BaseTopic", "TopicReferenceAttributeDescriptor", false);
201201

202202
var contentTypeDescriptor = TopicFactory.Create("ContentTypeDescriptor", "ContentTypeDescriptor", contentTypes);
203203

‎OnTopic.Tests/AttributeValueCollectionTest.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -712,19 +712,19 @@ public void GetValue_InheritFromParent_ReturnsParentValue() {
712712
}
713713

714714
/*==========================================================================================================================
715-
| TEST: GET VALUE: INHERIT FROM DERIVED: RETURNS DERIVED VALUE
715+
| TEST: GET VALUE: INHERIT FROM BASE: RETURNS INHERITED VALUE
716716
\-------------------------------------------------------------------------------------------------------------------------*/
717717
/// <summary>
718-
/// Establishes a long tree of derives topics, and ensures that the derived value is returned.
718+
/// Establishes a long tree of derived topics, and ensures that the inherited value is returned.
719719
/// </summary>
720720
[TestMethod]
721-
public void GetValue_InheritFromDerived_ReturnsDerivedValue() {
721+
public void GetValue_InheritFromBase_ReturnsInheritedValue() {
722722

723723
var topics = new Topic[5];
724724

725725
for (var i = 0; i <= 4; i++) {
726726
var topic = TopicFactory.Create("Topic" + i, "Container");
727-
if (i > 0) topics[i - 1].DerivedTopic = topic;
727+
if (i > 0) topics[i - 1].BaseTopic = topic;
728728
topics[i] = topic;
729729
}
730730

@@ -747,7 +747,7 @@ public void GetValue_ExceedsMaxHops_ReturnsDefault() {
747747

748748
for (var i = 0; i <= 7; i++) {
749749
var topic = TopicFactory.Create("Topic" + i, "Container");
750-
if (i > 0) topics[i - 1].DerivedTopic = topic;
750+
if (i > 0) topics[i - 1].BaseTopic = topic;
751751
topics[i] = topic;
752752
}
753753

‎OnTopic.Tests/BindingModels/InvalidReferenceTypeTopicBindingModel.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public class InvalidReferenceTypeTopicBindingModel : BasicTopicBindingModel {
2424

2525
public InvalidReferenceTypeTopicBindingModel(string? key = null) : base(key, "Page") { }
2626

27-
public TopicViewModel DerivedTopic { get; } = new();
27+
public TopicViewModel BaseTopic { get; } = new();
2828

2929
} //Class
3030
} //Namespace

‎OnTopic.Tests/BindingModels/ReferenceTopicBindingModel.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public class ReferenceTopicBindingModel : BasicTopicBindingModel {
2222

2323
public ReferenceTopicBindingModel(string key) : base(key, "TopicReferenceAttributeDescriptor") { }
2424

25-
public RelatedTopicBindingModel? DerivedTopic { get; set; }
25+
public RelatedTopicBindingModel? BaseTopic { get; set; }
2626

2727
} //Class
2828
} //Namespace

‎OnTopic.Tests/ReverseTopicMappingServiceTest.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -290,15 +290,15 @@ public async Task Map_TopicReferences_ReturnsMappedTopic() {
290290
var mappingService = new ReverseTopicMappingService(_topicRepository);
291291

292292
var bindingModel = new ReferenceTopicBindingModel("Test") {
293-
DerivedTopic = new() {
293+
BaseTopic = new() {
294294
UniqueKey = _topicRepository.Load("Root:Configuration:ContentTypes:Attributes:Title").GetUniqueKey()
295295
}
296296
};
297297

298298
var target = (TopicReferenceAttributeDescriptor?)await mappingService.MapAsync(bindingModel).ConfigureAwait(false);
299299

300-
Assert.IsNotNull(target.DerivedTopic);
301-
Assert.AreEqual<string>("Title", target.DerivedTopic.Key);
300+
Assert.IsNotNull(target.BaseTopic);
301+
Assert.AreEqual<string>("Title", target.BaseTopic.Key);
302302
Assert.AreEqual<string>("TopicReference", target.EditorType);
303303

304304
}

‎OnTopic.Tests/TopicReferenceDictionaryTest.cs

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -253,68 +253,68 @@ public void GetTopic_MissingReference_ReturnsNull() {
253253
}
254254

255255
/*==========================================================================================================================
256-
| TEST: GET TOPIC: DERIVED REFERENCE: RETURNS TOPIC
256+
| TEST: GET TOPIC: INHERITED REFERENCE: RETURNS TOPIC
257257
\-------------------------------------------------------------------------------------------------------------------------*/
258258
/// <summary>
259-
/// Assembles a new <see cref="TopicReferenceDictionary"/> with a <see cref="Topic.DerivedTopic"/>, adds a new <see
260-
/// cref="Topic"/> reference to the <see cref="Topic.DerivedTopic"/>, and confirms that <see cref=
259+
/// Assembles a new <see cref="TopicReferenceDictionary"/> with a <see cref="Topic.BaseTopic"/>, adds a new <see
260+
/// cref="Topic"/> reference to the <see cref="Topic.BaseTopic"/>, and confirms that <see cref=
261261
/// "TopicReferenceDictionary.GetTopic(String, Boolean)"/> correctly returns the related topic reference.
262262
/// </summary>
263263
[TestMethod]
264-
public void GetTopic_DerivedReference_ReturnsTopic() {
264+
public void GetTopic_InheritedReference_ReturnsTopic() {
265265

266266
var topic = TopicFactory.Create("Topic", "Page");
267-
var derived = TopicFactory.Create("Derived", "Page");
267+
var baseTopic = TopicFactory.Create("Base", "Page");
268268
var reference = TopicFactory.Create("Reference", "Page");
269269

270-
topic.DerivedTopic = derived;
271-
derived.References.Add("Reference", reference);
270+
topic.BaseTopic = baseTopic;
271+
baseTopic.References.Add("Reference", reference);
272272

273273
Assert.AreEqual(reference, topic.References.GetTopic("Reference"));
274274

275275
}
276276

277277
/*==========================================================================================================================
278-
| TEST: GET TOPIC: DERIVED REFERENCE: RETURNS NULL
278+
| TEST: GET TOPIC: INHERITED REFERENCE: RETURNS NULL
279279
\-------------------------------------------------------------------------------------------------------------------------*/
280280
/// <summary>
281-
/// Assembles a new <see cref="TopicReferenceDictionary"/> with a <see cref="Topic.DerivedTopic"/>, adds a new <see
282-
/// cref="Topic"/> reference to the <see cref="Topic.DerivedTopic"/>, and confirms that <see cref=
281+
/// Assembles a new <see cref="TopicReferenceDictionary"/> with a <see cref="Topic.BaseTopic"/>, adds a new <see
282+
/// cref="Topic"/> reference to the <see cref="Topic.BaseTopic"/>, and confirms that <see cref=
283283
/// "TopicReferenceDictionary.GetTopic(String, Boolean)"/> correctly returns <c>null</c> if an incorrect <c>referencedKey
284284
/// </c> is entered.
285285
/// </summary>
286286
[TestMethod]
287-
public void GetTopic_DerivedReference_ReturnsNull() {
287+
public void GetTopic_InheritedReference_ReturnsNull() {
288288

289289
var topic = TopicFactory.Create("Topic", "Page");
290-
var derived = TopicFactory.Create("Derived", "Page");
290+
var baseTopic = TopicFactory.Create("Base", "Page");
291291
var reference = TopicFactory.Create("Reference", "Page");
292292

293-
topic.DerivedTopic = derived;
294-
derived.References.Add("Reference", reference);
293+
topic.BaseTopic = baseTopic;
294+
baseTopic.References.Add("Reference", reference);
295295

296296
Assert.IsNull(topic.References.GetTopic("MissingReference"));
297297

298298
}
299299

300300
/*==========================================================================================================================
301-
| TEST: GET TOPIC: DERIVED REFERENCE WITHOUT INHERIT: RETURNS NULL
301+
| TEST: GET TOPIC: INHERITED REFERENCE WITHOUT INHERITANCE: RETURNS NULL
302302
\-------------------------------------------------------------------------------------------------------------------------*/
303303
/// <summary>
304-
/// Assembles a new <see cref="TopicReferenceDictionary"/> with a <see cref="Topic.DerivedTopic"/>, adds a new <see
305-
/// cref="Topic"/> reference to the <see cref="Topic.DerivedTopic"/>, and confirms that <see cref=
306-
/// "TopicReferenceDictionary.GetTopic(String, Boolean)"/> correctly returns <c>null</c> if <c>inheritFromDerived</c> is
304+
/// Assembles a new <see cref="TopicReferenceDictionary"/> with a <see cref="Topic.BaseTopic"/>, adds a new <see
305+
/// cref="Topic"/> reference to the <see cref="Topic.BaseTopic"/>, and confirms that <see cref=
306+
/// "TopicReferenceDictionary.GetTopic(String, Boolean)"/> correctly returns <c>null</c> if <c>inheritFromBase</c> is
307307
/// set to <c>false</c>.
308308
/// </summary>
309309
[TestMethod]
310-
public void GetTopic_DerivedReferenceWithoutInherit_ReturnsNull() {
310+
public void GetTopic_InheritedReferenceWithoutInheritance_ReturnsNull() {
311311

312312
var topic = TopicFactory.Create("Topic", "Page");
313-
var derived = TopicFactory.Create("Derived", "Page");
313+
var baseTopic = TopicFactory.Create("Base", "Page");
314314
var reference = TopicFactory.Create("Reference", "Page");
315315

316-
topic.DerivedTopic = derived;
317-
derived.References.Add("Reference", reference);
316+
topic.BaseTopic = baseTopic;
317+
baseTopic.References.Add("Reference", reference);
318318

319319
Assert.IsNull(topic.References.GetTopic("Reference", false));
320320

‎OnTopic.Tests/TopicRepositoryBaseTest.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,21 +50,21 @@ public TopicRepositoryBaseTest() {
5050
}
5151

5252
/*==========================================================================================================================
53-
| TEST: DELETE: DERIVED TOPIC: THROWS EXCEPTION
53+
| TEST: DELETE: BASE TOPIC: THROWS EXCEPTION
5454
\-------------------------------------------------------------------------------------------------------------------------*/
5555
/// <summary>
5656
/// Deletes a topic which other topics, outside of the graph, derive from. Expects exception.
5757
/// </summary>
5858
[TestMethod]
5959
[ExpectedException(typeof(ReferentialIntegrityException))]
60-
public void Delete_DerivedTopic_ThrowsException() {
60+
public void Delete_BaseTopic_ThrowsException() {
6161

6262
var root = TopicFactory.Create("Root", "Page");
6363
var topic = TopicFactory.Create("Topic", "Page", root);
6464
var child = TopicFactory.Create("Child", "Page", topic);
65-
var derived = TopicFactory.Create("Derived", "Page", root);
65+
var derivedTopic = TopicFactory.Create("Derived", "Page", root);
6666

67-
derived.DerivedTopic = child;
67+
derivedTopic.BaseTopic = child;
6868

6969
_topicRepository.Delete(topic, true);
7070

@@ -84,7 +84,7 @@ public void Delete_InternallyDerivedTopic_Succeeds() {
8484
var child = TopicFactory.Create("Child", "Page", topic);
8585
var derived = TopicFactory.Create("Derived", "Page", topic);
8686

87-
derived.DerivedTopic = child;
87+
derived.BaseTopic = child;
8888

8989
_topicRepository.Delete(topic, true);
9090

‎OnTopic.Tests/TopicTest.cs

Lines changed: 24 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ public void Create_ReturnsTopic() {
3939
| TEST: CREATE: CONTENT TYPE: RETURNS DERIVED TOPIC
4040
\-------------------------------------------------------------------------------------------------------------------------*/
4141
/// <summary>
42-
/// Creates a topic of a content type which has been derived, and ensures the derived version of <see cref="Topic"/> is
43-
/// returned.
42+
/// Creates a topic of a content type which maps to a class derived from <see cref="Topic"/>, and ensures the derived
43+
/// version of the <see cref="Topic"/> class is returned.
4444
/// </summary>
4545
[TestMethod]
4646
public void Create_ContentType_ReturnsDerivedTopic() {
@@ -271,49 +271,48 @@ public void LastModified_UpdateValue_ReturnsExpectedValue() {
271271
}
272272

273273
/*==========================================================================================================================
274-
| TEST: DERIVED TOPIC: UPDATE VALUE: RETURNS EXPECTED VALUE
274+
| TEST: BASE TOPIC: UPDATE VALUE: RETURNS EXPECTED VALUE
275275
\-------------------------------------------------------------------------------------------------------------------------*/
276276
/// <summary>
277-
/// Sets a derived topic to a topic entity, then replaces the references with a new topic entity. Ensures that both the
278-
/// derived topic as well as the underlying <see cref="AttributeValue"/> correctly reference the new value.
277+
/// Sets a base topic to a topic entity, then replaces the references with a new topic entity. Ensures that both the
278+
/// base topic as well as the underlying <see cref="AttributeValue"/> correctly reference the new value.
279279
/// </summary>
280280
[TestMethod]
281-
public void DerivedTopic_UpdateValue_ReturnsExpectedValue() {
281+
public void BaseTopic_UpdateValue_ReturnsExpectedValue() {
282282

283283
var topic = TopicFactory.Create("Topic", "Page");
284-
var firstDerivedTopic = TopicFactory.Create("DerivedTopic", "Page");
285-
var secondDerivedTopic = TopicFactory.Create("DerivedTopic", "Page", 1);
286-
var finalDerivedTopic = TopicFactory.Create("DerivedTopic", "Page", 2);
284+
var firstBaseTopic = TopicFactory.Create("BaseTopic", "Page");
285+
var secondBaseTopic = TopicFactory.Create("BaseTopic", "Page", 1);
286+
var finalBaseTopic = TopicFactory.Create("BaseTopic", "Page", 2);
287287

288-
topic.DerivedTopic = firstDerivedTopic;
289-
topic.DerivedTopic = secondDerivedTopic;
290-
topic.DerivedTopic = finalDerivedTopic;
288+
topic.BaseTopic = firstBaseTopic;
289+
topic.BaseTopic = secondBaseTopic;
290+
topic.BaseTopic = finalBaseTopic;
291291

292-
Assert.ReferenceEquals(topic.DerivedTopic, finalDerivedTopic);
293-
Assert.AreEqual<int>(2, topic.References.GetTopic("DerivedTopic").Id);
292+
Assert.ReferenceEquals(topic.BaseTopic, finalBaseTopic);
293+
Assert.AreEqual<int>(2, topic.References.GetTopic("BaseTopic").Id);
294294

295295
}
296296

297297
/*==========================================================================================================================
298-
| TEST: DERIVED TOPIC: RESAVED VALUE: RETURNS EXPECTED VALUE
298+
| TEST: BASE TOPIC: RESAVED VALUE: RETURNS EXPECTED VALUE
299299
\-------------------------------------------------------------------------------------------------------------------------*/
300300
/// <summary>
301-
/// Sets a derived topic to an unsaved topic entity, then saves the entity and reestablishes the relationship. Ensures
302-
/// that the derived topic is correctly set, including the <see cref="Topic.Id"/> as an underlying <see
303-
/// cref="AttributeValue"/>.
301+
/// Sets a base topic to an unsaved topic entity, then saves the entity and reestablishes the relationship. Ensures
302+
/// that the base topic is correctly set as a <see cref="Topic.References"/> entry.
304303
/// </summary>
305304
[TestMethod]
306-
public void DerivedTopic_ResavedValue_ReturnsExpectedValue() {
305+
public void BaseTopic_ResavedValue_ReturnsExpectedValue() {
307306

308307
var topic = TopicFactory.Create("Topic", "Page");
309-
var derivedTopic = TopicFactory.Create("DerivedTopic", "Page");
308+
var baseTopic = TopicFactory.Create("BaseTopic", "Page");
310309

311-
topic.DerivedTopic = derivedTopic;
312-
derivedTopic.Id = 5;
313-
topic.DerivedTopic = derivedTopic;
310+
topic.BaseTopic = baseTopic;
311+
baseTopic.Id = 5;
312+
topic.BaseTopic = baseTopic;
314313

315-
Assert.ReferenceEquals(topic.DerivedTopic, derivedTopic);
316-
Assert.AreEqual<int>(5, topic.References.GetTopic("DerivedTopic").Id);
314+
Assert.ReferenceEquals(topic.BaseTopic, baseTopic);
315+
Assert.AreEqual<int>(5, topic.References.GetTopic("BaseTopic").Id);
317316

318317
}
319318

0 commit comments

Comments
 (0)