Skip to content

Commit

Permalink
Fix default parameters not being used
Browse files Browse the repository at this point in the history
  • Loading branch information
luttje committed Nov 3, 2023
1 parent 15472b7 commit cc9ce13
Show file tree
Hide file tree
Showing 6 changed files with 110 additions and 43 deletions.
9 changes: 8 additions & 1 deletion Core/Key2Joy.Contracts/Plugins/PluginAction.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,22 @@ public virtual void LoadOptions(MappingAspectOptions options)
/// The types returned must be simple types that can be marshalled across
/// </summary>
/// <param name="methodName"></param>
/// <param name="parameterDefaultValues"></param>
/// <param name="isLastParameterParams"></param>
/// <returns></returns>
internal IList<Type> GetMethodParameterTypes(string methodName, out bool isLastParameterParams)
internal IList<Type> GetMethodParameterTypes(
string methodName,
out IList<object> parameterDefaultValues,
out bool isLastParameterParams)
{
var method = this.GetType().GetMethod(methodName);
var parameterInfos = method.GetParameters();

isLastParameterParams = parameterInfos.Length > 0
&& parameterInfos.Last().IsDefined(typeof(ParamArrayAttribute), false);

parameterDefaultValues = parameterInfos.Select(p => p.DefaultValue).ToList();

//var types = new List<Type>();
//foreach (var parameter in parameters)
//{
Expand Down
6 changes: 5 additions & 1 deletion Core/Key2Joy.Contracts/Plugins/PluginActionInsulator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,9 @@ public class PluginActionInsulator : MarshalByRefObject

public object GetPublicPropertyValue(string propertyName) => this.PluginAction.GetPublicPropertyValue(propertyName);

public IList<Type> GetMethodParameterTypes(string methodName, out bool isLastParameterParams) => this.PluginAction.GetMethodParameterTypes(methodName, out isLastParameterParams);
public IList<Type> GetMethodParameterTypes(
string methodName,
out IList<object> parameterDefaultValues,
out bool isLastParameterParams)
=> this.PluginAction.GetMethodParameterTypes(methodName, out parameterDefaultValues, out isLastParameterParams);
}
70 changes: 48 additions & 22 deletions Core/Key2Joy.Core/Mapping/Actions/Scripting/ExposedMethod.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ public abstract class ExposedMethod

private readonly Dictionary<Type, ParameterTransformerDelegate> parameterTransformers = new();
protected IList<Type> ParameterTypes { get; private set; } = new List<Type>();
protected IList<object> ParameterDefaultValues { get; private set; } = new List<object>();
protected bool IsLastParameterParams { get; private set; } = false;

public ExposedMethod(string functionName, string methodName)
Expand All @@ -32,11 +33,12 @@ public ExposedMethod(string functionName, string methodName)
public void Prepare(object instance)
{
this.Instance = instance;
this.ParameterTypes = this.GetParameterTypes(out var isLastParameterParams);
this.ParameterTypes = this.GetParameterTypes(out var parameterDefaultValues, out var isLastParameterParams);
this.ParameterDefaultValues = parameterDefaultValues;
this.IsLastParameterParams = isLastParameterParams;
}

public abstract IList<Type> GetParameterTypes(out bool isLastParameterParams);
public abstract IList<Type> GetParameterTypes(out IList<object> parameterDefaultValues, out bool isLastParameterParams);

public abstract object InvokeMethod(object[] transformedParameters);

Expand Down Expand Up @@ -87,29 +89,51 @@ public object TransformAndRedirect(params object[] parameters)
parameters = new object[] { parameters };
}

var transformedParameters = parameters.Select((parameter, parameterIndex) =>
{
var parameterType = this.ParameterTypes[parameterIndex];
if (this.parameterTransformers.TryGetValue(parameter.GetType(), out var transformer))
var transformedParameters = parameters
.Select((parameter, parameterIndex) =>
{
parameter = transformer(parameter, parameterType);
}
var parameterType = this.ParameterTypes[parameterIndex];
return TypeConverter.ConvertToType(parameter, parameterType);
}).ToArray();
if (this.parameterTransformers.TryGetValue(parameter.GetType(), out var transformer))
{
parameter = transformer(parameter, parameterType);
}
// Check if the last parameter is not provided, but is a params.
// If so, we'll create an empty array for it.
if (this.IsLastParameterParams)
return TypeConverter.ConvertToType(parameter, parameterType);
})
.ToList(); // ToList allows for easier manipulation than an array.

// Ensure the transformedParameters list has the same number of items as this.ParameterTypes
while (transformedParameters.Count < this.ParameterTypes.Count)
{
var lastParameter = this.ParameterTypes.Last();
var elementType = lastParameter.GetElementType();
var array = Array.CreateInstance(elementType, 0);
transformedParameters = transformedParameters.Append(array).ToArray();
transformedParameters.Add(this.GetDefaultParameterValue(transformedParameters.Count));
}

return this.InvokeMethod(transformedParameters);
return this.InvokeMethod(transformedParameters.ToArray());
}

