-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Allow users to specify resolution method for handlers, effects, and services #1870
Changes from 4 commits
e3a8d31
e33a440
2eaa4ba
76391ff
3d42cf8
30a8a35
75dd396
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,164 @@ | ||
using System; | ||
using System.Collections.Generic; | ||
using NUnit.Framework; | ||
|
||
namespace Xamarin.Forms.Core.UnitTests | ||
{ | ||
[TestFixture] | ||
public class DependencyResolutionTests : BaseTestFixture | ||
{ | ||
class MockElement { } | ||
|
||
class MockElementRenderer : IRegisterable { } | ||
|
||
class MockRendererWithParam : IRegisterable | ||
{ | ||
readonly object _aParameter; | ||
|
||
public MockRendererWithParam(object aParameter) | ||
{ | ||
_aParameter = aParameter ?? throw new ArgumentNullException(nameof(aParameter)); | ||
} | ||
} | ||
|
||
class MockEffect : Effect | ||
{ | ||
protected override void OnAttached() | ||
{ | ||
throw new NotImplementedException(); | ||
} | ||
|
||
protected override void OnDetached() | ||
{ | ||
throw new NotImplementedException(); | ||
} | ||
} | ||
|
||
interface IMockService { } | ||
|
||
class MockServiceImpl : IMockService { } | ||
|
||
// ReSharper disable once ClassNeverInstantiated.Local | ||
// (Just testing the type registration, don't need an instance) | ||
class MockServiceImpl2 : IMockService { } | ||
|
||
class MockContainer | ||
{ | ||
readonly Dictionary<Type, object> _services; | ||
readonly Dictionary<Type, Func<object, object>> _factories; | ||
|
||
public MockContainer() | ||
{ | ||
_services = new Dictionary<Type, object>(); | ||
_factories = new Dictionary<Type, Func<object, object>>(); | ||
} | ||
|
||
public void Register(Type type, object result) | ||
{ | ||
_services.Add(type, result); | ||
} | ||
|
||
public void Register(Type type, Func<object, object> factory) | ||
{ | ||
_factories.Add(type, factory); | ||
} | ||
|
||
public object Resolve(Type type, params object[] args) | ||
{ | ||
if (_services.ContainsKey(type)) | ||
{ | ||
return _services[type]; | ||
} | ||
|
||
if (_factories.ContainsKey(type)) | ||
{ | ||
return _factories[type].Invoke(args[0]); | ||
} | ||
|
||
return null; | ||
} | ||
} | ||
|
||
MockContainer _container; | ||
|
||
[SetUp] | ||
public void SetUp() | ||
{ | ||
_container = new MockContainer(); | ||
|
||
object ResolveDelegate(Type type, object[] args) => _container.Resolve(type, args); | ||
|
||
Internals.DependencyResolver.ResolveUsing(ResolveDelegate); | ||
} | ||
|
||
[Test] | ||
public void GetHandlerFromContainer() | ||
{ | ||
Internals.Registrar.Registered.Register(typeof(MockElement), typeof(MockElementRenderer)); | ||
var renderer = new MockElementRenderer(); | ||
_container.Register(typeof(MockElementRenderer), renderer); | ||
var result = Internals.Registrar.Registered.GetHandler(typeof(MockElement)); | ||
var typedRenderer = (MockElementRenderer)result; | ||
|
||
Assert.That(typedRenderer, Is.SameAs(renderer)); | ||
} | ||
|
||
[Test] | ||
public void GetEffectFromContainer() | ||
{ | ||
string effectName = "anEffect"; | ||
Internals.Registrar.Effects[effectName] = typeof(MockEffect); | ||
var effect = new MockEffect(); | ||
_container.Register(typeof(MockEffect), effect); | ||
var result = Effect.Resolve(effectName); | ||
|
||
Assert.That(result, Is.SameAs(effect)); | ||
} | ||
|
||
[Test] | ||
public void GetServiceFromContainer() | ||
{ | ||
MockServiceImpl impl = new MockServiceImpl(); | ||
_container.Register(typeof(IMockService), impl); | ||
DependencyService.Register<MockServiceImpl>(); | ||
var result = DependencyService.Resolve<IMockService>(); | ||
|
||
Assert.That(result, Is.SameAs(impl)); | ||
} | ||
|
||
[Test] | ||
public void PreferServiceTypeFromContainer() | ||
{ | ||
MockServiceImpl impl = new MockServiceImpl(); | ||
_container.Register(typeof(IMockService), impl); | ||
DependencyService.Register<IMockService, MockServiceImpl2>(); | ||
var result = DependencyService.Resolve<IMockService>(); | ||
|
||
Assert.That(result, Is.SameAs(impl)); | ||
} | ||
|
||
[Test] | ||
public void FallbackOnDependencyServiceIfNotInContainer() | ||
{ | ||
DependencyService.Register<MockServiceImpl>(); | ||
var result = DependencyService.Resolve<IMockService>(); | ||
|
||
Assert.That(result, Is.Not.Null); | ||
} | ||
|
||
[Test] | ||
public void HandlerWithParameter() | ||
{ | ||
Internals.Registrar.Registered.Register(typeof(MockElement), typeof(MockRendererWithParam)); | ||
|
||
_container.Register(typeof(MockRendererWithParam), (o) => new MockRendererWithParam(o)); | ||
|
||
var context = "Pretend this is an Android context"; | ||
|
||
var result = Internals.Registrar.Registered.GetHandler(typeof(MockElement), context); | ||
var typedRenderer = (MockRendererWithParam)result; | ||
|
||
Assert.That(typedRenderer, Is.InstanceOf(typeof(MockRendererWithParam))); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
using System; | ||
using System.Linq; | ||
using System.Reflection; | ||
|
||
namespace Xamarin.Forms.Internals | ||
{ | ||
public static class DependencyResolver | ||
{ | ||
public delegate object ResolveDelegate(Type type, params object[] args); | ||
|
||
static ResolveDelegate Resolver { get; set; } | ||
|
||
public static void ResolveUsing(ResolveDelegate resolveDelegate) | ||
{ | ||
Resolver = resolveDelegate; | ||
} | ||
|
||
public static void ResolveUsing(Func<Type, object> resolveFunc) | ||
{ | ||
object ResolveDelegate(Type type, object[] args) => resolveFunc.Invoke(type); | ||
Resolver = ResolveDelegate; | ||
} | ||
|
||
internal static object Resolve(Type type, params object[] args) | ||
{ | ||
return Resolver?.Invoke(type, args); | ||
} | ||
|
||
internal static object ForceResolve(Type type, params object[] args) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggest There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I went with |
||
{ | ||
var result = Resolve(type, args); | ||
|
||
if (result != null) return result; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggest we throw if Type is not assignable from the result. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
|
||
if (args.Length > 0) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure what case the following code handles; Suggest we add unit tests to cover 35:46. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The unit test at https://github.com/xamarin/Xamarin.Forms/pull/1870/files#diff-2b39f73ef46da5ad2d33b646bce4def9R150 covers this. |
||
{ | ||
// This is by no means a general solution to matching with the correct constructor, but it'll | ||
// do for finding Android renderers which need Context (vs older custom renderers which may still use | ||
// parameterless constructors) | ||
if (type.GetTypeInfo().DeclaredConstructors.Any(info => info.GetParameters().Length == args.Length)) | ||
{ | ||
return Activator.CreateInstance(type, args); | ||
} | ||
} | ||
|
||
return Activator.CreateInstance(type); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,11 +20,10 @@ internal Effect() | |
|
||
public static Effect Resolve(string name) | ||
{ | ||
Type effectType; | ||
Effect result = null; | ||
if (Internals.Registrar.Effects.TryGetValue(name, out effectType)) | ||
if (Internals.Registrar.Effects.TryGetValue(name, out Type effectType)) | ||
{ | ||
result = (Effect)Activator.CreateInstance(effectType); | ||
result = DependencyResolver.ForceResolve(effectType) as Effect; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggest considering cast instead of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep. Done. |
||
} | ||
|
||
if (result == null) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
<Type Name="DependencyResolver+ResolveDelegate" FullName="Xamarin.Forms.Internals.DependencyResolver+ResolveDelegate"> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We might consider if some dev time now drafting some isense docs might preempt questions/confusion in the forums. |
||
<TypeSignature Language="C#" Value="public delegate object DependencyResolver.ResolveDelegate(Type type, object[] args);" /> | ||
<TypeSignature Language="ILAsm" Value=".class nested public auto ansi sealed DependencyResolver/ResolveDelegate extends System.MulticastDelegate" /> | ||
<AssemblyInfo> | ||
<AssemblyName>Xamarin.Forms.Core</AssemblyName> | ||
<AssemblyVersion>2.0.0.0</AssemblyVersion> | ||
</AssemblyInfo> | ||
<Base> | ||
<BaseTypeName>System.Delegate</BaseTypeName> | ||
</Base> | ||
<Parameters> | ||
<Parameter Name="type" Type="System.Type" /> | ||
<Parameter Name="args" Type="System.Object[]"> | ||
<Attributes> | ||
<Attribute> | ||
<AttributeName>System.ParamArray</AttributeName> | ||
</Attribute> | ||
</Attributes> | ||
</Parameter> | ||
</Parameters> | ||
<ReturnValue> | ||
<ReturnType>System.Object</ReturnType> | ||
</ReturnValue> | ||
<Docs> | ||
<param name="type">To be added.</param> | ||
<param name="args">To be added.</param> | ||
<summary>To be added.</summary> | ||
<returns>To be added.</returns> | ||
<remarks>To be added.</remarks> | ||
</Docs> | ||
</Type> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
<Type Name="DependencyResolver" FullName="Xamarin.Forms.Internals.DependencyResolver"> | ||
<TypeSignature Language="C#" Value="public static class DependencyResolver" /> | ||
<TypeSignature Language="ILAsm" Value=".class public auto ansi abstract sealed beforefieldinit DependencyResolver extends System.Object" /> | ||
<AssemblyInfo> | ||
<AssemblyName>Xamarin.Forms.Core</AssemblyName> | ||
<AssemblyVersion>2.0.0.0</AssemblyVersion> | ||
</AssemblyInfo> | ||
<Base> | ||
<BaseTypeName>System.Object</BaseTypeName> | ||
</Base> | ||
<Interfaces /> | ||
<Docs> | ||
<summary>To be added.</summary> | ||
<remarks>To be added.</remarks> | ||
</Docs> | ||
<Members> | ||
<Member MemberName="ResolveUsing"> | ||
<MemberSignature Language="C#" Value="public static void ResolveUsing (Func<Type,object> resolveFunc);" /> | ||
<MemberSignature Language="ILAsm" Value=".method public static hidebysig void ResolveUsing(class System.Func`2<class System.Type, object> resolveFunc) cil managed" /> | ||
<MemberType>Method</MemberType> | ||
<AssemblyInfo> | ||
<AssemblyVersion>2.0.0.0</AssemblyVersion> | ||
</AssemblyInfo> | ||
<ReturnValue> | ||
<ReturnType>System.Void</ReturnType> | ||
</ReturnValue> | ||
<Parameters> | ||
<Parameter Name="resolveFunc" Type="System.Func<System.Type,System.Object>" /> | ||
</Parameters> | ||
<Docs> | ||
<param name="resolveFunc">To be added.</param> | ||
<summary>To be added.</summary> | ||
<remarks>To be added.</remarks> | ||
</Docs> | ||
</Member> | ||
<Member MemberName="ResolveUsing"> | ||
<MemberSignature Language="C#" Value="public static void ResolveUsing (Xamarin.Forms.Internals.DependencyResolver.ResolveDelegate resolveDelegate);" /> | ||
<MemberSignature Language="ILAsm" Value=".method public static hidebysig void ResolveUsing(class Xamarin.Forms.Internals.DependencyResolver/ResolveDelegate resolveDelegate) cil managed" /> | ||
<MemberType>Method</MemberType> | ||
<AssemblyInfo> | ||
<AssemblyVersion>2.0.0.0</AssemblyVersion> | ||
</AssemblyInfo> | ||
<ReturnValue> | ||
<ReturnType>System.Void</ReturnType> | ||
</ReturnValue> | ||
<Parameters> | ||
<Parameter Name="resolveDelegate" Type="Xamarin.Forms.Internals.DependencyResolver+ResolveDelegate" /> | ||
</Parameters> | ||
<Docs> | ||
<param name="resolveDelegate">To be added.</param> | ||
<summary>To be added.</summary> | ||
<remarks>To be added.</remarks> | ||
</Docs> | ||
</Member> | ||
</Members> | ||
</Type> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest we use
Func<Type, object[], object>
; Suggest we conserve the number of new Types introduced at the expense syntactic sugar at our call site; We can just put the args into a new object[] and save ourselves a bit of public surface area, no?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.