Skip to content

Commit ab4253d

Browse files
JeremyCaneyJeremyCaney
committed
Merge branch 'feature/ITypeLookupService.Lookup-fallbacks' into develop
Previously, the `ITypeLookupService` didn't directly handle fallbacks, and so implementations had instituted their own logic for handling alternatives and defaults. Namely, the `StaticTypeLookupService`—which most implementations use as a base class—accepted a `DefaultType` which would be returned if a match couldn't be found. In addition, more recently, the `DynamicTopicViewModelLookupService` ((902b380) and `TopicViewModelLookupService` (68a8635) overrode `Lookup()` to fallback to an alternate convention of `ViewModel`; and both `DefaultTopicLookupService` (aac3e97) and `DynamicTopicLookupService` (cd9e540) overrode `Lookup()` to fallback to `AttributeDescriptor` if the `typeName` ended in the `AttributeDescriptor` convention, but a specific type could not be found. While this fallback logic was valuable, it was also confusing. With the `DefaultType`, this meant that callers couldn't assume that a `null` result meant a miss, and needed to explicitly check the return type against the `typeName` to ensure it matched. With the `Lookup()` overrides, this meant repeating logic between implementations if they didn't share a type-specific base class. Further, all of these meant that callers had little control or transparency over the fallback process. This could result, for instance, in an alternate `ITypeLookupService` not providing the same fallbacks, which could be a confusing behavior. To mitigate that, all of the above have been removed, and instead replaced with a new `ITypeLookupService.Lookup()` signature that accepts a `param string[] typeNames` instead of a `string typeName`, thus allowing the _callers_ to specify the alternatives or logical defaults that _they_ are aware of and can accept, instead of relying on the `ITypeLookupService`—which they have no insight into or control over—to provide one on their behalf. This makes a lot more sense in terms of dependency management. So, for instance, the `TopicFactory.Create()` method now handles the fallback to `AttributeDescriptor` (if the `ContentType` ends in `AttributeDescriptor` but a specific type cannot be found) as well as `Topic`. And, similarly, the `TopicMappingService` now specifies a preference for `{ContentType}TopicViewModel`, but with alternative of `{ContentType}ViewModel`. As this update not only changes the constructor of `StaticTypeLookupService`, but also changes the signature of the `ITypeLookupService.Lookup()` method, it is a breaking change. External libraries, such as OnTopic Editor, will need to be updated. Customer implementations will also need to be updated if they a) pass a `DefaultType` to `StaticTypeLookupService`, b) override the `Lookup()` method, or c) rely on the `DefaultType`. (No customer implementations rely on the new `ViewModel` or `AttributeDescriptor` alternatives names, so there's no worries there.)
2 parents 69caaa2 + 33132b9 commit ab4253d

13 files changed

+60
-206
lines changed

‎OnTopic.Tests/ITypeLookupServiceTest.cs

Lines changed: 5 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@ public class ITypeLookupServiceTest {
2828
\-------------------------------------------------------------------------------------------------------------------------*/
2929
/// <summary>
3030
/// Assembles a new <see cref="CompositeTypeLookupService"/> with two instances of a <see cref="ITypeLookupService"/> and
31-
/// confirms that it returns the expected <see cref="Type"/> for a <see cref="ITypeLookupService.Lookup(String)"/> query.
31+
/// confirms that it returns the expected <see cref="Type"/> for a <see cref="ITypeLookupService.Lookup(String[])"/>
32+
/// query.
3233
/// </summary>
3334
[TestMethod]
3435
public void Composite_LookupValidType_ReturnsType() {
@@ -39,24 +40,7 @@ public void Composite_LookupValidType_ReturnsType() {
3940

4041
Assert.AreEqual(typeof(SlideshowTopicViewModel), compositeLookup.Lookup(nameof(SlideshowTopicViewModel)));
4142
Assert.AreEqual(typeof(MapToParentTopicViewModel), compositeLookup.Lookup(nameof(MapToParentTopicViewModel)));
42-
Assert.AreEqual(typeof(Object), compositeLookup.Lookup(nameof(Topic)));
43-
44-
}
45-
46-
/*==========================================================================================================================
47-
| TEST: DYNAMIC TOPIC LOOKUP SERVICE: LOOKUP MISSING ATTRIBUTE DESCRIPTOR: RETURNS ATTRIBUTE DESCRIPTOR
48-
\-------------------------------------------------------------------------------------------------------------------------*/
49-
/// <summary>
50-
/// Assembles a new <see cref="DynamicTopicLookupService"/> and requests a missing attribute type; confirms it correctly
51-
/// falls back to the expected <see cref="AttributeDescriptor"/> as a logical default.
52-
/// </summary>
53-
[TestMethod]
54-
public void DynamicTopicLookupService_LookupMissingAttributeDescriptor_ReturnsAttributeDescriptor() {
55-
56-
var lookupService = new DynamicTopicLookupService();
57-
var attributeType = lookupService.Lookup("ArbitraryAttributeDescriptor");
58-
59-
Assert.AreEqual(typeof(AttributeDescriptor), attributeType);
43+
Assert.AreEqual(null, compositeLookup.Lookup(nameof(Topic)));
6044

6145
}
6246

@@ -71,29 +55,12 @@ public void DynamicTopicLookupService_LookupMissingAttributeDescriptor_ReturnsAt
7155
public void DynamicTopicViewModelLookupService_LookupTopicViewModel_ReturnsFallbackViewModel() {
7256

7357
var lookupService = new DynamicTopicViewModelLookupService();
74-
var topicViewModel = lookupService.Lookup("FallbackTopicViewModel");
58+
var topicViewModel = lookupService.Lookup("FallbackTopicViewModel", "FallbackViewModel");
7559

7660
Assert.AreEqual(typeof(FallbackViewModel), topicViewModel);
7761

7862
}
7963

80-
/*==========================================================================================================================
81-
| TEST: DEFAULT TOPIC LOOKUP SERVICE: LOOKUP MISSING ATTRIBUTE DESCRIPTOR: RETURNS ATTRIBUTE DESCRIPTOR
82-
\-------------------------------------------------------------------------------------------------------------------------*/
83-
/// <summary>
84-
/// Assembles a new <see cref="DefaultTopicLookupService"/> and requests a missing attribute type; confirms it correctly
85-
/// falls back to the expected <see cref="AttributeDescriptor"/> as a logical default.
86-
/// </summary>
87-
[TestMethod]
88-
public void DefaultTopicLookupService_LookupMissingAttributeDescriptor_ReturnsAttributeDescriptor() {
89-
90-
var lookupService = new DefaultTopicLookupService();
91-
var attributeType = lookupService.Lookup("ArbitraryAttributeDescriptor");
92-
93-
Assert.AreEqual(typeof(AttributeDescriptor), attributeType);
94-
95-
}
96-
9764
/*==========================================================================================================================
9865
| TEST: DEFAULT TOPIC VIEW MODEL LOOKUP SERVICE: LOOKUP TOPIC VIEW MODEL: RETURNS FALLBACK VIEW MODEL
9966
\-------------------------------------------------------------------------------------------------------------------------*/
@@ -105,7 +72,7 @@ public void DefaultTopicLookupService_LookupMissingAttributeDescriptor_ReturnsAt
10572
public void TopicViewModelLookupService_LookupTopicViewModel_ReturnsFallbackViewModel() {
10673

10774
var lookupService = new FakeViewModelLookupService();
108-
var topicViewModel = lookupService.Lookup("FallbackTopicViewModel");
75+
var topicViewModel = lookupService.Lookup("FallbackTopicViewModel", "FallbackViewModel");
10976

11077
Assert.AreEqual(typeof(FallbackViewModel), topicViewModel);
11178

‎OnTopic.Tests/TestDoubles/FakeViewModelLookupService.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ public class FakeViewModelLookupService: TopicViewModelLookupService {
2828
/// Instantiates a new instance of the <see cref="FakeViewModelLookupService"/>.
2929
/// </summary>
3030
/// <returns>A new instance of the <see cref="FakeViewModelLookupService"/>.</returns>
31-
public FakeViewModelLookupService() : base(null, typeof(object)) {
31+
public FakeViewModelLookupService() : base() {
3232

3333
/*------------------------------------------------------------------------------------------------------------------------
3434
| Add test specific view models

‎OnTopic.ViewModels/TopicViewModelLookupService.cs

Lines changed: 1 addition & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,7 @@ public class TopicViewModelLookupService : StaticTypeLookupService {
3030
/// cref="MemberInfo.Name"/>; if they are not, they will be removed.
3131
/// </remarks>
3232
/// <param name="types">The list of <see cref="Type"/> instances to expose as part of this service.</param>
33-
/// <param name="defaultType">The default type to return if no match can be found. Defaults to object.</param>
34-
public TopicViewModelLookupService(IEnumerable<Type>? types = null, Type? defaultType = null) :
35-
base(types, defaultType?? typeof(TopicViewModel)) {
33+
public TopicViewModelLookupService(IEnumerable<Type>? types = null) : base(types) {
3634

3735
/*------------------------------------------------------------------------------------------------------------------------
3836
| Ensure local view models are accounted for
@@ -62,32 +60,5 @@ public TopicViewModelLookupService(IEnumerable<Type>? types = null, Type? defaul
6260

6361
}
6462

65-
/*==========================================================================================================================
66-
| METHOD: LOOKUP
67-
\-------------------------------------------------------------------------------------------------------------------------*/
68-
/// <inheritdoc/>
69-
/// <remarks>
70-
/// The <see cref="TopicViewModelLookupService"/> version of <see cref="Lookup(String)"/> will first look for
71-
/// the exact <paramref name="typeName"/>. If that fails, and the <paramref name="typeName"/> ends with <c>TopicViewModel
72-
/// </c>, then it will automatically fall back to look for the <paramref name="typeName"/> with the <c>ViewModel</c>
73-
/// suffix. If that cannot be found, it will return the <see cref="StaticTypeLookupService.DefaultType"/> of <see cref="
74-
/// TopicViewModel"/>. This allows implementors to use the shorter name, if preferred, without breaking compatibility with
75-
/// implementations which default to looking for <c>TopicViewModel</c>, such as the <see cref="TopicMappingService"/>.
76-
/// While this convention is not used by the <see cref="ViewModels"/>, this fallback provides support for derived classes
77-
/// which may prefer that convention.
78-
/// </remarks>
79-
public override Type? Lookup(string typeName) {
80-
if (typeName is null) {
81-
return DefaultType;
82-
}
83-
else if (Contains(typeName)) {
84-
return base.Lookup(typeName);
85-
}
86-
else if (typeName.EndsWith("TopicViewModel", StringComparison.OrdinalIgnoreCase)) {
87-
return base.Lookup(typeName.Replace("TopicViewModel", "ViewModel", StringComparison.CurrentCultureIgnoreCase));
88-
}
89-
return DefaultType;
90-
}
91-
9263
} //Class
9364
} //Namespace

‎OnTopic/Lookup/CompositeTypeLookupService.cs

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,16 @@ public CompositeTypeLookupService(params ITypeLookupService[] typeLookupServices
5252
| METHOD: LOOKUP
5353
\-------------------------------------------------------------------------------------------------------------------------*/
5454
/// <inheritdoc/>
55-
public Type? Lookup(string typeName) {
56-
var type = typeof(Object);
57-
foreach (var typeLookupService in _typeLookupServices) {
58-
type = typeLookupService.Lookup(typeName);
59-
if (type is not null && type.Name.Equals(typeName, StringComparison.OrdinalIgnoreCase)) {
60-
return type;
55+
public Type? Lookup(params string[] typeNames) {
56+
var type = typeof(object);
57+
if (typeNames is not null) {
58+
foreach (var typeName in typeNames) {
59+
foreach (var typeLookupService in _typeLookupServices) {
60+
type = typeLookupService.Lookup(typeName);
61+
if (type is not null && type.Name.Equals(typeName, StringComparison.OrdinalIgnoreCase)) {
62+
return type;
63+
}
64+
}
6165
}
6266
}
6367
//Default to default return type of last query

‎OnTopic/Lookup/DefaultTopicLookupService.cs

Lines changed: 1 addition & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,7 @@ public class DefaultTopicLookupService: StaticTypeLookupService {
3030
/// cref="MemberInfo.Name"/>; if they are not, they will be removed.
3131
/// </remarks>
3232
/// <param name="types">The list of <see cref="Type"/> instances to expose as part of this service.</param>
33-
/// <param name="defaultType">The default type to return if no match can be found. Defaults to object.</param>
34-
public DefaultTopicLookupService(IEnumerable<Type>? types = null, Type? defaultType = null) :
35-
base(types, defaultType?? typeof(Topic)) {
33+
public DefaultTopicLookupService(IEnumerable<Type>? types = null) : base(types) {
3634

3735
/*------------------------------------------------------------------------------------------------------------------------
3836
| Ensure editor types are accounted for
@@ -42,30 +40,5 @@ public DefaultTopicLookupService(IEnumerable<Type>? types = null, Type? defaultT
4240

4341
}
4442

45-
/*==========================================================================================================================
46-
| METHOD: LOOKUP
47-
\-------------------------------------------------------------------------------------------------------------------------*/
48-
/// <inheritdoc/>
49-
/// <remarks>
50-
/// The <see cref="DefaultTopicLookupService"/> version of <see cref="Lookup(String)"/> will automatically fall back to
51-
/// <see cref="AttributeDescriptor"/> if the <paramref name="typeName"/> ends with <c>AttributeDescriptor</c>, but a
52-
/// <see cref="AttributeDescriptor"/> with the specified name cannot be found. This accounts for the fact that strongly
53-
/// typed <see cref="AttributeDescriptor"/> classes are expected to be in external plugins which are not statically
54-
/// registered with the <see cref="DefaultTopicLookupService"/>. In that case, the base <see cref="AttributeDescriptor"/>
55-
/// class will provide access to the attributes needed by most applications, including the core OnTopic library.
56-
/// </remarks>
57-
public override Type? Lookup(string typeName) {
58-
if (typeName is null) {
59-
return DefaultType;
60-
}
61-
else if (Contains(typeName)) {
62-
return base.Lookup(typeName);
63-
}
64-
else if (typeName.EndsWith("AttributeDescriptor", StringComparison.OrdinalIgnoreCase)) {
65-
return typeof(AttributeDescriptor);
66-
}
67-
return DefaultType;
68-
}
69-
7043
} //Class
7144
} //Namespace

‎OnTopic/Lookup/DynamicTopicBindingModelLookupService.cs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,7 @@ public class DynamicTopicBindingModelLookupService : DynamicTypeLookupService {
2323
/// <summary>
2424
/// Establishes a new instance of a <see cref="DynamicTopicBindingModelLookupService"/>.
2525
/// </summary>
26-
public DynamicTopicBindingModelLookupService() : base(
27-
t => typeof(ITopicBindingModel).IsAssignableFrom(t),
28-
typeof(object)
29-
) { }
26+
public DynamicTopicBindingModelLookupService() : base(t => typeof(ITopicBindingModel).IsAssignableFrom(t)) { }
3027

3128
} //Class
3229
} //Namespace

‎OnTopic/Lookup/DynamicTopicLookupService.cs

Lines changed: 1 addition & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -24,35 +24,8 @@ public class DynamicTopicLookupService : DynamicTypeLookupService {
2424
/// Establishes a new instance of a <see cref="DynamicTopicLookupService"/>.
2525
/// </summary>
2626
public DynamicTopicLookupService() : base(
27-
t => typeof(Topic).IsAssignableFrom(t),
28-
typeof(Topic)
27+
t => typeof(Topic).IsAssignableFrom(t)
2928
) { }
3029

31-
/*==========================================================================================================================
32-
| METHOD: LOOKUP
33-
\-------------------------------------------------------------------------------------------------------------------------*/
34-
/// <inheritdoc/>
35-
/// <remarks>
36-
/// The <see cref="DynamicTopicLookupService"/> version of <see cref="Lookup(String)"/> will automatically fall back to
37-
/// <see cref="AttributeDescriptor"/> if the <paramref name="typeName"/> ends with <c>AttributeDescriptor</c>, but a
38-
/// <see cref="AttributeDescriptor"/> with the specified name cannot be found. This accounts for the fact that strongly
39-
/// typed <see cref="AttributeDescriptor"/> classes are expected to be in external plugins which may not be available
40-
/// unless the current application is configured to use the OnTopic Editor. In that case, the base <see cref="
41-
/// AttributeDescriptor"/> class will provide access to the attributes needed by most applications, including the core
42-
/// OnTopic library.
43-
/// </remarks>
44-
public override Type? Lookup(string typeName) {
45-
if (typeName is null) {
46-
return DefaultType;
47-
}
48-
else if (Contains(typeName)) {
49-
return base.Lookup(typeName);
50-
}
51-
else if (typeName.EndsWith("AttributeDescriptor", StringComparison.OrdinalIgnoreCase)) {
52-
return typeof(AttributeDescriptor);
53-
}
54-
return DefaultType;
55-
}
56-
5730
} //Class
5831
} //Namespace

‎OnTopic/Lookup/DynamicTopicViewModelLookupService.cs

Lines changed: 1 addition & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
| Project Topics Library
55
\=============================================================================================================================*/
66
using System;
7-
using OnTopic.Mapping;
87

98
namespace OnTopic.Lookup {
109

@@ -24,34 +23,8 @@ public class DynamicTopicViewModelLookupService : DynamicTypeLookupService {
2423
/// Establishes a new instance of a <see cref="DynamicTopicLookupService"/>.
2524
/// </summary>
2625
public DynamicTopicViewModelLookupService() : base(
27-
t => t.Name.EndsWith("ViewModel", StringComparison.OrdinalIgnoreCase),
28-
typeof(object)
26+
t => t.Name.EndsWith("ViewModel", StringComparison.OrdinalIgnoreCase)
2927
) { }
3028

31-
/*==========================================================================================================================
32-
| METHOD: LOOKUP
33-
\-------------------------------------------------------------------------------------------------------------------------*/
34-
/// <inheritdoc/>
35-
/// <remarks>
36-
/// The <see cref="DynamicTopicViewModelLookupService"/> version of <see cref="Lookup(String)"/> will first look for
37-
/// the exact <paramref name="typeName"/>. If that fails, and the <paramref name="typeName"/> ends with <c>TopicViewModel
38-
/// </c>, then it will automatically fall back to look for the <paramref name="typeName"/> with the <c>ViewModel</c>
39-
/// suffix. If that cannot be found, it will return the <see cref="StaticTypeLookupService.DefaultType"/> of <see cref="
40-
/// Object"/>. This allows implementors to use the shorter name, if preferred, without breaking compatibility with
41-
/// implementations which default to looking for <c>TopicViewModel</c>, such as the <see cref="TopicMappingService"/>.
42-
/// </remarks>
43-
public override Type? Lookup(string typeName) {
44-
if (typeName is null) {
45-
return DefaultType;
46-
}
47-
else if (Contains(typeName)) {
48-
return base.Lookup(typeName);
49-
}
50-
else if (typeName.EndsWith("TopicViewModel", StringComparison.OrdinalIgnoreCase)) {
51-
return base.Lookup(typeName.Replace("TopicViewModel", "ViewModel", StringComparison.CurrentCultureIgnoreCase));
52-
}
53-
return DefaultType;
54-
}
55-
5629
} //Class
5730
} //Namespace

‎OnTopic/Lookup/DynamicTypeLookupService.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,7 @@ public class DynamicTypeLookupService : StaticTypeLookupService {
2525
/// optionally, a default <see cref="Type"/> object to return if none is specified.
2626
/// </summary>
2727
/// <param name="predicate">The search condition to use to identify target classes.</param>
28-
/// <param name="defaultType">The default type to return if no match can be found. Defaults to object.</param>
29-
public DynamicTypeLookupService(Func<Type, bool> predicate, Type? defaultType = null) : base(null, defaultType) {
28+
public DynamicTypeLookupService(Func<Type, bool> predicate) : base() {
3029

3130
/*------------------------------------------------------------------------------------------------------------------------
3231
| Find target classes

‎OnTopic/Lookup/ITypeLookupService.cs

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,21 @@ public interface ITypeLookupService {
2727
| METHOD: LOOKUP
2828
\-------------------------------------------------------------------------------------------------------------------------*/
2929
/// <summary>
30-
/// Gets the requested <see cref="Type"/>.
30+
/// Attempts to retrieve a <see cref="Type"/> based on one or more supplied <paramref name="typeNames"/>.
3131
/// </summary>
32-
/// <param name="typeName">The name of the <see cref="Type"/> to retrieve.</param>
33-
Type? Lookup(string typeName);
32+
/// <remarks>
33+
/// No matter how many <paramref name="typeNames"/> are entered, at most one <see cref="Type"/> will be returned. Each
34+
/// subsequent <paramref name="typeNames"/> is treated as a fallback in case the previous one cannot be located. As such,
35+
/// the <paramref name="typeNames"/> offers a way for callers to provide a prioritized list of fallbacks. This is useful
36+
/// for scenarios where there are multiple accepted naming conventions, or there's a global default that can be accepted.
37+
/// </remarks>
38+
/// <param name="typeNames">
39+
/// The name of the <see cref="Type"/> to retrieve. If multiple names are supplied, then the first match is returned.
40+
/// </param>
41+
/// <returns>
42+
/// A <see cref="Type"/> corresponding to one of the specified <paramref name="typeNames"/>, if available.
43+
/// </returns>
44+
Type? Lookup(params string[] typeNames);
3445

3546
} //Class
3647
} //Namespace

0 commit comments

Comments
 (0)