/// <summary>
/// Method to get default parameter value or create an empty array for params
/// </summary>
/// <param name="parameterIndex"></param>
/// <returns></returns>
/// <exception cref="ArgumentException"></exception>
private object GetDefaultParameterValue(int parameterIndex)
{
if (this.IsLastParameterParams
&& parameterIndex == this.ParameterTypes.Count - 1)
{
var lastParameterType = this.ParameterTypes.Last();
var elementType = lastParameterType.GetElementType();
return Array.CreateInstance(elementType ?? typeof(object), 0);
}
else if (parameterIndex < this.ParameterDefaultValues.Count
&& this.ParameterDefaultValues[parameterIndex] != DBNull.Value)
{
return this.ParameterDefaultValues[parameterIndex];
}

throw new ArgumentException("No default value available for parameter at index " + parameterIndex);
}

/// <summary>
Expand All @@ -132,13 +156,15 @@ public TypeExposedMethod(string functionName, string methodName, Type type)
this.cachedMethodInfo = this.Type.GetMethod(this.MethodName);
}

public override IList<Type> GetParameterTypes(out bool isLastParameterParams)
public override IList<Type> GetParameterTypes(out IList<object> parameterDefaultValues, out bool isLastParameterParams)
{
var parameterInfos = this.cachedMethodInfo.GetParameters();

isLastParameterParams = parameterInfos.Length > 0
&& parameterInfos.Last().IsDefined(typeof(ParamArrayAttribute), false);

parameterDefaultValues = parameterInfos.Select(p => p.DefaultValue).ToList();

return parameterInfos.Select(p => p.ParameterType).ToList();
}

Expand All @@ -154,10 +180,10 @@ public PluginExposedMethod(string typeName, string functionName, string methodNa
: base(functionName, methodName)
=> this.TypeName = typeName;

public override IList<Type> GetParameterTypes(out bool isLastParameterParams)
public override IList<Type> GetParameterTypes(out IList<object> parameterDefaultValues, out bool isLastParameterParams)
{
var instance = (PluginActionProxy)this.Instance;
return instance.GetMethodParameterTypes(this.MethodName, out isLastParameterParams);
return instance.GetMethodParameterTypes(this.MethodName, out parameterDefaultValues, out isLastParameterParams);
}

public override object InvokeMethod(object[] transformedParameters)
Expand Down
4 changes: 2 additions & 2 deletions Core/Key2Joy.Core/Plugins/PluginActionProxy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ public override void LoadOptions(MappingAspectOptions options)

public override string GetNameDisplay() => this.source.GetNameDisplay(this.Name);

public IList<Type> GetMethodParameterTypes(string methodName, out bool isLastParameterParams)
=> this.source.GetMethodParameterTypes(methodName, out isLastParameterParams);
public IList<Type> GetMethodParameterTypes(string methodName, out IList<object> parameterDefaultValues, out bool isLastParameterParams)
=> this.source.GetMethodParameterTypes(methodName, out parameterDefaultValues, out isLastParameterParams);

public object InvokeScriptMethod(string methodName, object[] parameters)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public MockExposedMethod(string functionName, string methodName)
: base(functionName, methodName)
{ }

public override IList<Type> GetParameterTypes(out bool isLastParameterParams)
public override IList<Type> GetParameterTypes(out IList<object> parameterDefaultValues, out bool isLastParameterParams)
=> throw new NotImplementedException();

public override object InvokeMethod(object[] transformedParameters)
Expand All @@ -40,6 +40,9 @@ public string MethodThatConcatsFloatParameters(float p1, float p2)

public string MethodThatConcatsParametersAndHasParams(string p1, int p2, params string[] p3)
=> p1 + p2 + string.Join("/", p3);

public string MethodWithDefaultParameters(string p1, int p2 = 2)
=> $"{p1}|{p2}";
}

