Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Extend PartsOf to mock non-virtual methods implementing an i… #700

Merged
merged 1 commit into from
Aug 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/NSubstitute/Core/IProxyFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@ namespace NSubstitute.Core;

public interface IProxyFactory
{
object GenerateProxy(ICallRouter callRouter, Type typeToProxy, Type[]? additionalInterfaces, object?[]? constructorArguments);
object GenerateProxy(ICallRouter callRouter, Type typeToProxy, Type[]? additionalInterfaces, bool isPartial, object?[]? constructorArguments);
}
8 changes: 4 additions & 4 deletions src/NSubstitute/Core/SubstituteFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ public class SubstituteFactory(ISubstituteStateFactory substituteStateFactory, I
/// <returns></returns>
public object Create(Type[] typesToProxy, object?[] constructorArguments)
{
return Create(typesToProxy, constructorArguments, callBaseByDefault: false);
return Create(typesToProxy, constructorArguments, callBaseByDefault: false, isPartial: false);
}

/// <summary>
Expand All @@ -33,10 +33,10 @@ public object CreatePartial(Type[] typesToProxy, object?[] constructorArguments)
throw new CanNotPartiallySubForInterfaceOrDelegateException(primaryProxyType);
}

return Create(typesToProxy, constructorArguments, callBaseByDefault: true);
return Create(typesToProxy, constructorArguments, callBaseByDefault: true, isPartial: true);
}

