Skip to content

Commit c035ca0

Browse files
committed
Exclude empty values from GetAttributes()
Previously, `GetAttributes()` would return all attributes that met its criteria—even if their value was `null` or empty. In practice, however, `null` or empty attributes should be treated as deletions. The `UpdateTopic` stored procedure takes care of this scenario for _indexed_ attributes, but it introduces a problem for the _extended_ attributes. When an attribute is first deleted, the `Save()` will generate an empty attribute container for it. The next time the application `Load()`s, that attribute container will be discarded by the **OnTopic Library**. As such, if it's `Save()`d again, that attribute container will be removed. That process is largely transparent, and doesn't really hurt anything. But it does introduce some noise to the `ExtendedAttributes` table by introducing versions which might only vary by the introduction of an empty attribute—and especially now that `UpdateTopic` would otherwise eliminate consecutive duplicates. To prevent this, and to avoid confusion, `null` or empty attributes are now filtered out of `GetAttributes()` entirely so there's no risk of them polluting _any_ downstream providers, not just `SqlTopicRepository`. While this issue was exposed by the deduplication of consecutive versions code, it became even more noticeable when testing the Programmatic Support for Arbitrary Attributes (#19), since that introduces scenarios where values may be removed from the _extended_ attributes and added to the _indexed_ attributes (if their length is below 255), which would not only introduce this empty value in the _extended_ attribute storage, but also introduce ambiguity in the data model since different attribute values would be defined in two places for the same version (similar to 7a61005). This mitigates that redundancy and ambiguity.
1 parent 7a61005 commit c035ca0

2 files changed

Lines changed: 29 additions & 0 deletions

File tree

OnTopic.Tests/TopicRepositoryBaseTest.cs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,29 @@ public void GetAttributes_AnyAttributes_ReturnsAllAttributes() {
6868

6969
}
7070

71+
/*==========================================================================================================================
72+
| TEST: GET ATTRIBUTES: EMPTY ATTRIBUTES: SKIPS
73+
\-------------------------------------------------------------------------------------------------------------------------*/
74+
/// <summary>
75+
/// Retrieves a list of attributes from a topic, without any filtering by whether or not the attribute is an <see
76+
/// cref="AttributeDescriptor.IsExtendedAttribute"/>. Any <see cref="AttributeValue"/>s with a null or empty value should
77+
/// be skipped.
78+
/// </summary>
79+
[TestMethod]
80+
public void GetAttributes_EmptyAttributes_Skips() {
81+
82+
var topic = TopicFactory.Create("Test", "ContentTypes");
83+
84+
topic.Attributes.SetValue("EmptyAttribute", "");
85+
topic.Attributes.SetValue("NullAttribute", null);
86+
87+
var attributes = _topicRepository.GetAttributesProxy(topic, null);
88+
89+
Assert.IsFalse(attributes.Any(a => a.Key == "EmptyAttribute"));
90+
Assert.IsFalse(attributes.Any(a => a.Key == "NullAttribute"));
91+
92+
}
93+
7194
/*==========================================================================================================================
7295
| TEST: GET ATTRIBUTES: INDEXED ATTRIBUTES: RETURNS INDEXED ATTRIBUTES
7396
\-------------------------------------------------------------------------------------------------------------------------*/

OnTopic/Repositories/TopicRepositoryBase.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -502,6 +502,12 @@ protected IEnumerable<AttributeValue> GetAttributes(Topic topic, bool? isExtende
502502
attribute = contentType.AttributeDescriptors[key];
503503
}
504504

505+
//Skip if the value is null or empty; these values are not persisted to storage and should be treated as equivalent to
506+
//non-existent values.
507+
if (String.IsNullOrEmpty(attributeValue.Value)) {
508+
continue;
509+
}
510+
505511
//Add the attribute based on the isExtendedAttribute paramter. Add all parameters if isExtendedAttribute is null. Assume
506512
//an attribute is extended if the corresponding attribute descriptor cannot be located and the value is over 255
507513
//characters.

0 commit comments

Comments
 (0)