Skip to content

Commit 5cf13bb

Browse files
JeremyCaneyJeremyCaney
committed
Merge branch 'feature/MappedTopicCache' into develop
When mapping an object with relationships, it is possible that the same topic will be mapped more than one time in the resulting object graph. For instance, a nested topic may also be referenced as a relationship. When this occurs, the `TopicMappingService` implements a caching mechanism to prevent the topic from being mapped a second time. This improves performance, prevents potential bugs with different instances representing the same entity, and can even help avoid circular loops. Unfortunately, this cache introduces a potential bug of its own. If subsequent requests to the topic have a more comprehensive `[Follow()]` attribute, then the initial cached version of the topic will deprive the subsequent requests from having all relationships populated. So, for instance, if the topic is first referenced with `[Follows(Relationships.None)]`, and then a subsequent reference to it uses `[Follows(Relationships.All)]`, then the initial mapped instance will be returned with no relationships populated. To resolve this, the previous `ConcurrentDictionary<int, object>` cache is replaced with a new `MappedTopicCache()` object type which keeps track of not only the mapped object, but also the relationships that were used to map it (d027d35). If and when a subsequent request for the object is made, the difference between the original `Relationships` and the subsequent `Relationships` is calculated, and the missing relationships are mapped, while retaining the mapped values of all previous relationships and properties (49f0597). This allows us to maintain the benefits of caching already mapped objects, while also ensuring that the mapped object contains the union of relationships required of it.
2 parents e7cd22f + a2a281c commit 5cf13bb

4 files changed

Lines changed: 176 additions & 42 deletions

File tree

‎OnTopic.Tests/TopicMappingServiceTest.cs‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
using Microsoft.VisualStudio.TestTools.UnitTesting;
1212
using OnTopic.Attributes;
1313
using OnTopic.Data.Caching;
14+
using OnTopic.Internal.Mapping;
1415
using OnTopic.Mapping;
1516
using OnTopic.Mapping.Annotations;
1617
using OnTopic.Metadata;
@@ -280,6 +281,30 @@ public async Task Map_AlternateAttributeKey_ReturnsMappedModel() {
280281

281282
}
282283

284+
/*==========================================================================================================================
285+
| TEST: MAPPED TOPIC CACHE ENTRY: GET MISSING RELATIONSHIPS: RETURNS DIFFERENCE
286+
\-------------------------------------------------------------------------------------------------------------------------*/
287+
/// <summary>
288+
/// Establishes a <see cref="MappedTopicCacheEntry"/> with a set of <see cref="Relationships"/>, and then confirms that
289+
/// its <see cref="MappedTopicCacheEntry.GetMissingRelationships(Relationships)"/> correctly returns the missing
290+
/// relationships.
291+
/// </summary>
292+
[TestMethod]
293+
public void MappedTopicCacheEntry_GetMissingRelationships_ReturnsDifference() {
294+
295+
var cacheEntry = new MappedTopicCacheEntry() {
296+
Relationships = Relationships.Children | Relationships.Parents
297+
};
298+
var relationships = Relationships.Children | Relationships.References;
299+
300+
var difference = cacheEntry.GetMissingRelationships(relationships);
301+
302+
Assert.IsTrue(difference.HasFlag(Relationships.References));
303+
Assert.IsFalse(difference.HasFlag(Relationships.Children));
304+
Assert.IsFalse(difference.HasFlag(Relationships.Parents));
305+
306+
}
307+
283308
/*==========================================================================================================================
284309
| TEST: MAP: RELATIONSHIPS: RETURNS MAPPED MODEL
285310
\-------------------------------------------------------------------------------------------------------------------------*/

‎OnTopic/Internal/Collections/MappedTopicCache.cs‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/*==============================================================================================================================
2+
| Author Ignia, LLC
3+
| Client Ignia, LLC
4+
| Project Topics Library
5+
\=============================================================================================================================*/
6+
using System.Collections.Concurrent;
7+
using OnTopic.Mapping;
8+
using OnTopic.Internal.Mapping;
9+
10+
namespace OnTopic.Internal.Collections {
11+
12+
/*============================================================================================================================
13+
| CLASS: MAPPED TOPIC CACHE
14+
\---------------------------------------------------------------------------------------------------------------------------*/
15+
/// <summary>
16+
/// Provides a collection intended to track local caching of objects mapped using the <see cref="TopicMappingService"/>.
17+
/// </summary>
18+
public class MappedTopicCache: ConcurrentDictionary<int, MappedTopicCacheEntry> {
19+
20+
21+
} //Class
22+
} //Namespace

