Skip to content

Commit 95c27c1

Browse files
committed
Ensure content type descriptor initialization
With the introduction of the new `GetContentTypeDescriptors()` overload (7a5a243), we inadvertantly introduced the possibility of bypassing the main `GetContentTypeDescriptors()` logic entirely, thus preventing even checking to see if content types exist in the database before looking for them in the local cache. In practice, this probably wouldn't hurt anything. But it could potentially introduce problems if working with partial or disconnected topic graphs, where the 'Configuration:ContentTypes` is not part of the in-memory topic graph. To mitigate this, we now call `GetContentTypeDescriptors()` from the new overload. But we don't want to either a) call that multiple times, or b) introduce a circular loop. So, to address that risk, we now exclusively initialize the `_contentTypeDescriptors` cache from within `GetContentTypeDescriptors()` method, and always access the cache through that.
1 parent 5e50db4 commit 95c27c1

1 file changed

Lines changed: 20 additions & 7 deletions

File tree

OnTopic/Repositories/TopicRepositoryBase.cs

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,11 @@ public virtual ContentTypeDescriptorCollection GetContentTypeDescriptors() {
5959
\-----------------------------------------------------------------------------------------------------------------------*/
6060
if (_contentTypeDescriptors == null) {
6161

62+
/*----------------------------------------------------------------------------------------------------------------------
63+
| Initialize cache
64+
\---------------------------------------------------------------------------------------------------------------------*/
65+
_contentTypeDescriptors = new ContentTypeDescriptorCollection();
66+
6267
/*----------------------------------------------------------------------------------------------------------------------
6368
| Load configuration data
6469
\---------------------------------------------------------------------------------------------------------------------*/
@@ -105,10 +110,16 @@ public virtual ContentTypeDescriptorCollection GetContentTypeDescriptors() {
105110
protected virtual ContentTypeDescriptorCollection GetContentTypeDescriptors(ContentTypeDescriptor contentTypeDescriptors) {
106111

107112
/*------------------------------------------------------------------------------------------------------------------------
108-
| Initialize the collection
113+
| Initialize the collection from the repository
114+
>-------------------------------------------------------------------------------------------------------------------------
115+
| ### NOTE JJC2020519: We want to centralize the shared logic from the public GetContentTypeDescriptors() while still
116+
| ensuring the cache is first initialized from the underlying data store if this method is called directly. But we don't
117+
| want to repeatedly call the underlying data store if it's empty, nor do we want to create a circular loop if this is
118+
| being called from GetContentTypeDescriptors(). This is handled by initializing the _contentTypeDescriptors cache in the
119+
| GetContentTypeDescriptors() method as a way of tracking the initialization state.
109120
\-----------------------------------------------------------------------------------------------------------------------*/
110121
if (_contentTypeDescriptors == null) {
111-
_contentTypeDescriptors = new ContentTypeDescriptorCollection();
122+
_contentTypeDescriptors = GetContentTypeDescriptors();
112123
}
113124

114125
/*------------------------------------------------------------------------------------------------------------------------
@@ -280,19 +291,21 @@ public virtual int Save([ValidatedNotNull, NotNull]Topic topic, bool isRecursive
280291
/*------------------------------------------------------------------------------------------------------------------------
281292
| Validate content type
282293
\-----------------------------------------------------------------------------------------------------------------------*/
283-
_contentTypeDescriptors = GetContentTypeDescriptors();
284-
if (!_contentTypeDescriptors.Contains(topic.ContentType)) {
294+
var contentTypeDescriptors= GetContentTypeDescriptors();
295+
var contentTypeDescriptor = contentTypeDescriptors.GetTopic(topic.ContentType);
296+
297+
if (contentTypeDescriptor == null) {
285298
throw new ArgumentException(
286299
$"The Content Type \"{topic.ContentType}\" referenced by \"{topic.Key}\" could not be found under " +
287-
$"\"Configuration:ContentTypes\". There are currently {_contentTypeDescriptors.Count} ContentTypes in the Repository."
300+
$"\"Configuration:ContentTypes\". There are currently {contentTypeDescriptors.Count} ContentTypes in the Repository."
288301
);
289302
}
290303

291304
/*------------------------------------------------------------------------------------------------------------------------
292305
| Update content types collection, if appropriate
293306
\-----------------------------------------------------------------------------------------------------------------------*/
294-
if (topic is ContentTypeDescriptor && !_contentTypeDescriptors.Contains(topic.Key)) {
295-
_contentTypeDescriptors.Add((ContentTypeDescriptor)topic);
307+
if (topic is ContentTypeDescriptor && !contentTypeDescriptors.Contains(topic.Key)) {
308+
contentTypeDescriptors.Add((ContentTypeDescriptor)topic);
296309
}
297310

298311
/*------------------------------------------------------------------------------------------------------------------------

0 commit comments

Comments
 (0)