Skip to content

Commit c718adc

Browse files
JeremyCaneyJeremyCaney
committed
Merge branch 'improvement/ItemMetadata.MaybeCompatible' into develop
Introduced a new property, `MaybeCompatible`, to the `ItemMetadata` which attempts to determine if an item has a corresponding member on the `Topic` class (or a derivative with a matching name) and, if so, whether it is either a compatible or convertible data type. This can't guarantee compatibility since, potentially, a `Topic` may be derived, and the source topic type may not match the target model naming convention. Since derived topics are very rarely used by either the OnTopic library or customer implementations, however, this isn't a major concern. In the meanwhile, this allows us to limit more careful analysis of compatible and convertible types in the `TopicMappingService` by relying on `MaybeCompatible` to determine if further evaluation makes sense. This allows us to improve the threshold used for the `AttributeDictionary` constructor (Feature #99), thus evaluating whether there are any members that do _not_ correspond to properties on `Topic`. This is more intelligent than the previous approach, which simply evaluated the number of properties, since the base `TopicViewModel` class (if it's used) exceeded the threshold due to properties that map to `Topic` members, but not necessarily attributes (e.g., `Id`, `Key`, `ContentType`, `WebPath`). As a result, I was able to reduce the threshold from five to three, while actually making it more selective. Finally, the `TypeAccessor` was updated to exclude members inherited from `object` or added dynamically by the C# compiler to `record` objects. This helps reduce false positives in the above thresholds while also greatly reducing the number of cached properties that will never be used.
2 parents 78e5536 + 3a6820b commit c718adc

8 files changed

Lines changed: 383 additions & 18 deletions

File tree

‎OnTopic.Tests/TopicMappingServiceTest.cs‎

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -295,13 +295,29 @@ public async Task Map_AttributeDictionary_ReturnsNewModel() {
295295
var topic = new Topic("Test", "Page");
296296
var lastModified = new DateTime(2021, 12, 22);
297297

298-
topic.Attributes.SetValue("Subtitle", "Value");
298+
topic.Attributes.SetValue("Title", "Value");
299+
topic.Attributes.SetValue("ShortTitle", "Short Title");
300+
topic.Attributes.SetValue("Subtitle", "Subtitle");
301+
topic.Attributes.SetValue("MetaTitle", "Meta Title");
302+
topic.Attributes.SetValue("MetaDescription", "Meta Description");
303+
topic.Attributes.SetValue("MetaKeywords", "Load;Test;Keywords");
304+
topic.Attributes.SetValue("NoIndex", "0");
305+
topic.Attributes.SetValue("Body", "Body of test topic");
306+
topic.Attributes.SetValue("MappedProperty", "Mapped Value");
307+
topic.Attributes.SetValue("UnmappedProperty", "Unmapped Value");
299308
topic.VersionHistory.Add(lastModified);
300309

301-
var target = await _mappingService.MapAsync<PageTopicViewModel>(topic).ConfigureAwait(false);
302-
303-
Assert.Equal("Test", target?.Title);
304-
Assert.Equal("Value", target?.Subtitle);
310+
var target = await _mappingService.MapAsync<AttributeDictionaryConstructorTopicViewModel>(topic).ConfigureAwait(false);
311+
312+
Assert.Equal("Value", target?.Title);
313+
Assert.Equal("Short Title", target?.ShortTitle);
314+
Assert.Equal("Subtitle", target?.Subtitle);
315+
Assert.Equal("Meta Title", target?.MetaTitle);
316+
Assert.Equal("Meta Description", target?.MetaDescription);
317+
Assert.Equal(false, target?.NoIndex);
318+
Assert.Equal("Load;Test;Keywords", target?.MetaKeywords);
319+
Assert.Equal("Mapped Value", target?.MappedProperty);
320+
Assert.Null(target?.UnmappedProperty);
305321
Assert.Equal(lastModified, target?.LastModified);
306322

307323
}

‎OnTopic.Tests/TypeAccessorTest.cs‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
using OnTopic.Tests.BindingModels;
1212
using OnTopic.Tests.Fixtures;
1313
using OnTopic.Tests.ViewModels;
14+
using OnTopic.Tests.ViewModels.Metadata;
1415
using Xunit;
1516

1617
namespace OnTopic.Tests {
@@ -65,7 +66,7 @@ public void GetMembers_MixedValidity_ReturnsValidMembers() {
6566

6667
var members = _typeAccessor.GetMembers();
6768

68-
Assert.Equal(7+4, members.Count); // Includes four compliant members inherited from object
69+
Assert.Equal(7, members.Count);
6970

7071
}
7172

@@ -625,6 +626,27 @@ public void SetMethodValue_NullReferenceValue_DoesNotSetValue() {
625626

626627
}
627628

629+
/*==========================================================================================================================
630+
| TEST: MAYBE COMPATIBLE: CORRESPONDING TOPIC: CORRECTLY SET
631+
\-------------------------------------------------------------------------------------------------------------------------*/
632+
/// <summary>
633+
/// Establishes a <see cref="TypeAccessor"/> and based on the <see cref="ContentTypeDescriptorTopicViewModel"/> and
634+
/// confirms that the underlying <see cref="MemberAccessor"/> instances are correctly marked as <see cref="ItemMetadata.
635+
/// MaybeCompatible"/> based on the association with the <see cref="ContentTypeDescriptor"/>.
636+
/// </summary>
637+
[Fact]
638+
public void MaybeCompatible_CorrespondingTopic_CorrectlySet() {
639+
640+
var typeAccessor = TypeAccessorCache.GetTypeAccessor<CustomTopicTopicViewModel>();
641+
642+
Assert.Equal(9, typeAccessor.GetMembers(MemberTypes.Property).Count(m => m.MaybeCompatible));
643+
Assert.Equal(2, typeAccessor.GetMembers(MemberTypes.Property).Count(m => !m.MaybeCompatible));
644+
Assert.Equal(9, typeAccessor.ConstructorParameters.Count(m => m.MaybeCompatible));
645+
Assert.Equal(2, typeAccessor.ConstructorParameters.Count(m => !m.MaybeCompatible));
646+
Assert.True(typeAccessor.GetMember(nameof(CustomTopicTopicViewModel.TopicReference))?.MaybeCompatible);
647+
648+
}
649+
628650
/*==========================================================================================================================
629651
| TEST: SET PROPERTY VALUE: REFLECTION PERFORMANCE
630652
\-------------------------------------------------------------------------------------------------------------------------*/

‎OnTopic.Tests/ViewModels/AttributeDictionaryConstructorTopicViewModel.cs‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/*==============================================================================================================================
2+
| Author Ignia, LLC
3+
| Client Ignia, LLC
4+
| Project Topics Library
5+
\=============================================================================================================================*/
6+
7+
using OnTopic.Attributes;
8+
9+
namespace OnTopic.Tests.ViewModels {
10+
11+
/*============================================================================================================================
12+
| VIEW MODEL: ATTRIBUTE DICTIONARY CONSTRUCTOR
13+
\---------------------------------------------------------------------------------------------------------------------------*/
14+
/// <summary>
15+
/// Provides a strongly-typed data transfer object for testing a constructor with a <see cref="AttributeDictionary"/>.
16+
/// </summary>
17+
/// <remarks>
18+
/// This is a sample class intended for test purposes only; it is not designed for use in a production environment.
19+
/// </remarks>
20+
public record AttributeDictionaryConstructorTopicViewModel: PageTopicViewModel {
21+
22+
/*==========================================================================================================================
23+
| CONSTRUCTOR
24+
\-------------------------------------------------------------------------------------------------------------------------*/
25+
/// <summary>
26+
/// Initializes a new <see cref="AttributeDictionaryConstructorTopicViewModel"/> with an <paramref name="attributes"/>
27+
/// dictionary.
28+
/// </summary>
29+
/// <param name="attributes">An <see cref="AttributeDictionaryConstructorTopicViewModel"/> of attribute values.</param>
30+
public AttributeDictionaryConstructorTopicViewModel(AttributeDictionary attributes) : base(attributes) {
31+
Contract.Requires(attributes, nameof(attributes));
32+
MappedProperty = attributes.GetValue(nameof(MappedProperty));
33+
}
34+
35+
/// <summary>
36+
/// Initializes a new <see cref="AttributeDictionaryConstructorTopicViewModel"/> with no parameters.
37+
/// </summary>
38+
public AttributeDictionaryConstructorTopicViewModel() { }
39+
40+
/*==========================================================================================================================
41+
| PROPERTIES
42+
\-------------------------------------------------------------------------------------------------------------------------*/
43+
public string? MappedProperty { get; init; }
44+
public string? UnmappedProperty { get; init; }
45+
46+
47+
} //Class
48+
} //Namespace

‎OnTopic.Tests/ViewModels/CustomTopicTopicViewModel.cs‎

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
/*==============================================================================================================================
2+
| Author Ignia, LLC
3+
| Client Ignia, LLC
4+
| Project Topics Library
5+
\=============================================================================================================================*/
6+
using OnTopic.Attributes;
7+
using OnTopic.Internal.Reflection;
8+
using OnTopic.Tests.Entities;
9+
10+
namespace OnTopic.Tests.ViewModels {
11+
12+
/*============================================================================================================================
13+
| VIEW MODEL: CUSTOM TOPIC
14+
\---------------------------------------------------------------------------------------------------------------------------*/
15+
/// <summary>
16+
/// Provides a topic view model which maps to the <see cref="CustomTopic"/>.
17+
/// </summary>
18+
/// <remarks>
19+
/// <para>
20+
/// This is a sample class intended for test purposes only; it is not designed for use in a production environment.
21+
/// </para>
22+
/// <para>
23+
/// Typically, topics wouldn't end in "Topic" (outside of <see cref="Topic"/>). Because <see cref="CustomTopic"/> does,
24+
/// the corresponding topic view model must be named <see cref="CustomTopicTopicViewModel"/> (with two <c>Topic</c>).
25+
/// Otherwise, when the <see cref="TypeAccessor"/> attempts to match it to its implied content type, it will strip the
26+
/// <c>TopicViewModel</c>, per conventions, and discover that there isn't a match for <c>Custom</c>. This is a peculiarity
27+
/// of the test case, and not something we'd expect in real-life scenarios.
28+
/// </para>
29+
/// </remarks>
30+
public record CustomTopicTopicViewModel {
31+
32+
/*==========================================================================================================================
33+
| CONSTRUCTOR
34+
\-------------------------------------------------------------------------------------------------------------------------*/
35+
public CustomTopicTopicViewModel(
36+
string title,
37+
string webPath,
38+
string? textAttribute,
39+
bool booleanAttribute,
40+
string booleanAsStringAttribute,
41+
int numericAttribute,
42+
string nonNullableAttribute,
43+
DateTime dateTimeAttribute,
44+
Topic? topicReference,
45+
string? unmatchedMember,
46+
DateTime? isHidden
47+
) {
48+
Title = title;
49+
WebPath = webPath;
50+
TextAttribute = textAttribute;
51+
BooleanAttribute = booleanAttribute;
52+
BooleanAsStringAttribute = booleanAsStringAttribute;
53+
NumericAttribute = numericAttribute;
54+
NonNullableAttribute = nonNullableAttribute;
55+
DateTimeAttribute = dateTimeAttribute;
56+
TopicReference = topicReference;
57+
UnmatchedMember = unmatchedMember;
58+
IsHidden = isHidden;
59+
}
60+
61+
/*==========================================================================================================================
62+
| TITLE
63+
\-------------------------------------------------------------------------------------------------------------------------*/
64+
/// <summary>
65+
/// Provides a reference to the <see cref="Topic.Title"/> property.
66+
/// </summary>
67+
public string Title { get; init; }
68+
69+
/*==========================================================================================================================
70+
| WEB PATH
71+
\-------------------------------------------------------------------------------------------------------------------------*/
72+
/// <summary>
73+
/// Provides the root-relative web path of the Topic, based on an assumption that the root topic is bound to the root of
74+
/// the site.
75+
/// </summary>
76+
public string WebPath { get; init; }
77+
78+
/*==========================================================================================================================
79+
| TEXT ATTRIBUTE
80+
\-------------------------------------------------------------------------------------------------------------------------*/
81+
/// <summary>
82+
/// Provides a text property which is intended to be mapped to a text attribute.
83+
/// </summary>
84+
public string? TextAttribute { get; init; }
85+
86+
/*==========================================================================================================================
87+
| BOOLEAN ATTRIBUTE
88+
\-------------------------------------------------------------------------------------------------------------------------*/
89+
/// <summary>
90+
/// Provides a Boolean property which is intended to be mapped to a Boolean attribute.
91+
/// </summary>
92+
public bool BooleanAttribute { get; init; }
93+
94+
/*==========================================================================================================================
95+
| BOOLEAN AS STRING ATTRIBUTE
96+
\-------------------------------------------------------------------------------------------------------------------------*/
97+
/// <summary>
98+
/// Provides a string property which is intended to be mapped to a Boolean attribute.
99+
/// </summary>
100+
public string BooleanAsStringAttribute { get; init; }
101+
102+
/*==========================================================================================================================
103+
| NUMERIC ATTRIBUTE
104+
\-------------------------------------------------------------------------------------------------------------------------*/
105+
/// <summary>
106+
/// Provides a numeric property which is intended to be mapped to a numeric attribute.
107+
/// </summary>
108+
public int NumericAttribute { get; init; }
109+
110+
/*==========================================================================================================================
111+
| NON-NULLABLE ATTRIBUTE
112+
\-------------------------------------------------------------------------------------------------------------------------*/
113+
/// <summary>
114+
/// Provides a property which does not permit null values as a means of ensuring the business logic is enforced when
115+
/// removing values.
116+
/// </summary>
117+
public string NonNullableAttribute { get; init; }
118+
119+
/*==========================================================================================================================
120+
| DATE/TIME ATTRIBUTE
121+
\-------------------------------------------------------------------------------------------------------------------------*/
122+
/// <summary>
123+
/// Provides a date/time property which is intended to be mapped to a date/time attribute.
124+
/// </summary>
125+
public DateTime DateTimeAttribute { get; init; }
126+
127+
/*==========================================================================================================================
128+
| TOPIC REFERENCE
129+
\-------------------------------------------------------------------------------------------------------------------------*/
130+
/// <summary>
131+
/// Provides a topic reference property which is intended to be mapped to a topic reference.
132+
/// </summary>
133+
public Topic? TopicReference { get; init; }
134+
135+
/*==========================================================================================================================
136+
| UNMATCHED MEMBER
137+
\-------------------------------------------------------------------------------------------------------------------------*/
138+
/// <summary>
139+
/// Provides a member that doesn't match any members of either <see cref="CustomTopic"/> or <see cref="Topic"/>.
140+
/// </summary>
141+
public string? UnmatchedMember { get; init; }
142+
143+
/*==========================================================================================================================
144+
| MISMATCHED MEMBER
145+
\-------------------------------------------------------------------------------------------------------------------------*/
146+
/// <summary>
147+
/// Provides a member that corresponds to a property on <see cref="Topic"/>, but with an incompatible type.
148+
/// </summary>
149+
public DateTime? IsHidden { get; init; }
150+
151+
152+
} //Class
153+
} //Namespace

‎OnTopic/Internal/Reflection/ItemMetadata.cs‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,36 @@ internal ItemConfiguration Configuration {
155155
/// </summary>
156156
internal bool IsConvertible { get; init; }
157157

158+
/*==========================================================================================================================
159+
| MAYBE COMPATIBLE?
160+
\-------------------------------------------------------------------------------------------------------------------------*/
161+
/// <summary>
162+
/// Determine if the item corresponds to a member on the <see cref="Topic"/> class (or derivative) associated with the
163+
/// parent <see cref="TypeAccessor"/>.
164+
/// </summary>
165+
/// <remarks>
166+
/// <para>
167+
/// The <see cref="MaybeCompatible"/> does not <i>guarantee</i> that a corresponding compatible property will be found.
168+
/// First, it cannot know what the source <see cref="Topic"/> will be, so it relies only evaluates topics who match the
169+
/// naming convention (e.g., <c>{ContentType}TopicViewModel</c> maps to <c>{ContentType}</c>), and otherwise falls back
170+
/// to <see cref="Topic"/>; if a model is mapped to a different derivative of <see cref="Topic"/>, additional matches
171+
/// may be available. Second, it evaluates whether the <see cref="Topic"/> member is of a type that can be assigned to
172+
/// the model member, or otherwise <i>converted</i>—i.e., that the <see cref="Topic"/> member is a string, and the model
173+
/// member is one of the <see cref="AttributeValueConverter.ConvertibleTypes"/>. This provides a hint for avoiding type
174+
/// checks in cases we don't expect to be compatible, while deferring to the caller to provide more sophisticated checks
175+
/// based on the actual <see cref="Topic"/>.
176+
/// </para>
177+
/// <para>
178+
/// The <see cref="MaybeCompatible"/> property must be set from the <see cref="TypeAccessor"/>, which is best positioned
179+
/// to efficiently determine the corresponding attributes. To do this, it must rely on attributes accessed via the <see
180+
/// cref="Configuration"/> property. As a result, the <see cref="MaybeCompatible"/> must be set <i>after</i> the <see
181+
/// cref="ItemMetadata"/> instance has been constructed, instead of during initialization like the other properties.
182+
/// That isn't ideal, but isn't easy to avoid without making each <see cref="ItemMetadata"/> aware of the parent <see
183+
/// cref="TypeAccessor"/>.
184+
/// </para>
185+
/// </remarks>
186+
internal bool MaybeCompatible { get; set; }
187+
158188
/*==========================================================================================================================
159189
| CUSTOM ATTRIBUTES
160190
\-------------------------------------------------------------------------------------------------------------------------*/

0 commit comments

Comments
 (0)