‎OnTopic/Internal/Mapping/MappedTopicCacheEntry.cs‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
/*==============================================================================================================================
2+
| Author Ignia, LLC
3+
| Client Ignia, LLC
4+
| Project Topics Library
5+
\=============================================================================================================================*/
6+
using OnTopic.Mapping;
7+
using OnTopic.Mapping.Annotations;
8+
9+
namespace OnTopic.Internal.Mapping {
10+
11+
/*============================================================================================================================
12+
| CLASS: MAPPED TOPIC CACHE ENTRY
13+
\---------------------------------------------------------------------------------------------------------------------------*/
14+
/// <summary>
15+
/// Provides an entry to tracking an object mapped using the <see cref="TopicMappingService"/>.
16+
/// </summary>
17+
/// <remarks>
18+
/// In addition to the actual <see cref="MappedTopic"/>, this also includes a <see cref="Relationships"/> property for
19+
/// tracking what relationships were mapped to the <see cref="MappedTopic"/>. This allows the <see cref=
20+
/// "TopicMappingService"/> to be update the cached object with any missing relationships, which can be identified using the
21+
/// <see cref="GetMissingRelationships(Relationships)"/> method. In turn, the cache can then be updated to reflect those
22+
/// new relationships by using <see cref="AddMissingRelationships(Relationships)"/>. This ensures that even if a topic has
23+
/// already been mapped, its scope can be expanded without duplicating effort.
24+
/// </remarks>
25+
public class MappedTopicCacheEntry {
26+
27+
/*==========================================================================================================================
28+
| PROPERTY: MAPPED TOPIC
29+
\-------------------------------------------------------------------------------------------------------------------------*/
30+
/// <summary>
31+
/// Provides a reference to the mapped object.
32+
/// </summary>
33+
public object MappedTopic { get; set; } = null!;
34+
35+
/*==========================================================================================================================
36+
| PROPERTY: RELATIONSHIPS
37+
\-------------------------------------------------------------------------------------------------------------------------*/
38+
/// <summary>
39+
/// Provides a reference to the relationships that the <see cref="MappedTopic"/> was mapped with.
40+
/// </summary>
41+
public Relationships Relationships { get; set; } = Relationships.None;
42+
43+
/*==========================================================================================================================
44+
| METHOD: GET MISSING RELATIONSHIPS
45+
\-------------------------------------------------------------------------------------------------------------------------*/
46+
/// <summary>
47+
/// Given a target <paramref name="relationships"/>, identifies any relationships not covered by <see cref="Relationships"
48+
/// /> and returns them as a new <see cref="OnTopic.Mapping.Annotations.Relationships"/> instance.
49+
/// </summary>
50+
public Relationships GetMissingRelationships(Relationships relationships) => Relationships ^ (relationships | Relationships);
51+
52+
/*==========================================================================================================================
53+
| METHOD: ADD MISSING RELATIONSHIPS
54+
\-------------------------------------------------------------------------------------------------------------------------*/
55+
/// <summary>
56+
/// Given a target <paramref name="relationships"/>, adds any missing <see cref="OnTopic.Mapping.Annotations.
57+
/// Relationships"/> to the <see cref="Relationships"/> property.
58+
/// </summary>
59+
public void AddMissingRelationships(Relationships relationships) => Relationships = relationships | Relationships;
60+
61+
} //Class
62+
} //Namespace

‎OnTopic/Mapping/TopicMappingService.cs‎

