Skip to content

Commit 34a8c52

Browse files
JeremyCaneyJeremyCaney
committed
Merge branch 'feature/GetValue-nomenclature' into develop
Our naming conventions were inconsistent across classes, with a mix of `GetValue()` and `GetTopic()`—or, related, e.g. `RemoveTopic()`, `ClearTopics()`. Historically, this is due to these collections originally being purpose built for e.g. topics, and then some being generalized to e.g., `TrackedRecordCollection<>`, which isn't specific to topics. That said, `GetValue()` is more consistent with e.g., `KeyedCollection<>`'s similar `TryGetValue()`, and offers consistency across interfaces and predictability in terms of nomenclature. It is true that in changing this we're losing clarity of the type of value being retrieved, but this is almost always clear in context. E.g., `topic.Children.GetValue()`, `topic.Relationships.GetValues()`, and topic.Attributes.GetValue()` come with contextual clues. This change includes renaming: - `(ReadOnly)TopicMultiMap.GetValues()` to `GetTopics()` (ad3e5dd) - `ReadOnlyTopicMultipMap.GetAllTopics()` to `GetAllValues()` (2f01591) - `TopicRelationshipMultiMap.RemoveTopic()` to `Remove()` (9f65271) - `TopicRelationshipMultiMap.SetTopic()` to `SetValue()` (b99a04e) - `(ReadOnly)KeyedTopicCollection.GetTopic()` to `GetValue()` (6674c7d) These are all breaking changes that interfere with the standard way of engaging with the OnTopic library via well-established interfaces. As a result, I've maintained deprecated versions of the original interfaces so that implementors receive clear guidance on how to mitigate the errors.
2 parents a5cbf97 + f28fab2 commit 34a8c52

24 files changed

Lines changed: 202 additions & 153 deletions

‎OnTopic.Data.Sql/SqlDataReaderExtensions.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -379,10 +379,10 @@ private static void SetRelationships(this IDataReader reader, TopicIndex topics,
379379
| Set relationship on object
380380
\-----------------------------------------------------------------------------------------------------------------------*/
381381
if (!isDeleted) {
382-
current.Relationships.SetTopic(relationshipKey, related, isDirty);
382+
current.Relationships.SetValue(relationshipKey, related, isDirty);
383383
}
384384
else if (current.Relationships.Contains(relationshipKey, related)) {
385-
current.Relationships.RemoveTopic(relationshipKey, related);
385+
current.Relationships.Remove(relationshipKey, related);
386386
}
387387

388388
}

