Skip to content

Commit 682d328

Browse files
committed
Ensure lookup by Key, not Topic
Currently, the OnTopic library only enforces key uniqueness within a given `Parent` scope. That's normally fine. But it introduces issues when working with the `ContentTypes` hierarchy, as this is condenced down into a flat list of `ContentTypeDescriptor`s keyed off of the `Topic.Key`. Ideally, in the future, we should add validation to prevent nested `ContentTypeDescriptor` topics from having the same `Key` as an existing `ContentTypeDescriptor`. Until then, however, we should at least avoid trying to add duplicate keys when indexing the content type descriptors. To facilitate this, we do a `Contains(topic.Key)` check before inserting new `ContentTypeDescriptor` objects into the `GetContentTypeDescriptors()` cache. I missed one instance, however, where I instead check for the object. That doesn't work for multiple reasons—including the fact even the same exact entity can technically have multiple object instances (e.g., one pulled from the topic graph, another pulled from the `GetContentTypeDescriptors()` cache). By searching by `topic.Key` we resolve both issues.
1 parent d0a96d8 commit 682d328

1 file changed

Lines changed: 1 addition & 1 deletion

File tree

OnTopic/Repositories/TopicRepositoryBase.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,7 @@ public virtual int Save([ValidatedNotNull, NotNull]Topic topic, bool isRecursive
332332
topic.Id < 0 &&
333333
topic is ContentTypeDescriptor &&
334334
_contentTypeDescriptors != null &&
335-
!_contentTypeDescriptors.Contains(topic)
335+
!_contentTypeDescriptors.Contains(topic.Key)
336336
) {
337337
_contentTypeDescriptors.Add((ContentTypeDescriptor)topic);
338338
}

0 commit comments

Comments
 (0)