Lines changed: 67 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,13 @@
55
\=============================================================================================================================*/
66
using System;
77
using System.Collections;
8-
using System.Collections.Concurrent;
98
using System.Collections.Generic;
109
using System.Diagnostics.CodeAnalysis;
1110
using System.Linq;
1211
using System.Reflection;
1312
using System.Threading.Tasks;
1413
using OnTopic.Attributes;
14+
using OnTopic.Internal.Collections;
1515
using OnTopic.Internal.Diagnostics;
1616
using OnTopic.Internal.Mapping;
1717
using OnTopic.Internal.Reflection;
@@ -90,7 +90,7 @@ public TopicMappingService(ITopicRepository topicRepository, ITypeLookupService
9090
private async Task<object?> MapAsync(
9191
Topic? topic,
9292
Relationships relationships,
93-
ConcurrentDictionary<int, object> cache,
93+
MappedTopicCache cache,
9494
string? attributePrefix = null
9595
) {
9696

@@ -106,23 +106,32 @@ public TopicMappingService(ITopicRepository topicRepository, ITypeLookupService
106106
/*------------------------------------------------------------------------------------------------------------------------
107107
| Handle cached objects
108108
\-----------------------------------------------------------------------------------------------------------------------*/
109-
if (cache.TryGetValue(topic.Id, out var dto)) {
110-
return dto;
109+
object? target;
110+
111+
if (cache.TryGetValue(topic.Id, out var cacheEntry)) {
112+
target = cacheEntry.MappedTopic;
113+
if (cacheEntry.GetMissingRelationships(relationships) == Relationships.None) {
114+
return target;
115+
}
111116
}
112117

113118
/*------------------------------------------------------------------------------------------------------------------------
114119
| Instantiate object
115120
\-----------------------------------------------------------------------------------------------------------------------*/
116-
var viewModelType = _typeLookupService.Lookup($"{topic.ContentType}TopicViewModel");
121+
else {
117122

118-
if (viewModelType is null || !viewModelType.Name.EndsWith("TopicViewModel", StringComparison.CurrentCultureIgnoreCase)) {
119-
throw new InvalidOperationException(
120-
$"No class named '{topic.ContentType}TopicViewModel' could be located in any loaded assemblies. This is required " +
121-
$"to map the topic '{topic.GetUniqueKey()}'."
122-
);
123-
}
123+
var viewModelType = _typeLookupService.Lookup($"{topic.ContentType}TopicViewModel");
124124

125-
var target = Activator.CreateInstance(viewModelType);
125+
if (viewModelType is null || !viewModelType.Name.EndsWith("TopicViewModel", StringComparison.CurrentCultureIgnoreCase)) {
126+
throw new InvalidOperationException(
127+
$"No class named '{topic.ContentType}TopicViewModel' could be located in any loaded assemblies. This is required " +
128+
$"to map the topic '{topic.GetUniqueKey()}'."
129+
);
130+
}
131+
132+
target = Activator.CreateInstance(viewModelType);
133+
134+
}
126135

127136
/*------------------------------------------------------------------------------------------------------------------------
128137
| Provide mapping
@@ -168,10 +177,10 @@ public TopicMappingService(ITopicRepository topicRepository, ITypeLookupService
168177
/// The target view model with the properties appropriately mapped.
169178
/// </returns>
170179
private async Task<object> MapAsync(
171-
Topic? topic,
172-
object target,
173-
Relationships relationships,
174-
ConcurrentDictionary<int, object> cache,
180+
Topic? topic,
181+
object target,
182+
Relationships relationships,
183+
MappedTopicCache cache,
175184
string? attributePrefix = null
176185
) {
177186

@@ -193,20 +202,34 @@ private async Task<object> MapAsync(
193202

194203
/*------------------------------------------------------------------------------------------------------------------------
195204
| Handle cached objects
196-
\-----------------------------------------------------------------------------------------------------------------------*/
197-
if (cache.TryGetValue(topic.Id, out var dto)) {
198-
return dto;
205+
>-------------------------------------------------------------------------------------------------------------------------
206+
| If the cache contains an entry, check to make sure it includes all of the requested relationships. If it does, return
207+
| it. If it doesn't, determine the missing relationships and request to have those mapped.
208+
\-----------------------------------------------------------------------------------------------------------------------*/
209+
if (cache.TryGetValue(topic.Id, out var cacheEntry)) {
210+
relationships = cacheEntry.GetMissingRelationships(relationships);
211+
target = cacheEntry.MappedTopic;
212+
if (relationships == Relationships.None) {
213+
return cacheEntry.MappedTopic;
214+
}
215+
cacheEntry.AddMissingRelationships(relationships);
199216
}
200217
else if (!topic.IsNew) {
201-
cache.GetOrAdd(topic.Id, target);
218+
cache.GetOrAdd(
219+
topic.Id,
220+
new MappedTopicCacheEntry() {
221+
MappedTopic = target,
222+
Relationships = relationships
223+
}
224+
);
202225
}
203226

204227
/*------------------------------------------------------------------------------------------------------------------------
205228
| Loop through properties, mapping each one
206229
\-----------------------------------------------------------------------------------------------------------------------*/
207230
var taskQueue = new List<Task>();
208231
foreach (var property in _typeCache.GetMembers<PropertyInfo>(target.GetType())) {
209-
taskQueue.Add(SetPropertyAsync(topic, target, relationships, property, cache, attributePrefix));
232+
taskQueue.Add(SetPropertyAsync(topic, target, relationships, property, cache, attributePrefix, cacheEntry != null));
210233
}
211234
await Task.WhenAll(taskQueue.ToArray()).ConfigureAwait(false);
212235

@@ -230,13 +253,15 @@ private async Task<object> MapAsync(
230253
/// <param name="property">Information related to the current property.</param>
231254
/// <param name="cache">A cache to keep track of already-mapped object instances.</param>
232255
/// <param name="attributePrefix">The prefix to apply to the attributes.</param>
256+
/// <param name="mapRelationshipsOnly">Determines if properties not associated with properties should be mapped.</param>
233257
protected async Task SetPropertyAsync(
234-
Topic source,
235-
object target,
236-
Relationships relationships,
237-
PropertyInfo property,
238-
ConcurrentDictionary<int, object> cache,
239-
string? attributePrefix = null
258+
Topic source,
259+
object target,
260+
Relationships relationships,
261+
PropertyInfo property,
262+
MappedTopicCache cache,
263+
string? attributePrefix = null,
264+
bool mapRelationshipsOnly = false
240265
) {
241266

242267
/*------------------------------------------------------------------------------------------------------------------------
@@ -261,7 +286,7 @@ protected async Task SetPropertyAsync(
261286
/*------------------------------------------------------------------------------------------------------------------------
262287
| Assign default value
263288
\-----------------------------------------------------------------------------------------------------------------------*/
264-
if (configuration.DefaultValue is not null) {
289+
if (!mapRelationshipsOnly && configuration.DefaultValue is not null) {
265290
property.SetValue(target, configuration.DefaultValue);
266291
}
267292

@@ -274,7 +299,7 @@ protected async Task SetPropertyAsync(
274299
else if (SetCompatibleProperty(source, target, configuration)) {
275300
//Performed 1:1 mapping between source and target
276301
}
277-
else if (_typeCache.HasSettableProperty(target.GetType(), property.Name)) {
302+
else if (!mapRelationshipsOnly && _typeCache.HasSettableProperty(target.GetType(), property.Name)) {
278303
SetScalarValue(source, target, configuration);
279304
}
280305
else if (typeof(IList).IsAssignableFrom(property.PropertyType)) {
@@ -398,11 +423,11 @@ protected static void SetScalarValue(Topic source, object target, PropertyConfig
398423
/// </param>
399424
/// <param name="cache">A cache to keep track of already-mapped object instances.</param>
400425
protected async Task SetCollectionValueAsync(
401-
Topic source,
402-
object target,
403-
Relationships relationships,
404-
PropertyConfiguration configuration,
405-
ConcurrentDictionary<int, object> cache
426+
Topic source,
427+
object target,
428+
Relationships relationships,
429+
PropertyConfiguration configuration,
430+
MappedTopicCache cache
406431
) {
407432

408433
/*------------------------------------------------------------------------------------------------------------------------
@@ -590,10 +615,10 @@ IList<Topic> GetRelationship(RelationshipType relationship, Func<string, bool> c
590615
/// </param>
591616
/// <param name="cache">A cache to keep track of already-mapped object instances.</param>
592617
protected async Task PopulateTargetCollectionAsync(
593-
IList<Topic> sourceList,
594-
IList targetList,
595-
PropertyConfiguration configuration,
596-
ConcurrentDictionary<int, object> cache
618+
IList<Topic> sourceList,
619+
IList targetList,
620+
PropertyConfiguration configuration,
621+
MappedTopicCache cache
597622
) {
598623

599624
/*------------------------------------------------------------------------------------------------------------------------
@@ -698,10 +723,10 @@ void AddToList(object dto) {
698723
/// </param>
699724
/// <param name="cache">A cache to keep track of already-mapped object instances.</param>
700725
protected async Task SetTopicReferenceAsync(
701-
Topic source,
702-
object target,
703-
PropertyConfiguration configuration,
704-
ConcurrentDictionary<int, object> cache
726+
Topic source,
727+
object target,
728+
PropertyConfiguration configuration,
729+
MappedTopicCache cache
705730
) {
706731

707732
/*------------------------------------------------------------------------------------------------------------------------

0 commit comments

Comments
 (0)