[TestClass]
Expand All @@ -49,8 +52,10 @@ public class ExposedMethodTests
public void Test_Prepare_SetsInstanceAndParameterTypes()
{
var exposedMethod = new Mock<MockExposedMethod>() { CallBase = true };
bool _;
exposedMethod.Setup(m => m.GetParameterTypes(out _)).Returns(new List<Type> { typeof(string), typeof(int) });

IList<object> _;
bool __;
exposedMethod.Setup(m => m.GetParameterTypes(out _, out __)).Returns(new List<Type> { typeof(string), typeof(int) });

var instance = new object();
exposedMethod.Object.Prepare(instance);
Expand All @@ -75,8 +80,10 @@ public void Test_RegisterParameterTransformer_FailsIfNotPrepared()
public void Test_RegisterParameterTransformer_ReplacesExistingTransformer()
{
var exposedMethod = new Mock<MockExposedMethod>() { CallBase = true };
bool _;
exposedMethod.Setup(m => m.GetParameterTypes(out _)).Returns(new List<Type> { typeof(int) });

IList<object> _;
bool __;
exposedMethod.Setup(m => m.GetParameterTypes(out _, out __)).Returns(new List<Type> { typeof(int) });
var instance = new object();
exposedMethod.Object.Prepare(instance);

Expand All @@ -92,8 +99,10 @@ public void Test_RegisterParameterTransformer_ReplacesExistingTransformer()
public void Test_TransformAndRedirect_FailsIfNotPrepared()
{
var exposedMethod = new Mock<MockExposedMethod>() { CallBase = true };
bool _;
exposedMethod.Setup(m => m.GetParameterTypes(out _)).Returns(new List<Type> { typeof(DayOfWeek) });

IList<object> _;
bool __;
exposedMethod.Setup(m => m.GetParameterTypes(out _, out __)).Returns(new List<Type> { typeof(DayOfWeek) });
exposedMethod.Setup(m => m.InvokeMethod(It.IsAny<object[]>())).Returns<object[]>(p => p[0]);

var resultingParameters = (object[])exposedMethod.Object.TransformAndRedirect(nameof(DayOfWeek.Monday));
Expand All @@ -103,8 +112,10 @@ public void Test_TransformAndRedirect_FailsIfNotPrepared()
public void Test_TransformAndRedirect_EnumConversion()
{
var exposedMethod = new Mock<MockExposedMethod>() { CallBase = true };
bool _;
exposedMethod.Setup(m => m.GetParameterTypes(out _)).Returns(new List<Type> { typeof(DayOfWeek) });

IList<object> _;
bool __;
exposedMethod.Setup(m => m.GetParameterTypes(out _, out __)).Returns(new List<Type> { typeof(DayOfWeek) });

var instance = new object();
exposedMethod.Object.Prepare(instance);
Expand All @@ -120,8 +131,10 @@ public void Test_TransformAndRedirect_EnumConversion()
public void Test_TransformAndRedirect_ObjectArrayConversion()
{
var exposedMethod = new Mock<MockExposedMethod>() { CallBase = true };
bool _;
exposedMethod.Setup(m => m.GetParameterTypes(out _)).Returns(new List<Type> { typeof(string[]), typeof(int[]) });

IList<object> _;
bool __;
exposedMethod.Setup(m => m.GetParameterTypes(out _, out __)).Returns(new List<Type> { typeof(string[]), typeof(int[]) });

var instance = new object();
exposedMethod.Object.Prepare(instance);
Expand All @@ -142,8 +155,10 @@ public void Test_TransformAndRedirect_ObjectArrayConversion()
public void Test_TransformAndRedirect_InvalidOperationException()
{
var exposedMethod = new Mock<MockExposedMethod>() { CallBase = true };
bool _;
exposedMethod.Setup(m => m.GetParameterTypes(out _)).Returns(new List<Type> { typeof(MockExposedMethod) });

IList<object> _;
bool __;
exposedMethod.Setup(m => m.GetParameterTypes(out _, out __)).Returns(new List<Type> { typeof(MockExposedMethod) });

exposedMethod.Object.TransformAndRedirect(new object());
}
Expand All @@ -153,7 +168,7 @@ public void Test_TypeExposedMethod_GetParameterTypesIfNone()
{
var exposedMethod = new TypeExposedMethod("functionName", nameof(MockType.MethodWithNoParameters), typeof(MockType));

var parameterTypes = exposedMethod.GetParameterTypes(out var _);
var parameterTypes = exposedMethod.GetParameterTypes(out var _, out var _);

Assert.IsFalse(parameterTypes.Any());
}
Expand All @@ -163,7 +178,7 @@ public void Test_TypeExposedMethod_GetParameterTypes()
{
var exposedMethod = new TypeExposedMethod("functionName", nameof(MockType.MethodThatConcatsParameters), typeof(MockType));

var parameterTypes = exposedMethod.GetParameterTypes(out var _).ToList();
var parameterTypes = exposedMethod.GetParameterTypes(out var _, out var _).ToList();

CollectionAssert.AreEqual(parameterTypes, new Type[] { typeof(string), typeof(int) });
}
Expand Down Expand Up @@ -206,4 +221,17 @@ public void Test_TypeExposedMethod_InvokeMethodImpliedEmptyParams()

Assert.AreEqual("123", result);
}

[TestMethod]
public void Test_TypeExposedMethod_InvokeMethodWithDefaultParameters()
{
var exposedMethod = new TypeExposedMethod("functionName", nameof(MockType.MethodWithDefaultParameters), typeof(MockType));

var instance = new MockType();
exposedMethod.Prepare(instance);

var result = (string)exposedMethod.TransformAndRedirect(new object[] { "1" });

Assert.AreEqual("1|2", result);
}
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System.Collections.Generic;
using System.Linq;
using Key2Joy.Contracts.Mapping;
using Key2Joy.Tests.PluginHost;
Expand All @@ -24,8 +25,9 @@ public void Plugin_ScriptAction_CanGetParameterTypes()

exposedMethod.Prepare(actionProxy);

bool _;
var parameterTypes = exposedMethod.GetParameterTypes(out _).ToList();
IList<object> _;
bool __;
var parameterTypes = exposedMethod.GetParameterTypes(out _, out __).ToList();

CollectionAssert.AreEqual(parameterTypes, new[] { typeof(string), typeof(int[]) });
}
Expand Down

0 comments on commit cc9ce13

Please sign in to comment.