private object Create(Type[] typesToProxy, object?[] constructorArguments, bool callBaseByDefault)
private object Create(Type[] typesToProxy, object?[] constructorArguments, bool callBaseByDefault, bool isPartial)
{
var substituteState = substituteStateFactory.Create(this);
substituteState.CallBaseConfiguration.CallBaseByDefault = callBaseByDefault;
Expand All @@ -46,7 +46,7 @@ private object Create(Type[] typesToProxy, object?[] constructorArguments, bool

var callRouter = callRouterFactory.Create(substituteState, canConfigureBaseCalls);
var additionalTypes = typesToProxy.Where(x => x != primaryProxyType).ToArray();
var proxy = proxyFactory.GenerateProxy(callRouter, primaryProxyType, additionalTypes, constructorArguments);
var proxy = proxyFactory.GenerateProxy(callRouter, primaryProxyType, additionalTypes, isPartial, constructorArguments);
return proxy;
}

Expand Down
21 changes: 21 additions & 0 deletions src/NSubstitute/Exceptions/TypeForwardingException.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
namespace NSubstitute.Exceptions;

public abstract class TypeForwardingException(string message) : SubstituteException(message)
{
}

public sealed class CanNotForwardCallsToClassNotImplementingInterfaceException(Type type) : TypeForwardingException(DescribeProblem(type))
{
private static string DescribeProblem(Type type)
{
return string.Format("The provided class '{0}' doesn't implement all requested interfaces. ", type.Name);
}
Comment on lines +7 to +12
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (non-blocking): for a future change, I think it would be helpful for this to explain which of the requested interfaces has not been implemented. If we see "Class 'A' does not implement all requested interfaces. To support call forwarding 'A' must implement 'B', 'C', and 'D'." then it gives us a good hint on how to fix the exception.

}

public sealed class CanNotForwardCallsToAbstractClassException(Type type) : TypeForwardingException(DescribeProblem(type))
{
private static string DescribeProblem(Type type)
{
return string.Format("The provided class '{0}' is abstract. ", type.Name);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick (non-blocking): trailing space: "...abstract . " vs "...abstract ."..

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ public class CastleDynamicProxyFactory(ICallFactory callFactory, IArgumentSpecif
private readonly ProxyGenerator _proxyGenerator = new ProxyGenerator();
private readonly AllMethodsExceptCallRouterCallsHook _allMethodsExceptCallRouterCallsHook = new AllMethodsExceptCallRouterCallsHook();

public object GenerateProxy(ICallRouter callRouter, Type typeToProxy, Type[]? additionalInterfaces, object?[]? constructorArguments)
public object GenerateProxy(ICallRouter callRouter, Type typeToProxy, Type[]? additionalInterfaces, bool isPartial, object?[]? constructorArguments)
{
return typeToProxy.IsDelegate()
? GenerateDelegateProxy(callRouter, typeToProxy, additionalInterfaces, constructorArguments)
: GenerateTypeProxy(callRouter, typeToProxy, additionalInterfaces, constructorArguments);
: GenerateTypeProxy(callRouter, typeToProxy, additionalInterfaces, isPartial, constructorArguments);
}

private object GenerateTypeProxy(ICallRouter callRouter, Type typeToProxy, Type[]? additionalInterfaces, object?[]? constructorArguments)
private object GenerateTypeProxy(ICallRouter callRouter, Type typeToProxy, Type[]? additionalInterfaces, bool isPartial, object?[]? constructorArguments)
{
VerifyClassHasNotBeenPassedAsAnAdditionalInterface(additionalInterfaces);

Expand All @@ -31,7 +31,8 @@ private object GenerateTypeProxy(ICallRouter callRouter, Type typeToProxy, Type[
additionalInterfaces,
constructorArguments,
[proxyIdInterceptor, forwardingInterceptor],
proxyGenerationOptions);
proxyGenerationOptions,
isPartial);

forwardingInterceptor.SwitchToFullDispatchMode();
return proxy;
Expand All @@ -54,7 +55,8 @@ private object GenerateDelegateProxy(ICallRouter callRouter, Type delegateType,
additionalInterfaces: null,
constructorArguments: null,
interceptors: [proxyIdInterceptor, forwardingInterceptor],
proxyGenerationOptions);
proxyGenerationOptions,
isPartial: false);

forwardingInterceptor.SwitchToFullDispatchMode();

Expand All @@ -75,8 +77,13 @@ private CastleForwardingInterceptor CreateForwardingInterceptor(ICallRouter call
private object CreateProxyUsingCastleProxyGenerator(Type typeToProxy, Type[]? additionalInterfaces,
object?[]? constructorArguments,
IInterceptor[] interceptors,
ProxyGenerationOptions proxyGenerationOptions)
ProxyGenerationOptions proxyGenerationOptions,
bool isPartial)
{
if (isPartial)
return CreatePartialProxy(typeToProxy, additionalInterfaces, constructorArguments, interceptors, proxyGenerationOptions, isPartial);


if (typeToProxy.GetTypeInfo().IsInterface)
{
VerifyNoConstructorArgumentsGivenForInterface(constructorArguments);
Expand All @@ -96,13 +103,40 @@ private object CreateProxyUsingCastleProxyGenerator(Type typeToProxy, Type[]? ad
additionalInterfaces = interfaces;
}


return _proxyGenerator.CreateClassProxy(typeToProxy,
additionalInterfaces,
proxyGenerationOptions,
constructorArguments,
interceptors);
}

private object CreatePartialProxy(Type typeToProxy, Type[]? additionalInterfaces, object?[]? constructorArguments, IInterceptor[] interceptors, ProxyGenerationOptions proxyGenerationOptions, bool isPartial)
{
if (typeToProxy.GetTypeInfo().IsClass &&
additionalInterfaces != null &&
additionalInterfaces.Any())
{
VerifyClassIsNotAbstract(typeToProxy);
VerifyClassImplementsAllInterfaces(typeToProxy, additionalInterfaces);

var targetObject = Activator.CreateInstance(typeToProxy, constructorArguments);
typeToProxy = additionalInterfaces.First();

return _proxyGenerator.CreateInterfaceProxyWithTarget(typeToProxy,
additionalInterfaces,
target: targetObject,
options: proxyGenerationOptions,
interceptors: interceptors);
}

return _proxyGenerator.CreateClassProxy(typeToProxy,
additionalInterfaces,
proxyGenerationOptions,
constructorArguments,
interceptors);
}

private ProxyGenerationOptions GetOptionsToMixinCallRouterProvider(ICallRouter callRouter)
{
var options = new ProxyGenerationOptions(_allMethodsExceptCallRouterCallsHook);
Expand All @@ -116,6 +150,22 @@ private ProxyGenerationOptions GetOptionsToMixinCallRouterProvider(ICallRouter c
return options;
}

private static void VerifyClassImplementsAllInterfaces(Type classType, IEnumerable<Type> additionalInterfaces)
{
if (!additionalInterfaces.All(x => x.GetTypeInfo().IsAssignableFrom(classType.GetTypeInfo())))
{
throw new CanNotForwardCallsToClassNotImplementingInterfaceException(classType);
}
}

private static void VerifyClassIsNotAbstract(Type classType)
{
if (classType.GetTypeInfo().IsAbstract)
{
throw new CanNotForwardCallsToAbstractClassException(classType);
}
}

private static void VerifyNoConstructorArgumentsGivenForInterface(object?[]? constructorArguments)
{
if (HasItems(constructorArguments))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ public virtual ICall Map(IInvocation castleInvocation)
Func<object>? baseMethod = null;
if (castleInvocation.InvocationTarget != null &&
castleInvocation.MethodInvocationTarget.IsVirtual &&
!castleInvocation.MethodInvocationTarget.IsAbstract &&
!castleInvocation.MethodInvocationTarget.IsFinal)
!castleInvocation.MethodInvocationTarget.IsAbstract)
Comment on lines -14 to +13
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: dropping "not final" constraint here is required to support call forwarding?

{
baseMethod = CreateBaseResultInvocation(castleInvocation);
}
Expand Down
4 changes: 2 additions & 2 deletions src/NSubstitute/Proxies/DelegateProxy/DelegateProxyFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ public class DelegateProxyFactory(CastleDynamicProxyFactory objectProxyFactory)
{
private readonly CastleDynamicProxyFactory _castleObjectProxyFactory = objectProxyFactory ?? throw new ArgumentNullException(nameof(objectProxyFactory));

public object GenerateProxy(ICallRouter callRouter, Type typeToProxy, Type[]? additionalInterfaces, object?[]? constructorArguments)
public object GenerateProxy(ICallRouter callRouter, Type typeToProxy, Type[]? additionalInterfaces, bool isPartial, object?[]? constructorArguments)
{
// Castle factory can now resolve delegate proxies as well.
return _castleObjectProxyFactory.GenerateProxy(callRouter, typeToProxy, additionalInterfaces, constructorArguments);
return _castleObjectProxyFactory.GenerateProxy(callRouter, typeToProxy, additionalInterfaces, isPartial, constructorArguments);
}
}
6 changes: 3 additions & 3 deletions src/NSubstitute/Proxies/ProxyFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ namespace NSubstitute.Proxies;
[Obsolete("This class is deprecated and will be removed in future versions of the product.")]
public class ProxyFactory(IProxyFactory delegateFactory, IProxyFactory dynamicProxyFactory) : IProxyFactory
{
public object GenerateProxy(ICallRouter callRouter, Type typeToProxy, Type[]? additionalInterfaces, object?[]? constructorArguments)
public object GenerateProxy(ICallRouter callRouter, Type typeToProxy, Type[]? additionalInterfaces, bool isPartial, object?[]? constructorArguments)
{
var isDelegate = typeToProxy.IsDelegate();
return isDelegate
? delegateFactory.GenerateProxy(callRouter, typeToProxy, additionalInterfaces, constructorArguments)
: dynamicProxyFactory.GenerateProxy(callRouter, typeToProxy, additionalInterfaces, constructorArguments);
? delegateFactory.GenerateProxy(callRouter, typeToProxy, additionalInterfaces, isPartial, constructorArguments)
: dynamicProxyFactory.GenerateProxy(callRouter, typeToProxy, additionalInterfaces, isPartial, constructorArguments);
}
}
20 changes: 20 additions & 0 deletions src/NSubstitute/Substitute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -89,4 +89,24 @@ public static T ForPartsOf<T>(params object[] constructorArguments)
var substituteFactory = SubstitutionContext.Current.SubstituteFactory;
return (T)substituteFactory.CreatePartial([typeof(T)], constructorArguments);
}

/// <summary>
/// Creates a proxy for a class that implements an interface, forwarding methods and properties to an instance of the class, effectively mimicking a real instance.
Comment on lines +93 to +94
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

praise: great code docs here, thanks! 🙇

/// Both the interface and the class must be provided as parameters.
/// The proxy will log calls made to the interface members and delegate them to an instance of the class. Specific members can be substituted
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick (non-blocking): "will log calls" -> "will intercept calls"

/// by using <see cref="WhenCalled{T}.DoNotCallBase()">When(() => call).DoNotCallBase()</see> or by
/// <see cref="SubstituteExtensions.Returns{T}(T,T,T[])">setting a value to return value</see> for that member.
/// This extension supports sealed classes and non-virtual members, with some limitations. Since the substituted method is non-virtual, internal calls within the object will invoke the original implementation and will not be logged.
/// </summary>
/// <typeparam name="TInterface">The interface the substitute will implement.</typeparam>
/// <typeparam name="TClass">The class type implementing the interface. Must be a class; not a delegate or interface. </typeparam>
/// <param name="constructorArguments"></param>
/// <returns>An object implementing the selected interface. Calls will be forwarded to the actuall methods, but allows parts to be selectively
/// overridden via `Returns` and `When..DoNotCallBase`.</returns>
public static TInterface ForTypeForwardingTo<TInterface, TClass>(params object[] constructorArguments)
where TInterface : class
{
var substituteFactory = SubstitutionContext.Current.SubstituteFactory;
return (TInterface)substituteFactory.CreatePartial([typeof(TInterface), typeof(TClass)], constructorArguments);
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using NUnit.Framework;
using NUnit.Framework.Legacy;

namespace NSubstitute.Acceptance.Specs;

Expand Down Expand Up @@ -31,6 +32,30 @@ public void Can_sub_for_concrete_type_and_implement_other_interfaces()
subAsIFirst.Received().First();
}

[Test]
public void Can_sub_for_abstract_type_and_implement_other_two_interfaces()
{
// test from docs
var substitute = Substitute.For([typeof(IFirst), typeof(ISecond), typeof(ClassWithCtorArgs)],
["hello world", 5]);

ClassicAssert.IsInstanceOf<IFirst>(substitute);
ClassicAssert.IsInstanceOf<ISecond>(substitute);
ClassicAssert.IsInstanceOf<ClassWithCtorArgs>(substitute);
}

[Test]
public void Can_sub_for_concrete_type_and_implement_other_two_interfaces()
{
// test from docs
var substitute = Substitute.For([typeof(IFirst), typeof(ISecond), typeof(ConcreteClassWithCtorArgs)],
["hello world", 5]);

ClassicAssert.IsInstanceOf<IFirst>(substitute);
ClassicAssert.IsInstanceOf<ISecond>(substitute);
ClassicAssert.IsInstanceOf<ConcreteClassWithCtorArgs>(substitute);
}

[Test]
public void Partial_sub()
{
Expand Down Expand Up @@ -90,8 +115,13 @@ public class Partial
public virtual int Number() { return -1; }
public int GetNumberPlusOne() { return Number() + 1; }
}

public abstract class ClassWithCtorArgs(string s, int a)
{
public string StringFromCtorArg { get; set; } = s; public int IntFromCtorArg { get; set; } = a;
}

public class ConcreteClassWithCtorArgs(string s, int a) : ClassWithCtorArgs(s, a)
{
}
}
Loading
Loading