‎OnTopic.Data.Sql/SqlTopicRepository.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ public override Topic Load(int topicId, DateTime version, Topic? referenceTopic
221221

222222
if (topic is not null) {
223223
foreach (var relationship in topic.Relationships) {
224-
topic.Relationships.ClearTopics(relationship.Key);
224+
topic.Relationships.Clear(relationship.Key);
225225
}
226226
topic.References.Clear();
227227
}
@@ -641,7 +641,7 @@ private static void PersistRelationships(Topic topic, DateTime version, SqlConne
641641
CommandType = CommandType.StoredProcedure
642642
};
643643

644-
foreach (var targetTopic in topic.Relationships.GetTopics(key)) {
644+
foreach (var targetTopic in topic.Relationships.GetValues(key)) {
645645
if (!targetTopic.IsNew) {
646646
targetIds.AddRow(targetTopic.Id);
647647
}

‎OnTopic.TestDoubles/StubTopicRepository.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -228,8 +228,8 @@ private Topic CreateFakeData() {
228228
addAttribute(pageContentType, "IsHidden", "TextAttributeDescriptor", false);
229229
addAttribute(pageContentType, "TopicReference", "TopicReferenceAttributeDescriptor", false);
230230

231-
pageContentType.Relationships.SetTopic("ContentTypes", pageContentType);
232-
pageContentType.Relationships.SetTopic("ContentTypes", contentTypeDescriptor);
231+
pageContentType.Relationships.SetValue("ContentTypes", pageContentType);
232+
pageContentType.Relationships.SetValue("ContentTypes", contentTypeDescriptor);
233233

234234
var contactContentType = TopicFactory.Create("Contact", "ContentTypeDescriptor", contentTypes);
235235

@@ -247,7 +247,7 @@ AttributeDescriptor addAttribute(
247247
bool isExtended = true,
248248
bool isRequired = false
249249
) {
250-
var container = contentType.Children.GetTopic("Attributes");
250+
var container = contentType.Children.GetValue("Attributes");
251251
if (container is null) {
252252
container = TopicFactory.Create("Attributes", "List", contentType);
253253
container.Attributes.SetBoolean("IsHidden", true);

‎OnTopic.Tests/ReverseTopicMappingServiceTest.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ public async Task Map_Relationships_ReturnsMappedTopic() {
223223
var contentTypes = _topicRepository.GetContentTypeDescriptors();
224224
var topic = (ContentTypeDescriptor)TopicFactory.Create("Test", "ContentTypeDescriptor");
225225

226-
topic.Relationships.SetTopic("ContentTypes", contentTypes[4]);
226+
topic.Relationships.SetValue("ContentTypes", contentTypes[4]);
227227

228228
for (var i = 0; i < 3; i++) {
229229
bindingModel.ContentTypes.Add(
@@ -270,11 +270,11 @@ public async Task Map_NestedTopics_ReturnsMappedTopic() {
270270
var target = (ContentTypeDescriptor?)await mappingService.MapAsync(bindingModel, topic).ConfigureAwait(false);
271271

272272
Assert.AreEqual<int>(3, target.AttributeDescriptors.Count);
273-
Assert.IsNotNull(target.AttributeDescriptors.GetTopic("Attribute1"));
274-
Assert.IsNotNull(target.AttributeDescriptors.GetTopic("Attribute2"));
275-
Assert.IsNotNull(target.AttributeDescriptors.GetTopic("Attribute3"));
276-
Assert.AreEqual<string>("New Value", target.AttributeDescriptors.GetTopic("Attribute3").DefaultValue);
277-
Assert.IsNull(target.AttributeDescriptors.GetTopic("Attribute4"));
273+
Assert.IsNotNull(target.AttributeDescriptors.GetValue("Attribute1"));
274+
Assert.IsNotNull(target.AttributeDescriptors.GetValue("Attribute2"));
275+
Assert.IsNotNull(target.AttributeDescriptors.GetValue("Attribute3"));
276+
Assert.AreEqual<string>("New Value", target.AttributeDescriptors.GetValue("Attribute3").DefaultValue);
277+
Assert.IsNull(target.AttributeDescriptors.GetValue("Attribute4"));
278278

279279
}
280280

‎OnTopic.Tests/SqlTopicRepositoryTest.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ public void LoadTopicGraph_WithRelationship_ReturnsRelationship() {
9696

9797
Assert.IsNotNull(topic);
9898
Assert.AreEqual<int>(1, topic.Id);
99-
Assert.AreEqual<int?>(2, topic.Relationships.GetTopics("Test").FirstOrDefault()?.Id);
99+
Assert.AreEqual<int?>(2, topic.Relationships.GetValues("Test").FirstOrDefault()?.Id);
100100

101101
}
102102

@@ -203,7 +203,7 @@ public void LoadTopicGraph_WithDeletedRelationship_RemovesRelationship() {
203203
var child = TopicFactory.Create("Child", "Container", topic, 2);
204204
var related = TopicFactory.Create("Related", "Container", topic, 3);
205205

206-
child.Relationships.SetTopic("Test", related);
206+
child.Relationships.SetValue("Test", related);
207207

208208
using var empty = new AttributesDataTable();
209209
using var relationships = new RelationshipsDataTable();
@@ -214,7 +214,7 @@ public void LoadTopicGraph_WithDeletedRelationship_RemovesRelationship() {
214214

215215
tableReader.LoadTopicGraph(related);
216216

217-
Assert.AreEqual<int>(0, topic.Relationships.GetTopics("Test").Count);
217+
Assert.AreEqual<int>(0, topic.Relationships.GetValues("Test").Count);
218218

219219
}
220220

‎OnTopic.Tests/TopicMappingServiceTest.cs‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -339,9 +339,9 @@ public async Task Map_Relationships_ReturnsMappedModel() {
339339
var relatedTopic3 = TopicFactory.Create("Sibling", "Relation");
340340
var topic = TopicFactory.Create("Test", "Relation");
341341

342-
topic.Relationships.SetTopic("Cousins", relatedTopic1);
343-
topic.Relationships.SetTopic("Cousins", relatedTopic2);
344-
topic.Relationships.SetTopic("Siblings", relatedTopic3);
342+
topic.Relationships.SetValue("Cousins", relatedTopic1);
343+
topic.Relationships.SetValue("Cousins", relatedTopic2);
344+
topic.Relationships.SetValue("Siblings", relatedTopic3);
345345

346346
var target = await _mappingService.MapAsync<RelationTopicViewModel>(topic).ConfigureAwait(false);
347347

@@ -365,8 +365,8 @@ public async Task Map_Relationships_SkipsDisabled() {
365365
var relatedTopic2 = TopicFactory.Create("Cousin2", "Relation");
366366
var topic = TopicFactory.Create("Test", "Relation");
367367

368-
topic.Relationships.SetTopic("Cousins", relatedTopic1);
369-
topic.Relationships.SetTopic("Cousins", relatedTopic2);
368+
topic.Relationships.SetValue("Cousins", relatedTopic1);
369+
topic.Relationships.SetValue("Cousins", relatedTopic2);
370370

371371
topic.IsDisabled = true;
372372
relatedTopic2.IsDisabled = true;
@@ -403,12 +403,12 @@ public async Task Map_AlternateRelationship_ReturnsCorrectRelationship() {
403403
var topic = TopicFactory.Create("Test", "AmbiguousRelation");
404404

405405
//Set outgoing relationships
406-
topic.Relationships.SetTopic("RelationshipAlias", ambiguousRelation);
407-
topic.Relationships.SetTopic("AmbiguousRelationship", outgoingRelation);
406+
topic.Relationships.SetValue("RelationshipAlias", ambiguousRelation);
407+
topic.Relationships.SetValue("AmbiguousRelationship", outgoingRelation);
408408

409409
//Set incoming relationships
410-
ambiguousRelation.Relationships.SetTopic("RelationshipAlias", topic);
411-
incomingRelation.Relationships.SetTopic("AmbiguousRelationship", topic);
410+
ambiguousRelation.Relationships.SetValue("RelationshipAlias", topic);
411+
incomingRelation.Relationships.SetValue("AmbiguousRelationship", topic);
412412

413413
var target = await _mappingService.MapAsync<AmbiguousRelationTopicViewModel>(topic).ConfigureAwait(false);
414414

@@ -653,12 +653,12 @@ public async Task Map_RecursiveRelationships_ReturnsGraph() {
653653
var cousinOnceRemoved = TopicFactory.Create("CousinOnceRemoved", "Relation", childTopic3);
654654

655655
//Set first cousins
656-
topic.Relationships.SetTopic("Cousins", cousinTopic1);
657-
topic.Relationships.SetTopic("Cousins", cousinTopic2);
658-
topic.Relationships.SetTopic("Cousins", cousinTopic3);
656+
topic.Relationships.SetValue("Cousins", cousinTopic1);
657+
topic.Relationships.SetValue("Cousins", cousinTopic2);
658+
topic.Relationships.SetValue("Cousins", cousinTopic3);
659659

660660
//Set ancillary relationships
661-
cousinTopic3.Relationships.SetTopic("Cousins", secondCousin);
661+
cousinTopic3.Relationships.SetValue("Cousins", secondCousin);
662662

663663
var target = await _mappingService.MapAsync<RelationTopicViewModel>(topic).ConfigureAwait(false);
664664

@@ -719,9 +719,9 @@ public async Task Map_TopicEntities_ReturnsTopics() {
719719
var relatedTopic3 = TopicFactory.Create("RelatedTopic3", "KeyOnly");
720720
var topic = TopicFactory.Create("Test", "RelatedEntity");
721721

722-
topic.Relationships.SetTopic("RelatedTopics", relatedTopic1);
723-
topic.Relationships.SetTopic("RelatedTopics", relatedTopic2);
724-
topic.Relationships.SetTopic("RelatedTopics", relatedTopic3);
722+
topic.Relationships.SetValue("RelatedTopics", relatedTopic1);
723+
topic.Relationships.SetValue("RelatedTopics", relatedTopic2);
724+
topic.Relationships.SetValue("RelatedTopics", relatedTopic3);
725725

726726
var target = await _mappingService.MapAsync<RelatedEntityTopicViewModel>(topic).ConfigureAwait(false);
727727
var relatedTopic3copy = (getRelatedTopic(target, "RelatedTopic3"));

‎OnTopic.Tests/TopicReferenceCollectionTest.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ public void Add_NewReference_IncomingRelationshipSet() {
148148

149149
topic.References.SetValue("Reference", reference);
150150

151-
Assert.AreEqual<int>(1, reference.IncomingRelationships.GetTopics("Reference").Count);
151+
Assert.AreEqual<int>(1, reference.IncomingRelationships.GetValues("Reference").Count);
152152

153153
}
154154

@@ -170,7 +170,7 @@ public void Remove_ExistingReference_IncomingRelationshipRemoved() {
170170
topic.References.SetValue("Reference", reference);
171171
topic.References.Remove("Reference");
172172

173-
Assert.AreEqual<int>(0, reference.IncomingRelationships.GetTopics("Reference").Count);
173+
Assert.AreEqual<int>(0, reference.IncomingRelationships.GetValues("Reference").Count);
174174

175175
}
176176

0 commit comments

Comments
 (0)