Skip to content

Commit 8b9ae46

Browse files
committed
Merge branch 'feature/TrackedCollection' into develop
Introduced a new `TrackedCollection<>` type (e77d6e9) which centralizes the logic for both `AttributeValueCollection` (beb3d1f) and `TopicReferenceCollection` (previously `TopicReferenceDictionary`) (74d5907). This allows the two classes to share the following features: - Lookup of actual values by `key` via `GetValue()`, with inheritance from `BaseTopic` and, optionally, `Parent`. - Tracking of state via `IsDirty` and `Version` on a unified `TrackedItem<T>` metadata container. - Enforcement of business logic by calling appropriate properties on `Topic` via `SetValue()`. This also unifies the semantics and the syntax for working with these two collections, thus acknowledging that topic references are, ultimately, just topic-valued attributes, and should thus otherwise operate the same. This changes the semantics and syntax from the initial design of `TopicReferenceDictionary` which was, instead, modeled more off of `TopicRelationshipMultiMap`. Outside of returning `Topic` types, though, `TopicReferenceCollection` has more in common with `AttributeValueCollection`. As a result of this, the `TopicPropertyDispatcher` is now _exclusively_ used by the new `TrackedCollection<>`, which exclusively operates off of `TrackedItem<T>`. As such, I updated `TopicPropertyDispatcher` to operate off of that assumption, which thus eliminated the one-off hacks for both `AttributeValueCollection` and `TopicReferenceCollection` (11506c3). This also necessitated some bug fixes in the underlying `MemberDispatcher` to better handle generic type parameters (13d43d1, edc9f4b). Because of the change from `IDictionary<>` to `KeyedCollection<>`, the `TopicReferenceCollection` has some breaking changes. As it was first developed for OnTopic 5.x, which hasn't yet released, this won't impact any customers. It will require updating the OnTopic Editor, however, which has been developing its 5.x release in parallel based on pre-release packages. These changes are mostly trivial, pertaining to slight differences in interface. The most notable changes, however, include: 1. The `GetTopic()` and `SetTopic()` methods have been replaced with the more generically named `GetValue()` and `SetValue()` (0544353). 2. The `TryGetValue()` (4b00642) and `Add(string, Topic)` (956c9f4) methods no longer exist, and should be replaced with `GetValue()` and `SetValue()` respectively. 3. The `Value` is now nullable, which can be an issue when iterating over the collection (e7a5520). In exchange, the `TopicReferenceCollection` now enjoys nearly identical semantics and syntax as `AttributeValueCollection`, including some new features such as `inheritFromParent`, which it didn't previously support.
2 parents 2486ab2 + edc9f4b commit 8b9ae46

33 files changed

+1334
-1303
lines changed

OnTopic.AspNetCore.Mvc/Controllers/SitemapController.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ topic.References.Count is 0?
282282
from reference in topic.References
283283
select new XElement(_pagemapNamespace + "Attribute",
284284
new XAttribute("name", reference.Key),
285-
new XText(reference.Value.GetUniqueKey().Replace("Root:", "", StringComparison.OrdinalIgnoreCase))
285+
new XText(reference.Value?.GetUniqueKey().Replace("Root:", "", StringComparison.OrdinalIgnoreCase))
286286
)
287287
);
288288

OnTopic.Data.Sql/SqlDataReaderExtensions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -434,7 +434,7 @@ private static void SetReferences(this IDataReader reader, TopicIndex topics, bo
434434
/*------------------------------------------------------------------------------------------------------------------------
435435
| Set relationship on object
436436
\-----------------------------------------------------------------------------------------------------------------------*/
437-
current.References.SetTopic(relationshipKey, referenced, markDirty);
437+
current.References.SetValue(relationshipKey, referenced, markDirty);
438438

439439
}
440440

OnTopic.Data.Sql/SqlTopicRepository.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -710,8 +710,8 @@ private static void PersistReferences(Topic topic, DateTime version, SqlConnecti
710710
CommandType = CommandType.StoredProcedure
711711
};
712712

713-
foreach (var relatedTopic in topic.References.Where(t => !t.Value.IsNew)) {
714-
references.AddRow(relatedTopic.Key, relatedTopic.Value.Id);
713+
foreach (var relatedTopic in topic.References.Where(t => !t.Value?.IsNew?? false)) {
714+
references.AddRow(relatedTopic.Key, relatedTopic.Value!.Id);
715715
}
716716

717717
// Add Parameters

OnTopic.Tests/AttributeValueCollectionTest.cs

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using System.Globalization;
99
using Microsoft.VisualStudio.TestTools.UnitTesting;
1010
using OnTopic.Attributes;
11+
using OnTopic.Collections.Specialized;
1112
using OnTopic.Tests.Entities;
1213

1314
namespace OnTopic.Tests {
@@ -308,7 +309,7 @@ public void Clear_ExistingValues_IsDirty() {
308309
topic.Attributes.Clear();
309310

310311
Assert.IsTrue(topic.Attributes.IsDirty());
311-
Assert.IsTrue(topic.Attributes.DeletedAttributes.Contains("Foo"));
312+
Assert.IsTrue(topic.Attributes.DeletedItems.Contains("Foo"));
312313

313314
}
314315

@@ -317,7 +318,7 @@ public void Clear_ExistingValues_IsDirty() {
317318
\-------------------------------------------------------------------------------------------------------------------------*/
318319
/// <summary>
319320
/// Sets the value of a custom <see cref="AttributeValue"/> to the existing value and ensures it is <i>not</i> marked as
320-
/// <see cref="AttributeValue.IsDirty"/>.
321+
/// <see cref="TrackedItem{T}.IsDirty"/>.
321322
/// </summary>
322323
[TestMethod]
323324
public void SetValue_ValueUnchanged_IsNotDirty() {
@@ -336,7 +337,7 @@ public void SetValue_ValueUnchanged_IsNotDirty() {
336337
\-------------------------------------------------------------------------------------------------------------------------*/
337338
/// <summary>
338339
/// Populates the <see cref="AttributeValueCollection"/> with a <see cref="AttributeValue"/> that is marked as <see
339-
/// cref="AttributeValue.IsDirty"/>. Confirms that <see cref="AttributeValueCollection.IsDirty(Boolean)"/> returns
340+
/// cref="TrackedItem{T}.IsDirty"/>. Confirms that <see cref="AttributeValueCollection.IsDirty(Boolean)"/> returns
340341
/// <c>true</c>.
341342
/// </summary>
342343
[TestMethod]
@@ -374,7 +375,7 @@ public void IsDirty_DeletedValues_ReturnsTrue() {
374375
\-------------------------------------------------------------------------------------------------------------------------*/
375376
/// <summary>
376377
/// Populates the <see cref="AttributeValueCollection"/> with a <see cref="AttributeValue"/> that is <i>not</i> marked as
377-
/// <see cref="AttributeValue.IsDirty"/>. Confirms that <see cref="AttributeValueCollection.IsDirty(Boolean)"/> returns
378+
/// <see cref="TrackedItem{T}.IsDirty"/>. Confirms that <see cref="AttributeValueCollection.IsDirty(Boolean)"/> returns
378379
/// <c>false</c>/
379380
/// </summary>
380381
[TestMethod]
@@ -393,7 +394,7 @@ public void IsDirty_NoDirtyValues_ReturnsFalse() {
393394
\-------------------------------------------------------------------------------------------------------------------------*/
394395
/// <summary>
395396
/// Populates the <see cref="AttributeValueCollection"/> with a <see cref="AttributeValue"/> that is <i>not</i> marked as
396-
/// <see cref="AttributeValue.IsDirty"/> as well as a <c>LastModified</c> <see cref="AttributeValue"/> that is. Confirms
397+
/// <see cref="TrackedItem{T}.IsDirty"/> as well as a <c>LastModified</c> <see cref="AttributeValue"/> that is. Confirms
397398
/// that <see cref="AttributeValueCollection.IsDirty(Boolean)"/> returns <c>false</c>.
398399
/// </summary>
399400
[TestMethod]
@@ -414,8 +415,8 @@ public void IsDirty_ExcludeLastModified_ReturnsFalse() {
414415
\-------------------------------------------------------------------------------------------------------------------------*/
415416
/// <summary>
416417
/// Populates the <see cref="AttributeValueCollection"/> with a <see cref="AttributeValue"/> and then deletes it. Confirms
417-
/// that the <see cref="AttributeValue.LastModified"/> returns the new <c>version</c> after calling <see cref="
418-
/// AttributeValueCollection.MarkClean(DateTime?)"/>.
418+
/// that the <see cref="TrackedItem{T}.LastModified"/> returns the new <c>version</c> after calling <see cref="
419+
/// TrackedCollection{TItem, TValue, TAttribute}.MarkClean(DateTime?)"/>.
419420
/// </summary>
420421
[TestMethod]
421422
public void IsDirty_MarkClean_UpdatesLastModified() {
@@ -437,7 +438,7 @@ public void IsDirty_MarkClean_UpdatesLastModified() {
437438
/// <summary>
438439
/// Populates the <see cref="AttributeValueCollection"/> with a <see cref="AttributeValue"/> and then deletes it. Confirms
439440
/// that <see cref="AttributeValueCollection.IsDirty(Boolean)"/> returns <c>false</c> after calling <see cref="
440-
/// AttributeValueCollection.MarkClean(DateTime?)"/>.
441+
/// TrackedCollection{TItem, TValue, TAttribute}.MarkClean(DateTime?)"/>.
441442
/// </summary>
442443
[TestMethod]
443444
public void IsDirty_MarkClean_ReturnsFalse() {
@@ -460,8 +461,8 @@ public void IsDirty_MarkClean_ReturnsFalse() {
460461
\-------------------------------------------------------------------------------------------------------------------------*/
461462
/// <summary>
462463
/// Populates the <see cref="AttributeValueCollection"/> with a <see cref="AttributeValue"/> and then confirms that <see
463-
/// cref="AttributeValueCollection.IsDirty(String)"/> returns <c>false</c> for that attribute after calling <see cref="
464-
/// AttributeValueCollection.MarkClean(String, DateTime?)"/>.
464+
/// cref="TrackedCollection{TItem, TValue, TAttribute}.IsDirty(String)"/> returns <c>false</c> for that attribute after
465+
/// calling <see cref="TrackedCollection{TItem, TValue, TAttribute}.MarkClean(String, DateTime?)"/>.
465466
/// </summary>
466467
[TestMethod]
467468
public void IsDirty_MarkAttributeClean_ReturnsFalse() {
@@ -623,8 +624,8 @@ public void Add_InvalidAttributeValue_ThrowsException() {
623624
\-------------------------------------------------------------------------------------------------------------------------*/
624625
/// <summary>
625626
/// Adds a new <see cref="AttributeValue"/> which maps to <see cref="Topic.Key"/> directly to a <see cref=
626-
/// "AttributeValueCollection"/> and confirms that the original <see cref="AttributeValue.IsDirty"/> is replaced if the
627-
/// <see cref="AttributeValue.Value"/> changes.
627+
/// "AttributeValueCollection"/> and confirms that the original <see cref="TrackedItem{T}.IsDirty"/> is replaced if the
628+
/// <see cref="TrackedItem{T}.Value"/> changes.
628629
/// </summary>
629630
[TestMethod]
630631
public void Add_WithBusinessLogic_MaintainsIsDirty() {

OnTopic.Tests/Entities/CustomTopic.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public CustomTopic(string key, string contentType, Topic? parent = null, int id
3434
/// Provides a text property which is intended to be mapped to a text attribute.
3535
/// </summary>
3636
[AttributeSetter]
37-
public string TextAttribute {
37+
public string? TextAttribute {
3838
get => Attributes.GetValue("TextAttribute");
3939
set => SetAttributeValue("TextAttribute", value);
4040
}
@@ -95,13 +95,13 @@ public DateTime DateTimeAttribute {
9595
/// </summary>
9696
[ReferenceSetter]
9797
public Topic? TopicReference {
98-
get => References.GetTopic("TopicReference");
98+
get => References.GetValue("TopicReference");
9999
set {
100100
Contract.Requires<ArgumentOutOfRangeException>(
101101
value.ContentType == ContentType,
102102
$"{nameof(TopicReference)} expects a topic with the same content type as the parent: {ContentType}."
103103
);
104-
References.SetTopic("TopicReference", value);
104+
References.SetValue("TopicReference", value);
105105
}
106106
}
107107

OnTopic.Tests/SqlTopicRepositoryTest.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ public void LoadTopicGraph_WithReference_ReturnsReference() {
124124

125125
Assert.IsNotNull(topic);
126126
Assert.AreEqual<int>(1, topic.Id);
127-
Assert.AreEqual<int?>(2, topic.References.GetTopic("Test")?.Id);
127+
Assert.AreEqual<int?>(2, topic.References.GetValue("Test")?.Id);
128128
Assert.IsTrue(topic.References.IsDirty());
129129

130130
}
@@ -154,7 +154,7 @@ public void LoadTopicGraph_WithExternalReference_ReturnsReference() {
154154

155155
Assert.IsNotNull(topic);
156156
Assert.AreEqual<int>(1, topic.Id);
157-
Assert.AreEqual<int?>(2, topic.References.GetTopic("Test")?.Id);
157+
Assert.AreEqual<int?>(2, topic.References.GetValue("Test")?.Id);
158158
Assert.IsTrue(topic.References.IsFullyLoaded);
159159
Assert.IsFalse(topic.References.IsDirty());
160160

OnTopic.Tests/TopicMappingServiceTest.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -593,7 +593,7 @@ public async Task Map_TopicReferences_ReturnsMappedModel() {
593593

594594
var topic = TopicFactory.Create("Test", "TopicReference");
595595

596-
topic.References.SetTopic("TopicReference", topicReference);
596+
topic.References.SetValue("TopicReference", topicReference);
597597

598598
var target = (TopicReferenceTopicViewModel?)await mappingService.MapAsync(topic).ConfigureAwait(false);
599599

@@ -618,7 +618,7 @@ public async Task Map_TopicReferences_SkipsDisabled() {
618618

619619
topicReference.IsDisabled = true;
620620

621-
topic.References.SetTopic("TopicReference", topicReference);
621+
topic.References.SetValue("TopicReference", topicReference);
622622

623623
var target = (TopicReferenceTopicViewModel?)await mappingService.MapAsync(topic).ConfigureAwait(false);
624624

0 commit comments

Comments
 (0)